[RISCV] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address

[shiva0217]

Summary

Disable tail call if the callee function contain builtin_frame_address or builtin_return_address.
Otherwise, tail call optimization will remove the frame pointer and return address restoration which will make the above two builtin functions get incorrect value.

E.g.

void *callee (char *p) { return __builtin_return_address (1); }
void *caller (void) { char *save = (char*) alloca (4); return callee (save); }

[hfinkel]

Is this really a target-independent problem?

[mgrang]

lib/Target/RISCV/RISCVISelLowering.cpp

1312

Period at the end of comment.

1415

This check can be combined with the assignment:
if (const Function *CalleeFn = dyn_cast<Function>(G->getGlobal()))

1419

getFrameDepthArg asserts on isFrameAddressOrReturnAddressCall. So we end up calling isFrameAddressOrReturnAddressCall twice here. We could put them into one function but it is cleaner the way it is now.
@asb Do you think this should be cleaned up?

[shiva0217]

In D61626#1493735, @hfinkel wrote:

Is this really a target-independent problem?

Hi @hfinkel,
I think you're right, it should be a target-independent problem. I have created D61665, Thanks.

lib/Target/RISCV/RISCVISelLowering.cpp

1312

Got it, thanks.

1415

Got it, thanks.

1419

Hi @mgrang and @asb,
I created D61665 to replace this one and avoid calling isFrameAddressOrReturnAddressCall twice in the new patch.
Do you happy with the new implementation?

lib/Target/RISCV/RISCVISelLowering.cpp

SDValue RISCVTargetLowering::LowerFormalArguments(
   if (!OutChains.empty()) {
     OutChains.push_back(Chain);
     Chain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, OutChains);
   }
 
   return Chain;
 }
 
