Allow AsmLabel with -fno-gnu-inline-asm

[steven_wu]

Summary

AsmLabel is heavily used in system level and firmware to redirect
function and access platform specific labels. They are also extensively
used in system headers which makes this option unusable for many
users. Since AsmLabel doesn't introduce any assembly code into the
output binary, it shouldn't be considered as inline-asm.

[rnk]

lgtm

Yeah, the asm label syntax isn't really assembly.

[steven_wu]

Thanks, Reid. Committed in r237048.

Steven

[steven_wu]

Hi Reid

I did more digging in the darwin system headers and realize there are lots more than AsmLabel that prevents the user using this flag correctly.
For example, we have quite many empty file scope assembly blocks asm(“”) which are used to disable tail call optimization.
I think it will be better if we should just ignore system headers rather than special case them. Patch attached.

Steven

diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index ed27a9e..9c5d85d 100644

• a/lib/Parse/Parser.cpp

+++ b/lib/Parse/Parser.cpp
@@ -671,7 +671,8 @@ Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs,

SourceLocation EndLoc;
 
// Check if GNU-style InlineAsm is disabled.

• if (!getLangOpts().GNUAsm)

+ if (!(getLangOpts().GNUAsm ||
+ PP.getSourceManager().isInSystemHeader(StartLoc)))

  Diag(StartLoc, diag::err_gnu_inline_asm_disabled);
 
ExprResult Result(ParseSimpleAsm(&EndLoc));

diff --git a/test/Parser/no-gnu-inline-asm.c b/test/Parser/no-gnu-inline-asm.c
index 7089fa4..bae28de 100644

• a/test/Parser/no-gnu-inline-asm.c

+++ b/test/Parser/no-gnu-inline-asm.c
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 %s -triple i686-apple-darwin -verify -fsyntax-only -fno-gnu-inline-asm

+#include "no-gnu-inline-asm.h" file scope asm is ok in system header
+
asm ("INST r1, 0"); expected-error {{GNU-style inline assembly is disabled}}

void foo() asm("foo_func"); // AsmLabel is OK
diff --git a/test/Parser/no-gnu-inline-asm.h b/test/Parser/no-gnu-inline-asm.h
new file mode 100644
index 0000000..ba12e08

• /dev/null

+++ b/test/Parser/no-gnu-inline-asm.h
@@ -0,0 +1,3 @@
+ Test -fno-gnu-inline-asm in system header
+#pragma clang system_header
+asm(""); file scope asm is OK in system header

[rnk]

I think it might be better to special case empty or white-space only inline asm. That way this flag actually tells you that the .ll file produced doesn't have any instructions already baked into it.

cfe/trunk/lib/Parse/Parser.cpp

case tok::kw___extension__: {
     ConsumeToken();
     return ParseExternalDeclaration(attrs);
   }
   case tok::kw_asm: {
     ProhibitAttributes(attrs);
 
     SourceLocation StartLoc = Tok.getLocation();
     SourceLocation EndLoc;
+
+    // Check if GNU-style InlineAsm is disabled.
+    if (!getLangOpts().GNUAsm)
+      Diag(StartLoc, diag::err_gnu_inline_asm_disabled);
+
     ExprResult Result(ParseSimpleAsm(&EndLoc));
 
     ExpectAndConsume(tok::semi, diag::err_expected_after,
                      "top-level asm block");
 
     if (Result.isInvalid())
       return DeclGroupPtrTy();
     SingleDecl = Actions.ActOnFileScopeAsmDecl(Result.get(), StartLoc, EndLoc);

 ///
 /// [GNU] simple-asm-expr:
 ///         'asm' '(' asm-string-literal ')'
 ///
 ExprResult Parser::ParseSimpleAsm(SourceLocation *EndLoc) {
   assert(Tok.is(tok::kw_asm) && "Not an asm!");
   SourceLocation Loc = ConsumeToken();
 
-  // Check if GNU-style InlineAsm is disabled.
-  if (!getLangOpts().GNUAsm)
-    Diag(Loc, diag::err_gnu_inline_asm_disabled);
-
   if (Tok.is(tok::kw_volatile)) {
     // Remove from the end of 'asm' to the end of 'volatile'.
     SourceRange RemovalRange(PP.getLocForEndOfToken(Loc),
                              PP.getLocForEndOfToken(Tok.getLocation()));
 
     Diag(Tok, diag::warn_file_asm_volatile)
       << FixItHint::CreateRemoval(RemovalRange);
     ConsumeToken();

cfe/trunk/test/Parser/no-gnu-inline-asm.c

 // RUN: %clang_cc1 %s -triple i686-apple-darwin -verify -fsyntax-only -fno-gnu-inline-asm
 
 asm ("INST r1, 0"); // expected-error {{GNU-style inline assembly is disabled}}
+
+void foo() __asm("__foo_func"); // AsmLabel is OK
+int foo1 asm("bar1") = 0; // OK
+
 void f (void) {
   long long foo = 0, bar;
   asm volatile("INST %0, %1" : "=r"(foo) : "r"(bar)); // expected-error {{GNU-style inline assembly is disabled}}
   return;
 }