-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Conversation
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
@llvm/pr-subscribers-clang Author: Erich Keane (erichkeane) ChangesThese 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:
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
+}
|
There was a problem hiding this 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this 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?
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
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
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
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
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
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