[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:2011
+
+  llvm_unreachable("Didn't find DW_AT_addr_base in cloned DIE!");
+}
----------------
avl wrote:
> rastogishubham wrote:
> > 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.
> This is also should be check for internal consistency: We should not miss DW_AT_addr_base when we generate .debug_addr. The patchAddrBase is called from DIECloner::emitDebugAddrSection, thus we check for DW_AT_addr_base exactly when we are going to generate .debug_addr. The only problem with current implementation is this check will fail when original DWARF does not have .debug_addr but we call emitDebugAddrSection for generation of empty .debug_addr. The check for empty addresses set needs to be added into emitDebugAddrSection.
> 
> For the case of invalid DWARF(DW_AT_addr_base missed in input file) - we should not generate any addresses as we could not correctly read original addresses. DWARFLinker should report warning here and do not generate any address:
> 
> DIECloner::cloneAddressAttribute
> ```
>   std::optional<uint64_t> Addr = AddrAttribute->getAsAddress();
>   if (!Addr) {
>     Linker.reportWarning("Cann't read address attribute value.", ObjFile);
>     Addr = 0;
>   }
> 
> ```
> 
> Though, this place looks a bit incorrect. Instead of using zero Addr, the attribute should probably be skipped.
DW_AT_addr_base is of form DW_FORM_sec_offset, so it will call `DWARFLinker::DIECloner::cloneScalarAttribute(`, are you suggesting that I add a warning there if there is no addr_base found?


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

https://reviews.llvm.org/D154638