Bug 2216906

@thuth , Would you have a link on how this code is being used in the wild, please? Including compiler flags.

While investigating this issue, I found out that LLVM_UNREACHABLE_OPTIMIZE is enabled (default behavior).
That explains why clang is crashing without providing a better error message.

(In reply to Tulio Magno Quites Machado Filho from comment #1)
> @thuth , Would you have a link on how this code is being used in
> the wild, please? Including compiler flags.

I'm not aware of any real occurrences in the wild - I just came around this crash while I tried to find a solution for the problem that we hit in https://bugzilla.redhat.com/show_bug.cgi?id=2216662

tl;dr; I believe there are 3 independent bugs in this bug report:

1. I believe this asm construction is invalid in s390x, but it's exposing the following 2 issues.
2. GNU AS is trying to workaround limitations in ELF by computing offsets of defined symbols, but this may lead to errors later.
3. Clang assembler is trying to workaround the same limitations by computing PC-relative offsets, which isn't available for this kind of asm construction on s390x. It crashes instead of printing an error message.

Rationale
---------

AFAIU, ELF is not able to compute a relocation of the type: SymbolA - SymbolB + ConstantC. However, it can do SymbolA - ConstantK.
If both symbols were defined in the same object file, this wouldn't be an issue because their offsets are known at compilation time.
But one of the symbols is undefined.

GNU AS appears to work around this limitation by doing the following transformation:

   ConstantK = OffsetB + ConstantC

While this works on some scenarios, it isn't correct for all the cases.

The Clang assembler tries to work around the same limitation by using PC-relative relocation. AFAIU, this is guaranteed to work, but it isn't available for the kind of asm construction used here.

One way to fix this for non-pic code is to load the addresses separately using PC-relative loads, i.e. https://godbolt.org/z/hr758PK1j

Let me ask IBM to review this comment. They will be able to correct me.

I will drive changes to clang in order to provide a proper error output.

------- Comment From Andreas.Krebbel.com 2023-06-29 02:21 EDT-------
I think the difference you observed for your example between GCC and LLVM comes from the section anchors optimization in GCC we have enabled a while back for s390x. With section anchors GCC tries to address all local symbols using a section anchor to make better use of memory operands which we have in many instructions. You can turn that optimization off with -fno-section-anchors. Then both GCC and LLVM generate exactly the same code for your example.

To me both variants look ok though. If the extern symbol b comes from another .o file it is in fact local in the final link step and addressing it with larl is ok. If it comes from a shared library a copy relocation will be generated what also makes it addressable from the main binary with larl. If you build with -fPIC both compilers fetch the symbol addresses from the GOT and subtract it.

------- Comment From Andreas.Krebbel.com 2023-06-29 03:19 EDT-------
wrt "la %r2,_end-start(%r2)"

this is only legal if both symbols are defined in the same .s file so that the computation can be done by the assembler. Otherwise there would be no way to express this in a .o file. I'm not aware of any workaround gas is applying in that case. I would expect it just responses with "can't resolve". Not sure why this doesn't happen in that case though.

------- Comment From Andreas.Krebbel.com 2023-06-29 03:59 EDT-------
Tulio, please ignore my last two comments. I went into an utterly wrong direction. I should have read more carefully first.

You are right that GNU as appears to apply some questionable magic here instead of responding with "can't resolve" as it does for other cases I've tested.

For:

start:
.quad 4
la %r2,end-start(%r2)

gas tries to insert the address of end with an addend of 8 (which apparently is determined by the ".quad 4") as a 12 bit reloc

Relocation section '.rela.text' at offset 0xf0 contains 1 entry:
Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000a  0000000500000002 R_390_12               0000000000000000 end + 8

gas only does that if start is defined in the code section. I need to have a closer look what's going on and whether it can be made to work in some cases but issuing an error instead is probably the right thing.

Thanks for looking into it and sorry for the confusion.

------- Comment From Ulrich.Weigand.com 2023-06-29 07:00 EDT-------
(In reply to comment #8)
> For:
>
> start:
>         .quad 4
>         la %r2,end-start(%r2)
>
> gas tries to insert the address of end with an addend of 8 (which apparently
> is determined by the ".quad 4") as a 12 bit reloc

I believe what's going on here is that GAS recognized that "start" is in the .text section and can therefore expressed relative to the PC:  start = PC - 8.

It then correctly transforms "end-start" into "end+8 - PC", which *could* be expressed as a PC-relative relocation --- if we *had* a 12-bit PC-relative reloc (something like "R_390_PC12").

Since we don't actually have this reloc, we cannot do this transformation here.  Both GAS and LLVM don't handle this correctly, but the details of the bug are different: GAS seems to simply ignore the PC-relative requirement and just uses the absolute relocation instead, while LLVM simply crashes because the relocation is not available ...

------- Comment From Ulrich.Weigand.com 2023-06-30 10:07 EDT-------
So it turned out LLVM already checked for these scenarios, but it would simply throw an assertion (llvm_unreachable).  Seems like in the RPM release builds, those assert messages don't even show up, so it just looks like a crash ...

In any case, as this can be triggered by invalid user input, it should never result in an assertion, but rather a regular compiler error message.

I've now fixes this, checked in as commit efbaf8bc61f4c0e29a3eaafb11ac0ddda8bd3dff.

In the test case, this now results in:

test.s:5:9: error: Unsupported PC-relative address
la %r2,end-start(%r2)
^

Thank you!
Let me backport this.

------- Comment From Andreas.Krebbel.com 2023-07-03 14:14 EDT-------
I've committed a Binutils fix to address this:
https://sourceware.org/pipermail/binutils/2023-July/128252.html

I created merge requests for the LLVM fixed on c9s: https://gitlab.com/redhat/centos-stream/rpms/llvm/-/merge_requests/45

And on Rawhide: https://src.fedoraproject.org/rpms/llvm/pull-request/174

Tested with llvm-16.0.6-1.el9. When trying to assemble the code provided in the bug description clang reports the unsupported PC-relative relocation as expected.

++ mktemp -d
+ cd /tmp/tmp.h9xqb2nHNl
+ echo 'start: la %r2,_end-start(%r2)'
+ clang -c -o test.o test.S
+ tee clang.output
/tmp/test-1836fc.s:4:8: error: Unsupported PC-relative address
start: la %r2,_end-start(%r2)
       ^
+ grep -q 'Unsupported PC-relative address' clang.output

Testcase: https://src.fedoraproject.org/tests/llvm/blob/main/f/s390x-asm-unsupported-pc-relative-addr

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (python-lit, lldb, lld, libomp, compiler-rt, clang, and llvm update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:6360