[clang] [Parser][BoundsSafety] Print attribute as macro if it's system defined (PR #107619)

Yeoul Na via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 6 14:17:18 PDT 2024


https://github.com/rapidsna updated https://github.com/llvm/llvm-project/pull/107619

>From 1a0e2dede95bd69ab6badc9aafbd60d5eaf5d407 Mon Sep 17 00:00:00 2001
From: Yeoul Na <yeoul_na at apple.com>
Date: Thu, 5 Sep 2024 09:52:41 -0700
Subject: [PATCH] [Parser][BoundsSafety] Print attribute as macro if it's
 system defined

This correctly adds macro identifier for late parsed attrs, which has
been missing. 'operator<<' for StreamingDiagnostic and ParsedAttr
checks if Attr should be printed as macro and does it.

This also fixes the bug that the macro identifier included the
parenthesis and arguments.
---
 clang/include/clang/Sema/ParsedAttr.h         | 39 +++++++++++++------
 clang/lib/Parse/ParseCXXInlineMethods.cpp     | 11 ++++++
 clang/lib/Parse/ParseDecl.cpp                 | 18 ++++++++-
 .../test/Frontend/Inputs/macro_defined_type.h |  4 ++
 clang/test/Frontend/macro_defined_type.cpp    | 18 ++++++++-
 .../Inputs/warn-thread-safety-parsing.h       |  3 ++
 .../SemaCXX/warn-thread-safety-parsing.cpp    | 21 ++++++++--
 7 files changed, 96 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/Frontend/Inputs/macro_defined_type.h
 create mode 100644 clang/test/SemaCXX/Inputs/warn-thread-safety-parsing.h

diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index 22cbd0d90ee432..29ddfada0f9371 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -191,6 +191,10 @@ class ParsedAttr final
   LLVM_PREFERRED_TYPE(bool)
   mutable unsigned IsPragmaClangAttribute : 1;
 
+  /// Determines if the attribute will be printed as macro in the diagnostics.
+  LLVM_PREFERRED_TYPE(bool)
+  mutable unsigned PrintAsMacro : 1;
+
   /// The location of the 'unavailable' keyword in an
   /// availability attribute.
   SourceLocation UnavailableLoc;
@@ -225,7 +229,7 @@ class ParsedAttr final
         UsedAsTypeAttr(false), IsAvailability(false),
         IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
         HasProcessingCache(false), IsPragmaClangAttribute(false),
