[clang] Differentiate between identifier and string EnumArgument (llvm#68550)

s-barannikov · web-flow · commit 2d0137dd6401 · 2024-02-18T17:44:19.000+03:00
EnumArgument may be a string or an identifier. If it is a string, it
should be parsed as unevaluated string literal. Add IsString flag to
EnumArgument so that the parser can choose the correct parsing method.

Target-specific attributes that share spelling may have different
attribute "prototypes". For example, ARM's version of "interrupt"
attribute accepts a string enum, while MSP430's version accepts an
unsigned integer. Adjust ClangAttrEmitter so that the generated
`attributeStringLiteralListArg` returns the correct mask depending on
target triple.

It is worth noting that even after this change some string arguments are
still parsed as identifiers or, worse, as expressions. This is because
of some special logic in `ParseAttributeArgsCommon`. Fixing it is out of
scope of this patch.
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
@@ -291,7 +291,7 @@ static bool attributeHasIdentifierArg(const IdentifierInfo &II) {
 
 /// Determine whether the given attribute has an identifier argument.
 static ParsedAttributeArgumentsProperties
-attributeStringLiteralListArg(const IdentifierInfo &II) {
+attributeStringLiteralListArg(const llvm::Triple &T, const IdentifierInfo &II) {
 #define CLANG_ATTR_STRING_LITERAL_ARG_LIST
   return llvm::StringSwitch<uint32_t>(normalizeAttrName(II.getName()))
 #include "clang/Parse/AttrParserStringSwitches.inc"
@@ -550,7 +550,7 @@ unsigned Parser::ParseAttributeArgsCommon(
 
       ExprVector ParsedExprs;
       ParsedAttributeArgumentsProperties ArgProperties =
-          attributeStringLiteralListArg(*AttrName);
+          attributeStringLiteralListArg(getTargetInfo().getTriple(), *AttrName);
       if (ParseAttributeArgumentList(*AttrName, ParsedExprs, ArgProperties)) {
         SkipUntil(tok::r_paren, StopAtSemi);
         return 0;
diff --git a/clang/test/Sema/attr-function-return.c b/clang/test/Sema/attr-function-return.c
@@ -13,7 +13,7 @@ __attribute__((function_return("thunk-extern"))) void w(void) {}
 // expected-warning@+1 {{'function_return' attribute argument not supported: invalid}}
 __attribute__((function_return("invalid"))) void v(void) {}
 
-// expected-error@+1 {{'function_return' attribute requires a string}}
+// expected-error@+1 {{expected string literal as argument of 'function_return' attribute}}
 __attribute__((function_return(5))) void a(void) {}
 
 // expected-error@+1 {{'function_return' attribute takes one argument}}
diff --git a/clang/test/Sema/callingconv-iamcu.c b/clang/test/Sema/callingconv-iamcu.c
@@ -36,7 +36,7 @@ int __attribute__((pcs("aapcs", "aapcs"))) pcs1(void); // expected-error {{'pcs'
 int __attribute__((pcs())) pcs2(void); // expected-error {{'pcs' attribute takes one argument}}
 int __attribute__((pcs(pcs1))) pcs3(void); // expected-error {{'pcs' attribute requires a string}} \
                                            // expected-error {{invalid PCS type}}
-int __attribute__((pcs(0))) pcs4(void); // expected-error {{'pcs' attribute requires a string}}
+int __attribute__((pcs(0))) pcs4(void); // expected-error {{expected string literal as argument of 'pcs' attribute}}
 /* These are ignored because the target is i386 and not ARM */
 int __attribute__((pcs("aapcs"))) pcs5(void); // expected-warning {{'pcs' calling convention is not supported for this target}}
 int __attribute__((pcs("aapcs-vfp"))) pcs6(void); // expected-warning {{'pcs' calling convention is not supported for this target}}
diff --git a/clang/test/Sema/callingconv.c b/clang/test/Sema/callingconv.c
@@ -45,7 +45,7 @@ int __attribute__((pcs("aapcs", "aapcs"))) pcs1(void); // expected-error {{'pcs'
 int __attribute__((pcs())) pcs2(void); // expected-error {{'pcs' attribute takes one argument}}
 int __attribute__((pcs(pcs1))) pcs3(void); // expected-error {{'pcs' attribute requires a string}} \
                                            // expected-error {{invalid PCS type}}
-int __attribute__((pcs(0))) pcs4(void); // expected-error {{'pcs' attribute requires a string}}
+int __attribute__((pcs(0))) pcs4(void); // expected-error {{expected string literal as argument of 'pcs' attribute}}
 /* These are ignored because the target is i386 and not ARM */
 int __attribute__((pcs("aapcs"))) pcs5(void); // expected-warning {{'pcs' calling convention is not supported for this target}}
 int __attribute__((pcs("aapcs-vfp"))) pcs6(void); // expected-warning {{'pcs' calling convention is not supported for this target}}
diff --git a/clang/test/Sema/mips-interrupt-attr.c b/clang/test/Sema/mips-interrupt-attr.c
@@ -3,6 +3,7 @@ struct a { int b; };
 
 struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions and methods}}
 
+__attribute((interrupt(42))) void foo0(void) {} // expected-error {{expected string literal as argument of 'interrupt' attribute}} 
 __attribute__((interrupt("EIC"))) void foo1(void) {} // expected-warning {{'interrupt' attribute argument not supported: 'EIC'}}
 
 __attribute__((interrupt("eic", 1))) void foo2(void) {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
diff --git a/clang/test/Sema/riscv-interrupt-attr.c b/clang/test/Sema/riscv-interrupt-attr.c
@@ -25,6 +25,7 @@ struct a { int b; };
 
 struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}}
 
+__attribute__((interrupt(42))) void foo0(void) {} // expected-error {{expected string literal as argument of 'interrupt' attribute}}
 __attribute__((interrupt("USER"))) void foo1(void) {} // expected-warning {{'interrupt' attribute argument not supported: USER}}
 __attribute__((interrupt("user"))) void foo1b(void) {} // expected-warning {{'interrupt' attribute argument not supported: user}}
 __attribute__((interrupt("MACHINE"))) void foo1c(void) {} // expected-warning {{'interrupt' attribute argument not supported: MACHINE}}
diff --git a/clang/test/Sema/zero_call_used_regs.c b/clang/test/Sema/zero_call_used_regs.c
@@ -4,7 +4,7 @@
 
 void failure1(void) _zero_call_used_regs();                   // expected-error {{takes one argument}}
 void failure2(void) _zero_call_used_regs("used", "used-gpr"); // expected-error {{takes one argument}}
-void failure3(void) _zero_call_used_regs(0);                  // expected-error {{requires a string}}
+void failure3(void) _zero_call_used_regs(0);                  // expected-error {{expected string literal}}
 void failure4(void) _zero_call_used_regs("hello");            // expected-warning {{argument not supported: hello}}
 
 void success1(void) _zero_call_used_regs("skip");
diff --git a/clang/test/SemaCXX/warn-consumed-parsing.cpp b/clang/test/SemaCXX/warn-consumed-parsing.cpp
@@ -35,7 +35,7 @@ void function3() CONSUMABLE(consumed); // expected-warning {{'consumable' attrib
 
 class CONSUMABLE(unknown) AttrTester1 {
   void callableWhen0()  CALLABLE_WHEN("unconsumed");
-  void callableWhen1()  CALLABLE_WHEN(42); // expected-error {{'callable_when' attribute requires a string}}
+  void callableWhen1()  CALLABLE_WHEN(42); // expected-error {{expected string literal as argument of 'callable_when' attribute}}
   void callableWhen2()  CALLABLE_WHEN("foo"); // expected-warning {{'callable_when' attribute argument not supported: foo}}
   void callableWhen3()  CALLABLE_WHEN(unconsumed);
   void consumes()       SET_TYPESTATE(consumed);
diff --git a/clang/test/SemaHLSL/shader_type_attr.hlsl b/clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -47,9 +47,9 @@ int forwardDecl() {
 
 // expected-error@+1 {{'shader' attribute takes one argument}}
 [shader()]
-// expected-error@+1 {{'shader' attribute takes one argument}}
+// expected-error@+1 {{expected string literal as argument of 'shader' attribute}}
 [shader(1, 2)]
-// expected-error@+1 {{'shader' attribute requires a string}}
+// expected-error@+1 {{expected string literal as argument of 'shader' attribute}}
 [shader(1)]
 // expected-warning@+1 {{'shader' attribute argument not supported: cs}}
 [shader("cs")]
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -171,12 +171,13 @@ static StringRef NormalizeGNUAttrSpelling(StringRef AttrSpelling) {
 typedef std::vector<std::pair<std::string, const Record *>> ParsedAttrMap;
 
 static ParsedAttrMap getParsedAttrList(const RecordKeeper &Records,
-                                       ParsedAttrMap *Dupes = nullptr) {
+                                       ParsedAttrMap *Dupes = nullptr,
+                                       bool SemaOnly = true) {
   std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
   std::set<std::string> Seen;
   ParsedAttrMap R;
   for (const auto *Attr : Attrs) {
-    if (Attr->getValueAsBit("SemaHandler")) {
+    if (!SemaOnly || Attr->getValueAsBit("SemaHandler")) {
       std::string AN;
       if (Attr->isSubClassOf("TargetSpecificAttr") &&
           !Attr->isValueUnset("ParseKind")) {
@@ -2358,19 +2359,21 @@ static bool isVariadicExprArgument(const Record *Arg) {
 }
 
 static bool isStringLiteralArgument(const Record *Arg) {
-  return !Arg->getSuperClasses().empty() &&
-         llvm::StringSwitch<bool>(
-             Arg->getSuperClasses().back().first->getName())
-             .Case("StringArgument", true)
-             .Default(false);
+  if (Arg->getSuperClasses().empty())
+    return false;
+  StringRef ArgKind = Arg->getSuperClasses().back().first->getName();
+  if (ArgKind == "EnumArgument")
+    return Arg->getValueAsBit("IsString");
+  return ArgKind == "StringArgument";
 }
 
 static bool isVariadicStringLiteralArgument(const Record *Arg) {
-  return !Arg->getSuperClasses().empty() &&
-         llvm::StringSwitch<bool>(
-             Arg->getSuperClasses().back().first->getName())
-             .Case("VariadicStringArgument", true)
-             .Default(false);
+  if (Arg->getSuperClasses().empty())
+    return false;
+  StringRef ArgKind = Arg->getSuperClasses().back().first->getName();
+  if (ArgKind == "VariadicEnumArgument")
+    return Arg->getValueAsBit("IsString");
+  return ArgKind == "VariadicStringArgument";
 }
 
 static void emitClangAttrVariadicIdentifierArgList(RecordKeeper &Records,
@@ -2393,14 +2396,18 @@ static void emitClangAttrVariadicIdentifierArgList(RecordKeeper &Records,
   OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
 }
 
+static bool GenerateTargetSpecificAttrChecks(const Record *R,
+                                             std::vector<StringRef> &Arches,
+                                             std::string &Test,
+                                             std::string *FnName);
+
 // Emits the list of arguments that should be parsed as unevaluated string
 // literals for each attribute.
 static void emitClangAttrUnevaluatedStringLiteralList(RecordKeeper &Records,
                                                       raw_ostream &OS) {
   OS << "#if defined(CLANG_ATTR_STRING_LITERAL_ARG_LIST)\n";
-  std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
-  for (const auto *Attr : Attrs) {
-    std::vector<Record *> Args = Attr->getValueAsListOfDefs("Args");
+
+  auto MakeMask = [](ArrayRef<Record *> Args) {
     uint32_t Bits = 0;
     assert(Args.size() <= 32 && "unsupported number of arguments in attribute");
     for (uint32_t N = 0; N < Args.size(); ++N) {
@@ -2411,11 +2418,46 @@ static void emitClangAttrUnevaluatedStringLiteralList(RecordKeeper &Records,
         break;
       }
     }
-    if (!Bits)
+    return Bits;
+  };
+
+  auto AddMaskWithTargetCheck = [](const Record *Attr, uint32_t Mask,
+                                   std::string &MaskStr) {
+    const Record *T = Attr->getValueAsDef("Target");
+    std::vector<StringRef> Arches = T->getValueAsListOfStrings("Arches");
+    std::string Test;
+    GenerateTargetSpecificAttrChecks(T, Arches, Test, nullptr);
+    MaskStr.append(Test + " ? " + std::to_string(Mask) + " : ");
+  };
+
+  ParsedAttrMap Dupes;
+  ParsedAttrMap Attrs = getParsedAttrList(Records, &Dupes, /*SemaOnly=*/false);
+  for (const auto &[AttrName, Attr] : Attrs) {
+    std::string MaskStr;
+    if (Attr->isSubClassOf("TargetSpecificAttr") &&
+        !Attr->isValueUnset("ParseKind")) {
+      if (uint32_t Mask = MakeMask(Attr->getValueAsListOfDefs("Args")))
+        AddMaskWithTargetCheck(Attr, Mask, MaskStr);
+      StringRef ParseKind = Attr->getValueAsString("ParseKind");
+      for (const auto &[DupeParseKind, DupAttr] : Dupes) {
+        if (DupeParseKind != ParseKind)
+          continue;
+        if (uint32_t Mask = MakeMask(DupAttr->getValueAsListOfDefs("Args")))
+          AddMaskWithTargetCheck(DupAttr, Mask, MaskStr);
+      }
+      if (!MaskStr.empty())
+        MaskStr.append("0");
+    } else {
+      if (uint32_t Mask = MakeMask(Attr->getValueAsListOfDefs("Args")))
+        MaskStr = std::to_string(Mask);
+    }
+
+    if (MaskStr.empty())
       continue;
+
     // All these spellings have at least one string literal has argument.
     forEachUniqueSpelling(*Attr, [&](const FlattenedSpelling &S) {
-      OS << ".Case(\"" << S.name() << "\", " << Bits << ")\n";
+      OS << ".Case(\"" << S.name() << "\", " << MaskStr << ")\n";
     });
   }
   OS << "#endif // CLANG_ATTR_STRING_LITERAL_ARG_LIST\n\n";
@@ -3404,6 +3446,8 @@ void EmitClangAttrPCHWrite(RecordKeeper &Records, raw_ostream &OS) {
   OS << "  }\n";
 }
 
+} // namespace clang
+
 // Helper function for GenerateTargetSpecificAttrChecks that alters the 'Test'
 // parameter with only a single check type, if applicable.
 static bool GenerateTargetSpecificAttrCheck(const Record *R, std::string &Test,
@@ -3570,6 +3614,8 @@ static void GenerateHasAttrSpellingStringSwitch(
   OS << "    .Default(0);\n";
 }
 
+namespace clang {
+
 // Emits list of regular keyword attributes with info about their arguments.
 void EmitClangRegularKeywordAttributeInfo(RecordKeeper &Records,
                                           raw_ostream &OS) {
