Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][Sema] improve sema check of clang::musttail attribute #77727

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

jcsxky
Copy link
Contributor

@jcsxky jcsxky commented Jan 11, 2024

Call function with no-return attribute generates code with unreachable instruction instead of return and if the call statement has clang::musttail attribute, it would crash in VerifyPass. Check this condition in Sema ahead.
Fix issue

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

Call function with no-return attribute generate code with unreachable instruction instead of return and if the call statement has clang::musttail attribute, it would crash in Verify. Check this condition in Sema.


Full diff: https://github.com/llvm/llvm-project/pull/77727.diff

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
  • (modified) clang/lib/Sema/SemaStmt.cpp (+6)
  • (added) clang/test/SemaCXX/PR76631.cpp (+7)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3884dca59e2f3b..b40befc5c1cfb5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3113,6 +3113,8 @@ def err_musttail_scope : Error<
   "cannot perform a tail call from this return statement">;
 def err_musttail_no_variadic : Error<
   "%0 attribute may not be used with variadic functions">;
+def err_musttail_no_return : Error<
+  "%0 attribute may not be used with no-return-attribute functions">;
 
 def err_nsobject_attribute : Error<
   "'NSObject' attribute is for pointer types only">;
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 21efe25ed84a3d..9e7c8c7e4e8c12 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -786,6 +786,12 @@ bool Sema::checkMustTailAttr(const Stmt *St, const Attr &MTA) {
     return false;
   }
 
+  const auto *CalleeDecl = CE->getCalleeDecl();
+  if (CalleeDecl && CalleeDecl->hasAttr<CXX11NoReturnAttr>()) {
+    Diag(St->getBeginLoc(), diag::err_musttail_no_return) << &MTA;
+    return false;
+  }
+
   // Caller and callee must match in whether they have a "this" parameter.
   if (CallerType.This.isNull() != CalleeType.This.isNull()) {
     if (const auto *ND = dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) {
diff --git a/clang/test/SemaCXX/PR76631.cpp b/clang/test/SemaCXX/PR76631.cpp
new file mode 100644
index 00000000000000..df89753bd12580
--- /dev/null
+++ b/clang/test/SemaCXX/PR76631.cpp
@@ -0,0 +1,7 @@
+[[noreturn]] void throw_int() {
+  throw int();
+}
+
+void throw_int_wrapper() {
+  [[clang::musttail]] return throw_int(); // expected-error {{'musttail' attribute may not be used with no-return-attribute functions}}
+}

@jcsxky jcsxky force-pushed the improve_sema_check_musttail branch 4 times, most recently from cdc04d0 to 75e9a81 Compare January 11, 2024 08:55
@llvmbot llvmbot added the HLSL HLSL Language Support label Jan 11, 2024
@jcsxky jcsxky changed the title [Clang][SemaCXX] improve sema check of clang::musttail attribute [Clang][Sema] improve sema check of clang::musttail attribute Jan 11, 2024
@jcsxky jcsxky force-pushed the improve_sema_check_musttail branch from 75e9a81 to 0281f6d Compare January 12, 2024 01:56
@jcsxky jcsxky removed the HLSL HLSL Language Support label Jan 12, 2024
Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just needs a release note, else LGTM.

@jcsxky jcsxky force-pushed the improve_sema_check_musttail branch from 0281f6d to 537228e Compare January 14, 2024 09:28
@jcsxky
Copy link
Contributor Author

jcsxky commented Jan 14, 2024

Just needs a release note, else LGTM.

Release note has been added.

@jcsxky jcsxky force-pushed the improve_sema_check_musttail branch from 537228e to 67396b5 Compare January 17, 2024 05:10
@jcsxky jcsxky force-pushed the improve_sema_check_musttail branch from 67396b5 to 17e2f6d Compare January 17, 2024 12:56
@jcsxky jcsxky merged commit 2fe5b15 into llvm:main Jan 17, 2024
4 of 5 checks passed
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
…7727)

Call function with no-return attribute generates code with unreachable
instruction instead of return and if the call statement has
`clang::musttail` attribute, it would crash in `VerifyPass`. Check this
condition in Sema ahead.
Fix [issue](llvm#76631)

Co-authored-by: huqizhi <836744285@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang] crash when using [[clang::musttail]] on function marked [[noreturn]]
3 participants