[PATCH] libdw: handle DW_FORM_indirect when reading attributes

[Mark Wielaard]

Hi Omar,

On Fri, 2021-04-23 at 16:36 -0700, Omar Sandoval wrote:
> Whenever we encounter an attribute with DW_FORM_indirect, we need to
> read its true form from the DIE data. Then, we can continue normally.
> This adds support to the most obvious places: __libdw_find_attr() and
> dwarf_getattrs().

Very nice. I wasn't aware anybody actually used DW_FORM_indirect.

In your patch you only handle one indirection. I think that in theory a
DW_FORM_indirect could be an DW_FORM_indirect itself (but now in the
DIE instead of the Abbrev). But this is probably silly, so not
supporting that seems correct. Same for an indirect
DW_FORM_implicit_const. It doesn't really make sense to do that because
if you wanted to put the value in the DIE instead of an Abbrev you
would have used an actual FORM that did that.

> There may be more places that need to be updated.

There is __libdw_form_val_compute_len which already handles
DW_FORM_indirect:

    case DW_FORM_indirect:
      get_uleb128 (u128, valp, endp);
      // XXX Is this really correct?
      result = __libdw_form_val_len (cu, u128, valp);
      if (result != (size_t) -1)
        result += valp - startp;
      else
        return (size_t) -1;
      break;

I believe the XXX question can be answered with: Yes, the result is the
size of the actual FORM data plus the size of the uleb128 encoding that
FORM (which is valp - startp). And it probably should check like your
code does that valp != DW_FORM_indirect && valp !=
DW_FORM_implicit_const. I'll sent a patch to do that.

But, now that dwarf_child and dwarf_getattrs handle DW_FORM_indirectly
directly (haha, pun intended) I think __libdw_form_val_compute_len is
never called for DW_FORM_indirect anymore.

There is dwarf_hasattr, but that doesn't care about the value, just the
attribute name, so it doesn't have to follow any DW_FORM_indirect.

And there are the "raw" dwarf_getabbrev functions, but those aren't
associated with any DIE/.debug_info data, so leaving DW_FORM_indirect
as is in the Dwarf_Abbrev for those seems correct.

> I encountered this when inspecting a file that was processed by our BOLT
> tool: https://github.com/facebookincubator/BOLT. This also adds a couple
> of test cases using a file generated by that tool.

Thanks for that test case. It really helps seeing an actual example of
indirection. I note that all Abbrevs/DIEs using DW_FORM_indirect are
for DW_AT_low_pc and that the indirect DW_AT_addr are all the zero
address. Is that intended?

Pushed you patch.

Thanks,

Mark