[PATCH 07/14] Add dwarf2_per_cu_data::index

Tom Tromey tom@tromey.com
Fri Feb 21 23:36:00 GMT 2020


>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

>> static dwarf2_per_cu_data *
>> create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
>> -                          struct dwarf2_section_info *section,
>> -                          int is_dwz,
>> -                          sect_offset sect_off, ULONGEST length)
>> +			   struct dwarf2_section_info *section,
>> +			   int is_dwz,
>> +			   sect_offset sect_off, ULONGEST length)

Luis> Did only formatting change above?

Yeah.  I backed this bit out.

>> +  /* A convenience function to allocate a dwarf2_per_cu_data (or
>> +     object holding one, using C-style derivation).  The returned
>> +     object has its "index" field set properly.  The object is
>> +     allocated on the dwarf2_per_objfile obstack.  FIXME the index
>> +     here is weird since it may not (!?!?) be the same as the other
>> +     index  */

Luis> I'm a bit lost on the above comment. Instead of having a FIXME here,
Luis> is there something we can do better at this point? Or maybe it will be 
Luis> addressed later in the series?

Luis> I couldn't quite understand the problems with the indexes, but you're
Luis> more familiar with the DWARF reading code than i am.

I updated the comment, and added a comment by allocate_signatured_type.
Part of that comment referred to some earlier code that also tried to
unify signatured_type and type_unit_group into this allocation function.

The FIXME was more of a note to myself about whether this index could
somehow be reused, because the reader sometimes uses an index to look in
all_comp_units and all_type_units.

I think it turns out they are equivalent, but it seems like it doesn't
matter, because there's no need to use this value to index into the
all_type_units / all_comp_units vectors.

>> +  /* Our index in the unshared "all_cutus" vector.  */
>> +  unsigned index;

Luis> What is the all_cutus vector? I don't see it being added later in the
Luis> series. Should we make it more clear? Maybe you meant "all CU's/TU's"?

I changed it to "symtabs" as Simon pointed out down-thread.

Tom



More information about the Gdb-patches mailing list