Add a tag to indicate whether to work around this binutils ADD/SUB/SE…

[palmer-dabbelt]

…T bug

See https://sourceware.org/bugzilla/show_bug.cgi?id=31179 for slightly more info.

[MaskRay]

This R_RISCV_SUB* addend bug (fixed now) is certainly unfortunate but it is not severe. It affects .reloc directives using R_RISCV_SUB*, which are extremely uncommon (never encountered if I did not write assembler/linker tests).

If an object is subject to this gas bug, what can the toolchain do? I believe it's a non-starter for other toolchain components (LLVM) to detect the bug. We cannot retrospectively add a tag to the relocatable file with the bug. It's also not feasible to force new relocatable files without the bug to add a tag.

The R_RISCV_SUB_ULEB128 bug "RISC-V: Clarify the behaviors of SET/ADD/SUB relocations." fixed is worse as it affects .uleb128 (A + 3) - (B + 2) (not just .reloc). However, such subtraction expressions are relatively uncommon as well and ULEB128 relocations are very new.

Personally I think we should just treat the gas bug as a defect and live with it. Usually we'd not run into a problem at all. If we use advanced assembly tricks (primarily ULEB128), we should be aware of the binutils<2.41 problem.

Moving forward, I wish that binutils has better tests :) The gas bug fix doesn't come with a test, so a future regression cannot be detected. Perhaps RISC-V folks may have more bandwidth to improve the status quo (see also my feature request binutils/testsuite/lib/binutils-common.exp: Support free-form shell commands and check patterns to solve some pain so that gas contributors will become more willing to add tests).

[Nelson1225]

This R_RISCV_SUB addend bug (fixed now) is certainly unfortunate but it is not severe. It affects .reloc directives using R_RISCV_SUB, which are extremely uncommon (never encountered if I did not write assembler/linker tests).

~If this is rare, then I think I can have a workaround/hack in GNU assembler to make .reloc ADD/SET/SUB/ULEB128 friendly with the 2.41 linker behavior.~

[rui314]

I'm not sure if making a modification to the spec is the best way to handle the situation, but this proposal, as it stands, is incomplete because it does not specify anything other than defining a new tag.

What should the linker do if the tag is present in an object file? I think the spec needs to address that question.

[kito-cheng]

Has short chart with Nelson about the bug and this tag, and we both don't think the tag resolve the problem unless all other toolchain also always emit that tag as well.

Otherwise object file produced by LLVM (and all other toolchain) will handle incorrectly with newer binutils, because that object has no tag, and then it will use legacy way to handle, which is wrong...

Update: Let me sort out this issue again, it little mess to me

[kito-cheng]

More detail here:

Use this uleb128 directive as example:

.uleb128 bar - foo + 1

The assembler should emit R_RISCV_SET_ULEB128 bar + 1 and R_RISCV_SUB_ULEB128 foo + 0, but binutils 2.41 has emit R_RISCV_SET_ULEB128 bar + 1 and R_RISCV_SUB_ULEB128 foo + 1, it will become (bar + 1) - (foo + 1) which is bar - foo, but why it still work? because ld.bfd will ignore the Addend of R_RISCV_SUB_ULEB128.

So it will become problem once new binutils (2.42, not release yet) link the object files which produced by binutils (2.41), because that will evaluate the (bar + 1) - (foo + 1) right in above example.

And lld is implement that right, so it will got problem when it link object came from bintuils 2.41.

[Nelson1225]

because ld.bfd will ignore the Addend of R_RISCV_SUB_ULEB128

Should be ignore the addend of SET_ULEB128, so need this patch to fix the problem, and let ld.bfd ignore the addend of SUB_ULEB128 in binutils 2.42 to workaround it.

But after applying this patch, the behavior of .reloc *SUB* with addend will fail like before, since the commit was used to fix the .reloc *SUB* behaviors in bfd.ld (V - S + A) to the behavior that mentioned in psabi (V - S - A), but it causes uleb128 problem in this pr.

The ld.bfd always calculate (V - S + A) for SUB relocation, so this looks wrong and conflict with psabi. Maybe we should limit people cannot use the .reloc *SUB* with non-zero addend in psabi or some where? Or we just wait people rebuild their stuff to get the zero-addend of SUB_ULEB128 from .uleb128 directives, and then fix the behavior of bfd.ld to (V -S - A) for SUB relocations maybe in binutils 2.43 or 2.44 release.

[jrtc27]

We can’t go adding tags to document historic bugs. If binutils is broken then that’s pretty sad and I have questions about the testing methodology (and review process) in use, but that’s for the binutils maintainers to deal with like any other bug.

[kito-cheng]

Discussed in last psABI meeting, and we reach consensus to close this PR since we believe this tag isn't help that issue too much.