Diagnose the usage of an MS inline assembly label not defined in the same assembly block

[• ehsan]

Summary

This addresses PR21035. For now, we are unable to represent the
possibility of jumping between asm IR blocks in LLVM, so these kinds of
jumps don't play well with the SSA model. This patch adds a diagnostic
that detect those kinds of jumps, based on the information that the LLVM
side patch exposes from MC.

Note that this diagnostic is unable to differentiate between branches
versus other kinds of label access, so it covers them all.

[rnk]

I think recording the MSAsmStmt in the statement field of the LabelDecl is probably the way to go here. Then we can wait until the end of the function to check what label went where.

lib/Parse/ParseStmtAsm.cpp

50

This can be SmallPtrSet<LabelDecl*, 4>. Sorry, we have a silly STL allergy. :)

118

Do we reject this?

void f(int x) {
  __asm {
  label:
    nop
  }
  x++;
  __asm {
    cmp x, 10
    je label
  }
}

test/Sema/ms-inline-asm.c

109

Hm, this diagnostic is pretty bad. :( We should try returning the found declaration even when we generate the first diagnostic to suppress these follow on diagnostics.

[• ehsan]

In D5694#4, @rnk wrote:

I think recording the MSAsmStmt in the statement field of the LabelDecl is probably the way to go here. Then we can wait until the end of the function to check what label went where.

OK, I'll try doing that.

lib/Parse/ParseStmtAsm.cpp

50

np, I'm not new to that allergy. ;)

118

Ah, no. :( Sorry I must have caught this myself. I'll fix it.

test/Sema/ms-inline-asm.c

109

Can I do that in a separate patch? It's only tangentially related to this change.

[rnk]

test/Sema/ms-inline-asm.c

109

Yeah.

[• ehsan]

Playing with the idea of checking these LabelDecl's at the end of the function, here's another issue I just thought about. At the end of the function we will know which MSAsmStmt has defined a given LabelDecl, but how do we know which LabelDecl has *used* it? Should I record that info in LabelDecl too?

include/clang/Basic/DiagnosticSemaKinds.td

def warn_missing_braces : Warning<
   InGroup<MissingBraces>, DefaultIgnore;
 
 def err_redefinition_of_label : Error<"redefinition of label %0">;
 def err_undeclared_label_use : Error<"use of undeclared label %0">;
 def err_goto_ms_asm_label : Error<
   "cannot jump from this goto statement to label %0 inside an inline assembly block">;
 def note_goto_ms_asm_label : Note<
   "inline assembly label %0 declared here">;
+def err_cross_block_asm_label_use : Error<
+  "cannot use inline assembly label %0 defined outside of an inline assembly block">;
 def warn_unused_label : Warning<"unused label %0">,
   InGroup<UnusedLabel>, DefaultIgnore;
 
 def err_goto_into_protected_scope : Error<
   "cannot jump from this goto statement to its label">;
 def ext_goto_into_protected_scope : ExtWarn<
   "jump from this goto statement to its label is a Microsoft extension">,
   InGroup<Microsoft>;

include/clang/Sema/Sema.h

StmtResult ActOnMSAsmStmt(SourceLocation AsmLoc, SourceLocation LBraceLoc,
                             unsigned NumOutputs, unsigned NumInputs,
                             ArrayRef<StringRef> Constraints,
                             ArrayRef<StringRef> Clobbers,
                             ArrayRef<Expr*> Exprs,
                             SourceLocation EndLoc);
   LabelDecl *GetOrCreateMSAsmLabel(StringRef ExternalLabelName,
                                    SourceLocation Location,
                                    bool AlwaysCreate);
+  void DiagnoseCrossBlockLabelUse(const LabelDecl *Label);
 
   VarDecl *BuildObjCExceptionDecl(TypeSourceInfo *TInfo, QualType ExceptionType,
                                   SourceLocation StartLoc,
                                   SourceLocation IdLoc, IdentifierInfo *Id,
                                   bool Invalid = false);
 
   Decl *ActOnObjCExceptionDecl(Scope *S, Declarator &D);
 

lib/Parse/ParseStmtAsm.cpp

 //===---- ParseStmtAsm.cpp - Assembly Statement Parser --------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements parsing for GCC and Microsoft inline assembly. 
 //
 //===----------------------------------------------------------------------===//
 
+#include <set>
 #include "clang/Parse/Parser.h"
 #include "RAIIObjectsForParser.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
class ClangAsmParserCallback : public llvm::MCAsmParserSemaCallback {
   StringRef AsmString;
 
   /// The tokens we streamed into AsmString and handed off to MC.
   ArrayRef<Token> AsmToks;
 
   /// The offset of each token in AsmToks within AsmString.
   ArrayRef<unsigned> AsmTokOffsets;
 
+  /// The LabelDecls encountered when parsing the asm block.
+  std::set<LabelDecl*> LabelDecls;
+
 public:
   ClangAsmParserCallback(Parser &P, SourceLocation Loc, StringRef AsmString,
                          ArrayRef<Token> Toks, ArrayRef<unsigned> Offsets)
       : TheParser(P), AsmLoc(Loc), AsmString(AsmString), AsmToks(Toks),
         AsmTokOffsets(Offsets) {
     assert(AsmToks.size() == AsmTokOffsets.size());
   }
 
public:
   }
 
   StringRef LookupInlineAsmLabel(StringRef Identifier, llvm::SourceMgr &LSM,
                                  llvm::SMLoc Location,
                                  bool Create) override {
     SourceLocation Loc = translateLocation(LSM, Location);
     LabelDecl *Label =
       TheParser.getActions().GetOrCreateMSAsmLabel(Identifier, Loc, Create);
+    LabelDecls.insert(Label);
     return Label->getMSAsmLabel();
   }
 
   bool LookupInlineAsmField(StringRef Base, StringRef Member,
                             unsigned &Offset) override {
     return TheParser.getActions().LookupInlineAsmField(Base, Member, Offset,
                                                        AsmLoc);
   }
 
+  void CheckExternalLabels() {
+    for (const auto *Label : LabelDecls) {
+      if (!Label->isResolvedMSAsmLabel()) {
+        TheParser.getActions().DiagnoseCrossBlockLabelUse(Label);
+      }
+    }
+  }
+
   static void DiagHandlerCallback(const llvm::SMDiagnostic &D, void *Context) {
     ((ClangAsmParserCallback *)Context)->handleDiagnostic(D);
   }
 
 private:
   /// Collect the appropriate tokens for the given string.
   void findTokensForString(StringRef Str, SmallVectorImpl<Token> &TempToks,
                            const Token *&FirstOrigToken) const {
for (unsigned i = 0, e = NumExprs; i != e; ++i) {
     if (OpExprs[i].second)
       OpExpr =
           Actions.BuildUnaryOp(getCurScope(), AsmLoc, UO_AddrOf, OpExpr).get();
 
     ConstraintRefs[i] = StringRef(Constraints[i]);
     Exprs[i] = OpExpr;
   }
 
+  Callback.CheckExternalLabels();
+
   // FIXME: We should be passing source locations for better diagnostics.
   return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLocs[0], AsmToks, AsmStringIR,
                                 NumOutputs, NumInputs, ConstraintRefs,
                                 ClobberRefs, Exprs, EndLoc);
 }
 
 /// ParseAsmStatement - Parse a GNU extended asm statement.
 ///       asm-statement:

lib/Sema/SemaStmtAsm.cpp

if (AlwaysCreate) {
     // them as resolved.
     Label->setMSAsmLabelResolved();
   }
   // Adjust their location for being able to generate accurate diagnostics.
   Label->setLocation(Location);
 
   return Label;
 }
