[PATCH] libdw: Fix __libdw_form_val_len endp pointer.

Mark Wielaard mjw at redhat.com
Fri Dec 12 15:54:21 UTC 2014


On Thu, 2014-12-11 at 15:34 -0800, Josh Stone wrote:
> On 12/11/2014 03:05 PM, Mark Wielaard wrote:
> > While rebasing my work on top of Josh attrabbrev reading speedups
> > I noticed a typo/thinko in my form_val_len bound checking patch.
> > 
> > We should check against the die->cu->endp, not the abbrev endp.
> 
> In that case, I don't think you need endp as a parameter at all.  Just
> use cu->endp as needed, and prevent this mistake from ever happening.

Yes we can. But it is slightly tricky since we have a couple of places
where we have "fake" attributes that get their form data not from
the .debug_info CU data. So we need to make sure those attributes have a
fake cu with the data pointers properly initialized.

We have one "static" empty location expression that we have to give a
static "one byte" fake CU. The new debug macros also have form data,
which comes from a fake debug_macro CU. The somewhat tricky one is
dwarf_getlocation_attr, which gives a fake attribute that represents a
location given an Dwarf_Op. In the case of DW_OP_implicit_value,
DW_OP_GNU_entry_value and DW_OP_GNU_const_type this returns an attribute
that represents a (expression) block. The block data can come from
either the .debug_info cu data or from the location list data
in .debug_loc. For the last case we need a fake loc CU for each Dwarf
that covers the .debug_loc data.

The attached patch implements this. It also adds bounds checking for
dwarf_form_block. Which did actually try to do some checking to see
whether the returned block was completely in the CU. Luckily this check
was wrong and for the above tricky cases not notice the block data was
beyond the .debug_info data. Correcting the check without the proper
fake CUs in place would make some testcases fail.

> Maybe you could also check valp >= cu->startp while you're at it, but
> that's probably an ok invariant to just assume.

I think that is redundant checking in a place where we want to do as
little work as possible. Also that is checked earlier in the code.

> Also along these lines, the dbg parameter could just use cu->dbg as
> needed.  Saving parameters probably won't do much for performance in
> this case, nor is changing to a pointer read likely to hurt.  It does
> help code clarity though, IMO.

Yes, much nicer to read.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libdw-Make-sure-all-attributes-come-with-a-fake-CU-f.patch
Type: text/x-patch
Size: 15915 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/elfutils-devel/attachments/20141212/5827b7e9/attachment-0001.bin>


More information about the elfutils-devel mailing list