[PATCH] D88228: [yaml2obj][obj2yaml] - Add support for SHT_ARM_EXIDX section.

Fri Sep 25 01:33:56 PDT 2020

grimar added inline comments.

================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:1322
+  const auto &Obj = *static_cast<ELFYAML::Object *>(IO.getContext());
+  if (Obj.getMachine() == ELF::EM_ARM && Type == ELF::SHT_ARM_EXIDX) {
+    if (!IO.outputting())
----------------
jhenderson wrote:
> I think you should drop the `EM_ARM` requirement here. It doesn't really add anything, and makes it more restrictive on what one can use this code for. Imagine I have a downstream target that is ARM-based - I might have a different EM_* value, but still want to be able to use SHT_ARM_EXIDX, and therefore I'd want to be able to write tests using this format.
The problem is that `SHT_ARM_EXIDX` shares the value with `SHT_X86_64_UNWIND` (0x70000001U). We might have other machine specific conflicts, e.g. `SHT_ARM_ATTRIBUTES` vs `SHT_MSP430_ATTRIBUTES` vs `SHT_RISCV_ATTRIBUTES` (0x70000003U).
For your case the best solution I believe is to have a private patch to expand this condition.

================
Comment at: llvm/test/tools/obj2yaml/ELF/arm-exidx-section.yaml:24-27
+# BE-NEXT:     - Offset: 0xAABBCCDD
+# BE-NEXT:       Value:  EXIDX_CANTUNWIND
+# BE-NEXT:     - Offset: 0xEEFF8899
+# BE-NEXT:       Value:  0x01000000
----------------
jhenderson wrote:
> I might be missing something, but why are the entries in a different order here? I wouldn't expect endianness ot have any impact outside individual values.
They are not in different order. See, the input data is:

```
## 4 words: <arbitrary>, EXIDX_CANTUNWIND in big-endian,
##          <arbitrary> and EXIDX_CANTUNWIND in little-endian.
    Content: "AABBCCDD00000001EEFF889901000000"
```

We have: <AABBCCD, 00000001>, <EEFF8899, 01000000>

`0x01000000` for a little-endian object is just a value, while for big-endian object it is `EXIDX_CANTUNWIND` (0x1).
The same happens for `0x00000001`.

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

https://reviews.llvm.org/D88228