[clang] 55a51e1 - Disallow an empty string literal in an asm label

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 8 05:39:04 PST 2020


Author: Aaron Ballman
Date: 2020-01-08T08:38:02-05:00
New Revision: 55a51e1c79a21080289ba88d5eac4bbe54ec4272

URL: https://github.com/llvm/llvm-project/commit/55a51e1c79a21080289ba88d5eac4bbe54ec4272
DIFF: https://github.com/llvm/llvm-project/commit/55a51e1c79a21080289ba88d5eac4bbe54ec4272.diff

LOG: Disallow an empty string literal in an asm label

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.

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 63221d8758fc..5f3821fcecb6 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -265,7 +265,7 @@ def err_label_end_of_compound_statement : Error<
 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">;

diff  --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e7130d2fe68e..d41bbd86561c 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1532,10 +1532,9 @@ class Parser : public CodeCompletionHandler {
                  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);

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index a90147ca4692..514a2ede936e 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2197,7 +2197,7 @@ bool Parser::ParseAsmAttributesAfterDeclarator(Declarator &D) {
   // 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;

diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index ca2497ef6121..af3403403c11 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2325,7 +2325,7 @@ bool Parser::ParseCXXMemberDeclaratorBeforeInitializer(
   // 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);
 

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index fecd70b17e5b..a39998482e95 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -2017,7 +2017,7 @@ Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
   // 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();

diff  --git a/clang/lib/Parse/ParseStmtAsm.cpp b/clang/lib/Parse/ParseStmtAsm.cpp
index d35973df921b..ea2c871d6a82 100644
--- a/clang/lib/Parse/ParseStmtAsm.cpp
+++ b/clang/lib/Parse/ParseStmtAsm.cpp
@@ -743,7 +743,7 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
   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 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
     // 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 @@ bool Parser::ParseAsmOperandsOpt(SmallVectorImpl<IdentifierInfo *> &Names,
     } else
       Names.push_back(nullptr);
 
-    ExprResult Constraint(ParseAsmStringLiteral());
+    ExprResult Constraint(ParseAsmStringLiteral(/*ForAsmLabel*/ false));
     if (Constraint.isInvalid()) {
       SkipUntil(tok::r_paren, StopAtSemi);
       return true;

diff  --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index ed4e6ff0fc53..4249de361b89 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -803,7 +803,7 @@ Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
     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 @@ void Parser::ParseKNRParamDeclarations(Declarator &D) {
 
 /// 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 @@ ExprResult Parser::ParseAsmStringLiteral() {
         << 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 @@ ExprResult Parser::ParseAsmStringLiteral() {
 /// [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 @@ ExprResult Parser::ParseSimpleAsm(SourceLocation *EndLoc) {
     return ExprError();
   }
 
-  ExprResult Result(ParseAsmStringLiteral());
+  ExprResult Result(ParseAsmStringLiteral(ForAsmLabel));
 
   if (!Result.isInvalid()) {
     // Close the paren and get the location of the end bracket

diff  --git a/clang/test/AST/ast-print-attr.c b/clang/test/AST/ast-print-attr.c
index 6448437c5eab..d223d607ed50 100644
--- a/clang/test/AST/ast-print-attr.c
+++ b/clang/test/AST/ast-print-attr.c
@@ -11,7 +11,7 @@ using B = int ** __ptr32 *[3];
 // 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");

diff  --git a/clang/test/CodeGen/asm-label.c b/clang/test/CodeGen/asm-label.c
index f944d368f8b3..c06f11fd2d24 100644
--- a/clang/test/CodeGen/asm-label.c
+++ b/clang/test/CodeGen/asm-label.c
@@ -17,15 +17,3 @@ int *test(void) {
 // 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;
-}

diff  --git a/clang/test/Parser/asm.c b/clang/test/Parser/asm.c
index 637f9d7ed42f..b15aae1d9c6f 100644
--- a/clang/test/Parser/asm.c
+++ b/clang/test/Parser/asm.c
@@ -20,6 +20,10 @@ void f2() {
   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'}}


        


More information about the cfe-commits mailing list