+// Return true if the instruction is __builtin_frame_address or
+// __builtin_return_address function call.
+static bool isFrameAddressOrReturnAddressCall(const Instruction *I) {
+  if (const CallInst *CI = dyn_cast<CallInst>(I))
+    if (Function *IntrinsicF = CI->getCalledFunction()) {
+      Intrinsic::ID ID = IntrinsicF->getIntrinsicID();
+      if ((ID == Intrinsic::frameaddress) ||
+          (ID == Intrinsic::returnaddress))
+        return true;
+    }
+  return false;
+}
+
+// Return frame depth argument of __builtin_frame_address or
+// __builtin_return_address
+static unsigned getFrameDepthArg(const Instruction *I) {
+  assert (isFrameAddressOrReturnAddressCall(I));
+  const CallInst *CI = cast<CallInst>(I);
+  Constant *C = cast<Constant>(*CI->arg_begin());
+  return C->getUniqueInteger().getZExtValue();
+}
+
 /// IsEligibleForTailCallOptimization - Check whether the call is eligible
 /// for tail call optimization.
 /// Note: This is modelled after ARM's IsEligibleForTailCallOptimization.
 bool RISCVTargetLowering::IsEligibleForTailCallOptimization(
   CCState &CCInfo, CallLoweringInfo &CLI, MachineFunction &MF,
   const SmallVector<CCValAssign, 16> &ArgLocs) const {
 
   auto &Callee = CLI.Callee;
bool RISCVTargetLowering::IsEligibleForTailCallOptimization(
 
   // Byval parameters hand the function a pointer directly into the stack area
   // we want to reuse during a tail call. Working around this *is* possible
   // but less efficient and uglier in LowerCall.
   for (auto &Arg : Outs)
     if (Arg.Flags.isByVal())
       return false;
 
+  // Do not do tail call if the callee function contain __builtin_frame_address
+  // or __builtin_return_address.
+  //
+  // Enabling tail call may remove the frame pointer and return address
+  // restoration which will make the above two builtin functions get incorrect
+  // value.
+  //
+  // void *callee (char *p) { return __builtin_return_address (1); }
+  // void *caller (void) { char *save = (char*) alloca (4);
+  //                       return callee (save); }
+  // caller:
+  //       tail callee <= caller didn't store fp and ra in caller stack.
+  // callee:
+  //       Try to get return address from caller stack.
+  if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee)) {
+    const Function *CalleeFn = dyn_cast<Function>(G->getGlobal());
+    if (CalleeFn)
+      for (const BasicBlock &BB : *CalleeFn)
+        for (const Instruction &I : BB)
+          if (isFrameAddressOrReturnAddressCall(&I) &&
+              (getFrameDepthArg(&I) > 0))
+            return false;
+  }
+
   return true;
 }
 
 // Lower a call to a callseq_start + CALL + callseq_end chain, and add input
 // and output parameter nodes.
 SDValue RISCVTargetLowering::LowerCall(CallLoweringInfo &CLI,
                                        SmallVectorImpl<SDValue> &InVals) const {
   SelectionDAG &DAG = CLI.DAG;

test/CodeGen/RISCV/builtin-frame-address.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32I
+
+; The test case check that tail call optimization will be suppressed for
+; @llvm.frameaddress with depth > 0.
+; Otherwise, @llvm.frameaddress(i32 1) will get wrong frame address.
+
+; Tail call will be suppressed in caller1 because callee1 contain
+; @llvm.rameaddress(i32 1).
+define i8* @caller1() {
+; RV32I-LABEL: caller1:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    addi sp, sp, -16
+; RV32I-NEXT:    sw ra, 12(sp)
+; RV32I-NEXT:    call callee1
+; RV32I-NEXT:    lw ra, 12(sp)
+; RV32I-NEXT:    addi sp, sp, 16
+; RV32I-NEXT:    ret
+entry:
+  %call = tail call i8* @callee1(i8* undef)
+  ret i8* %call
+}
+
+define i8* @callee1(i8* nocapture readnone %p) {
+; RV32I-LABEL: callee1:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    addi sp, sp, -16
+; RV32I-NEXT:    sw ra, 12(sp)
+; RV32I-NEXT:    sw s0, 8(sp)
+; RV32I-NEXT:    addi s0, sp, 16
+; RV32I-NEXT:    lw a0, -8(s0)
+; RV32I-NEXT:    lw s0, 8(sp)
+; RV32I-NEXT:    lw ra, 12(sp)
+; RV32I-NEXT:    addi sp, sp, 16
+; RV32I-NEXT:    ret
+entry:
+  %0 = tail call i8* @llvm.frameaddress(i32 1)
+  ret i8* %0
+}
+
+; Tail call won't be suppressed in caller0 because callee0 contain
+; @llvm.frameaddress(i32 0) which will not backtrace to caller0's stack.
+define i8* @caller0() {
+; RV32I-LABEL: caller0:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    tail callee0
+entry:
+  %call = tail call i8* @callee0(i8* undef)
+  ret i8* %call
+}
+
+define i8* @callee0(i8* nocapture readnone %p) {
+; RV32I-LABEL: callee0:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    addi sp, sp, -16
+; RV32I-NEXT:    sw ra, 12(sp)
+; RV32I-NEXT:    sw s0, 8(sp)
+; RV32I-NEXT:    addi s0, sp, 16
+; RV32I-NEXT:    mv a0, s0
+; RV32I-NEXT:    lw s0, 8(sp)
+; RV32I-NEXT:    lw ra, 12(sp)
+; RV32I-NEXT:    addi sp, sp, 16
+; RV32I-NEXT:    ret
+entry:
+  %0 = tail call i8* @llvm.frameaddress(i32 0)
+  ret i8* %0
+}
+
+declare i8* @llvm.frameaddress(i32)

test/CodeGen/RISCV/builtin-return-address.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32I
+
+; The test case check that tail call optimization will be suppressed for
+; @llvm.returnaddress with depth > 0.
+; Otherwise, @llvm.returnaddress(i32 1) will get wrong return address.
+
+; Tail call will be suppressed in caller1 because callee1 contain
+; @llvm.returnaddress(i32 1).
+define i8* @caller1() {
+; RV32I-LABEL: caller1:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    addi sp, sp, -16
+; RV32I-NEXT:    sw ra, 12(sp)
+; RV32I-NEXT:    call callee1
+; RV32I-NEXT:    lw ra, 12(sp)
+; RV32I-NEXT:    addi sp, sp, 16
+; RV32I-NEXT:    ret
+entry:
+  %call = tail call i8* @callee1(i8* undef)
+  ret i8* %call
+}
+
+define i8* @callee1(i8* nocapture readnone %p) {
+; RV32I-LABEL: callee1:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    addi sp, sp, -16
+; RV32I-NEXT:    sw ra, 12(sp)
+; RV32I-NEXT:    sw s0, 8(sp)
+; RV32I-NEXT:    addi s0, sp, 16
+; RV32I-NEXT:    lw a0, -8(s0)
+; RV32I-NEXT:    lw a0, -4(a0)
+; RV32I-NEXT:    lw s0, 8(sp)
+; RV32I-NEXT:    lw ra, 12(sp)
+; RV32I-NEXT:    addi sp, sp, 16
+; RV32I-NEXT:    ret
+entry:
+  %0 = tail call i8* @llvm.returnaddress(i32 1)
+  ret i8* %0
+}
+
+; Tail call won't be suppressed in caller0 because callee0 contain
+; @llvm.returnaddress(i32 0) which will not backtrace to caller0's stack.
+define i8* @caller0() {
+; RV32I-LABEL: caller0:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    tail callee0
+entry:
+  %call = tail call i8* @callee0(i8* undef)
+  ret i8* %call
+}
+
+define i8* @callee0(i8* nocapture readnone %p) {
+; RV32I-LABEL: callee0:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    mv a0, ra
+; RV32I-NEXT:    ret
+entry:
+  %0 = tail call i8* @llvm.returnaddress(i32 0)
+  ret i8* %0
+}
+
+declare i8* @llvm.returnaddress(i32)