Missing Linker Relaxation Relocations after Assembly Relaxation

[lenary]

The following example, when assembled with llvm-mc --triple=riscv32 -mattr=+relax,+experimental-xqcilb seems to have a bug:

.global foo

bar:
  jal x1, foo
  bne a0, a1, bar
  ret

Looking at the llvm-objdump -dr output (this has not been truncated)

jal.o:  file format elf32-littleriscv

Disassembly of section .text:

00000000 <bar>:
       0: c01f 0000 0000        qc.e.jal        0x0 <bar>
                        00000000:  R_RISCV_VENDOR       QUALCOMM
                        00000000:  R_RISCV_CUSTOM195    foo
                        00000000:  R_RISCV_RELAX        *ABS*
       6: feb51de3      bne     a0, a1, 0x0 <bar>
       a: 8082          ret

The bne is branching over a relaxable instruction, but has been resolved instead of having a relocation on it. This means that if the linker relaxes the qc.e.jal instruction, the bne will be incorrect.

I think the sequence of what is happening is:

• jal x1, foo is parsed as JAL, as would be expected.
• JAL is compressed to C_JAL in RISCVAsmParser::emitToStreamer
• C_JAL is emitted, with a RISCV::fixup_riscv_rvc_jump fixup, which is not marked as linker relaxable.
• C_JAL is jumping to an undefined symbol, so will be changed with relaxInstruction
• relaxInstruction first relaxes to JAL and then the second time to QC_E_JAL
• QC_E_JAL has a RISCV::fixup_riscv_qc_e_call_plt fixup, which is marked as linker relaxable!
• nothing now marks the fragment and the section as linker relaxable.
• When it comes time to apply the fixup to bne, I think RISCVAsmBackend::isPCRelFixupResolved is returning that the fixup is fully resolved, when it shouldn't be.

I think the relaxation of JAL to QC_E_JAL might be the only one we have right now which changes the fixup from being non-linker-relaxable to being linker-relaxable. (This is not going to be the case indefinitely, as there are now linker relaxations for R_RISCV_JAL, so the C_JAL to JAL relaxation will have the same bug once we update RISCVMCCodeEmitter::getImmOpValue for new relaxations in the psABI since we did this last time)

I think this is still violating invariants of the code, that fragments (and their parent sections) with linker relaxable fixups should be marked as linker relaxable.

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Sam Elliott (lenary)

The following example, when assembled with `llvm-mc --triple=riscv32 -mattr=+relax,+experimental-xqcilb` seems to have a bug:

.global foo

bar:
  jal x1, foo
  bne a0, a1, bar
  ret

Looking at the llvm-objdump -dr output (this has not been truncated)

jal.o:  file format elf32-littleriscv

Disassembly of section .text:

00000000 <bar>:
       0: c01f 0000 0000        qc.e.jal        0x0 <bar>
                        00000000:  R_RISCV_VENDOR       QUALCOMM
                        00000000:  R_RISCV_CUSTOM195    foo
                        00000000:  R_RISCV_RELAX        *ABS*
       6: feb51de3      bne     a0, a1, 0x0 <bar>
       a: 8082          ret

The bne is branching over a relaxable instruction, but has been resolved instead of having a relocation on it. This means that if the linker relaxes the qc.e.jal instruction, the bne will be incorrect.

I think the sequence of what is happening is:

• jal x1, foo is parsed as JAL, as would be expected.
• JAL is compressed to C_JAL in RISCVAsmParser::emitToStreamer
• C_JAL is emitted, with a RISCV::fixup_riscv_rvc_jump fixup, which is not marked as linker relaxable.
• C_JAL is jumping to an undefined symbol, so will be changed with relaxInstruction
• relaxInstruction first relaxes to JAL and then the second time to QC_E_JAL
• QC_E_JAL has a RISCV::fixup_riscv_qc_e_call_plt fixup, which is marked as linker relaxable!
• nothing now marks the fragment and the section as linker relaxable.
• When it comes time to apply the fixup to bne, I think RISCVAsmBackend::isPCRelFixupResolved is returning that the fixup is fully resolved, when it shouldn't be.

I think the relaxation of JAL to QC_E_JAL might be the only one we have right now which changes the fixup from being non-linker-relaxable to being linker-relaxable. (This is not going to be the case indefinitely, as there are now linker relaxations for R_RISCV_JAL, so the C_JAL to JAL relaxation will have the same bug once we update RISCVMCCodeEmitter::getImmOpValue for new relaxations in the psABI since we did this last time)

I think this is still violating invariants of the code, that fragments (and their parent sections) with linker relaxable fixups should be marked as linker relaxable.

[lenary]

This is the relevant debug output from the assembly above, on main:

assembler backend - pre-layout
--
Sections:[
MCSection Name:.text
0 Align Size:0+0 []
  Align:2 Fill:0 FillLen:1 MaxBytesToEmit:2 Nops
  Symbol @0 .text
0 Relaxable Size:0+2 [01,20] <MCInst #12621 <MCOperand Expr:foo>>
  Fixup @0 Value:foo Kind:4019
  Symbol @0 bar
  Symbol @0 $x
0 Relaxable Size:0+4 [63,10,b5,00] <MCInst #12252 <MCOperand Reg:70> <MCOperand Reg:71> <MCOperand Expr:bar>>
  Fixup @0 Value:bar Kind:4018
0 Data Size:2 [82,80]
]
assembler backend - final-layout
--
Sections:[
MCSection Name:.text
0 Align Size:0+0 []
  Align:2 Fill:0 FillLen:1 MaxBytesToEmit:2 Nops
  Symbol @0 .text
0 Relaxable Size:0+6 [1f,c0,00,00,00,00] <MCInst #13231 <MCOperand Expr:foo>>
  Fixup @0 Value:foo Kind:4027
  Symbol @0 bar
  Symbol @0 $x
6 Relaxable Size:0+4 [63,10,b5,00] <MCInst #12252 <MCOperand Reg:70> <MCOperand Reg:71> <MCOperand Expr:bar>>
  Fixup @0 Value:bar Kind:4018
10 Data Size:2 [82,80]
]

I found it helpful to print LinkerRelaxable for both sections and fixups, in addition to fragments, but that's just debug output. Fixup @0 Value:foo Kind:4027 is Linker Relaxable.

---

I tried modifying MCAssembler::relaxInstruction to mark F as linker relaxable if Fixups contained a linker relaxable fixup, but that didn't seem to work.

I think this might be because attemptToFoldSymbolOffsetDifference, the code around BBeforeRelax and AAfterRelax seems to only expect data fragments to be linker relaxable, whereas this is a linker-relaxable relaxable fragment, but I'm not sure.

[MaskRay]

The LinkerRelaxable property should be set exclusively by MCObjectStreamer and remain unchanged. Modifying it during relaxation is inappropriate. For example, MCObjectStreamer::emitDwarfAdvanceLineAddr absoluteSymbolDiff relies on LinkerRelaxable being correct at streaming time.

At MCObjectStreamer time, jal x1, foo should be marked with linker-relaxable.

[lenary]

At MCObjectStreamer time, jal x1, foo should be marked with linker-relaxable.

The problem is that what is passed to the streamer is c.jal foo, which is not linker-relaxable, and I'm not sure the abi allows you to emit R_RISCV_RELAX on the R_RISCV_RVC_JUMP relaxation.

[lenary]

I don't quite understand the issue with absoluteSymbolDiff - LoF->getKind() will be FT_Relaxable, so the function will return std::nullopt, right?

This was an analysis from an old version of the code, I'm juggling too many branches right now.