[PATCH] D52444: [DWARF] Make llvm-dwarfdump display the .debug_loc.dwo section (fixes PR38991)

[Wolfgang Pieb via Phabricator via llvm-commits]

wolfgangp added a comment.

In https://reviews.llvm.org/D52444#1245696, @dblaikie wrote:

> In https://reviews.llvm.org/D52444#1245403, @wolfgangp wrote:
>
> > In https://reviews.llvm.org/D52444#1244247, @dblaikie wrote:
> >
> > > Should the address size be required/used for parsing the debug_loc.dwo
> > >  section? I would think there shouldn't be any addresses, or otherwise
> > >  address-size-dependent there?
> >
> >
> > Yes, but currently DWARFExpression is oblivious to DWO and insists on having a valid Address Size. This would have to be fixed before we can omit the address size.
>
>
> Is that sufficiently expensive to be worth doing this workaround in the interim?


The question is whether to bring it up to DWARF5 level right away or to just have it deal with DWO with what's currently supported. The former would certainly be more expensive.

> What if the address size was hardcoded to 4 here, for example? What would go wrong in that case?

A DW_OP_addr or DW_OP_call_ref opcode with an 8-byte operand would cause the rest of the expression to be parsed incorrectly. That's unlikely to occur in a location list, but it would definitely fail.
As a matter of fact, DW_OP_call_ref uses AddrSize to decide whether to get 32 or 64 bits, when this should be dependent on the DWARF format (32 vs. 64). So instead of addrsize we should pass the format instead in a DWO context.

> (could leave a fixit that this shouldn't be needed - maybe use Optional for the address size, so it asserts if someone doesn't provide an address size but it ends up being needed down the line)

I had originally thought to get location lists up to DWARF 5 level (i.e. just the table and range aspect), but deal with expressions later. Perhaps in the interim it's best to have what's currently there deal with DWO correctly so we don't have anything half-working out there.


https://reviews.llvm.org/D52444