[PATCH] D143269: [DWARFLinker] Refactor cloneAddressAttribute().

Sat Feb 4 00:36:31 PST 2023

avl added a comment.

In D143269#4103095 <https://reviews.llvm.org/D143269#4103095>, @friss wrote:

> This feels like a nice generalization (and I like the shape of the new code better), but I'm wondering if it's not too general. For example a `DT_AT_location` can be `DW_FORM_addr`, and applying PCOffset to it would be wrong. I'm not sure what PCOffset is set to in a variable DIE, but it does feel somewhat wrong even if it's 0. I cannot be 100% sure, but I believe that fear of mishandling non-PC addresses was the original thinking that led to spelling out all the supported cases.

It looks like `DW_AT_location` can not have `DW_FORM_addr` attribute. DWARF spec says that `DW_AT_location` has "exprloc, loclistptr" classes, while `DW_FORM_addr` is used for "address" class. It is also said for `DW_FORM_addr`:

  • Debugging information entries may have attributes with the form
  DW_FORM_addr (see Section 7.5.4 on page 207). These attributes represent
  locations within the virtual address space of the program, and require
  relocation.
  • A DWARF expression may contain a DW_OP_addr (see Section 2.5.1.1 on
  page 26) which contains a location within the virtual address space of the
  program, and require relocation.

Thus, it looks like lt is Ok to always apply PCOffset to attribute of `DW_FORM_addr` form.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143269