-        Info(ParsedAttrInfo::get(*this)) {
+        PrintAsMacro(false), Info(ParsedAttrInfo::get(*this)) {
     if (numArgs)
       memcpy(getArgsBuffer(), args, numArgs * sizeof(ArgsUnion));
   }
@@ -242,8 +246,8 @@ class ParsedAttr final
         NumArgs(1), Invalid(false), UsedAsTypeAttr(false), IsAvailability(true),
         IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
         HasProcessingCache(false), IsPragmaClangAttribute(false),
-        UnavailableLoc(unavailable), MessageExpr(messageExpr),
-        Info(ParsedAttrInfo::get(*this)) {
+        PrintAsMacro(false), UnavailableLoc(unavailable),
+        MessageExpr(messageExpr), Info(ParsedAttrInfo::get(*this)) {
     ArgsUnion PVal(Parm);
     memcpy(getArgsBuffer(), &PVal, sizeof(ArgsUnion));
     new (getAvailabilityData())
@@ -260,7 +264,8 @@ class ParsedAttr final
         NumArgs(3), Invalid(false), UsedAsTypeAttr(false),
         IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(false),
         HasParsedType(false), HasProcessingCache(false),
-        IsPragmaClangAttribute(false), Info(ParsedAttrInfo::get(*this)) {
+        IsPragmaClangAttribute(false), PrintAsMacro(false),
+        Info(ParsedAttrInfo::get(*this)) {
     ArgsUnion *Args = getArgsBuffer();
     Args[0] = Parm1;
     Args[1] = Parm2;
@@ -276,7 +281,8 @@ class ParsedAttr final
         NumArgs(1), Invalid(false), UsedAsTypeAttr(false),
         IsAvailability(false), IsTypeTagForDatatype(true), IsProperty(false),
         HasParsedType(false), HasProcessingCache(false),
-        IsPragmaClangAttribute(false), Info(ParsedAttrInfo::get(*this)) {
+        IsPragmaClangAttribute(false), PrintAsMacro(false),
+        Info(ParsedAttrInfo::get(*this)) {
     ArgsUnion PVal(ArgKind);
     memcpy(getArgsBuffer(), &PVal, sizeof(ArgsUnion));
     detail::TypeTagForDatatypeData &ExtraData = getTypeTagForDatatypeDataSlot();
@@ -294,7 +300,7 @@ class ParsedAttr final
         UsedAsTypeAttr(false), IsAvailability(false),
         IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(true),
         HasProcessingCache(false), IsPragmaClangAttribute(false),
-        Info(ParsedAttrInfo::get(*this)) {
+        PrintAsMacro(false), Info(ParsedAttrInfo::get(*this)) {
     new (&getTypeBuffer()) ParsedType(typeArg);
   }
 
@@ -306,7 +312,8 @@ class ParsedAttr final
         NumArgs(0), Invalid(false), UsedAsTypeAttr(false),
         IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(true),
         HasParsedType(false), HasProcessingCache(false),
-        IsPragmaClangAttribute(false), Info(ParsedAttrInfo::get(*this)) {
+        IsPragmaClangAttribute(false), PrintAsMacro(false),
+        Info(ParsedAttrInfo::get(*this)) {
     new (&getPropertyDataBuffer()) detail::PropertyData(getterId, setterId);
   }
 
@@ -493,9 +500,11 @@ class ParsedAttr final
   /// Set the macro identifier info object that this parsed attribute was
   /// declared in if it was declared in a macro. Also set the expansion location
   /// of the macro.
-  void setMacroIdentifier(IdentifierInfo *MacroName, SourceLocation Loc) {
+  void setMacroIdentifier(IdentifierInfo *MacroName, SourceLocation Loc,
+                          bool PrintAsMac) {
     MacroII = MacroName;
     MacroExpansionLoc = Loc;
+    PrintAsMacro = PrintAsMac;
   }
 
   /// Returns true if this attribute was declared in a macro.
@@ -511,6 +520,12 @@ class ParsedAttr final
     return MacroExpansionLoc;
   }
 
+  bool printAsMacro() const {
+    if (!hasMacroIdentifier())
+      return false;
+    return PrintAsMacro;
+  }
+
   /// Check if the attribute has exactly as many args as Num. May output an
   /// error. Returns false if a diagnostic is produced.
   bool checkExactlyNumArgs(class Sema &S, unsigned Num) const;
@@ -1105,16 +1120,16 @@ enum AttributeDeclKind {
 
 inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                              const ParsedAttr &At) {
-  DB.AddTaggedVal(reinterpret_cast<uint64_t>(At.getAttrName()),
+  const IdentifierInfo *AttrName =
+      At.printAsMacro() ? At.getMacroIdentifier() : At.getAttrName();
+  DB.AddTaggedVal(reinterpret_cast<uint64_t>(AttrName),
                   DiagnosticsEngine::ak_identifierinfo);
   return DB;
 }
 
 inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                              const ParsedAttr *At) {
-  DB.AddTaggedVal(reinterpret_cast<uint64_t>(At->getAttrName()),
-                  DiagnosticsEngine::ak_identifierinfo);
-  return DB;
+  return DB << *At;
 }
 
 /// AttributeCommonInfo has a non-explicit constructor which takes an
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index b461743833c82a..1600e5b719ae4b 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -835,6 +835,17 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA,
     Diag(Tok, diag::warn_attribute_on_function_definition)
       << &LA.AttrName;
 
+  const auto &SM = PP.getSourceManager();
+  CharSourceRange ExpansionRange = SM.getExpansionRange(LA.AttrNameLoc);
+  StringRef FoundName =
+      Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts())
+          .split('(')
+          .first;
+  IdentifierInfo *MacroII = PP.getIdentifierInfo(FoundName);
+  for (unsigned i = 0; i < Attrs.size(); ++i)
+    Attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin(),
+                                SM.isInSystemMacro(LA.AttrNameLoc));
+
   for (unsigned i = 0, ni = LA.Decls.size(); i < ni; ++i)
     Actions.ActOnFinishDelayedAttribute(getCurScope(), LA.Decls[i], Attrs);
 
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 78d729c5ef7d8a..c08a3cc2c49bd9 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -297,11 +297,14 @@ void Parser::ParseGNUAttributes(ParsedAttributes &Attrs,
         FindLocsWithCommonFileID(PP, AttrTokLoc, Loc)) {
       CharSourceRange ExpansionRange = SM.getExpansionRange(AttrTokLoc);
       StringRef FoundName =
-          Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts());
+          Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts())
+              .split('(')
+              .first;
       IdentifierInfo *MacroII = PP.getIdentifierInfo(FoundName);
 
       for (unsigned i = OldNumAttrs; i < Attrs.size(); ++i)
-        Attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin());
+        Attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin(),
+                                    SM.isInSystemMacro(AttrTokLoc));
 
       if (LateAttrs) {
         for (unsigned i = OldNumLateAttrs; i < LateAttrs->size(); ++i)
@@ -5055,6 +5058,17 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute &LA, bool EnterScope,
   ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, nullptr, nullptr,
                         SourceLocation(), ParsedAttr::Form::GNU(), nullptr);
 
+  const auto &SM = PP.getSourceManager();
+  CharSourceRange ExpansionRange = SM.getExpansionRange(LA.AttrNameLoc);
+  StringRef FoundName =
+      Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts())
+          .split('(')
+          .first;
+  IdentifierInfo *MacroII = PP.getIdentifierInfo(FoundName);
+  for (unsigned i = 0; i < Attrs.size(); ++i)
+    Attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin(),
+                                SM.isInSystemMacro(LA.AttrNameLoc));
+
   for (auto *D : LA.Decls)
     Actions.ActOnFinishDelayedAttribute(getCurScope(), D, Attrs);
 
diff --git a/clang/test/Frontend/Inputs/macro_defined_type.h b/clang/test/Frontend/Inputs/macro_defined_type.h
new file mode 100644
index 00000000000000..fa90b5e9e6a1c8
--- /dev/null
+++ b/clang/test/Frontend/Inputs/macro_defined_type.h
@@ -0,0 +1,4 @@
+#pragma clang system_header
+
+#define _SYS_NODEREF __attribute__((noderef))
+#define _SYS_LIBCPP_FLOAT_ABI __attribute__((pcs("aapcs")))
diff --git a/clang/test/Frontend/macro_defined_type.cpp b/clang/test/Frontend/macro_defined_type.cpp
index 71a0ff18477d8c..e796d9b37132ca 100644
--- a/clang/test/Frontend/macro_defined_type.cpp
+++ b/clang/test/Frontend/macro_defined_type.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-gnu %s
+// RUN: %clang_cc1 -I%S/Inputs -fsyntax-only -verify -triple x86_64-linux-gnu %s
+
+#include <macro_defined_type.h>
 
 #define NODEREF __attribute__((noderef))
 
@@ -14,12 +16,26 @@ void Func() {
   auto NODEREF auto_i2 = i; // expected-warning{{'noderef' can only be used on an array or pointer type}}
 }
 
+// The diagnostic message is hard-coded as 'noderef' so using a system macro doesn't change the behavior
+void Func_system_macro() {
+  int _SYS_NODEREF i; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int _SYS_NODEREF *i_ptr;
+
+  auto _SYS_NODEREF *auto_i_ptr2 = i_ptr;
+  auto _SYS_NODEREF auto_i2 = i; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+
 // Added test for fix for P41835
 #define _LIBCPP_FLOAT_ABI __attribute__((pcs("aapcs")))
 struct A {
   _LIBCPP_FLOAT_ABI int operator()() throw(); // expected-warning{{'pcs' calling convention is not supported for this target}}
 };
 
+struct A_system_macro {
+  _SYS_LIBCPP_FLOAT_ABI int operator()() throw(); // expected-warning{{'_SYS_LIBCPP_FLOAT_ABI' calling convention is not supported for this target}}
+};
+
 // Added test for fix for PR43315
 #define a __attribute__((__cdecl__, __regparm__(0)))
 int(a b)();
diff --git a/clang/test/SemaCXX/Inputs/warn-thread-safety-parsing.h b/clang/test/SemaCXX/Inputs/warn-thread-safety-parsing.h
new file mode 100644
index 00000000000000..1c55e9f6684788
--- /dev/null
+++ b/clang/test/SemaCXX/Inputs/warn-thread-safety-parsing.h
@@ -0,0 +1,3 @@
+#pragma clang system_header
+
+#define _SYS_GUARDED_BY(x)       __attribute__ ((guarded_by(x)))
\ No newline at end of file
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 0c5b0cc85897be..9bd3b9eb108aaf 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s -D CPP11
+// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -verify -Wthread-safety %s
+// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -verify -Wthread-safety -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -verify -Wthread-safety -std=c++11 %s -D CPP11
+
+#include <warn-thread-safety-parsing.h>
 
 #define LOCKABLE            __attribute__ ((lockable))
 #define SCOPED_LOCKABLE     __attribute__ ((scoped_lockable))
@@ -340,6 +342,19 @@ int gb_testfn(int y){
   return x;
 }
 
+void gb_function_sys_macro() _SYS_GUARDED_BY(mu1); // \
+  // expected-warning {{'_SYS_GUARDED_BY' attribute only applies to}}
+
+void gb_function_params_sys_macro(int gv_lvar _SYS_GUARDED_BY(mu1)); // \
+  // expected-warning {{'_SYS_GUARDED_BY' attribute only applies to}}
+
+int gb_testfn_sys_macro(int y){
+  int x _SYS_GUARDED_BY(mu1) = y; // \
+    // expected-warning {{'_SYS_GUARDED_BY' attribute only applies to}}
+  return x;
+}
+
+
 //2. Check argument parsing.
 
 // legal attribute arguments



More information about the cfe-commits mailing list