[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)

[Timothy Herchen via cfe-commits]

https://github.com/anematode created https://github.com/llvm/llvm-project/pull/82966

Will resolve https://github.com/llvm/llvm-project/issues/66059 . GCC's behavior in the case of inlining (https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html) is that a caller's return/frame address may be returned, if the function in question gets inlined. Their docs encourage the function to be marked noinline. Therefore, it would be courteous to produce a warning if the function containing the call to __builtin_frame_address and __builtin_return_address is not marked noinline.

Marking this as draft because it's not fully ready, and I'd like to see whether this is a welcome change.

>From ef4c372a4d9364e6457c88a940c9af8666de0b74 Mon Sep 17 00:00:00 2001
From: Timothy Herchen <timothy.herchen at gmail.com>
Date: Mon, 26 Feb 2024 00:01:27 -0800
Subject: [PATCH] Add noinline check for __builtin_frame_address and
 __builtin_return_address

Resolves https://github.com/llvm/llvm-project/issues/66059 . GCC's behavior in the case of inlining (https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html) is that a caller's return/frame address may be returned, if the function in question gets inlined. Their docs encourage the function to be marked noinline. Therefore, produce a warning if the function containing the call to __builtin_frame_address and __builtin_return_address is not marked noinline.
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  4 +++
 clang/lib/Sema/SemaChecking.cpp               | 29 +++++++++++++----
 clang/test/Sema/builtin-returnaddress.c       | 32 +++++++++++++++----
 3 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a7f2858477bee6..8f1cc611203694 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2029,6 +2029,10 @@ def warn_frame_address : Warning<
   "calling '%0' with a nonzero argument is unsafe">,
   InGroup<FrameAddress>, DefaultIgnore;
 
+def warn_frame_address_missing_noinline: Warning<
+  "calling '%0' in function not marked __attribute__((noinline)) may return a caller's %1 address">
+  InGroup<FrameAddress>, DefaultIgnore;
+
 def warn_cxx98_compat_nontrivial_union_or_anon_struct_member : Warning<
   "%select{anonymous struct|union}0 member %1 with a non-trivial "
   "%sub{select_special_member_kind}2 is incompatible with C++98">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 7fa295ebd94044..1861f7dace3c6e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2733,13 +2733,28 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
     // return/frame address.
     Expr::EvalResult Result;
     if (!TheCall->getArg(0)->isValueDependent() &&
-        TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
-        Result.Val.getInt() != 0)
-      Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
-          << ((BuiltinID == Builtin::BI__builtin_return_address)
-                  ? "__builtin_return_address"
-                  : "__builtin_frame_address")
-          << TheCall->getSourceRange();
+        TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext())) {
+      const char *BuiltinName =
+          (BuiltinID == Builtin::BI__builtin_return_address)
+               ? "__builtin_return_address"
+               : "__builtin_frame_address";
+
+      if (Result.Val.getInt() != 0) {
+        Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
+            << BuiltinName << TheCall->getSourceRange();
+      }
+
+      // Check if enclosing function is marked noinline
+      if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CurContext)) {
+        if (!FD->hasAttr<NoInlineAttr>() && !FD->isMain()) {
+          const char *ShortName =
+              (BuiltinID == Builtin::BI__builtin_return_address)
+                  ? "return" : "frame";
+          Diag(TheCall->getBeginLoc(), diag::warn_frame_address_missing_noinline)
+              << BuiltinName << ShortName << TheCall->getSourceRange();
+        }
+      }
+    }
     break;
   }
 
diff --git a/clang/test/Sema/builtin-returnaddress.c b/clang/test/Sema/builtin-returnaddress.c
index 16d2a517ac12f2..feb5a2ae05def7 100644
--- a/clang/test/Sema/builtin-returnaddress.c
+++ b/clang/test/Sema/builtin-returnaddress.c
@@ -2,24 +2,32 @@
 // RUN: %clang_cc1 -fsyntax-only -Wmost -verify %s
 // RUN: %clang_cc1 -x c++ -fsyntax-only -Wframe-address -verify %s
 
-void* a(unsigned x) {
+__attribute__((noinline)) void* a(unsigned x) {
 return __builtin_return_address(0);
 }
 
-void* b(unsigned x) {
+__attribute__((noinline)) void* b(unsigned x) {
 return __builtin_return_address(1); // expected-warning{{calling '__builtin_return_address' with a nonzero argument is unsafe}}
 }
 
-void* c(unsigned x) {
+__attribute__((noinline)) void* c(unsigned x) {
 return __builtin_frame_address(0);
 }
 
-void* d(unsigned x) {
+__attribute__((noinline)) void* d(unsigned x) {
 return __builtin_frame_address(1); // expected-warning{{calling '__builtin_frame_address' with a nonzero argument is unsafe}}
 }
 
+void* e(unsigned x) {
+  return __builtin_frame_address(0); // expected-warning{{calling '__builtin_frame_address' in function that is not marked __attribute__((noinline)) might return a caller's frame address}}
+}
+
+void* f(unsigned x) {
+  return __builtin_return_address(0); // expected-warning{{calling '__builtin_return_address' in function that is not marked __attribute__((noinline)) might return a caller's return address}}
+}
+
 #ifdef __cplusplus
-template<int N> void *RA()
+__attribute__((noinline)) template<int N> void *RA()
 {
   return __builtin_return_address(N); // expected-warning{{calling '__builtin_return_address' with a nonzero argument is unsafe}}
 }
@@ -28,4 +36,16 @@ void *foo()
 {
  return RA<2>(); // expected-note{{in instantiation of function template specialization 'RA<2>' requested here}}
 }
-#endif
+
+void* f() {
+  return ([&] () {
+    return __builtin_frame_address(0); // expected-warning{{calling '__builtin_frame_address' in function that is not marked __attribute__((noinline)) might return a caller's frame address}}
+  })();
+}
+
+void* g() {
+  return ([&] () __attribute__((noinline)) {
+    return __builtin_frame_address(0);
+  })();
+}
+#endif
\ No newline at end of file