Skip to content

Fix crash when an attribute is applied to pragma attribute/pragma dump #137880

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

Merged
merged 2 commits into from
Apr 30, 2025

Conversation

erichkeane
Copy link
Collaborator

These two don't result in a statement, so the attempt to apply the attributes to them was crashing. This patch correctly prohibits the use of attributes on these clauses.

Fixes: #137861

These two don't result in a statement, so the attempt to apply the
attributes to them was crashing.  This patch correctly prohibits the use
of attributes on these clauses.

Fixes: llvm#137861
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 29, 2025

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

These two don't result in a statement, so the attempt to apply the attributes to them was crashing. This patch correctly prohibits the use of attributes on these clauses.

Fixes: #137861


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

2 Files Affected:

  • (modified) clang/lib/Parse/ParseStmt.cpp (+4)
  • (added) clang/test/Parser/gh137861.cpp (+33)
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 4e801f4ef890f..97924f093240f 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -526,10 +526,14 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
     return ParsePragmaLoopHint(Stmts, StmtCtx, TrailingElseLoc, CXX11Attrs);
 
   case tok::annot_pragma_dump:
+    ProhibitAttributes(CXX11Attrs);
+    ProhibitAttributes(GNUAttrs);
     HandlePragmaDump();
     return StmtEmpty();
 
   case tok::annot_pragma_attribute:
+    ProhibitAttributes(CXX11Attrs);
+    ProhibitAttributes(GNUAttrs);
     HandlePragmaAttribute();
     return StmtEmpty();
   }
diff --git a/clang/test/Parser/gh137861.cpp b/clang/test/Parser/gh137861.cpp
new file mode 100644
index 0000000000000..9354aef919650
--- /dev/null
+++ b/clang/test/Parser/gh137861.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 %s -verify 
+
+void foo() {
+  // expected-error@+1{{an attribute list cannot appear here}}
+__attribute__((aligned(64)))
+#pragma clang attribute push(__attribute__((uninitialized)), apply_to = any(variable(is_local)))
+{
+  int f;
+}
+#pragma clang attribute pop
+}
+
+void foo2() {
+  // expected-error@+1{{an attribute list cannot appear here}}
+__attribute__((aligned(64)))
+#pragma clang __debug dump foo
+}
+
+void foo3() {
+  // expected-error@+1{{an attribute list cannot appear here}}
+  [[nodiscard]]
+#pragma clang attribute push(__attribute__((uninitialized)), apply_to = any(variable(is_local)))
+{
+  int f;
+}
+#pragma clang attribute pop
+}
+
+void foo4() {
+  // expected-error@+1{{an attribute list cannot appear here}}
+  [[nodiscard]]
+#pragma clang __debug dump foo
+}

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

I think we need a release note since we are fixing a bug, right?

@@ -526,10 +526,14 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
return ParsePragmaLoopHint(Stmts, StmtCtx, TrailingElseLoc, CXX11Attrs);

case tok::annot_pragma_dump:
ProhibitAttributes(CXX11Attrs);
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL

Does kw_try and kw___leave also need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, both of those successfully produce a statement out of this, so they won't assert. So any attributes you try to apply will either be valid, or fail because they don't apply to a statement.

The difference in these two is that they don't result in either an error OR a valid statement.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM, but the changes should come with a release note, right?

@erichkeane erichkeane merged commit 84a5435 into llvm:main Apr 30, 2025
12 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm#137880)

These two don't result in a statement, so the attempt to apply the
attributes to them was crashing. This patch correctly prohibits the use
of attributes on these clauses.

Fixes: llvm#137861
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm#137880)

These two don't result in a statement, so the attempt to apply the
attributes to them was crashing. This patch correctly prohibits the use
of attributes on these clauses.

Fixes: llvm#137861
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm#137880)

These two don't result in a statement, so the attempt to apply the
attributes to them was crashing. This patch correctly prohibits the use
of attributes on these clauses.

Fixes: llvm#137861
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
llvm#137880)

These two don't result in a statement, so the attempt to apply the
attributes to them was crashing. This patch correctly prohibits the use
of attributes on these clauses.

Fixes: llvm#137861
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
llvm#137880)

These two don't result in a statement, so the attempt to apply the
attributes to them was crashing. This patch correctly prohibits the use
of attributes on these clauses.

Fixes: llvm#137861
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 21 crash (stack dump) when parsing #pragma clang attribute push inside function body
4 participants