Re: [PATCH V5] Fix issues with reading rnglists, especially from dwo files, for DWARF v5

[Simon Marchi <simark@simark.ca>]

On 2020-07-14 11:47 a.m., Caroline Tice wrote:
> "This time for sure!" -- Bullwinkle Moose
> 
> I think I've got all of your requested changes in now, and I've
> attached the updated patch.   About what you said at the very end of
> your last message:
> 
>> I thought that somehow, when reading a CU that uses a DWO, we were creating
>> a "logical" DIE tree by combining the DW_TAG_skeleton_unit DIE and the
>> children of the DWO's DW_TAG_compile_unit DIE, and while doing this,
>> overwriting the DW_TAG_skeleton_unit's DIE to use the DW_TAG_compile_unit
>> tag instead.  Therefore making it appear to the rest of the DWARF reader
>> as if it was a "standard" DW_TAG_compile_unit DIE.  But no, maybe I just dreamed
>> all of this, or I can't find it anymore.
>>
> 
> Actually your first thought was absolutely correct.  This is done in
> cutu_reader::cutu_reader.  In my patched
> read.c this is at line 7244:
> 
> comp_unit_die = dwo_comp_unit_die;

Ah ok, we just the full tree from the dwo.  And in read_cutu_die_from_dwo we copy
over some useful attributes from the skeleton to the dwo's root DIE, like the ranges.

Looks like I need to read and understand something a few times before it stays :).

>> In fact, the reason the code was checking for DW_TAG_compile_unit must be that
>> in the GCC/pre-standard version, the skeleton DIE in the executable is a
>> DW_TAG_compile_unit.  With DWARF5, we'll see DW_TAG_skeleton_unit here.
>>
>> So I believe we should use
>>
>>  (tag != DW_TAG_compile_unit && tag != DW_TAG_skeleton_unit)
>>
>> to cover both versions, GCC pre-standard and DWARF 5.  Does that make sense?
> 
> I agree that we need to check both cases in cu_debug_rnglists_section,
> because sometimes it gets called before the line above in cutu_reader,
> and sometimes it gets called after (now that I'm also calling it in
> dwarf2_rnglists_process).

Ok.

>> Wherever we use the logic:
>>
>>          int need_ranges_base = (die->tag != DW_TAG_compile_unit
>>                                  && attr->form != DW_FORM_rnglistx);
>>
>> we should maybe check for DW_TAG_skeleton_unit as well?"
> 
> I don't think there's any point in checking for DW_TAG_skeleton_unit
> in the need_ranges_base checks, because I believe that all of those
> checks are called after the call to cutu_reader, so we never have a
> DW_TAG_skeleton_unit by the time we get to those checks.

Makes sense, I think.

I don't think I have any more comments.  Tom, are you ok with this?

Simon