grimar added inline comments.

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:351
+  if (!addr_base)
+    addr_base =
+       cu_die.GetAttributeValueAsUnsigned(m_dwarf, this, DW_AT_GNU_addr_base, 
0);
----------------
jankratochvil wrote:
> Here I would find good also a comment:
> ``` // pre-DWARF v5 attributes DW_AT_GNU_* applied only to the DWO unit while 
> DWARF v5 attributes DW_AT_* apply also to the main unit
> ```
> Based on a [[ https://gcc.gnu.org/wiki/DebugFission | DebugFission ]] comment:
> ```Note the following difference between the current GCC implementation and 
> the DWARF v5 specification: In the current GCC implementation (based on DWARF 
> v4), if DW_AT_ranges is present, the offset into the ranges table is not 
> relative to the value given by DW_AT_ranges_base (i.e., DW_AT_ranges_base is 
> used only for references to the range table from the dwo sections). In DWARF 
> v5, the DW_AT_ranges_base attribute is used for all references to the range 
> table -- both from dwo sections and from skeleton compile units.
> ```
> 
> I was also thinking to fetch the attributes just once by some `(m_version >= 
> 5 ? DW_AT_addr_base : DW_AT_GNU_addr_base)` but clang-7.0 does produce 
> DWARF-5 still using `DW_AT_GNU_addr_base`.
> 
I rewrote the comment.

> I was also thinking to fetch the attributes just once by some (m_version >= 5 
> ? DW_AT_addr_base : DW_AT_GNU_addr_base) but clang-7.0 does produce DWARF-5 
> still using DW_AT_GNU_addr_base.

Yeah, it would not be safe to do such check I think. But it should not be an 
issue, in practice, I believe it should be fine to check both the attributes in 
order.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54751/new/

https://reviews.llvm.org/D54751



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to