[RISC-V] Do not emit cm.popret[z] with zicfiss

[nemanjai]

When emitting shadow call stack protection instructions, the push/pop optimization needs to be turned off because an sspopchk before a cm.popret[z] is guaranteed to fail in a non-leaf function. In addition, the sspopchk must be emitted after a cm.pop so that the ra has the correct value when the check is performed.

Fixes: #196261

[llvmorg-github-actions]

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

Author: Nemanja Ivanovic (nemanjai)

Changes

When emitting shadow call stack protection instructions, the push/pop optimization needs to be turned off because an sspopchk before a cm.popret[z] is guaranteed to fail in a non-leaf function. In addition, the sspopchk must be emitted after a cm.pop so that the ra has the correct value when the check is performed.

Fixes: #196261

---

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

3 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+4)
• (modified) llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp (+6)
• (added) llvm/test/CodeGen/RISCV/shadow-stack-zcmp.ll (+52)

diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 1be81ea93129a..4b683e05659fe 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -12,6 +12,7 @@
 
 #include "RISCVFrameLowering.h"
 #include "MCTargetDesc/RISCVBaseInfo.h"
+#include "MCTargetDesc/RISCVMCTargetDesc.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVSubtarget.h"
 #include "llvm/BinaryFormat/Dwarf.h"
@@ -197,6 +198,9 @@ static void emitSCSEpilogue(MachineFunction &MF, MachineBasicBlock &MBB,
 
   const RISCVInstrInfo *TII = STI.getInstrInfo();
   if (HasHWShadowStack) {
+    // The shadow call stack popchk needs to happen after cm.pop that loads ra.
+    if (MI->getOpcode() == RISCV::CM_POP || MI->getOpcode() == RISCV::QC_CM_POP)
+      MI = std::next(MI);
     BuildMI(MBB, MI, DL, TII->get(RISCV::PseudoMOP_SSPOPCHK)).addReg(RAReg);
     return;
   }
diff --git a/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp
index eae7e8697f0ad..97f7c2d387b21 100644
--- a/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp
@@ -138,6 +138,12 @@ bool RISCVPushPopOpt::runOnMachineFunction(MachineFunction &Fn) {
   if (!Subtarget->hasStdExtZcmp() && !Subtarget->hasVendorXqccmp())
     return false;
 
+  // We don't want any popret[z] instructions when emitting code with hardware
+  // shadow stack protection.
+  if (Fn.getFunction().hasFnAttribute("hw-shadow-stack") &&
+      Subtarget->hasStdExtZimop())
+    return false;
+
   TII = Subtarget->getInstrInfo();
   TRI = Subtarget->getRegisterInfo();
 
diff --git a/llvm/test/CodeGen/RISCV/shadow-stack-zcmp.ll b/llvm/test/CodeGen/RISCV/shadow-stack-zcmp.ll
new file mode 100644
index 0000000000000..75e9362dd9124
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/shadow-stack-zcmp.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s --check-prefix=RV32
+@.str = private unnamed_addr constant [13 x i8] c"Val[%d]: %d\0A\00", align 1
+
+; Function Attrs: nofree nounwind
+define dso_local noundef i32 @printSomething(ptr noundef readonly captures(none) %arr, i32 noundef returned %len) local_unnamed_addr #0 {
+; RV32-LABEL: printSomething:
+; RV32:       # %bb.0: # %entry
+; RV32-NEXT:    sspush ra
+; RV32-NEXT:    cm.push {ra, s0-s3}, -32
+; RV32-NEXT:    mv s2, a1
+; RV32-NEXT:    blez a1, .LBB0_3
+; RV32-NEXT:  # %bb.1: # %for.body.preheader
+; RV32-NEXT:    mv s1, a0
+; RV32-NEXT:    li s0, 0
+; RV32-NEXT:    lui s3, %hi(.L.str)
+; RV32-NEXT:    addi s3, s3, %lo(.L.str)
+; RV32-NEXT:  .LBB0_2: # %for.body
+; RV32-NEXT:    # =>This Inner Loop Header: Depth=1
+; RV32-NEXT:    lw a2, 0(s1)
+; RV32-NEXT:    cm.mva01s s3, s0
+; RV32-NEXT:    call printf
+; RV32-NEXT:    addi s0, s0, 1
+; RV32-NEXT:    addi s1, s1, 4
+; RV32-NEXT:    bne s2, s0, .LBB0_2
+; RV32-NEXT:  .LBB0_3: # %for.cond.cleanup
+; RV32-NEXT:    mv a0, s2
+; RV32-NEXT:    cm.pop {ra, s0-s3}, 32
+; RV32-NEXT:    mop.r.28 zero, ra
+; RV32-NEXT:    ret
+entry:
+  %cmp5 = icmp sgt i32 %len, 0
+  br i1 %cmp5, label %for.body, label %for.cond.cleanup
+
+for.cond.cleanup:                                 ; preds = %for.body, %entry
+  ret i32 %len
+
+for.body:                                         ; preds = %entry, %for.body
+  %i.06 = phi i32 [ %inc, %for.body ], [ 0, %entry ]
+  %arrayidx = getelementptr inbounds nuw [4 x i8], ptr %arr, i32 %i.06
+  %0 = load i32, ptr %arrayidx, align 4
+  %call = tail call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str, i32 noundef %i.06, i32 noundef %0)
+  %inc = add nuw nsw i32 %i.06, 1
+  %exitcond.not = icmp eq i32 %inc, %len
+  br i1 %exitcond.not, label %for.cond.cleanup, label %for.body
+}
+
+; Function Attrs: nofree nounwind
+declare dso_local noundef i32 @printf(ptr noundef readonly captures(none), ...) local_unnamed_addr #0
+
+attributes #0 = { nofree nounwind "hw-shadow-stack" "target-cpu"="generic-rv32" "target-features"="+32bit,+c,+experimental-zicfiss,+i,+zcmop,+zcmp,+zimop" }

[topperc]

++MI?

[nemanjai]

I wrote it this way for consistency as other updates all use std::next in the adjacent code. Of course, the other uses have a good reason to use it (i.e. advancing by some variable number of instructions). I'll change it to the simpler ++MI;.

[lenary]

Nice catch.

[topperc]

Remove the word "hardware" from this sentence since we also block the software case?

[topperc]

Attribute::ShadowCallStack doesn't require Zimop does it?

[lenary]

Approving because I will be afk. I do not have more comments, but please wait for @topperc to approve before merging this.