[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