[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
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
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, DefaultIgnore;
+def warn_frame_address_missing_noinline: Warning<
+ "calling '%0' in function not marked __attribute__((noinline)) may return a
caller's %1 address">
+ InGroup, 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(CurContext)) {
+if (!FD->hasAttr() && !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__((
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
https://github.com/anematode updated
https://github.com/llvm/llvm-project/pull/82966
>From ef4c372a4d9364e6457c88a940c9af8666de0b74 Mon Sep 17 00:00:00 2001
From: Timothy Herchen
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, DefaultIgnore;
+def warn_frame_address_missing_noinline: Warning<
+ "calling '%0' in function not marked __attribute__((noinline)) may return a
caller's %1 address">
+ InGroup, 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(CurContext)) {
+if (!FD->hasAttr() && !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}
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
https://github.com/anematode updated
https://github.com/llvm/llvm-project/pull/82966
>From cd68484e851040fc84e66933454f6fd910566b81 Mon Sep 17 00:00:00 2001
From: Timothy Herchen
Date: Mon, 26 Feb 2024 12:21:40 -0800
Subject: [PATCH] Add noinline check for __builtin_frame_address and
__builtin_return_address
Resolves #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 to prevent this
behavior. 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 | 34 ++
clang/test/Sema/builtin-returnaddress.c | 44 ---
3 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a7f2858477bee6..6afd34d50086ce 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, DefaultIgnore;
+def warn_frame_address_missing_noinline: Warning<
+ "calling '%0' in function not marked __attribute__((noinline)) may return a
caller's %1 address">,
+ InGroup, 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..7fd34816dbcda9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2729,17 +2729,33 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl,
unsigned BuiltinID,
if (SemaBuiltinConstantArgRange(TheCall, 0, 0, 0x))
return ExprError();
-// -Wframe-address warning if non-zero passed to builtin
-// 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";
+
+ // -Wframe-address warning if non-zero passed to builtin
+ // return/frame address.
+ if (Result.Val.getInt() != 0) {
+Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
+<< BuiltinName << TheCall->getSourceRange();
+ }
+
+ // -Wframe-address warning if enclosing function is not marked noinline.
+ if (const FunctionDecl *FD = dyn_cast(CurContext)) {
+if (!FD->hasAttr() && !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..0b6733f9381c9f 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 functio
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
https://github.com/anematode updated
https://github.com/llvm/llvm-project/pull/82966
>From e5646f3f4967847ef68d428c84b30dbd4d2e8e49 Mon Sep 17 00:00:00 2001
From: Timothy Herchen
Date: Mon, 26 Feb 2024 12:21:40 -0800
Subject: [PATCH] Add noinline check for __builtin_frame_address and
__builtin_return_address
Resolves #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 to prevent this
behavior. 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 | 34 ++
clang/test/Sema/builtin-returnaddress.c | 44 ---
3 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 57784a4ba2e388..21cfd69dc69fc4 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, DefaultIgnore;
+def warn_frame_address_missing_noinline: Warning<
+ "calling '%0' in function not marked __attribute__((noinline)) may return a
caller's %1 address">,
+ InGroup, 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 984088e345c806..3c0a5a8a402ae9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2730,17 +2730,33 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl,
unsigned BuiltinID,
if (SemaBuiltinConstantArgRange(TheCall, 0, 0, 0x))
return ExprError();
-// -Wframe-address warning if non-zero passed to builtin
-// 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";
+
+ // -Wframe-address warning if non-zero passed to builtin
+ // return/frame address.
+ if (Result.Val.getInt() != 0) {
+Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
+<< BuiltinName << TheCall->getSourceRange();
+ }
+
+ // -Wframe-address warning if enclosing function is not marked noinline.
+ if (const FunctionDecl *FD = dyn_cast(CurContext)) {
+if (!FD->hasAttr() && !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..0b6733f9381c9f 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 functio
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
https://github.com/anematode ready_for_review https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
https://github.com/anematode edited https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
https://github.com/anematode edited https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
anematode wrote: > I'm seeing evidence that this might be a chatty diagnostic in practice: > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/kernel/fork.c?L311 > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/mm/util.c?L644 > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/arch/arm/mm/nommu.c?L224 > > https://sourcegraph.com/github.com/torvalds/linux@90d35da658da8cff0d4ecbb5113f5fac9d00eb72/-/blob/kernel/scs.c?L48 > (and quite a few others). Are any of these logic errors if the function gets inlined? https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
anematode wrote: > Although I seem to remember having seen code that uses `always_inline` in > order to force `__builtin_return_address` to actually apply to its caller. Right; that's why I thought just disabling inlining would be a suboptimal choice, although I guess you could have `always_inline` override it. If the diagnostic is so noisy then maybe I don't see the point of it. Is there any case in Linux where it's a correctness issue to use a caller's address? (Presumably in this case it would just give a confusing call site when you're debugging.) https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add noinline check for __builtin_frame_address and __builtin_return_address (PR #82966)
anematode wrote: It's used 100+ more times through the macro `_RET_IP_`. https://elixir.bootlin.com/linux/latest/source/include/linux/instruction_pointer.h#L7 https://elixir.bootlin.com/linux/latest/source/include/linux/kasan.h#L164 has some example uses where `always_inline` is important for correctness. https://elixir.bootlin.com/linux/latest/source/kernel/kcsan/core.c#L912 might be interesting? (This is way out of my depth.) Function isn't marked noinline and the return pointer is actually important. Also, many other functions in that file seem to be marked noinline, e.g. https://elixir.bootlin.com/linux/latest/source/kernel/kcsan/core.c#L1327, but not that one. https://github.com/llvm/llvm-project/pull/82966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] Fix typos concerning wasm __funcref (PR #124365)
https://github.com/anematode created
https://github.com/llvm/llvm-project/pull/124365
The docs conflate `__funcref` with an actual type in a couple places.
>From 2508da6fd7f38101011573460724f13df1c1616d Mon Sep 17 00:00:00 2001
From: Timothy Herchen
Date: Fri, 24 Jan 2025 15:13:30 -0800
Subject: [PATCH] [clang][docs] Fix typos concerning wasm __funcref
---
clang/docs/LanguageExtensions.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/docs/LanguageExtensions.rst
b/clang/docs/LanguageExtensions.rst
index c42b88015e2695..ddb11cc6efb232 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2632,7 +2632,7 @@ with the current table size.
.. code-block:: c++
typedef void (*__funcref funcref_t)();
- static __funcref table[0];
+ static funcref_t table[0];
size_t getSize() {
return __builtin_wasm_table_size(table);
@@ -2654,10 +2654,10 @@ or -1. It will return -1 if not enough space could be
allocated.
.. code-block:: c++
typedef void (*__funcref funcref_t)();
- static __funcref table[0];
+ static funcref_t table[0];
// grow returns the new table size or -1 on error.
- int grow(__funcref fn, int delta) {
+ int grow(funcref_t fn, int delta) {
int prevSize = __builtin_wasm_table_grow(table, fn, delta);
if (prevSize == -1)
return -1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] Fix typos concerning wasm __funcref (PR #124365)
https://github.com/anematode updated
https://github.com/llvm/llvm-project/pull/124365
>From 2508da6fd7f38101011573460724f13df1c1616d Mon Sep 17 00:00:00 2001
From: Timothy Herchen
Date: Fri, 24 Jan 2025 15:13:30 -0800
Subject: [PATCH] [clang][docs] Fix typos concerning wasm __funcref
---
clang/docs/LanguageExtensions.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/docs/LanguageExtensions.rst
b/clang/docs/LanguageExtensions.rst
index c42b88015e2695..ddb11cc6efb232 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2632,7 +2632,7 @@ with the current table size.
.. code-block:: c++
typedef void (*__funcref funcref_t)();
- static __funcref table[0];
+ static funcref_t table[0];
size_t getSize() {
return __builtin_wasm_table_size(table);
@@ -2654,10 +2654,10 @@ or -1. It will return -1 if not enough space could be
allocated.
.. code-block:: c++
typedef void (*__funcref funcref_t)();
- static __funcref table[0];
+ static funcref_t table[0];
// grow returns the new table size or -1 on error.
- int grow(__funcref fn, int delta) {
+ int grow(funcref_t fn, int delta) {
int prevSize = __builtin_wasm_table_grow(table, fn, delta);
if (prevSize == -1)
return -1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][docs] Fix typos concerning wasm __funcref (PR #124365)
@@ -1211,6 +1211,15 @@ WebAssemblyTargetLowering::LowerCall(CallLoweringInfo
&CLI,
}
}
}
+
+// If outgoing arguments are passed via the stack, we cannot tail call
+for (const ISD::OutputArg &Out : CLI.Outs) {
anematode wrote:
Whoops! 😅
https://github.com/llvm/llvm-project/pull/124365
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][docs] Fix typos concerning wasm __funcref (PR #124365)
https://github.com/anematode updated
https://github.com/llvm/llvm-project/pull/124365
>From 2508da6fd7f38101011573460724f13df1c1616d Mon Sep 17 00:00:00 2001
From: Timothy Herchen
Date: Fri, 24 Jan 2025 15:13:30 -0800
Subject: [PATCH 1/2] [clang][docs] Fix typos concerning wasm __funcref
---
clang/docs/LanguageExtensions.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/docs/LanguageExtensions.rst
b/clang/docs/LanguageExtensions.rst
index c42b88015e2695..ddb11cc6efb232 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2632,7 +2632,7 @@ with the current table size.
.. code-block:: c++
typedef void (*__funcref funcref_t)();
- static __funcref table[0];
+ static funcref_t table[0];
size_t getSize() {
return __builtin_wasm_table_size(table);
@@ -2654,10 +2654,10 @@ or -1. It will return -1 if not enough space could be
allocated.
.. code-block:: c++
typedef void (*__funcref funcref_t)();
- static __funcref table[0];
+ static funcref_t table[0];
// grow returns the new table size or -1 on error.
- int grow(__funcref fn, int delta) {
+ int grow(funcref_t fn, int delta) {
int prevSize = __builtin_wasm_table_grow(table, fn, delta);
if (prevSize == -1)
return -1;
>From 5721ec3228edebb3170df3a8dc80ef12076f5ea8 Mon Sep 17 00:00:00 2001
From: Timothy Herchen
Date: Sat, 25 Jan 2025 20:19:00 -0800
Subject: [PATCH 2/2] [wasm] disallow tail calls when passing structs on the
stack
---
llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp | 9 +
1 file changed, 9 insertions(+)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 02db1b142a22b5..ec745ebabb4f63 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1211,6 +1211,15 @@ WebAssemblyTargetLowering::LowerCall(CallLoweringInfo
&CLI,
}
}
}
+
+// If outgoing arguments are passed via the stack, we cannot tail call
+for (const ISD::OutputArg &Out : CLI.Outs) {
+ if (Out.Flags.isByVal() && Out.Flags.getByValSize() != 0) {
+NoTail(
+"WebAssembly does not support tail calling with stack arguments");
+break;
+ }
+}
}
SmallVectorImpl &Ins = CLI.Ins;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] Fix typos concerning wasm __funcref (PR #124365)
anematode wrote: Pardon? (Last time I made a PR my commit was merged directly, but I don't mind if you commit a trivial one like this) https://github.com/llvm/llvm-project/pull/124365 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
