https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/83611
>From 8152ad56b320719553701edf020c30aea8c3213e Mon Sep 17 00:00:00 2001 From: erichkeane <eke...@nvidia.com> Date: Fri, 1 Mar 2024 11:36:14 -0800 Subject: [PATCH 1/4] Fix null-deref thanks to an attribute on a global declarator chunk This was reported (sort of) in a PR: #77703. The problem is that a declarator 'owns' an attributes allocation via an `AttributePool`. However, this example tries to copy a DeclaratorChunk from one Declarator to another, so when the temporary Declarator goes out of scope, it deletes the attribute it has tried to pass on via the chunk. This patch ensures that we copy the 'ownership' of the attribute correctly, and adds an assert to catch any other casess where this happens. --- clang/docs/ReleaseNotes.rst | 5 ++++- clang/include/clang/Sema/DeclSpec.h | 16 ++++++++++++++++ clang/include/clang/Sema/ParsedAttr.h | 5 +++++ clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/ParsedAttr.cpp | 6 ++++++ .../Parser/cxx-declarator-attribute-crash.cpp | 8 ++++++++ 6 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 clang/test/Parser/cxx-declarator-attribute-crash.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 1286263700963c..3476342ef6f6e4 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -306,7 +306,10 @@ Bug Fixes to C++ Support instead of only on class, alias, and variable templates, as last updated by CWG2032. Fixes (`#83461 <https://github.com/llvm/llvm-project/issues/83461>`_) - +- Fixed an issue where an attribute on a declarator would cause the attribute to + be destructed prematurely. This fixes a pair of Chromium that were brought to + our attention by an attempt to fix in + `#77703 <https://github.com/llvm/llvm-project/pull/77703>`_. Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h index 316e8071169a3a..a176159707486c 100644 --- a/clang/include/clang/Sema/DeclSpec.h +++ b/clang/include/clang/Sema/DeclSpec.h @@ -2359,11 +2359,27 @@ class Declarator { SetRangeEnd(EndLoc); } + /// AddTypeInfo - Add a chunk to this declarator. Also extend the range to + /// EndLoc, which should be the last token of the chunk. This overload is for + /// copying a 'chunk' from another declarator, so it takes the pool that the + /// other Declarator owns so that it can 'take' the attributes from it. + void AddTypeInfo(const DeclaratorChunk &TI, AttributePool &OtherPool, + SourceLocation EndLoc) { + DeclTypeInfo.push_back(TI); + getAttributePool().takeFrom(DeclTypeInfo.back().getAttrs(), OtherPool); + + if (!EndLoc.isInvalid()) + SetRangeEnd(EndLoc); + } + /// AddTypeInfo - Add a chunk to this declarator. Also extend the range to /// EndLoc, which should be the last token of the chunk. void AddTypeInfo(const DeclaratorChunk &TI, SourceLocation EndLoc) { DeclTypeInfo.push_back(TI); + assert(TI.AttrList.empty() && + "Cannot add a declarator chunk with attributes with this overload"); + if (!EndLoc.isInvalid()) SetRangeEnd(EndLoc); } diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h index 8c3ba39031aad8..53cc76da34df27 100644 --- a/clang/include/clang/Sema/ParsedAttr.h +++ b/clang/include/clang/Sema/ParsedAttr.h @@ -680,6 +680,7 @@ class AttributeFactory { ~AttributeFactory(); }; +class ParsedAttributesView; class AttributePool { friend class AttributeFactory; friend class ParsedAttributes; @@ -734,6 +735,9 @@ class AttributePool { pool.Attrs.clear(); } + /// Take a list of attributes from another pool and add them to this pool. + void takeFrom(ParsedAttributesView &List, AttributePool &Pool); + ParsedAttr *create(IdentifierInfo *attrName, SourceRange attrRange, IdentifierInfo *scopeName, SourceLocation scopeLoc, ArgsUnion *args, unsigned numArgs, ParsedAttr::Form form, @@ -816,6 +820,7 @@ class AttributePool { }; class ParsedAttributesView { + friend class AttributePool; using VecTy = llvm::SmallVector<ParsedAttr *>; using SizeType = decltype(std::declval<VecTy>().size()); diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index edfab11c37cf0f..ccbfea6a66fba7 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -7928,7 +7928,7 @@ void Parser::ParseMisplacedBracketDeclarator(Declarator &D) { // Adding back the bracket info to the end of the Declarator. for (unsigned i = 0, e = TempDeclarator.getNumTypeObjects(); i < e; ++i) { const DeclaratorChunk &Chunk = TempDeclarator.getTypeObject(i); - D.AddTypeInfo(Chunk, SourceLocation()); + D.AddTypeInfo(Chunk, TempDeclarator.getAttributePool(), SourceLocation()); } // The missing identifier would have been diagnosed in ParseDirectDeclarator. diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp index 06c213267c7ef5..978d4e7bf4d392 100644 --- a/clang/lib/Sema/ParsedAttr.cpp +++ b/clang/lib/Sema/ParsedAttr.cpp @@ -100,6 +100,12 @@ void AttributePool::takePool(AttributePool &pool) { pool.Attrs.clear(); } +void AttributePool::takeFrom(ParsedAttributesView &List, AttributePool &Pool) { + assert(&Pool != this && "AttributePool can't take attributes from itself"); + llvm::for_each(List.AttrList, [&Pool](ParsedAttr *A) { Pool.remove(A); }); + Attrs.insert(Attrs.end(), List.AttrList.begin(), List.AttrList.end()); +} + namespace { #include "clang/Sema/AttrParsedAttrImpl.inc" diff --git a/clang/test/Parser/cxx-declarator-attribute-crash.cpp b/clang/test/Parser/cxx-declarator-attribute-crash.cpp new file mode 100644 index 00000000000000..3b989a659db569 --- /dev/null +++ b/clang/test/Parser/cxx-declarator-attribute-crash.cpp @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// expected-error@+5{{brackets are not allowed here}} +// expected-error@+4{{a type specifier is required for all declarations}} +// expected-warning@+3{{unknown attribute 'h' ignored}} +// expected-error@+2{{definition of variable with array type}} +// expected-error@+1{{expected ';'}} +[][[h]]l >From 3f05e51851d861d45e8bad65a8dabe72f23fb194 Mon Sep 17 00:00:00 2001 From: erichkeane <eke...@nvidia.com> Date: Fri, 1 Mar 2024 11:46:24 -0800 Subject: [PATCH 2/4] clang-format --- clang/lib/Sema/ParsedAttr.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp index 978d4e7bf4d392..6abc90336c9946 100644 --- a/clang/lib/Sema/ParsedAttr.cpp +++ b/clang/lib/Sema/ParsedAttr.cpp @@ -101,9 +101,9 @@ void AttributePool::takePool(AttributePool &pool) { } void AttributePool::takeFrom(ParsedAttributesView &List, AttributePool &Pool) { - assert(&Pool != this && "AttributePool can't take attributes from itself"); - llvm::for_each(List.AttrList, [&Pool](ParsedAttr *A) { Pool.remove(A); }); - Attrs.insert(Attrs.end(), List.AttrList.begin(), List.AttrList.end()); + assert(&Pool != this && "AttributePool can't take attributes from itself"); + llvm::for_each(List.AttrList, [&Pool](ParsedAttr *A) { Pool.remove(A); }); + Attrs.insert(Attrs.end(), List.AttrList.begin(), List.AttrList.end()); } namespace { >From 28c0b7da5050f84aca01cc2d308b24bd0b8f85c1 Mon Sep 17 00:00:00 2001 From: erichkeane <eke...@nvidia.com> Date: Mon, 4 Mar 2024 06:10:29 -0800 Subject: [PATCH 3/4] Update release note to include Shafik's bug report --- clang/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3476342ef6f6e4..1ee7fbdcb8d6e9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -310,6 +310,7 @@ Bug Fixes to C++ Support be destructed prematurely. This fixes a pair of Chromium that were brought to our attention by an attempt to fix in `#77703 <https://github.com/llvm/llvm-project/pull/77703>`_. + Fixes '#83611 <https://github.com/llvm/llvm-project/pull/83611>`_. Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ >From da48f028ca73262eb0d4097dd88ea9ede0891f87 Mon Sep 17 00:00:00 2001 From: erichkeane <eke...@nvidia.com> Date: Mon, 4 Mar 2024 06:35:38 -0800 Subject: [PATCH 4/4] Update comment suggested by Aaron --- clang/include/clang/Sema/ParsedAttr.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h index 53cc76da34df27..e3857b2f07d9e0 100644 --- a/clang/include/clang/Sema/ParsedAttr.h +++ b/clang/include/clang/Sema/ParsedAttr.h @@ -735,7 +735,8 @@ class AttributePool { pool.Attrs.clear(); } - /// Take a list of attributes from another pool and add them to this pool. + /// Removes the attributes from \c List, which are owned by \c Pool, and adds + /// them at the end of this \c AttributePool. void takeFrom(ParsedAttributesView &List, AttributePool &Pool); ParsedAttr *create(IdentifierInfo *attrName, SourceRange attrRange, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
