Fix null-deref thanks to an attribute on a global declarator chunk (#…

…83611)

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.

Additionally, this was put in as a bug report, so this
Fixes #83611

clang/docs/ReleaseNotes.rst

@@ -295,7 +295,9 @@ Bug Fixes to C++ Support
   of templates. Previously we were diagnosing on any non-function template
   instead of only on class, alias, and variable templates, as last updated by
   CWG2032. Fixes (#GH83461)
-
+- 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 (#GH77703). Fixes (#GH83611).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

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);
   }

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,10 @@ class AttributePool {
     pool.Attrs.clear();
   }
 
+  /// 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,
                      IdentifierInfo *scopeName, SourceLocation scopeLoc,
                      ArgsUnion *args, unsigned numArgs, ParsedAttr::Form form,
@@ -816,6 +821,7 @@ class AttributePool {
 };
 
 class ParsedAttributesView {
+  friend class AttributePool;
   using VecTy = llvm::SmallVector<ParsedAttr *>;
   using SizeType = decltype(std::declval<VecTy>().size());
 

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.

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"

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