[PATCH] D75527: [yaml2obj] - Add `ELFYAML::YAMLInt` to fix how we parse a relocation `Addend` key.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 14 07:29:08 PDT 2020
grimar added a comment.
In D75527#1921825 <https://reviews.llvm.org/D75527#1921825>, @MaskRay wrote:
> > For an 64-bit object any hex/decimal addends in range [-(2^64 - 1), 2^64 - 1)] is accepted.
> > For an 64-bit object any hex/decimal addends in range [-(2^32 - 1), 2^32 - 1)] is accepted.
>
> My feeling is that the ranges should be:
> [-(2^63 - 1), 2^64 - 1)]
> [-(2^31 - 1), 2^32 - 1)]
>
> > 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.
>
>
But it does not work good for other relocations, isn't? Different relocations allows different ranges.
We are anyways not 100% correct with such approach until we don't handle each possible relocation individually.
Currently (this diff), `YAMLInt` is kind of universal type. It can be used not only for addends, but as a
type or a base for type for other keys probably.
> For example, for an ELFCLASS32 object, an addend of -0xffffffff or -0x80000001 is invalid.
I have to say about what I think about negative hex numbers again, sorry.
Negitive hex numbers in YAML is something I suggested to ban in the first version of this diff.
I can read `-0xffffffff` as `--1` or as `-4294967296` or as a `-0x00000000ffffffff` at the same time.
But why actually we want to allow users to do this and think about values that hard?
Who will use negative hex numbers in a YAML tests and for what? I think it accepts less readable
tests and probably gives no any benefits. I think we should not focus too much on them (or just ban).
Speaking about decimal version of `-0xffffffff`: I have a test in this diff:
## Addend == -(2^32 - 1)
# RUN: yaml2obj --docnum=2 %s -o %t32.decimal.min -DADDEND=-4294967295
# RUN: llvm-readobj -r %t32.decimal.min | FileCheck %s --check-prefix=TEST -DADDEND=0x1
# RUN: yaml2obj --docnum=2 %s -o %t32.hex.min -DADDEND=-0xFFFFFFFF
# RUN: llvm-readobj -r %t32.hex.min | FileCheck %s --check-prefix=TEST -DADDEND=0x1
`-0xffffffff` is accepted for 32-bits platform and reads the same as `-4294967295`, i.e. as `0x1`.
While I understand why you think that `-0xffffffff`` should be restricted in this case,
the current behavior looks acceptable for me in general.
First of all I need this patch for D75671 <https://reviews.llvm.org/D75671> which wants to do `Addend: 0xffffffffffffffff`
for example and fails:
YAML:17:17: error: invalid number
Addend: 0xffffffffffffffff
^~~~~~~~~~~~~~~~~~
yaml2obj: error: failed to parse YAML input: Invalid argument
This is a problem I am trying to solve first of all.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75527/new/
https://reviews.llvm.org/D75527
More information about the llvm-commits
mailing list