[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967 --- Comment #4 from Aleksei Vetrov --- (In reply to Frank Ch. Eigler from comment #3) > Is there some reason not to just bump up that bitfield width from :24 to :32 > or more for now, until a deeper analysis of llvm informs us as to how wide > these discriminator codes can really be? For me it is ok to bump that bitfield, but there is a warning in the code: > All the flags and other bit fields should add up to 48 bits > to give the whole struct a nice round size. So this question should be directed to the author of this code: Roland McGrath . I think there may be slight performance/memory issues, but as a temporary solution it looks good. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967
Mark Wielaard changed:
What|Removed |Added
CC||roland at gnu dot org
--- Comment #5 from Mark Wielaard ---
(In reply to Aleksei Vetrov from comment #4)
> (In reply to Frank Ch. Eigler from comment #3)
> > Is there some reason not to just bump up that bitfield width from :24 to :32
> > or more for now, until a deeper analysis of llvm informs us as to how wide
> > these discriminator codes can really be?
>
> For me it is ok to bump that bitfield, but there is a warning in the code:
>
> > All the flags and other bit fields should add up to 48 bits
> > to give the whole struct a nice round size.
>
> So this question should be directed to the author of this code: Roland
> McGrath . I think there may be slight performance/memory
> issues, but as a temporary solution it looks good.
Added Roland (now with another email address) to the CC.
Note that we have blown the size of this struct already to support the NVIDIA
extensions :{
The issue here is that we create (very) large arrays of struct Dwarf_Line_s and
we do that eagerly, see bug #27405
So we would like to keep that struct as small as possible.
Another "solution"/workaround would be to just ignore such crazy big
discriminator values and just set them to zero or store them modulo 24 bits
(they are probably still unique then).
--
You are receiving this mail because:
You are on the CC list for the bug.
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967 --- Comment #6 from Mark Wielaard --- So my preferred workaround: diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c index df003c5f..69e10c7b 100644 --- a/libdw/dwarf_getsrclines.c +++ b/libdw/dwarf_getsrclines.c @@ -129,6 +129,12 @@ add_new_line (struct line_state *state, struct linelist *new_line) return true; \ } while (0) + /* Same as above, but don't flag as "invalid" just use truncated + value. Used for discriminator for which llvm might use a value + that won't fit 24 bits. */ +#define SETX(field) \ + new_line->line.field = state->field;\ + SET (addr); SET (op_index); SET (file); @@ -140,7 +146,7 @@ add_new_line (struct line_state *state, struct linelist *new_line) SET (prologue_end); SET (epilogue_begin); SET (isa); - SET (discriminator); + SETX (discriminator); SET (context); SET (function_name); https://code.wildebeest.org/git/user/mjw/elfutils/commit/?h=really-large-disc -- You are receiving this mail because: You are on the CC list for the bug.
Preparing for a 0.190 release end of next week
Hi all, There are various patch reviews and outstanding bugs being worked on. And we'll try to get to all of them. But it would also be good to get a new release out. The last one was in March. There have been 70+ commits by 20 people since 0.189. Lets get a few more patches in and then aim for a release Friday 3 November. Don't worry if a patch doesn't make it for this release, we'll try to do another early next year (normally there are just 3 or 4 months between releases, this one took 6, which is an exception). Cheers, Mark
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967 --- Comment #7 from Frank Ch. Eigler --- > So my preferred workaround: appears to be based on the assumption that truncated bitfields will not collide. Has this assumption been tested? -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967
--- Comment #8 from Frank Ch. Eigler ---
> The issue here is that we create (very) large arrays of struct Dwarf_Line_s
> and we do that eagerly, see bug #27405
> So we would like to keep that struct as small as possible.
Do we have an estimate about the numbers we're talking about here? The current
Dwarf_Line_s object is 40 bytes long (on x86-64). Even if packing becomes less
efficient, it could cost us say 2 bytes more per record. These records are
packed together in allocation via realloc() et al. How many records do we see
in programs of interest? readelf -wL /bin/emacs indicates about 800 thousand;
libxul.so about 8 million. Is this about single digit megabytes more RAM in
grand total?
Note that bug #27405 was a request for optimization, not in order to save a few
percent of memory for used data, but to save ALL the related memory & time for
totally unused data.
struct Dwarf_Line_s {
Dwarf_Files * files;/* 0 8 */
Dwarf_Addr addr; /* 8 8 */
unsigned int file; /*16 4 */
intline; /*20 4 */
short unsigned int column; /*24 2 */
/* Bitfield combined with previous fields */
unsigned int is_stmt:1;/*24:16 4 */
unsigned int basic_block:1;/*24:17 4 */
unsigned int end_sequence:1; /*24:18 4 */
unsigned int prologue_end:1; /*24:19 4 */
unsigned int epilogue_begin:1; /*24:20 4 */
unsigned int op_index:8; /*24:21 4 */
/* XXX 3 bits hole, try to pack */
/* Force alignment to the next boundary: */
unsigned int :0;
unsigned int isa:8;/*28: 0 4 */
unsigned int discriminator:24; /*28: 8 4 */
unsigned int context; /*32 4 */
unsigned int function_name;/*36 4 */
/* size: 40, cachelines: 1, members: 15 */
/* sum members: 34 */
/* sum bitfield members: 45 bits, bit holes: 1, sum bit holes: 3 bits
*/
/* last cacheline: 40 bytes */
};
--
You are receiving this mail because:
You are on the CC list for the bug.
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967 --- Comment #9 from Aleksei Vetrov --- (In reply to Frank Ch. Eigler from comment #7) > So my preferred workaround: I like this workaround and it works in our use case. > appears to be based on the assumption that truncated bitfields will not > collide. Has this assumption been tested? No such assumption can be made, there is no guarantees from DWARF standard and it is possible to manually generate DWARF with discriminators exceeding even "unsigned int": just use any library and shift all values by 32 bits. It will be well within standard, however compilers are not expected to behave like this. In an "liblog.so" attachment truncated values don't collide. I'm more interested in this: what is worst that can happen on discriminator collision? We are not using discriminator at all in ABI monitoring, so I'm not familiar with its use cases. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967 --- Comment #10 from Mark Wielaard --- (In reply to Frank Ch. Eigler from comment #7) > > So my preferred workaround: > > appears to be based on the assumption that truncated bitfields will not > collide. Has this assumption been tested? Most of the values used as discriminators in the test file are unique when truncated to their 24bit value. Note that the values only need to be unique per source line. I haven't tested that they are. But this is already much better than rejecting the whole line table. And all this only really matters for something actually using the discriminator (worst case some instructions are grouped together). -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/30967] Discriminator in Dwarf_Line_s may exceed 24 bits
https://sourceware.org/bugzilla/show_bug.cgi?id=30967 --- Comment #11 from Frank Ch. Eigler --- > I'm more interested in this: what is worst that can happen on discriminator > collision? We are not using discriminator at all in ABI monitoring, so I'm > not familiar with its use cases. That's a good point. There is an api call dwarf_linediscriminator() to fetch it for a client. In a bit of searching here and there (github etc.), I have not found any user so far, other than readelf itself to print. In gnu binutils, the field is "unsigned int" wide, and is used in bfd & gdb. -- You are receiving this mail because: You are on the CC list for the bug.
