[PATCH] D74612: [DebugInfo] Read CIE pointer as a relocatable value.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/DebugInfo/debug-frame-cieid-reloc.test:1
+# RUN: yaml2obj %s -o - | \
+# RUN:   llvm-dwarfdump -debug-frame - | \
----------------
As this is testing the CIE_pointer field of FDEs, it's porbably best to rename the test to debug-frame-cie-pointer-reloc.test.


================
Comment at: llvm/test/DebugInfo/debug-frame-cieid-reloc.test:7
+## The second FDE references the second CIE. The value for the CIE pointer
+## field in the raw section data is 0, thus, to recover the real reference
+## it is required to read the addend from the corresponding RELA relocation.
----------------
Add a comma after "reference"


================
Comment at: llvm/test/DebugInfo/debug-frame-cieid-reloc.test:18-21
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
----------------
I'm guessing you used obj2yaml to generate this file? It generates excessive whitespace between keys and values, which I find can make it harder to read, and would prefer be removed to make something like this:

```
FileHeader:
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_REL
  Machine: EM_X86_64
```

Same goes further below.


================
Comment at: llvm/test/DebugInfo/debug-frame-cieid-reloc.test:25
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         0C000000FFFFFFFF02000178100000001400000000000000000000000000000001000000000000000C000000FFFFFFFF0200017810000000140000000000000010000000000000000100000000000000
----------------
No need for the AddressAlign field. Delete it.


================
Comment at: llvm/test/DebugInfo/debug-frame-cieid-reloc.test:29-31
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    EntSize:         0x0000000000000018
----------------
No need for these three fields.


================
Comment at: llvm/test/DebugInfo/debug-frame-cieid-reloc.test:40
+        Type:            R_X86_64_32
+        Addend:          40
+Symbols:
----------------
Does `Addend` allow a hex number? If so, I feel like 0x28 would be clearer, as it matches the value in the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74612