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.herc...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits