[RISCV] Don't delete all fixups in RISCVMCCodeEmitter::expandLongCondBr.

[topperc]

The Fixups vector passed into this function may already have fixups in it from earlier instructions. We should not erase those. We just want to erase fixups added by this function.

Fixes #108612.

[llvmbot]

@llvm/pr-subscribers-mc

Author: Craig Topper (topperc)

Changes

The Fixups vector passed into this function may already have fixups in it from earlier instructions. We should not erase those. We just want to erase fixups added by this function.

Fixes #108612.

---

Full diff: https://github.com/llvm/llvm-project/pull/109513.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp (+6-1)
• (added) llvm/test/MC/RISCV/long-conditional-jump-crash.s (+19)

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index 75323632dd5333..66394dc8cd138b 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -283,13 +283,18 @@ void RISCVMCCodeEmitter::expandLongCondBr(const MCInst &MI,
     Offset = 4;
   }
 
+  // Save the number fixups.
+  size_t N = Fixups.size();
+
   // Emit an unconditional jump to the destination.
   MCInst TmpInst =
       MCInstBuilder(RISCV::JAL).addReg(RISCV::X0).addOperand(SrcSymbol);
   uint32_t Binary = getBinaryCodeForInstr(TmpInst, Fixups, STI);
   support::endian::write(CB, Binary, llvm::endianness::little);
 
-  Fixups.clear();
+  // Drop any fixup added so we can add the correct one.
+  Fixups.resize(N);
+
   if (SrcSymbol.isExpr()) {
     Fixups.push_back(MCFixup::create(Offset, SrcSymbol.getExpr(),
                                      MCFixupKind(RISCV::fixup_riscv_jal),
diff --git a/llvm/test/MC/RISCV/long-conditional-jump-crash.s b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
new file mode 100644
index 00000000000000..bac0036ca5568f
--- /dev/null
+++ b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
@@ -0,0 +1,19 @@
+# RUN: llvm-mc %s -mc-relax-all -triple=riscv64 -filetype=obj \
+# RUN:     | llvm-objdump -d -M no-aliases - \
+# RUN:     | FileCheck --check-prefix=CHECK %s
+
+# This test previously crashed because expanding a conditional branch deleted
+# all fixups in the fragment.
+
+# CHECK:      beq     s0, zero, 0x8
+# CHECK-NEXT: jal     zero, 0x14
+# CHECK-NEXT: jal     zero, 0x14
+# CHECK-NEXT: bne     s0, zero, 0x14
+# CHECK-NEXT: jal     zero, 0x14
+
+# CHECK:      jalr    zero, 0x0(ra)
+  bnez s0, .foo
+  j    .foo
+  beqz s0, .foo
+.foo:
+  ret

[llvmbot]

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

Author: Craig Topper (topperc)

Changes

The Fixups vector passed into this function may already have fixups in it from earlier instructions. We should not erase those. We just want to erase fixups added by this function.

Fixes #108612.

---

Full diff: https://github.com/llvm/llvm-project/pull/109513.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp (+6-1)
• (added) llvm/test/MC/RISCV/long-conditional-jump-crash.s (+19)

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index 75323632dd5333..66394dc8cd138b 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -283,13 +283,18 @@ void RISCVMCCodeEmitter::expandLongCondBr(const MCInst &MI,
     Offset = 4;
   }
 
+  // Save the number fixups.
+  size_t N = Fixups.size();
+
   // Emit an unconditional jump to the destination.
   MCInst TmpInst =
       MCInstBuilder(RISCV::JAL).addReg(RISCV::X0).addOperand(SrcSymbol);
   uint32_t Binary = getBinaryCodeForInstr(TmpInst, Fixups, STI);
   support::endian::write(CB, Binary, llvm::endianness::little);
 
-  Fixups.clear();
+  // Drop any fixup added so we can add the correct one.
+  Fixups.resize(N);
+
   if (SrcSymbol.isExpr()) {
     Fixups.push_back(MCFixup::create(Offset, SrcSymbol.getExpr(),
                                      MCFixupKind(RISCV::fixup_riscv_jal),
diff --git a/llvm/test/MC/RISCV/long-conditional-jump-crash.s b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
new file mode 100644
index 00000000000000..bac0036ca5568f
--- /dev/null
+++ b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
@@ -0,0 +1,19 @@
+# RUN: llvm-mc %s -mc-relax-all -triple=riscv64 -filetype=obj \
+# RUN:     | llvm-objdump -d -M no-aliases - \
+# RUN:     | FileCheck --check-prefix=CHECK %s
+
+# This test previously crashed because expanding a conditional branch deleted
+# all fixups in the fragment.
+
+# CHECK:      beq     s0, zero, 0x8
+# CHECK-NEXT: jal     zero, 0x14
+# CHECK-NEXT: jal     zero, 0x14
+# CHECK-NEXT: bne     s0, zero, 0x14
+# CHECK-NEXT: jal     zero, 0x14
+
+# CHECK:      jalr    zero, 0x0(ra)
+  bnez s0, .foo
+  j    .foo
+  beqz s0, .foo
+.foo:
+  ret

[MaskRay]

Perhaps FixupStartIndex to be similar to MCELFStreamer.cpp:550

[MaskRay]

llvm/test/MC/RISCV/rv64-relax-all.s tests -mc-relax-all. Add the test there, perhaps rename the file?