diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -91,6 +91,13 @@ 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 @@ - 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 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1090,9 +1090,8 @@ // 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 --- 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; -def warn_file_asm_volatile : Warning< - "meaningless 'volatile' on asm outside function">, CatInlineAsm, - InGroup; +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 @@ "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 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3201,6 +3201,26 @@ 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 --- a/clang/lib/Parse/ParseStmtAsm.cpp +++ b/clang/lib/Parse/ParseStmtAsm.cpp @@ -349,31 +349,13 @@ 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 @@ 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 auto A = getGNUAsmQualifier(Tok); + if (A == GNUAsmQualifiers::AQ_unspecified) { + if (Tok.isNot(tok::l_paren)) { + SkipUntil(tok::r_paren, StopAtSemi); + Diag(Tok.getLocation(), diag::err_asm_qualifier_ignored); + 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 @@ 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 @@ 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 @@ } } } - 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 @@ 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 @@ 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 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1528,13 +1528,13 @@ 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 --- a/clang/test/CodeGen/inline-asm-mixed-style.c +++ b/clang/test/CodeGen/inline-asm-mixed-style.c @@ -14,11 +14,6 @@ // 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 --- /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 (""); +// +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@-1 {{expected ')'}} +// expected-note@-2 {{to match this '('}} diff --git a/clang/test/Parser/asm.c b/clang/test/Parser/asm.c --- a/clang/test/Parser/asm.c +++ b/clang/test/Parser/asm.c @@ -12,12 +12,6 @@ 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 --- a/clang/test/Sema/asm.c +++ b/clang/test/Sema/asm.c @@ -91,9 +91,6 @@ return a; } -// -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.