Error when we overwrite data on the stack

LucasSte · Jul 3, 2024 · fa8865c · fa8865c
1 parent 8bc30e8
commit fa8865c
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 20 deletions.
diff --git a/llvm/lib/Target/SBF/SBFISelLowering.cpp b/llvm/lib/Target/SBF/SBFISelLowering.cpp
@@ -625,37 +625,39 @@ SDValue SBFTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
       EVT PtrVT = DAG.getTargetLoweringInfo().getPointerTy(DAG.getDataLayout());
       SDValue DstAddr;
       MachinePointerInfo DstInfo;
+      int FrameIndex;
+      int64_t Offset;
       if (Subtarget->getHasDynamicFrames()) {
         // In the new call convention, arguments are stored in the callee frame
         // The positive offset signals that the variable does not occupy space
         // in the caller frame.
-        int64_t Offset = static_cast<int64_t>(VA.getLocMemOffset() +
-                                              PtrVT.getFixedSizeInBits() / 8);
+        Offset = static_cast<int64_t>(VA.getLocMemOffset() +
+                                      PtrVT.getFixedSizeInBits() / 8);
         if (!Subtarget->getEnableNewCallConvention())
           // In the old call convention, we place argument at the start of the
           // frame in a fixed stack offset.
           Offset = -Offset;
 
-        int FrameIndex = MF.getFrameInfo().CreateFixedObject(
+        FrameIndex = MF.getFrameInfo().CreateFixedObject(
             VA.getLocVT().getFixedSizeInBits() / 8, Offset, false);
-        SBFFuncInfo->storeFrameIndexArgument(FrameIndex);
-        DstAddr = DAG.getFrameIndex(FrameIndex, PtrVT);
-        DstInfo = MachinePointerInfo::getFixedStack(MF, FrameIndex, Offset);
       } else {
-        SDValue Const =
-            DAG.getConstant(SBFRegisterInfo::FrameLength - VA.getLocMemOffset(),
-                            CLI.DL, MVT::i64);
-        DstAddr = DAG.getNode(ISD::SUB, CLI.DL, PtrVT, FramePtr, Const);
-        DstInfo = MachinePointerInfo();
+        Offset = static_cast<int64_t>(VA.getLocMemOffset());
+        FrameIndex = MF.getFrameInfo().CreateFixedObject(
+            VA.getLocVT().getFixedSizeInBits() / 8, Offset, false);
       }
 
+      SBFFuncInfo->storeFrameIndexArgument(FrameIndex);
+      DstAddr = DAG.getFrameIndex(FrameIndex, PtrVT);
+      DstInfo = MachinePointerInfo::getFixedStack(MF, FrameIndex, Offset);
       Chain = DAG.getStore(Chain, CLI.DL, Arg, DstAddr, DstInfo);
     }
 
-    // Pass the current stack frame pointer via SBF::R5, gluing the
-    // instruction to instructions passing the first 4 arguments in
-    // registers below.
-    Chain = DAG.getCopyToReg(Chain, CLI.DL, SBF::R5, FramePtr, InGlue);
+    if (!Subtarget->getEnableNewCallConvention())
+      // Pass the current stack frame pointer via SBF::R5, gluing the
+      // instruction to instructions passing the first 4 arguments in
+      // registers below.
+      Chain = DAG.getCopyToReg(Chain, CLI.DL, SBF::R5, FramePtr, InGlue);
+
     InGlue = Chain.getValue(1);
   }
 

