vsk added inline comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:431
const size_t num_ranges =
- die->GetAttributeAddressRanges(dwarf, this, ranges, false);
+ die->GetAttributeAddressRanges(dwarf, this, ranges, check_hi_lo_pc);
if (num_ranges > 0) {
----------------
sgraenitz wrote:
> clayborg wrote:
> > sgraenitz wrote:
> > > @JDevlieghere Thanks for your feedback!
> > >
> > > @everyone:
> > > This parameter is, actually, one of the key questions in this review: If
> > > there is no DW_AT_range attribute for the compile unit, can we safely
> > > fall back to DW_AT_low/high_pc?
> > I have seen DWARF where the DW_AT_ranges is incorrect and I have seen cases
> > where the low pc is just set to zero and the high pc is set to the max
> > address. So you might have many overlapping ranges between different CUs in
> > the latter case. We might need to be more vigilant with high/low pc values
> > though and that was the reason that we previously ignored high/low pc
> > values. If the stuff is missing, it should index the DWARF and generate the
> > address ranges table manually right? What was the reasoning behind this
> > change?
> I had a closer look at the individual fallbacks and you are right. After
> fixing `SymbolFileDWARF::ParseCompileUnit()` we have the correct
> `CompileUnit` pointers in `DWARFUnit::m_user_data` for all CUs. Thus, the
> below `die->BuildAddressRangeTable(dwarf, this, debug_aranges);` now
> correctly constructs the ranges! Great, with this we can keep
> `check_hi_lo_pc=false`.
>
> I didn't notice this while testing, because of the following issue that kept
> my tests failing: 2 of my LTO object's CUs have no code remaining after
> optimization, so we step into the next fallback assuming 'a line tables only
> situation' and eventually call `AddOSOARanges()`, which adds to
> `debug_aranges` all entries of the CU's FileRangeMap with a zero offset.
>
> Thus, my `debug_aranges` had 28000 entries too much and most requests would
> continue to return `0` as before. For now I commented out the call below.
>
> **What do you think is a good way to avoid this for empty CUs?
> IIUC they should have no DW_AT_low/high_pc right? :-D**
+1 to @clayborg's suggestion here that we either use AT_ranges, or build up the
ranges by parsing the low/high pc's in subprograms. ISTM that it'd be
acceptable to implement the fallback path as a follow-up, provided that doing
so isn't a regression.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:489
+ } //else
+ //debug_map_sym_file->AddOSOARanges(dwarf, debug_aranges);
}
----------------
Could you leave an in-source comment explaining why this is commented out?
https://reviews.llvm.org/D52375
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
