Skip to content

Commit

Permalink
[clang] Differentiate between identifier and string EnumArgument (#68550
Browse files Browse the repository at this point in the history
)

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.
  • Loading branch information
s-barannikov committed Feb 18, 2024
1 parent 3496927 commit 2d0137d
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 62 deletions.
84 changes: 48 additions & 36 deletions clang/include/clang/Basic/Attr.td

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/attr-function-return.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/callingconv-iamcu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/callingconv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
1 change: 1 addition & 0 deletions clang/test/Sema/mips-interrupt-attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
1 change: 1 addition & 0 deletions clang/test/Sema/riscv-interrupt-attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/zero_call_used_regs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/warn-consumed-parsing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaHLSL/shader_type_attr.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
80 changes: 63 additions & 17 deletions clang/utils/TableGen/ClangAttrEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 2d0137d

Please sign in to comment.