diff --git a/llvm/lib/Target/SBF/SBFRegisterInfo.cpp b/llvm/lib/Target/SBF/SBFRegisterInfo.cpp
@@ -47,7 +47,7 @@ static void WarnSize(int Offset, MachineFunction &MF, DebugLoc& DL)
 {
   static Function *OldMF = nullptr;
   int MaxOffset = -1 * SBFRegisterInfo::FrameLength;
-  if (Offset <= MaxOffset) {
+  if (Offset < MaxOffset) {
 
     if (&(MF.getFunction()) == OldMF) {
       return;
@@ -156,8 +156,21 @@ int SBFRegisterInfo::resolveInternalFrameIndex(
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   const SBFFunctionInfo *SBFFuncInfo = MF.getInfo<SBFFunctionInfo>();
   int Offset = MFI.getObjectOffset(FI);
-  if (MF.getSubtarget<SBFSubtarget>().getEnableNewCallConvention() &&
+
+  if (!MF.getSubtarget<SBFSubtarget>().getHasDynamicFrames() &&
       SBFFuncInfo->containsFrameIndex(FI)) {
+    Offset = SBFRegisterInfo::FrameLength - Offset;
+    if (static_cast<uint64_t>(Offset) < MFI.getStackSize()) {
+      dbgs() << "Error: A function call in method "
+             << MF.getFunction().getName()
+             << " overwrites values in the frame. Please, decrease stack usage "
+             << "or remove parameters from the call.\n\n";
+      report_fatal_error(
+          "The function call may cause undefined behavior during execution.");
+    }
+    Offset = -Offset;
+  } else if (MF.getSubtarget<SBFSubtarget>().getEnableNewCallConvention() &&
+             SBFFuncInfo->containsFrameIndex(FI)) {
     uint64_t StackSize = MFI.getStackSize();
     Offset = -static_cast<int>(StackSize) - Offset;
   } else if (Imm.has_value()) {

diff --git a/llvm/test/CodeGen/SBF/func_call_error.ll b/llvm/test/CodeGen/SBF/func_call_error.ll
@@ -0,0 +1,46 @@
+; RUN: not --crash llc < %s -march=sbf 2>&1 | FileCheck %s
+
+; Function Attrs: noinline nounwind optnone ssp uwtable(sync)
+define i32 @func(i32 noundef %0, i32 noundef %1, i32 noundef %2, i32 noundef %3, i32 noundef %4, i32 noundef %5) #0 {
+  ret i32 %2
+}
+
+; Function Attrs: noinline nounwind optnone ssp uwtable(sync)
+define i32 @func1(i32 noundef %0, i32 noundef %1, i32 noundef %2, i32 noundef %3) #0 {
+  %5 = alloca i32, align 4
+  %6 = alloca i32, align 4
+  %7 = alloca i32, align 4
+  %8 = alloca i32, align 4
+  %9 = alloca [4070 x i8], align 1
+  store i32 %0, ptr %5, align 4
+  store i32 %1, ptr %6, align 4
+  store i32 %2, ptr %7, align 4
+  store i32 %3, ptr %8, align 4
+  %10 = load i32, ptr %5, align 4
+  %11 = load i32, ptr %6, align 4
+  %12 = load i32, ptr %7, align 4
+  %13 = load i32, ptr %8, align 4
+  %14 = getelementptr inbounds [4070 x i8], ptr %9, i64 0, i64 5
+  %15 = load i8, ptr %14, align 1
+  %16 = sext i8 %15 to i32
+  %17 = getelementptr inbounds [4070 x i8], ptr %9, i64 0, i64 9
+  %18 = load i8, ptr %17, align 1
+  %19 = sext i8 %18 to i32
+  %20 = call i32 @func(i32 noundef %10, i32 noundef %11, i32 noundef %12, i32 noundef %13, i32 noundef %16, i32 noundef %19)
+  ret i32 %20
+
+; CHECK: A function call in method func1 overwrites values in the frame.
+; CHECK: Please, decrease stack usage or remove parameters from the call.
+; CHECK: The function call may cause undefined behavior during execution.
+}
+
+
+attributes #0 = { noinline nounwind optnone ssp uwtable(sync) "frame-pointer"="non-leaf" "min-legal-vector-width"="0" "no-trapping-math"="true" "probe-stack"="__chkstk_darwin" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+crc,+crypto,+dotprod,+fp-armv8,+fp16fml,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+sha2,+sha3,+sm4,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8a,+zcm,+zcz" }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4}
+
+!0 = !{i32 2, !"SDK Version", [2 x i32] [i32 14, i32 4]}
+!1 = !{i32 1, !"wchar_size", i32 4}
+!2 = !{i32 8, !"PIC Level", i32 2}
+!3 = !{i32 7, !"uwtable", i32 1}
+!4 = !{i32 7, !"frame-pointer", i32 1}
diff --git a/llvm/test/CodeGen/SBF/warn-stack.ll b/llvm/test/CodeGen/SBF/warn-stack.ll
@@ -24,10 +24,10 @@ declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #1
 ; CHECK: Error: warn_stack.c
 ; CHECK: please minimize large stack variables 
 define void @warn() local_unnamed_addr #0 !dbg !20 {
-  %1 = alloca [512 x i8], align 1
-  %2 = getelementptr inbounds [512 x i8], [512 x i8]* %1, i64 0, i64 0, !dbg !26
+  %1 = alloca [4124 x i8], align 1
+  %2 = getelementptr inbounds [4124 x i8], [4124 x i8]* %1, i64 0, i64 0, !dbg !26
   call void @llvm.lifetime.start.p0i8(i64 512, i8* nonnull %2) #4, !dbg !26
-  tail call void @llvm.dbg.declare(metadata [512 x i8]* %1, metadata !22, metadata !16), !dbg !27
+  tail call void @llvm.dbg.declare(metadata [4124 x i8]* %1, metadata !22, metadata !16), !dbg !27
   call void @doit(i8* nonnull %2) #4, !dbg !28
   call void @llvm.lifetime.end.p0i8(i64 512, i8* nonnull %2) #4, !dbg !29
   ret void, !dbg !29