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
> - 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
> 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
> 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,
> 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 :)