[PATCH] D75527: [yaml2obj] - Add `ELFYAML::YAMLInt` to fix how we parse a relocation `Addend` key.

Wed Mar 4 00:39:07 PST 2020

grimar marked an inline comment as done.
grimar added a comment.

In D75527#1903438 <https://reviews.llvm.org/D75527#1903438>, @MaskRay wrote:

> It should probably accept the union of int32_t and uint32_t, i.e. [-2147483648, 4294967295]. Examples: R_AARCH64_ABS32, R_AARCH64_PREL32, R_PPC64_ADDR32.

I think your suggestion is fine. I'll go this way if there will be no objections.

But what should we print (obj2yaml)? We print int64_t decimal currently.
Should we switch to printing a hex form then?

================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:998
+    // Do not allow "-0xFE". It looks strange.
+    if (Scalar.drop_front().startswith("0x"))
+      return ErrMsg;
----------------
MaskRay wrote:
> Not very necessary, I think.
This syntax is just strange.
Imagine you do `Addend: 0xFFFFFFFF`, i.e. you mean `-1`.
If you write `Addend: -0xFFFFFFFF`, the result will be `1`, or `0x00000001` in hex. The hex sequence is
very different because of that minus. I think it is a bad practice to use `-0xFFFFFFFF` form for `Addend` value
or any other key probably.

We either need a test case for this, or to restrict it explicitly probably.
(Doesn't seem we can just ignore it? And I do not think we want to support it).
I selected to restrict it.

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

https://reviews.llvm.org/D75527