+
+void Sema::DiagnoseCrossBlockLabelUse(const LabelDecl *Label) {
+  Diag(Label->getLocStart(), diag::err_cross_block_asm_label_use)
+    << Label->getIdentifier();
+}

test/Parser/ms-inline-asm.c

__asm {
       }
     }
   }
 }
 void t11() {
   do { __asm mov eax, 0 __asm { __asm mov edx, 1 } } while(0);
 }
 void t12() {
-  __asm jmp label // expected-error {{use of undeclared label 'label'}}
+  __asm jmp label // expected-error {{use of undeclared label 'label'}} expected-error {{cannot use inline assembly label 'label' defined outside of an inline assembly block}}
+}
+void t13() {
+  __asm {
+    jmp lbl1 // expected-error {{cannot use inline assembly label 'lbl1' defined outside of an inline assembly block}}
+  }
+  __asm {
+    lbl1:
+    nop
+  }
+}
+void t14() {
+  __asm {
+    jmp lbl2 // expected-error {{cannot use inline assembly label 'lbl2' defined outside of an inline assembly block}}
+  }
+  for (;;) {
+    __asm {
+      lbl2:
+      nop
+    }
+  }
+}
+void t15() {
+  for (;;) {
+    __asm {
+      jmp lbl2 // expected-error {{cannot use inline assembly label 'lbl2' defined outside of an inline assembly block}}
+    }
+  }
+  __asm {
+    lbl2:
+    nop
+  }
 }
 int t_fail() { // expected-note {{to match this}}
   __asm 
   __asm { // expected-error 2 {{expected}} expected-note {{to match this}}

test/Sema/ms-inline-asm.c

void t4() {
   __asm { mov eax, fs:[0]. A.a } // expected-error {{Unexpected token type!}}
 }
 
 void test_operand_size() {
   __asm { call word t4 } // expected-error {{Expected 'PTR' or 'ptr' token!}}
 }
 
 __declspec(naked) int t5(int x) { // expected-note {{attribute is here}}
-  asm { movl eax, x } // expected-error {{parameter references not allowed in naked functions}} expected-error {{use of undeclared label 'x'}}
+  asm { movl eax, x } // expected-error {{parameter references not allowed in naked functions}} expected-error {{use of undeclared label 'x'}} expected-error {{cannot use inline assembly label 'x' defined outside of an inline assembly block}}
   asm { retl }
 }
 
 int y;
 __declspec(naked) int t6(int x) {
   asm { mov eax, y } // No error.
   asm { ret }
 }

 void t10() {
   goto foo; // expected-error {{cannot jump from this goto statement to label 'foo' inside an inline assembly block}}
   __asm foo: // expected-note {{inline assembly label 'foo' declared here}}
   __asm mov eax, 0
 }
 
 void t11() {
 foo:
-  __asm mov eax, foo // expected-error {{use of undeclared label 'foo'}} expected-warning {{unused label 'foo'}}
+  __asm mov eax, foo // expected-error {{use of undeclared label 'foo'}} expected-error {{cannot use inline assembly label 'foo' defined outside of an inline assembly block}}
 }
 
 void t12() {
   __asm foo:
   __asm bar: // expected-warning {{unused label 'bar'}}
   __asm jmp foo
 }