[PATCH] D154638: Emit a .debug_addr section with dsymutil

[Shubham Sandeep Rastogi via Phabricator via llvm-commits]

rastogishubham added inline comments.


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:1393-1405
   Die.addValue(DIEAlloc, static_cast<dwarf::Attribute>(AttrSpec.Attr),
-               AttrSpec.Form, DIEInteger(*Addr));
-  return Unit.getOrigUnit().getAddressByteSize();
+               AttrSpec.Form, DIEInteger(AddrPool.getAddrIndex(*Addr)));
+
+  assert((AttrSpec.Form == dwarf::DW_FORM_addrx) ||
+         (AttrSpec.Form == dwarf::DW_FORM_addrx1 &&
+          AddrPool.getAddrIndex(*Addr) <= UINT8_MAX) ||
+         (AttrSpec.Form == dwarf::DW_FORM_addrx2 &&
----------------
JDevlieghere wrote:
> Let's extract `AddrPool.getAddrIndex(*Addr)` and assign it to a variable that you can reuse in the assert. Also, is the assert actually checking internal consistency or catching invalid DWARF? If it's the latter we should emit an error/warning instead. 
It is checking internal consistency.


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:2011
+
+  llvm_unreachable("Didn't find DW_AT_addr_base in cloned DIE!");
+}
----------------
JDevlieghere wrote:
> Similar question as for the assert: is this checking internal consistency (i.e. that we have created a `DW_AT_addr_base` in the cloned DIE) or can this trip for invalid input?
This one can trip for invalid input, if the input dwarf doesn't have an addr base, then the cloned die would not either.


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

https://reviews.llvm.org/D154638