[clang] 246398e - [clang][Parse] properly parse asm-qualifiers, asm inline
Nick Desaulniers via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 12 15:25:49 PDT 2020
Author: Nick Desaulniers
Date: 2020-03-12T15:13:59-07:00
New Revision: 246398ece7115b57a02dbe7876d86ae8e72406ef
URL: https://github.com/llvm/llvm-project/commit/246398ece7115b57a02dbe7876d86ae8e72406ef
DIFF: https://github.com/llvm/llvm-project/commit/246398ece7115b57a02dbe7876d86ae8e72406ef.diff
LOG: [clang][Parse] properly parse asm-qualifiers, asm inline
Summary:
The parsing of GNU C extended asm statements was a little brittle and
had a few issues:
- It was using Parse::ParseTypeQualifierListOpt to parse the `volatile`
qualifier. That parser is really meant for TypeQualifiers; an asm
statement doesn't really have a type qualifier. This is still maybe
nice to have, but not necessary. We now can check for the `volatile`
token by properly expanding the grammer, rather than abusing
Parse::ParseTypeQualifierListOpt.
- The parsing of `goto` was position dependent, so `asm goto volatile`
wouldn't parse. The qualifiers should be position independent to one
another. Now they are.
- We would warn on duplicate `volatile`, but the parse error for
duplicate `goto` was a generic parse error and wasn't clear.
- We need to add support for the recent GNU C extension `asm inline`.
Adding support to the parser with the above issues highlighted the
need for this refactoring.
Link: https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: aheejin, jfb, nathanchance, cfe-commits, echristo, efriedma, rsmith, chandlerc, craig.topper, erichkeane, jyu2, void, srhines
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75563
Added:
clang/test/Parser/asm-qualifiers.c
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseStmtAsm.cpp
clang/lib/Parse/Parser.cpp
clang/test/CodeGen/inline-asm-mixed-style.c
clang/test/Parser/asm.c
clang/test/Sema/asm.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 664ae4e4167c..710f005985da 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@ Modified Compiler Flags
of a variable in a header file. In some cases, no specific translation unit
provides a definition of the variable. The previous behavior can be restored by
specifying ``-fcommon``.
+- -Wasm-ignored-qualifier (ex. `asm const ("")`) has been removed and replaced
+ with an error (this matches a recent change in GCC-9).
+- -Wasm-file-asm-volatile (ex. `asm volatile ("")` at global scope) has been
+ removed and replaced with an error (this matches GCC's behavior).
+- Duplicate qualifiers on asm statements (ex. `asm volatile volatile ("")`) no
+ longer produces a warning via -Wduplicate-decl-specifier, but now an error
+ (this matches GCC's behavior).
New Pragmas in Clang
--------------------
@@ -111,6 +118,9 @@ C Language Changes in Clang
- The default C language standard used when `-std=` is not specified has been
upgraded from gnu11 to gnu17.
+- Clang now supports the GNU C extension `asm inline`; it won't do anything
+ *yet*, but it will be parsed.
+
- ...
C++ Language Changes in Clang
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 2b11298bfcfa..2c28789aac1d 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1090,9 +1090,8 @@ def ObjCSignedCharBool : DiagGroup<"objc-signed-char-bool",
// Inline ASM warnings.
def ASMOperandWidths : DiagGroup<"asm-operand-widths">;
-def ASMIgnoredQualifier : DiagGroup<"asm-ignored-qualifier">;
def ASM : DiagGroup<"asm", [
- ASMOperandWidths, ASMIgnoredQualifier
+ ASMOperandWidths
]>;
// OpenMP warnings.
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index ec33a0fcfdc7..f22d3a18d254 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -12,11 +12,10 @@
let Component = "Parse" in {
-def warn_asm_qualifier_ignored : Warning<
- "ignored %0 qualifier on asm">, CatInlineAsm, InGroup<ASMIgnoredQualifier>;
-def warn_file_asm_volatile : Warning<
- "meaningless 'volatile' on asm outside function">, CatInlineAsm,
- InGroup<ASMIgnoredQualifier>;
+def err_asm_qualifier_ignored : Error<
+ "expected 'volatile', 'inline', 'goto', or '('">, CatInlineAsm;
+def err_global_asm_qualifier_ignored : Error<
+ "meaningless '%0' on asm outside function">, CatInlineAsm;
let CategoryName = "Inline Assembly Issue" in {
def err_asm_empty : Error<"__asm used with no assembly instructions">;
@@ -29,6 +28,7 @@ def err_gnu_inline_asm_disabled : Error<
"GNU-style inline assembly is disabled">;
def err_asm_goto_cannot_have_output : Error<
"'asm goto' cannot have output constraints">;
+def err_asm_duplicate_qual : Error<"duplicate asm qualifier '%0'">;
}
let CategoryName = "Parse Issue" in {
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 8802bf9bb90a..85844e2edb07 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3201,6 +3201,27 @@ class Parser : public CodeCompletionHandler {
unsigned ArgumentIndex) override;
void CodeCompleteIncludedFile(llvm::StringRef Dir, bool IsAngled) override;
void CodeCompleteNaturalLanguage() override;
+
+ class GNUAsmQualifiers {
+ unsigned Qualifiers = AQ_unspecified;
+
+ public:
+ enum AQ {
+ AQ_unspecified = 0,
+ AQ_volatile = 1,
+ AQ_inline = 2,
+ AQ_goto = 4,
+ };
+ static const char *getQualifierName(AQ Qualifier);
+ bool setAsmQualifier(AQ Qualifier);
+ inline bool isVolatile() const { return Qualifiers & AQ_volatile; };
+ inline bool isInline() const { return Qualifiers & AQ_inline; };
+ inline bool isGoto() const { return Qualifiers & AQ_goto; }
+ };
+ bool isGCCAsmStatement(const Token &TokAfterAsm) const;
+ bool isGNUAsmQualifier(const Token &TokAfterAsm) const;
+ GNUAsmQualifiers::AQ getGNUAsmQualifier(const Token &Tok) const;
+ bool parseGNUAsmQualifierListOpt(GNUAsmQualifiers &AQ);
};
} // end namespace clang
diff --git a/clang/lib/Parse/ParseStmtAsm.cpp b/clang/lib/Parse/ParseStmtAsm.cpp
index bf02dd6c02d4..2e369448ab6a 100644
--- a/clang/lib/Parse/ParseStmtAsm.cpp
+++ b/clang/lib/Parse/ParseStmtAsm.cpp
@@ -349,31 +349,13 @@ static bool buildMSAsmString(Preprocessor &PP, SourceLocation AsmLoc,
return false;
}
-/// isTypeQualifier - Return true if the current token could be the
-/// start of a type-qualifier-list.
-static bool isTypeQualifier(const Token &Tok) {
- switch (Tok.getKind()) {
- default: return false;
- // type-qualifier
- case tok::kw_const:
- case tok::kw_volatile:
- case tok::kw_restrict:
- case tok::kw___private:
- case tok::kw___local:
- case tok::kw___global:
- case tok::kw___constant:
- case tok::kw___generic:
- case tok::kw___read_only:
- case tok::kw___read_write:
- case tok::kw___write_only:
- return true;
- }
+// Determine if this is a GCC-style asm statement.
+bool Parser::isGCCAsmStatement(const Token &TokAfterAsm) const {
+ return TokAfterAsm.is(tok::l_paren) || isGNUAsmQualifier(TokAfterAsm);
}
-// Determine if this is a GCC-style asm statement.
-static bool isGCCAsmStatement(const Token &TokAfterAsm) {
- return TokAfterAsm.is(tok::l_paren) || TokAfterAsm.is(tok::kw_goto) ||
- isTypeQualifier(TokAfterAsm);
+bool Parser::isGNUAsmQualifier(const Token &TokAfterAsm) const {
+ return getGNUAsmQualifier(TokAfterAsm) != GNUAsmQualifiers::AQ_unspecified;
}
/// ParseMicrosoftAsmStatement. When -fms-extensions/-fasm-blocks is enabled,
@@ -684,13 +666,41 @@ StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
ClobberRefs, Exprs, EndLoc);
}
+/// parseGNUAsmQualifierListOpt - Parse a GNU extended asm qualifier list.
+/// asm-qualifier:
+/// volatile
+/// inline
+/// goto
+///
+/// asm-qualifier-list:
+/// asm-qualifier
+/// asm-qualifier-list asm-qualifier
+bool Parser::parseGNUAsmQualifierListOpt(GNUAsmQualifiers &AQ) {
+ while (1) {
+ const GNUAsmQualifiers::AQ A = getGNUAsmQualifier(Tok);
+ if (A == GNUAsmQualifiers::AQ_unspecified) {
+ if (Tok.isNot(tok::l_paren)) {
+ Diag(Tok.getLocation(), diag::err_asm_qualifier_ignored);
+ SkipUntil(tok::r_paren, StopAtSemi);
+ return true;
+ }
+ return false;
+ }
+ if (AQ.setAsmQualifier(A))
+ Diag(Tok.getLocation(), diag::err_asm_duplicate_qual)
+ << GNUAsmQualifiers::getQualifierName(A);
+ ConsumeToken();
+ }
+ return false;
+}
+
/// ParseAsmStatement - Parse a GNU extended asm statement.
/// asm-statement:
/// gnu-asm-statement
/// ms-asm-statement
///
/// [GNU] gnu-asm-statement:
-/// 'asm' type-qualifier[opt] '(' asm-argument ')' ';'
+/// 'asm' asm-qualifier-list[opt] '(' asm-argument ')' ';'
///
/// [GNU] asm-argument:
/// asm-string-literal
@@ -712,34 +722,11 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
return ParseMicrosoftAsmStatement(AsmLoc);
}
- DeclSpec DS(AttrFactory);
SourceLocation Loc = Tok.getLocation();
- ParseTypeQualifierListOpt(DS, AR_VendorAttributesParsed);
-
- // GNU asms accept, but warn, about type-qualifiers other than volatile.
- if (DS.getTypeQualifiers() & DeclSpec::TQ_const)
- Diag(Loc, diag::warn_asm_qualifier_ignored) << "const";
- if (DS.getTypeQualifiers() & DeclSpec::TQ_restrict)
- Diag(Loc, diag::warn_asm_qualifier_ignored) << "restrict";
- // FIXME: Once GCC supports _Atomic, check whether it permits it here.
- if (DS.getTypeQualifiers() & DeclSpec::TQ_atomic)
- Diag(Loc, diag::warn_asm_qualifier_ignored) << "_Atomic";
-
- // Remember if this was a volatile asm.
- bool isVolatile = DS.getTypeQualifiers() & DeclSpec::TQ_volatile;
- // Remember if this was a goto asm.
- bool isGotoAsm = false;
-
- if (Tok.is(tok::kw_goto)) {
- isGotoAsm = true;
- ConsumeToken();
- }
-
- if (Tok.isNot(tok::l_paren)) {
- Diag(Tok, diag::err_expected_lparen_after) << "asm";
- SkipUntil(tok::r_paren, StopAtSemi);
+ GNUAsmQualifiers GAQ;
+ if (parseGNUAsmQualifierListOpt(GAQ))
return StmtError();
- }
+
BalancedDelimiterTracker T(*this, tok::l_paren);
T.consumeOpen();
@@ -767,11 +754,10 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
if (Tok.is(tok::r_paren)) {
// We have a simple asm expression like 'asm("foo")'.
T.consumeClose();
- return Actions.ActOnGCCAsmStmt(AsmLoc, /*isSimple*/ true, isVolatile,
- /*NumOutputs*/ 0, /*NumInputs*/ 0, nullptr,
- Constraints, Exprs, AsmString.get(),
- Clobbers, /*NumLabels*/ 0,
- T.getCloseLocation());
+ return Actions.ActOnGCCAsmStmt(
+ AsmLoc, /*isSimple*/ true, GAQ.isVolatile(),
+ /*NumOutputs*/ 0, /*NumInputs*/ 0, nullptr, Constraints, Exprs,
+ AsmString.get(), Clobbers, /*NumLabels*/ 0, T.getCloseLocation());
}
// Parse Outputs, if present.
@@ -829,7 +815,7 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
}
}
}
- if (!isGotoAsm && (Tok.isNot(tok::r_paren) || AteExtraColon)) {
+ if (!GAQ.isGoto() && (Tok.isNot(tok::r_paren) || AteExtraColon)) {
Diag(Tok, diag::err_expected) << tok::r_paren;
SkipUntil(tok::r_paren, StopAtSemi);
return StmtError();
@@ -862,16 +848,16 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
if (!TryConsumeToken(tok::comma))
break;
}
- } else if (isGotoAsm) {
+ } else if (GAQ.isGoto()) {
Diag(Tok, diag::err_expected) << tok::colon;
SkipUntil(tok::r_paren, StopAtSemi);
return StmtError();
}
T.consumeClose();
- return Actions.ActOnGCCAsmStmt(
- AsmLoc, false, isVolatile, NumOutputs, NumInputs, Names.data(),
- Constraints, Exprs, AsmString.get(), Clobbers, NumLabels,
- T.getCloseLocation());
+ return Actions.ActOnGCCAsmStmt(AsmLoc, false, GAQ.isVolatile(), NumOutputs,
+ NumInputs, Names.data(), Constraints, Exprs,
+ AsmString.get(), Clobbers, NumLabels,
+ T.getCloseLocation());
}
/// ParseAsmOperands - Parse the asm-operands production as used by
@@ -942,3 +928,28 @@ bool Parser::ParseAsmOperandsOpt(SmallVectorImpl<IdentifierInfo *> &Names,
return false;
}
}
+
+const char *Parser::GNUAsmQualifiers::getQualifierName(AQ Qualifier) {
+ switch (Qualifier) {
+ case AQ_volatile: return "volatile";
+ case AQ_inline: return "inline";
+ case AQ_goto: return "goto";
+ case AQ_unspecified: return "unspecified";
+ }
+ llvm_unreachable("Unkown GNUAsmQualifier");
+}
+
+Parser::GNUAsmQualifiers::AQ
+Parser::getGNUAsmQualifier(const Token &Tok) const {
+ switch (Tok.getKind()) {
+ case tok::kw_volatile: return GNUAsmQualifiers::AQ_volatile;
+ case tok::kw_inline: return GNUAsmQualifiers::AQ_inline;
+ case tok::kw_goto: return GNUAsmQualifiers::AQ_goto;
+ default: return GNUAsmQualifiers::AQ_unspecified;
+ }
+}
+bool Parser::GNUAsmQualifiers::setAsmQualifier(AQ Qualifier) {
+ bool IsDuplicate = Qualifiers & Qualifier;
+ Qualifiers |= Qualifier;
+ return IsDuplicate;
+}
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index ffe31f82a693..27cb8a2a5e76 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1529,13 +1529,13 @@ ExprResult Parser::ParseSimpleAsm(bool ForAsmLabel, SourceLocation *EndLoc) {
assert(Tok.is(tok::kw_asm) && "Not an asm!");
SourceLocation Loc = ConsumeToken();
- if (Tok.is(tok::kw_volatile)) {
- // Remove from the end of 'asm' to the end of 'volatile'.
+ if (isGNUAsmQualifier(Tok)) {
+ // Remove from the end of 'asm' to the end of the asm qualifier.
SourceRange RemovalRange(PP.getLocForEndOfToken(Loc),
PP.getLocForEndOfToken(Tok.getLocation()));
-
- Diag(Tok, diag::warn_file_asm_volatile)
- << FixItHint::CreateRemoval(RemovalRange);
+ Diag(Tok, diag::err_global_asm_qualifier_ignored)
+ << GNUAsmQualifiers::getQualifierName(getGNUAsmQualifier(Tok))
+ << FixItHint::CreateRemoval(RemovalRange);
ConsumeToken();
}
diff --git a/clang/test/CodeGen/inline-asm-mixed-style.c b/clang/test/CodeGen/inline-asm-mixed-style.c
index a9e111cd5ddc..b11507fd4eca 100644
--- a/clang/test/CodeGen/inline-asm-mixed-style.c
+++ b/clang/test/CodeGen/inline-asm-mixed-style.c
@@ -14,11 +14,6 @@ void f() {
// CHECK: movl %ebx, %eax
// CHECK: movl %ecx, %edx
- __asm mov eax, ebx
- __asm const ("movl %ecx, %edx"); // expected-warning {{ignored const qualifier on asm}}
- // CHECK: movl %ebx, %eax
- // CHECK: movl %ecx, %edx
-
__asm volatile goto ("movl %ecx, %edx");
// CHECK: movl %ecx, %edx
diff --git a/clang/test/Parser/asm-qualifiers.c b/clang/test/Parser/asm-qualifiers.c
new file mode 100644
index 000000000000..d18336dfd7c8
--- /dev/null
+++ b/clang/test/Parser/asm-qualifiers.c
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify %s
+
+void qualifiers(void) {
+ asm("");
+ asm volatile("");
+ asm inline("");
+ asm goto("" ::::foo);
+foo:;
+}
+
+void unknown_qualifiers(void) {
+ asm noodle(""); // expected-error {{expected 'volatile', 'inline', 'goto', or '('}}
+ asm goto noodle("" ::::foo); // expected-error {{expected 'volatile', 'inline', 'goto', or '('}}
+ asm volatile noodle inline(""); // expected-error {{expected 'volatile', 'inline', 'goto', or '('}}
+foo:;
+}
+
+void underscores(void) {
+ __asm__("");
+ __asm__ __volatile__("");
+ __asm__ __inline__("");
+ // Note: goto is not supported with underscore prefix+suffix.
+ __asm__ goto("" ::::foo);
+foo:;
+}
+
+void permutations(void) {
+ asm goto inline volatile("" ::::foo);
+ asm goto inline("");
+ asm goto volatile inline("" ::::foo);
+ asm goto volatile("");
+ asm inline goto volatile("" ::::foo);
+ asm inline goto("" ::::foo);
+ asm inline volatile goto("" ::::foo);
+ asm inline volatile("");
+ asm volatile goto("" ::::foo);
+ asm volatile inline goto("" ::::foo);
+ asm volatile inline("");
+foo:;
+}
+
+void duplicates(void) {
+ asm volatile volatile(""); // expected-error {{duplicate asm qualifier 'volatile'}}
+ __asm__ __volatile__ __volatile__(""); // expected-error {{duplicate asm qualifier 'volatile'}}
+ asm inline inline(""); // expected-error {{duplicate asm qualifier 'inline'}}
+ __asm__ __inline__ __inline__(""); // expected-error {{duplicate asm qualifier 'inline'}}
+ asm goto goto("" ::::foo); // expected-error {{duplicate asm qualifier 'goto'}}
+ __asm__ goto goto("" ::::foo); // expected-error {{duplicate asm qualifier 'goto'}}
+foo:;
+}
+
+// globals
+asm ("");
+// <rdar://problem/7574870>
+asm volatile (""); // expected-error {{meaningless 'volatile' on asm outside function}}
+asm inline (""); // expected-error {{meaningless 'inline' on asm outside function}}
+asm goto (""::::noodle); // expected-error {{meaningless 'goto' on asm outside function}}
+// expected-error at -1 {{expected ')'}}
+// expected-note at -2 {{to match this '('}}
diff --git a/clang/test/Parser/asm.c b/clang/test/Parser/asm.c
index b15aae1d9c6f..b4cadee4669d 100644
--- a/clang/test/Parser/asm.c
+++ b/clang/test/Parser/asm.c
@@ -12,12 +12,6 @@ void f1() {
void f2() {
asm("foo" : "=r" (a)); // expected-error {{use of undeclared identifier 'a'}}
asm("foo" : : "r" (b)); // expected-error {{use of undeclared identifier 'b'}}
-
- asm const (""); // expected-warning {{ignored const qualifier on asm}}
- asm volatile ("");
- asm restrict (""); // expected-warning {{ignored restrict qualifier on asm}}
- // FIXME: Once GCC supports _Atomic, check whether it allows this.
- asm _Atomic (""); // expected-warning {{ignored _Atomic qualifier on asm}}
}
void a() __asm__(""); // expected-error {{cannot use an empty string literal in 'asm'}}
diff --git a/clang/test/Sema/asm.c b/clang/test/Sema/asm.c
index 29a55c610de4..c5620fd2dc6b 100644
--- a/clang/test/Sema/asm.c
+++ b/clang/test/Sema/asm.c
@@ -91,9 +91,6 @@ int test7(unsigned long long b) {
return a;
}
-// <rdar://problem/7574870>
-asm volatile (""); // expected-warning {{meaningless 'volatile' on asm outside function}}
-
// PR3904
void test8(int i) {
// A number in an input constraint can't point to a read-write constraint.
More information about the cfe-commits
mailing list