CHERICompartmentName: only take into account attr from DeclChunk instead of Decl

Otherwise, the attribute is silently ignored when the attribute
follows a pointer return type such as:

void *__attribute__((cheri_compartment("my_compartment"))) f();

A unittest case is added to never let that slip again.

Author: v01dxyz

clang/lib/Sema/SemaDeclAttr.cpp

@@ -2392,11 +2392,28 @@ static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr,
                                        Sema::DeclAttributeLocation DAL) {
   // cheri_compartment is both:
   //
-  // * a Declaration attribute: marks the function as a compartment entry point
+  // * a Declaration attribute: marks the function as a compartment
+  //   entry point
   // * a Function Type attribute: affects the calling convention
   //
-  // That's the reason why we don't short-circuit with hasDeclarator
-  // (as other handlers do).
+  // That's the reason why we don't short-circuit using hasDeclarator
+  // (as other handlers do) as Sema::GetTypeForDeclarator only does
+  // the Function Type part.
+  //
+  // BUT because it is a Function Type attribute, when the attribute
+  // is initially attached to the DeclSpec, Sema::GetTypeForDeclarator
+  // distributes it to the first DeclChunk which is a function.  Thus,
+  // the attribute is always be present at the DeclChunk level (but
+  // not always at the DeclSpec level). In order to not possibly
+  // process it twice, we always skip the DeclSpec level.
+  //
+  // **Attention**: The attribute is not always initially attached to
+  // the DeclSpec especially when it follows a pointer:
+  //
+  // - cheri_compartment(...) void f(): attached to DeclSpec
+  // - void * cheri_compartment(...) f(): attached to DeclChunk
+  if (DAL != Sema::DAL_DeclSpec)
+    return;
 
   StringRef Str;
   SourceLocation LiteralLoc;

clang/unittests/AST/CMakeLists.txt

@@ -36,6 +36,8 @@ add_clang_unittest(ASTTests
   TemplateNameTest.cpp
   TypePrinterTest.cpp
   UnresolvedSetTest.cpp
+
+  cheri/AttrTest.cpp
   )
 
 clang_target_link_libraries(ASTTests

clang/unittests/AST/cheri/AttrTest.cpp

@@ -0,0 +1,77 @@
+//===- unittests/AST/cheri/AttrTests.cpp --- CHERI Attribute tests --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/Attr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/AttrKinds.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+
+namespace {
+
+using clang::ast_matchers::constantExpr;
+using clang::ast_matchers::equals;
+using clang::ast_matchers::functionDecl;
+using clang::ast_matchers::has;
+using clang::ast_matchers::hasDescendant;
+using clang::ast_matchers::hasName;
+using clang::ast_matchers::integerLiteral;
+using clang::ast_matchers::match;
+using clang::ast_matchers::selectFirst;
+using clang::ast_matchers::stringLiteral;
+using clang::ast_matchers::varDecl;
+using clang::tooling::buildASTFromCode;
+using clang::tooling::buildASTFromCodeWithArgs;
+
+template <typename Attr>
+testing::AssertionResult HasAttribute(const FunctionDecl *FD,
+                                      const std::string attrName) {
+  if (FD->hasAttr<Attr>())
+    return testing::AssertionSuccess();
+
+  return testing::AssertionFailure()
+         << *FD << " doesn't have a " << attrName << " attribute";
+}
+
+TEST(Attr, CHERICompartmentName) {
+  // cheri_compartment name is a strange attribute which is both:
+  //
+  // * a function type attribute: affects the calling convention
+  // * a function declaration attribute: marks the function as an compartment
+  // entrypoint
+  //
+  // Thus this attribute is both handled by SemaType and SemaDeclAttr and in
+  // order to not output twice a diagnosis we have to handle it specially. The
+  // following test ensure we don't skip silently the attribute.
+
+  auto AST = buildASTFromCode(R"cpp(
+    #define cheri_compartment(name) __attribute__((cheri_compartment(name)))
+    #define default_compartment cheri_compartment("default_compartment")
+
+    void default_compartment      f_00();
+    default_compartment void      f_01();
+    default_compartment void     *f_03();
+    void default_compartment     *f_04();
+    void * default_compartment    f_05();
+ )cpp");
+
+  {
+    for (auto Node : match(functionDecl().bind("fn"), AST->getASTContext())) {
+      const FunctionDecl *FD = Node.getNodeAs<FunctionDecl>("fn");
+
+      EXPECT_TRUE(
+          HasAttribute<CHERICompartmentNameAttr>(FD, "cheri_compartment"));
+    };
+  }
+}
+
+} // namespace