aaron.ballman created this revision. aaron.ballman added reviewers: rjmccall, joerg, eli.friedman.
An empty string literal in an asm label does not make a whole lot of sense. GCC does not diagnose such a construct, but it also generates code that cannot be assembled by gas should two symbols have an empty asm label within the same TU. This does not affect an asm statement with an empty string literal, which is still a useful construct. The default parameter was converted to a mandated parameter in `ParseSimpleAsm()` because all callers passed the argument anyway. The test code in CodeGen\asm-label.c was removed because it was no longer needed to satisfy the PR (it's covered by test code in Parser\asm.c). https://reviews.llvm.org/D72375 Files: clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Parse/Parser.h clang/lib/Parse/ParseDecl.cpp clang/lib/Parse/ParseDeclCXX.cpp clang/lib/Parse/ParseExprCXX.cpp clang/lib/Parse/ParseStmtAsm.cpp clang/lib/Parse/Parser.cpp clang/test/AST/ast-print-attr.c clang/test/CodeGen/asm-label.c clang/test/Parser/asm.c
Index: clang/test/Parser/asm.c =================================================================== --- clang/test/Parser/asm.c +++ clang/test/Parser/asm.c @@ -20,6 +20,10 @@ asm _Atomic (""); // expected-warning {{ignored _Atomic qualifier on asm}} } +void a() __asm__(""); // expected-error {{cannot use an empty string literal in 'asm'}} +void a() { + __asm__(""); // ok +} // rdar://5952468 __asm ; // expected-error {{expected '(' after 'asm'}} Index: clang/test/CodeGen/asm-label.c =================================================================== --- clang/test/CodeGen/asm-label.c +++ clang/test/CodeGen/asm-label.c @@ -17,15 +17,3 @@ // DARWIN: @"\01bar" = internal global i32 0 // DARWIN: @"\01foo" = common global i32 0 // DARWIN: declare i8* @"\01alias"(i32) - -// PR7887 -int pr7887_1 asm(""); -extern int pr7887_2 asm(""); -int pr7887_3 () asm(""); - -int pt7887_4 () { - static int y asm(""); - y = pr7887_3(); - pr7887_2 = 1; - return pr7887_1; -} Index: clang/test/AST/ast-print-attr.c =================================================================== --- clang/test/AST/ast-print-attr.c +++ clang/test/AST/ast-print-attr.c @@ -11,7 +11,7 @@ // CHECK: using C = int ((*))() __attribute__((cdecl)); using C = int (*)() [[gnu::cdecl]]; -// CHECK: int fun_asm() asm(""); -int fun_asm() asm(""); -// CHECK: int var_asm asm(""); -int var_asm asm(""); +// CHECK: int fun_asm() asm("test"); +int fun_asm() asm("test"); +// CHECK: int var_asm asm("test"); +int var_asm asm("test"); Index: clang/lib/Parse/Parser.cpp =================================================================== --- clang/lib/Parse/Parser.cpp +++ clang/lib/Parse/Parser.cpp @@ -803,7 +803,7 @@ SourceLocation StartLoc = Tok.getLocation(); SourceLocation EndLoc; - ExprResult Result(ParseSimpleAsm(&EndLoc)); + ExprResult Result(ParseSimpleAsm(/*ForAsmLabel*/ false, &EndLoc)); // Check if GNU-style InlineAsm is disabled. // Empty asm string is allowed because it will not introduce @@ -1467,11 +1467,14 @@ /// ParseAsmStringLiteral - This is just a normal string-literal, but is not /// allowed to be a wide string, and is not subject to character translation. +/// Unlike GCC, we also diagnose an empty string literal when parsing for an +/// asm label as opposed to an asm statement, because such a construct does not +/// behave well. /// /// [GNU] asm-string-literal: /// string-literal /// -ExprResult Parser::ParseAsmStringLiteral() { +ExprResult Parser::ParseAsmStringLiteral(bool ForAsmLabel) { if (!isTokenStringLiteral()) { Diag(Tok, diag::err_expected_string_literal) << /*Source='in...'*/0 << "'asm'"; @@ -1487,6 +1490,11 @@ << SL->getSourceRange(); return ExprError(); } + if (ForAsmLabel && SL->getString().empty()) { + Diag(Tok, diag::err_asm_operand_wide_string_literal) + << 2 /* an empty */ << SL->getSourceRange(); + return ExprError(); + } } return AsmString; } @@ -1496,7 +1504,7 @@ /// [GNU] simple-asm-expr: /// 'asm' '(' asm-string-literal ')' /// -ExprResult Parser::ParseSimpleAsm(SourceLocation *EndLoc) { +ExprResult Parser::ParseSimpleAsm(bool ForAsmLabel, SourceLocation *EndLoc) { assert(Tok.is(tok::kw_asm) && "Not an asm!"); SourceLocation Loc = ConsumeToken(); @@ -1516,7 +1524,7 @@ return ExprError(); } - ExprResult Result(ParseAsmStringLiteral()); + ExprResult Result(ParseAsmStringLiteral(ForAsmLabel)); if (!Result.isInvalid()) { // Close the paren and get the location of the end bracket Index: clang/lib/Parse/ParseStmtAsm.cpp =================================================================== --- clang/lib/Parse/ParseStmtAsm.cpp +++ clang/lib/Parse/ParseStmtAsm.cpp @@ -743,7 +743,7 @@ BalancedDelimiterTracker T(*this, tok::l_paren); T.consumeOpen(); - ExprResult AsmString(ParseAsmStringLiteral()); + ExprResult AsmString(ParseAsmStringLiteral(/*ForAsmLabel*/ false)); // Check if GNU-style InlineAsm is disabled. // Error on anything other than empty string. @@ -823,7 +823,7 @@ // Parse the asm-string list for clobbers if present. if (!AteExtraColon && isTokenStringLiteral()) { while (1) { - ExprResult Clobber(ParseAsmStringLiteral()); + ExprResult Clobber(ParseAsmStringLiteral(/*ForAsmLabel*/ false)); if (Clobber.isInvalid()) break; @@ -920,7 +920,7 @@ } else Names.push_back(nullptr); - ExprResult Constraint(ParseAsmStringLiteral()); + ExprResult Constraint(ParseAsmStringLiteral(/*ForAsmLabel*/ false)); if (Constraint.isInvalid()) { SkipUntil(tok::r_paren, StopAtSemi); return true; Index: clang/lib/Parse/ParseExprCXX.cpp =================================================================== --- clang/lib/Parse/ParseExprCXX.cpp +++ clang/lib/Parse/ParseExprCXX.cpp @@ -2017,7 +2017,7 @@ // simple-asm-expr[opt] if (Tok.is(tok::kw_asm)) { SourceLocation Loc; - ExprResult AsmLabel(ParseSimpleAsm(&Loc)); + ExprResult AsmLabel(ParseSimpleAsm(/*ForAsmLabel*/ true, &Loc)); if (AsmLabel.isInvalid()) { SkipUntil(tok::semi, StopAtSemi); return Sema::ConditionError(); Index: clang/lib/Parse/ParseDeclCXX.cpp =================================================================== --- clang/lib/Parse/ParseDeclCXX.cpp +++ clang/lib/Parse/ParseDeclCXX.cpp @@ -2325,7 +2325,7 @@ // If a simple-asm-expr is present, parse it. if (Tok.is(tok::kw_asm)) { SourceLocation Loc; - ExprResult AsmLabel(ParseSimpleAsm(&Loc)); + ExprResult AsmLabel(ParseSimpleAsm(/*ForAsmLabel*/ true, &Loc)); if (AsmLabel.isInvalid()) SkipUntil(tok::comma, StopAtSemi | StopBeforeMatch); Index: clang/lib/Parse/ParseDecl.cpp =================================================================== --- clang/lib/Parse/ParseDecl.cpp +++ clang/lib/Parse/ParseDecl.cpp @@ -2197,7 +2197,7 @@ // If a simple-asm-expr is present, parse it. if (Tok.is(tok::kw_asm)) { SourceLocation Loc; - ExprResult AsmLabel(ParseSimpleAsm(&Loc)); + ExprResult AsmLabel(ParseSimpleAsm(/*ForAsmLabel*/ true, &Loc)); if (AsmLabel.isInvalid()) { SkipUntil(tok::semi, StopBeforeMatch); return true; Index: clang/include/clang/Parse/Parser.h =================================================================== --- clang/include/clang/Parse/Parser.h +++ clang/include/clang/Parse/Parser.h @@ -1508,10 +1508,9 @@ const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo(), LateParsedAttrList *LateParsedAttrs = nullptr); void ParseKNRParamDeclarations(Declarator &D); - // EndLoc, if non-NULL, is filled with the location of the last token of - // the simple-asm. - ExprResult ParseSimpleAsm(SourceLocation *EndLoc = nullptr); - ExprResult ParseAsmStringLiteral(); + // EndLoc is filled with the location of the last token of the simple-asm. + ExprResult ParseSimpleAsm(bool ForAsmLabel, SourceLocation *EndLoc); + ExprResult ParseAsmStringLiteral(bool ForAsmLabel); // Objective-C External Declarations void MaybeSkipAttributes(tok::ObjCKeywordKind Kind); Index: clang/include/clang/Basic/DiagnosticParseKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticParseKinds.td +++ clang/include/clang/Basic/DiagnosticParseKinds.td @@ -265,7 +265,7 @@ def err_address_of_label_outside_fn : Error< "use of address-of-label extension outside of a function body">; def err_asm_operand_wide_string_literal : Error< - "cannot use %select{unicode|wide}0 string literal in 'asm'">; + "cannot use %select{unicode|wide|an empty}0 string literal in 'asm'">; def err_expected_selector_for_method : Error< "expected selector for Objective-C method">; def err_expected_property_name : Error<"expected property name">;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits