Re: dwarflint

[Petr Machata <pmachata at redhat dot com>]

I had a few spare moments today, so I fixed a couple things.

Roland McGrath wrote:
> I'll try to review the actual code soon.  A few thoughts for now:
> 
>>      - It loads .debug_abbrev, doing checks for validity of various 
>> fields along the way.  LEB128s are checked for overflow.  All reads are 
>> checked that they don't pass outside the section data.
> 
> For LEB128 there is an additional warning check to do.  That is for a
> longer-than-necessary encoding.  For ULEB128, that means any trailing bytes
> all of whose low 7 bits are 0.  For SLEB128, any that are all 1 when the
> last high bit was 1 or are all 0 when the last high bit was 0.
> 
> This is not an invalid encoding, but is a red flag.  Make it a warning
> rather than error, and/or enable the check with a --strict flag.

I implemented the check and made it a warning.

>> DIEs whose abbreviation has_children are checked to actually have
>> non-zero children.
> 
> This also is a strict warning rather an error.  fpc used to generate DWARF
> like this (maybe still does).  It's suboptimal but technically kosher.

I made it a warning present in strict mode.

>>      - String references are marked and holes of unused bytes in 
>> .dwarf_str are reported.
> 
> This is going off all DW_FORM_strp uses?  I guess that does cover it.
> My first reaction was to think you can't find all references without
> higher-level grokage of DIEs.  But in fact the only references to the
> string table are via DW_FORM_strp, I think.
> 
> This of course is also a warning only, since unused bytes are kosher.
> The only hard errors are an offset too large, or if the last byte of
> .debug_str is not zero (so the final string would be unterminated).

It's only --strict level check, but I made it a warning anyway.

>>      - Most of that can't be considered tested.  I tried a couple 
>> debuginfo files (emacs, libX11, libsc680lx from OpenOffice).  debuginfo 
>> files often seem to contain unused .debug_str bytes.
> 
> Check whether any unused bytes are not zero, and flag that distinctly.  If
> they are all zero, it's likely to be alignment padding that the linker or
> assembler or compiler thought was necessary (though it isn't).

All cases that I've seen so far were suffixes of previous strings.  For 
example this is a typical case (in libX11):

  [ 14fd]  "/usr/src/debug/libX11-1.1.4/src"
  [ 151d]  ".4/src"
  [ 1524]  "xbutton"

dwarflint claims that 0x151d..0x1523 are unused.  I'm thinking it's the 
compiler squeezing the reference to ".4/src" into previous string, and 
failing to garbage collect the fragment.

> You can notice the highest power of two to which the first used
> offset after the extra zeros is aligned, and mention that in the
> message.  (In future, we might add flags to suppress the warning

Didn't do that yet, but made an XXX comment.

>> Non-zero DIEs followed by zero DIEs are reported for libsc680lx, which is
>> suspicious and likely a bug in lint.  False positives of this type will
>> have to be fixed as the producer development proceeds.
> 
> Not sure I grok "nonzero DIEs followed by zero DIEs".  Does this mean
> haschildren=true in a DIE with no children?  Or something else?
> (What is a "zero DIE"?)

Sorry, that should have been a "NULL abbrev," an abbrev with code 0, 
used for alignment purposes.  The open office library contains sequences 
of non-NULL abbrevs.  These technically form empty abbreviation 
sections, so maybe make it a warning instead?

The above is on dwarflint branch.  I had a small battle with git about 
the merges.  I think I didn't screw anything, all commits on master are 
yours.  monotone-viz was quite a bit better at showing inter-branch 
relations, in gitk everything looks like a particularly messy subway 
map.  But a couple more commits and I'll have branches figured out :)

PM

Attachment: signature.asc
Description: PGP signature