[PATCH] D80185: [xray] Add llvm-xray extract support for 32 bit ARM

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/XRay/InstrumentationMap.cpp:120
         if (SupportsRelocation && SupportsRelocation(Reloc.getType())) {
-          auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend();
-          auto A = AddendOrErr ? *AddendOrErr : 0;
+          auto A = object::ELFRelocationRef(Reloc).getOptionalAddend();
           Expected<uint64_t> ValueOrErr = Reloc.getSymbol()->getValue();
----------------
R_ARM_RELATIVE does not need this.


================
Comment at: llvm/lib/XRay/InstrumentationMap.cpp:127
         } else if (Reloc.getType() == RelativeRelocation) {
-          if (auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend())
-            Relocs.insert({Reloc.getOffset(), *AddendOrErr});
+          auto A = object::ELFRelocationRef(Reloc).getOptionalAddend();
+          Relocs.insert({Reloc.getOffset(), A});
----------------
Perhaps we can special case R_ARM_* for now. This can avoid a `getOptionalAddend` change.

I am not sure `getOptionalAddend` will have more use cases.


================
Comment at: llvm/test/tools/llvm-xray/ARM/extract-instrmap.test:73
+    EntSize:         0x0000000000000010
+  - Name:            .hash
+    Type:            SHT_HASH
----------------
Please check whether more sections can be dropped. For example, `.hash` is not read. Many tags in .dynamic are probably not needed. `xray_fn_idx` may not be needed.

Sorry for letting you do more, but I hope we can get a minimal test case which can be used as a template to improve tests for other architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80185