ms-inline-asm: Scope inline asm labels to functions

[• ehsan]

Summary

This fixes PR20023. In order to implement this scoping rule, we maintain
a map from the label's name as it appears in the source code to an internal
name which is guaranteed to be an invalid mangled name. Every time that we
encounter a label, we add an entry to this internal map, and when we fail
to parse something as a valid identifier, we get a LookupInlineAsmLabel
callback from MC which allows us to optimistically interpret the identifier
as a label name. By the time that we pop the function's scope, we go
through this map, and for each entry verify that we have seen a label
somewhere in the function body, and emit a diagnostic otherwise. This has
the undesired side effect that for some unknown names, we may issue an
extra diagnostic (as can be seen in test/Sema/ms-inline-asm.c.

[rnk]

include/clang/Sema/Sema.h

3134 ↗

(On Diff #11659)

micro nit: & on the right.

lib/Parse/ParseStmtAsm.cpp

308–312 ↗

(On Diff #11659)

This seems like the responsibility of the assembly parser. IMO it should call back into clang when it encounters a label in inline assembly mode, and then clang can set up the mapping.

test/CodeGen/ms-inline-asm.c

499–500 ↗

(On Diff #11659)

Can you use the asm { ... } syntax for the test case? In the future, we need to lower consecutive asm statements into a single LLVM inline assembly blob, because right now LLVM can insert spills and restores between the two blobs. In other words, you can't rely on this today:

__asm mov eax, x
__asm test eax, eax ; eax may be different, but we usually have no reason to spill here

If you use the __asm { } syntax, we won't have to change this test in the future.

[• ehsan]

lib/Parse/ParseStmtAsm.cpp

308–312 ↗

(On Diff #11659)

Do we have access to the sema callback there? AFAICT we don't... :/

test/CodeGen/ms-inline-asm.c

499–500 ↗

(On Diff #11659)

Ah, this reminds me that I intended to work on a patch to merge these __asm blocks. :-)

[• ehsan]

lib/Parse/ParseStmtAsm.cpp

308–312 ↗

(On Diff #11659)

Actually I think I can make this work... Will upload a new patch later today.

[• ehsan]

Addressed the review comments.

[• ehsan]

ping?

[rnk]

lib/Parse/ParseStmtAsm.cpp

96 ↗

(On Diff #11687)

You can return a StringRef directly.

lib/Sema/SemaDecl.cpp

1492 ↗

(On Diff #11687)

I believe ActOnPopScope happens on any closing curly brace, so this won't do the "right" thing:

void f() {
  bool failed;
  __asm retry:
  {
    failed = g();
  } // The label is cleared here.
  if (failed)
    __asm jmp retry
}

Thinking more carefully, I don't think having the label map right on Sema works at all in the general case of nested functions. Consider this C++ example with inner classes:

void f() {
  __asm some_label:
  struct A {
    static void g() {
      __asm jmp some_label ; This should jump forwards
      __asm some_label:
      __asm nop
    }
  };
}

You can build similar test cases from C++11 lambdas and Obj-C blocks, and now OpenMP captured statements.

We need to associate this label map with the function we're currently processing. Now that I'm considering the actual implementation difficulties, I'm going back to your "proposal #3" on PR20023, where we model these labels as regular label statements. That way, we reuse all the right name lookup machinery. We won't be able to 'goto' a label from an inline asm statement or 'jmp' to a non-asm label, so if you go this way, please diagnose both cases. You can probably add a bit to LabelDecl to track whether the label was from MS inline asm or regular source code.

Apologies for the runaround, but I think it's probably better to rewrite along these lines. What do you think?

[• ehsan]

Updated the patch to use the LabelDecl machinery.

Note that there are a couple of issues with the diagnostics that I'm generating (see the TODO comments.)

The first issue is that I currently diagnose jumping to an asm label from a goto statement in ActOnGotoStmt, which means that we currently can't diagnose the case where we first see the goto statement, generate an implicit label from it, and later on see an asm block which defines a label with that name. The reason why the proper diagnostic is hard is that I don't know where the right place to perform it is. Note that we need access to the SourceLocation of the GotoStmt and also to the SourceLocation of the place where the label is declared in the asm block.

The second issue with the diagnostics is that as I'm generating the "use of undeclared label" error, I only have access to the LabelDecl and its location, which means that for example the diagnostic in t10 in test/Sema/ms-inline-asm.c is misplaced.

Any help with fixing the above two issues is appreciated.

Also, I have tested most of the combinations of goto's, jumps and asm/non-asm labels that I have thought of, but I may have missed something. If you see cases for which I have not included a test case, please point them out!

[rnk]

Nice, this completely eliminates the lookup difficulties in the previous approach.

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

This will cause a memory leak for names > 16 chars. When the ASTContext is destroyed, it doesn't actually call destructors of individual AST nodes, it merely frees the memory. This should be a StringRef that points to memory from 'new (Context, 1) char[Len]', probably.

lib/Parse/ParseStmtAsm.cpp

99–101 ↗

(On Diff #13793)

I think you can sink this into translateLocation if you make it take a SourceMgr and SMLoc.

lib/Sema/SemaStmt.cpp

2357–2358 ↗

(On Diff #13793)

Does it help if you issue the same diagnostic when processing 'foo:' in this example?

void f() {
  goto foo;
  __asm {
  foo:
    nop
  }
}

After issuing the diagnostic, you can avoid the "undeclared label" knock-on errors by pretending that you did in fact define the label.

lib/Sema/SemaStmtAsm.cpp

522 ↗

(On Diff #13793)

This should be a member variable of Sema, rather than a global.

534 ↗

(On Diff #13793)

We should be able to use the 'L' assembler prefix to suppress the symbol table entry for the label. I'm happy as long as both 'nm' and 'dumpbin /symbols' agree that there are no MSASMLABEL symbols in the object file. I think it will with the code as written.

I'd also try to get the user-written label in there, so that the -S output is more intelligible, something like:

OS << "L__MSASMLABEL_" << counter++ << "__" << ExternalLabelName;

Users can't make labels that start with 'L', so we should be safe from collisions, we just need a unique counter to make sure we don't collide with ourselves.

test/Sema/ms-inline-asm.c

148 ↗

(On Diff #13793)

Can you add a .cpp test case involving nested scopes, like:

void f() {
  __asm some_label:
  struct A {
    static void g() {
      __asm jmp some_label ; This should jump forwards
      __asm some_label:
      __asm nop
    }
  };
}

[• ehsan]

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

Oh, I didn't know that! What's this 'new (Contex, 1) char[Len]' trick? I'm not familiar with it, I don't think...

lib/Parse/ParseStmtAsm.cpp

99–101 ↗

(On Diff #13793)

Yeah looks like it. Will do.

lib/Sema/SemaStmt.cpp

2357–2358 ↗

(On Diff #13793)

I cannot issue the diagnostic there because I will not have a GotoStmt pointer when processing foo:. That's really the main issue.

What I would ideally do would be to not issue any diagnostics in ActOnGotoStmt, and wait until the scope is fully processed, then visit all of the GotoStmts in the scope, check their LabelDecls to see if they're asm labels and issue the diagnostic there. But I don't know where I would hook into in order to be able to visit all of the GotoStmts at the end of a scope.

lib/Sema/SemaStmtAsm.cpp

522 ↗

(On Diff #13793)

Sure.

534 ↗

(On Diff #13793)

I think you brought this issue up before. :) I am turning these names into local symbol names in http://reviews.llvm.org/D4587 by using getPrivateGlobalPrefix() when rewriting AOK_Label. I don't think that we need to store that prefix in the AST, do we?

Your suggestion to add the user-written label in the name is a good one, it just needs me to change all of these tests again! ;) But I'll do it in the next iteration.

On the issue of counter uniqueness, the current global counter obviously ensures that. If we have one Sema object per each compilation, then making it a Sema member won't change things, so we should be good there.

test/Sema/ms-inline-asm.c

145 ↗

(On Diff #13793)

Any ideas how I can fix this? Or is the current misplaced diagnostic acceptable?

148 ↗

(On Diff #13793)

Certainly!

[rnk]

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

It's an operator new overload declared in ASTContext.h. Basically, it's equivalent to 'new char[Len]', except it goes calls the 'void* operator new(size_t Bytes, ASTContext &Context, size_t Alignment)' overload.

lib/Sema/SemaStmt.cpp

2357–2358 ↗

(On Diff #13793)

Looks like we have JumpDiagnostics.cpp for this purpose... See if you can work it in there?

lib/Sema/SemaStmtAsm.cpp

534 ↗

(On Diff #13793)

OK, doing this in LLVM MC seems great.

test/Sema/ms-inline-asm.c

145 ↗

(On Diff #13793)

Moving to JumpDiagnostics.cpp will probably fix it.

[• ehsan]

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

Hmm, who is responsible for freeing this memory? The comments in ASTContext.h seem to suggest that I should call ASTContext::Deallocate on the pointer, but I'm not sure when I should do that if the destructor is not invoked. Looking at other parts of the code base, it seems like nobody worries about deleting memory allocated this way...

lib/Sema/SemaStmt.cpp

2357–2358 ↗

(On Diff #13793)

Sure.

[rnk]

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

When the ASTContext gets destroyed, it frees all memory allocated with it. It's basically a simple pool allocation scheme. The AST lives until compilation is over (i.e. forever). The clang driver even passes a -disable-free flag to prevent the -cc1 process from destroying the AST because even that is slower than just ending the process.

[• ehsan]

I think this addresses all of the review comments...

[• ehsan]

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

Thanks! This makes perfect sense. Should I submit another patch to improve the documentation about this in ASTContext.h?

[rnk]

lgtm

include/clang/Sema/ScopeInfo.h

323–331 ↗

(On Diff #13816)

Is it enough to say setHasBranchProtectedScope() instead of setHasInlineMSAsmBlocks()? I would say that an MS inline asm blob is a "protected scope" and add it to the list of things above HasBranchProtectedScope.

[rnk]

include/clang/AST/Decl.h

312 ↗

(On Diff #13793)

Maybe, but I could swear it's in there somewhere, under Ownership.h or something.

[• ehsan]

Addressed the review comment.

[• ehsan]

Closed by commit rL218230 (authored by @ehsan).

cfe/trunk/include/clang/AST/Decl.h

 /// LabelDecl - Represents the declaration of a label.  Labels also have a
 /// corresponding LabelStmt, which indicates the position that the label was
 /// defined at.  For normal labels, the location of the decl is the same as the
 /// location of the statement.  For GNU local labels (__label__), the decl
 /// location is where the __label__ is.
 class LabelDecl : public NamedDecl {
   void anchor() override;
   LabelStmt *TheStmt;
+  StringRef MSAsmName;
+  bool MSAsmNameResolved;
   /// LocStart - For normal labels, this is the same as the main declaration
   /// label, i.e., the location of the identifier; for GNU local labels,
   /// this is the location of the __label__ keyword.
   SourceLocation LocStart;
 
   LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,
             LabelStmt *S, SourceLocation StartL)
-    : NamedDecl(Label, DC, IdentL, II), TheStmt(S), LocStart(StartL) {}
+    : NamedDecl(Label, DC, IdentL, II),
+      TheStmt(S),
+      MSAsmNameResolved(false),
+      LocStart(StartL) {}
 
 public:
   static LabelDecl *Create(ASTContext &C, DeclContext *DC,
                            SourceLocation IdentL, IdentifierInfo *II);
   static LabelDecl *Create(ASTContext &C, DeclContext *DC,
                            SourceLocation IdentL, IdentifierInfo *II,
                            SourceLocation GnuLabelL);
   static LabelDecl *CreateDeserialized(ASTContext &C, unsigned ID);
   
   LabelStmt *getStmt() const { return TheStmt; }
   void setStmt(LabelStmt *T) { TheStmt = T; }
 
   bool isGnuLocal() const { return LocStart != getLocation(); }
   void setLocStart(SourceLocation L) { LocStart = L; }
 
   SourceRange getSourceRange() const override LLVM_READONLY {
     return SourceRange(LocStart, getLocation());
   }
 
+  bool isMSAsmLabel() const { return MSAsmName.size() != 0; }
+  bool isResolvedMSAsmLabel() const { return isMSAsmLabel() && MSAsmNameResolved; }
+  void setMSAsmLabel(StringRef Name);
+  StringRef getMSAsmLabel() const { return MSAsmName; }
+  void setMSAsmLabelResolved() { MSAsmNameResolved = true; }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == Label; }
 };
 
 /// NamespaceDecl - Represent a C++ namespace.
 class NamespaceDecl : public NamedDecl, public DeclContext, 
                       public Redeclarable<NamespaceDecl> 

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def warn_anon_bitfield_width_exceeds_type_size : Warning<
   "be truncated to %1 bits">;
 
 def warn_missing_braces : Warning<
   "suggest braces around initialization of subobject">,
   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 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>;

cfe/trunk/include/clang/Sema/Sema.h

public:
   /// \brief The declaration for + (id) alloc method used in [NSDictionary alloc]
   ObjCMethodDecl *DictAllocObjectsMethod;
 
   /// \brief id<NSCopying> type.
   QualType QIDNSCopying;
 
   /// \brief will hold 'respondsToSelector:'
   Selector RespondsToSelectorSel;
-  
+  /// \brief counter for internal MS Asm label names.
+  unsigned MSAsmLabelNameCounter;
+
   /// A flag to remember whether the implicit forms of operator new and delete
   /// have been declared.
   bool GlobalNewDeleteDeclared;
 
   /// A flag to indicate that we're in a context that permits abstract
   /// references to fields.  This is really a 
   bool AllowAbstractFieldReference;
 
public:
   StmtResult ActOnMSAsmStmt(SourceLocation AsmLoc, SourceLocation LBraceLoc,
                             ArrayRef<Token> AsmToks,
                             StringRef AsmString,
                             unsigned NumOutputs, unsigned NumInputs,
                             ArrayRef<StringRef> Constraints,
                             ArrayRef<StringRef> Clobbers,
                             ArrayRef<Expr*> Exprs,
                             SourceLocation EndLoc);
+  LabelDecl *GetOrCreateMSAsmLabel(StringRef ExternalLabelName,
+                                   SourceLocation Location,
+                                   bool AlwaysCreate);
 
   VarDecl *BuildObjCExceptionDecl(TypeSourceInfo *TInfo, QualType ExceptionType,
                                   SourceLocation StartLoc,
                                   SourceLocation IdLoc, IdentifierInfo *Id,
                                   bool Invalid = false);
 
   Decl *ActOnObjCExceptionDecl(Scope *S, Declarator &D);
 

cfe/trunk/lib/AST/Decl.cpp

LabelDecl *LabelDecl::Create(ASTContext &C, DeclContext *DC,
   return new (C, DC) LabelDecl(DC, IdentL, II, nullptr, GnuLabelL);
 }
 
 LabelDecl *LabelDecl::CreateDeserialized(ASTContext &C, unsigned ID) {
   return new (C, ID) LabelDecl(nullptr, SourceLocation(), nullptr, nullptr,
                                SourceLocation());
 }
 
+void LabelDecl::setMSAsmLabel(StringRef Name) {
+  char *Buffer = new (getASTContext(), 1) char[Name.size() + 1];
+  memcpy(Buffer, Name.data(), Name.size());
+  Buffer[Name.size()] = '\0';
+  MSAsmName = Buffer;
+}
+
 void ValueDecl::anchor() { }
 
 bool ValueDecl::isWeak() const {
   for (const auto *I : attrs())
     if (isa<WeakAttr>(I) || isa<WeakRefAttr>(I))
       return true;
 
   return isWeakImported();

cfe/trunk/lib/Parse/ParseStmtAsm.cpp

if (NumConsumedToks == 0 || NumConsumedToks == LineToks.size()) {
       LineBuf = LineBuf.substr(0, TotalOffset);
     }
 
     // Initialize the "decl" with the lookup result.
     Info.OpDecl = static_cast<void *>(Result.get());
     return Info.OpDecl;
   }
 
+  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);
+    return Label->getMSAsmLabel();
+  }
+
   bool LookupInlineAsmField(StringRef Base, StringRef Member,
                             unsigned &Offset) override {
     return TheParser.getActions().LookupInlineAsmField(Base, Member, Offset,
                                                        AsmLoc);
   }
 
   static void DiagHandlerCallback(const llvm::SMDiagnostic &D, void *Context) {
     ((ClangAsmParserCallback *)Context)->handleDiagnostic(D);
void findTokensForString(StringRef Str, SmallVectorImpl<Token> &TempToks,
     unsigned LastCharOffset = Str.end() - AsmString.begin();
     for (unsigned i = FirstTokIndex, e = AsmTokOffsets.size(); i != e; ++i) {
       if (AsmTokOffsets[i] >= LastCharOffset)
         break;
       TempToks.push_back(AsmToks[i]);
     }
   }
 
-  void handleDiagnostic(const llvm::SMDiagnostic &D) {
+  SourceLocation translateLocation(const llvm::SourceMgr &LSM, llvm::SMLoc SMLoc) {
     // Compute an offset into the inline asm buffer.
     // FIXME: This isn't right if .macro is involved (but hopefully, no
     // real-world code does that).
-    const llvm::SourceMgr &LSM = *D.getSourceMgr();
     const llvm::MemoryBuffer *LBuf =
-        LSM.getMemoryBuffer(LSM.FindBufferContainingLoc(D.getLoc()));
-    unsigned Offset = D.getLoc().getPointer() - LBuf->getBufferStart();
+        LSM.getMemoryBuffer(LSM.FindBufferContainingLoc(SMLoc));
+    unsigned Offset = SMLoc.getPointer() - LBuf->getBufferStart();
 
     // Figure out which token that offset points into.
     const unsigned *TokOffsetPtr =
         std::lower_bound(AsmTokOffsets.begin(), AsmTokOffsets.end(), Offset);
     unsigned TokIndex = TokOffsetPtr - AsmTokOffsets.begin();
     unsigned TokOffset = *TokOffsetPtr;
 
     // If we come up with an answer which seems sane, use it; otherwise,
     // just point at the __asm keyword.
     // FIXME: Assert the answer is sane once we handle .macro correctly.
     SourceLocation Loc = AsmLoc;
     if (TokIndex < AsmToks.size()) {
       const Token &Tok = AsmToks[TokIndex];
       Loc = Tok.getLocation();
       Loc = Loc.getLocWithOffset(Offset - TokOffset);
     }
+    return Loc;
+  }
+
+  void handleDiagnostic(const llvm::SMDiagnostic &D) {
+    const llvm::SourceMgr &LSM = *D.getSourceMgr();
+    SourceLocation Loc = translateLocation(LSM, D.getLoc());
     TheParser.Diag(Loc, diag::err_inline_ms_asm_parsing) << D.getMessage();
   }
 };
 }
 
 /// Parse an identifier in an MS-style inline assembly block.
 ///
 /// \param CastInfo - a void* so that we don't have to teach Parser.h

cfe/trunk/lib/Sema/JumpDiagnostics.cpp

private:
   void VerifyJumps();
   void VerifyIndirectJumps();
   void NoteJumpIntoScopes(ArrayRef<unsigned> ToScopes);
   void DiagnoseIndirectJump(IndirectGotoStmt *IG, unsigned IGScope,
                             LabelDecl *Target, unsigned TargetScope);
   void CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
                  unsigned JumpDiag, unsigned JumpDiagWarning,
                  unsigned JumpDiagCXX98Compat);
+  void CheckGotoStmt(GotoStmt *GS);
 
   unsigned GetDeepestCommonScope(unsigned A, unsigned B);
 };
 } // end anonymous namespace
 
 #define CHECK_PERMISSIVE(x) (assert(Permissive || !(x)), (Permissive && (x)))
 
 JumpScopeChecker::JumpScopeChecker(Stmt *Body, Sema &s)

 /// VerifyJumps - Verify each element of the Jumps array to see if they are
 /// valid, emitting diagnostics if not.
 void JumpScopeChecker::VerifyJumps() {
   while (!Jumps.empty()) {
     Stmt *Jump = Jumps.pop_back_val();
 
     // With a goto,
     if (GotoStmt *GS = dyn_cast<GotoStmt>(Jump)) {
+      // The label may not have a statement if it's coming from inline MS ASM.
+      if (GS->getLabel()->getStmt()) {
         CheckJump(GS, GS->getLabel()->getStmt(), GS->getGotoLoc(),
                   diag::err_goto_into_protected_scope,
                   diag::ext_goto_into_protected_scope,
                   diag::warn_cxx98_compat_goto_into_protected_scope);
+      }
+      CheckGotoStmt(GS);
       continue;
     }
 
     // We only get indirect gotos here when they have a constant target.
     if (IndirectGotoStmt *IGS = dyn_cast<IndirectGotoStmt>(Jump)) {
       LabelDecl *Target = IGS->getConstantTarget();
       CheckJump(IGS, Target->getStmt(), IGS->getGotoLoc(),
                 diag::err_goto_into_protected_scope,
void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
 
   // Handle -Wc++98-compat warnings if the jump is well-formed.
   if (ToScopesError.empty() && !ToScopesCXX98Compat.empty()) {
     S.Diag(DiagLoc, JumpDiagCXX98Compat);
     NoteJumpIntoScopes(ToScopesCXX98Compat);
   }
 }
 
+void JumpScopeChecker::CheckGotoStmt(GotoStmt *GS) {
+  if (GS->getLabel()->isMSAsmLabel()) {
+    S.Diag(GS->getGotoLoc(), diag::err_goto_ms_asm_label)
+        << GS->getLabel()->getIdentifier();
+    S.Diag(GS->getLabel()->getLocation(), diag::note_goto_ms_asm_label)
+        << GS->getLabel()->getIdentifier();
+  }
+}
+
 void Sema::DiagnoseInvalidJumps(Stmt *Body) {
   (void)JumpScopeChecker(Body, *this);
 }

cfe/trunk/lib/Sema/Sema.cpp

: ExternalSource(nullptr),
     NSNumberDecl(nullptr),
     NSStringDecl(nullptr), StringWithUTF8StringMethod(nullptr),
     NSArrayDecl(nullptr), ArrayWithObjectsMethod(nullptr),
     InitArrayWithObjectsMethod(nullptr),
     NSDictionaryDecl(nullptr), DictionaryWithObjectsMethod(nullptr),
     InitDictionaryWithObjectsMethod(nullptr),
     ArrayAllocObjectsMethod(nullptr),
     DictAllocObjectsMethod(nullptr),
+    MSAsmLabelNameCounter(0),
     GlobalNewDeleteDeclared(false),
     TUKind(TUKind),
     NumSFINAEErrors(0),
     AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false),
     NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1),
     CurrentInstantiationScope(nullptr), DisableTypoCorrection(false),
     TyposCorrected(0), AnalysisWarnings(*this),
     VarDataSharingAttributesStack(nullptr), CurScope(nullptr),

cfe/trunk/lib/Sema/SemaDecl.cpp

else
     DiagID = diag::warn_unused_variable;
 
   Diag(D->getLocation(), DiagID) << D->getDeclName() << Hint;
 }
 
 static void CheckPoppedLabel(LabelDecl *L, Sema &S) {
   // Verify that we have no forward references left.  If so, there was a goto
   // or address of a label taken, but no definition of it.  Label fwd
-  // definitions are indicated with a null substmt.
-  if (L->getStmt() == nullptr)
+  // definitions are indicated with a null substmt which is also not a resolved
+  // MS inline assembly label name.
+  bool Diagnose = false;
+  if (L->isMSAsmLabel())
+    Diagnose = !L->isResolvedMSAsmLabel();
+  else
+    Diagnose = L->getStmt() == nullptr;
+  if (Diagnose)
     S.Diag(L->getLocation(), diag::err_undeclared_label_use) <<L->getDeclName();
 }
 
 void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
   S->mergeNRVOIntoParent();
 
   if (S->decl_empty()) return;
   assert((S->getFlags() & (Scope::DeclScope | Scope::TemplateParamScope)) &&

cfe/trunk/lib/Sema/SemaStmt.cpp

if (TheDecl->getStmt()) {
     return SubStmt;
   }
 
   // Otherwise, things are good.  Fill in the declaration and return it.
   LabelStmt *LS = new (Context) LabelStmt(IdentLoc, TheDecl, SubStmt);
   TheDecl->setStmt(LS);
   if (!TheDecl->isGnuLocal()) {
     TheDecl->setLocStart(IdentLoc);
+    if (!TheDecl->isMSAsmLabel()) {
+      // Don't update the location of MS ASM labels.  These will result in
+      // a diagnostic, and changing the location here will mess that up.
       TheDecl->setLocation(IdentLoc);
     }
+  }
   return LS;
 }
 
 StmtResult Sema::ActOnAttributedStmt(SourceLocation AttrLoc,
                                      ArrayRef<const Attr*> Attrs,
                                      Stmt *SubStmt) {
   // Fill in the declaration and return it.
   AttributedStmt *LS = AttributedStmt::Create(Context, AttrLoc, Attrs, SubStmt);

cfe/trunk/lib/Sema/SemaStmtAsm.cpp

 //  This file implements semantic analysis for inline asm statements.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Sema/SemaInternal.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 using namespace clang;
StmtResult Sema::ActOnMSAsmStmt(SourceLocation AsmLoc, SourceLocation LBraceLoc,
                                 ArrayRef<Token> AsmToks,
                                 StringRef AsmString,
                                 unsigned NumOutputs, unsigned NumInputs,
                                 ArrayRef<StringRef> Constraints,
                                 ArrayRef<StringRef> Clobbers,
                                 ArrayRef<Expr*> Exprs,
                                 SourceLocation EndLoc) {
   bool IsSimple = (NumOutputs != 0 || NumInputs != 0);
+  getCurFunction()->setHasBranchProtectedScope();
   MSAsmStmt *NS =
     new (Context) MSAsmStmt(Context, AsmLoc, LBraceLoc, IsSimple,
                             /*IsVolatile*/ true, AsmToks, NumOutputs, NumInputs,
                             Constraints, Exprs, AsmString,
                             Clobbers, EndLoc);
   return NS;
 }
+
+LabelDecl *Sema::GetOrCreateMSAsmLabel(StringRef ExternalLabelName,
+                                       SourceLocation Location,
+                                       bool AlwaysCreate) {
+  LabelDecl* Label = LookupOrCreateLabel(PP.getIdentifierInfo(ExternalLabelName),
+                                         Location);
+
+  if (!Label->isMSAsmLabel()) {
+    // Otherwise, insert it, but only resolve it if we have seen the label itself.
+    std::string InternalName;
+    llvm::raw_string_ostream OS(InternalName);
+    // Create an internal name for the label.  The name should not be a valid mangled
+    // name, and should be unique.  We use a dot to make the name an invalid mangled
+    // name.
+    OS << "__MSASMLABEL_." << MSAsmLabelNameCounter++ << "__" << ExternalLabelName;
+    Label->setMSAsmLabel(OS.str());
+  }
+  if (AlwaysCreate) {
+    // The label might have been created implicitly from a previously encountered
+    // goto statement.  So, for both newly created and looked up labels, we mark
+    // them as resolved.
+    Label->setMSAsmLabelResolved();
+  }
+  // Adjust their location for being able to generate accurate diagnostics.
+  Label->setLocation(Location);
+
+  return Label;
+}

cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 %s -triple i386-apple-darwin10 -fasm-blocks -emit-llvm -o - | opt -strip -S | FileCheck %s
 
 // Some test cases for MS inline asm support from Mozilla code base.
 
+void invoke_copy_to_stack() {}
+
 void invoke(void* that, unsigned methodIndex,
             unsigned paramCount, void* params)
 {
 // CHECK: @invoke
 // CHECK: %5 = alloca i8*, align 4
 // CHECK: %6 = alloca i32, align 4
 // CHECK: %7 = alloca i32, align 4
 // CHECK: %8 = alloca i8*, align 4
 // CHECK: store i8* %0, i8** %5, align 4
 // CHECK: store i32 %1, i32* %6, align 4
 // CHECK: store i32 %2, i32* %7, align 4
 // CHECK: store i8* %3, i8** %8, align 4
 // CHECK: call void asm sideeffect inteldialect
 // CHECK: mov edx,dword ptr $1
 // CHECK: test edx,edx
-// CHECK: jz noparams
+// CHECK: jz {{[^_]*}}__MSASMLABEL_.0__noparams
+//             ^ Can't use {{.*}} here because the matching is greedy.
 // CHECK: mov eax,edx
 // CHECK: shl eax,$$3
 // CHECK: sub esp,eax
 // CHECK: mov ecx,esp
 // CHECK: push dword ptr $0
-// CHECK: call invoke_copy_to_stack
-// CHECK: noparams:
-// CHECK: mov ecx,dword ptr $2
+// CHECK: call dword ptr $2
+// CHECK: {{.*}}__MSASMLABEL_.0__noparams:
+// CHECK: mov ecx,dword ptr $3
 // CHECK: push ecx
 // CHECK: mov edx,[ecx]
-// CHECK: mov eax,dword ptr $3
+// CHECK: mov eax,dword ptr $4
 // CHECK: call dword ptr[edx+eax*$$4]
 // CHECK: mov esp,ebp
 // CHECK: pop ebp
 // CHECK: ret
-// CHECK: "=*m,*m,*m,*m,~{eax},~{ebp},~{ecx},~{edx},~{flags},~{esp},~{dirflag},~{fpsr},~{flags}"
-// CHECK: (i8** %8, i32* %7, i8** %5, i32* %6)
+// CHECK: "=*m,*m,*m,*m,*m,~{eax},~{ebp},~{ecx},~{edx},~{flags},~{esp},~{dirflag},~{fpsr},~{flags}"
+// CHECK: (i8** %8, i32* %7, void (...)* bitcast (void ()* @invoke_copy_to_stack to void (...)*), i8** %5, i32* %6)
 // CHECK: ret void
     __asm {
         mov     edx,paramCount
         test    edx,edx
         jz      noparams
         mov     eax,edx
         shl     eax,3
         sub     esp,eax

cfe/trunk/test/CodeGen/ms-inline-asm.c

 // CHECK: call void asm sideeffect inteldialect "mov esp, ebx\0A\09pop ebx", "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t23() {
   __asm {
   the_label:
   }
 // CHECK: t23
-// CHECK: call void asm sideeffect inteldialect "the_label:", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "{{.*}}__MSASMLABEL_.0__the_label:", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t24_helper(void) {}
 void t24() {
   __asm call t24_helper
 // CHECK: t24
 // CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @t24_helper)
 }
void call_clobber() {
   // CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void (i16)* @t41)
 }
 
 void xgetbv() {
   __asm xgetbv
 }
 // CHECK-LABEL: define void @xgetbv()
 // CHECK: call void asm sideeffect inteldialect "xgetbv", "~{eax},~{edx},~{dirflag},~{fpsr},~{flags}"()
+
+void label1() {
+  __asm {
+    label:
+    jmp label
+  }
+  // CHECK-LABEL: define void @label1
+  // CHECK: call void asm sideeffect inteldialect "{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", "~{dirflag},~{fpsr},~{flags}"()
+}
+
+void label2() {
+  __asm {
+    jmp label
+    label:
+  }
+  // CHECK-LABEL: define void @label2
+  // CHECK: call void asm sideeffect inteldialect "jmp {{.*}}__MSASMLABEL_.2__label\0A\09{{.*}}__MSASMLABEL_.2__label:", "~{dirflag},~{fpsr},~{flags}"()
+}
+
+void label3() {
+  __asm {
+    label:
+    mov eax, label
+  }
+  // CHECK-LABEL: define void @label3
+  // CHECK: call void asm sideeffect inteldialect "{{.*}}__MSASMLABEL_.3__label:\0A\09mov eax, {{.*}}__MSASMLABEL_.3__label", "~{eax},~{dirflag},~{fpsr},~{flags}"()
+}

cfe/trunk/test/CodeGen/ms-inline-asm.cpp

void t7_using() {
   using A = struct {
     int a;
     int b;
   };
   __asm mov eax, [eax].A.b
   // CHECK-LABEL: define void @_Z8t7_usingv
   // CHECK: call void asm sideeffect inteldialect "mov eax, [eax].4", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
+
+void t8() {
+  __asm some_label:
+  // CHECK-LABEL: define void @_Z2t8v()
+  // CHECK: call void asm sideeffect inteldialect "L__MSASMLABEL_.1__some_label:", "~{dirflag},~{fpsr},~{flags}"()
+  struct A {
+    static void g() {
+      __asm jmp some_label ; This should jump forwards
+      __asm some_label:
+      __asm nop
+      // CHECK-LABEL: define internal void @_ZZ2t8vEN1A1gEv()
+      // CHECK: call void asm sideeffect inteldialect "jmp L__MSASMLABEL_.2__some_label\0A\09L__MSASMLABEL_.2__some_label:\0A\09nop", "~{dirflag},~{fpsr},~{flags}"()
+    }
+  };
+  A::g();
+}
+

cfe/trunk/test/Parser/ms-inline-asm.c

__asm {
         mov eax, 2
       }
     }
   }
 }
 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'}}
+}
 int t_fail() { // expected-note {{to match this}}
   __asm 
   __asm { // expected-error 2 {{expected}} expected-note {{to match this}}

cfe/trunk/test/Sema/ms-inline-asm.c

__asm {
     mov eax, LENGTH bar // expected-error {{unable to lookup expression}}
   }
   f();
   __asm {
     mov eax, SIZE bar // expected-error {{unable to lookup expression}}
   }
   f();
   __asm {
-    mov eax, TYPE bar // expected-error {{unable to lookup expression}}
+    mov eax, TYPE bar // expected-error {{unable to lookup expression}} expected-error {{use of undeclared label 'bar'}}
   }
 }
 
 void rdar15318432(void) {
   // We used to crash on this.  When LLVM called back to Clang to parse a name
   // and do name lookup, if parsing failed, we did not restore the lexer state
   // properly.
 

 }
 
 typedef struct {
   int a;
   int b;
 } A;
 
 void t3() {
-  __asm { mov eax, [eax] UndeclaredId } // expected-error {{unknown token in expression}}
+  __asm { mov eax, [eax] UndeclaredId } // expected-error {{unknown token in expression}} expected-error {{use of undeclared label 'UndeclaredId'}}
 
   // FIXME: Only emit one diagnostic here.
+  // expected-error@+3 {{use of undeclared label 'A'}}
   // expected-error@+2 {{unexpected type name 'A': expected expression}}
   // expected-error@+1 {{unknown token in expression}}
   __asm { mov eax, [eax] A }
 }
 
 void t4() {
   // The dot in the "intel dot operator" is optional in MSVC.  MSVC also does
   // global field lookup, but we don't.
   __asm { mov eax, [0] A.a }
   __asm { mov eax, [0].A.a }
   __asm { mov eax, [0].a } // expected-error {{Unable to lookup field reference!}}
   __asm { mov eax, fs:[0] A.a }
   __asm { mov eax, fs:[0].A.a }
   __asm { mov eax, fs:[0].a } // expected-error {{Unable to lookup field reference!}}
   __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}}
+  asm { movl eax, x } // expected-error {{parameter references not allowed in naked functions}} expected-error {{use of undeclared label 'x'}}
   asm { retl }
 }
 
 int y;
 __declspec(naked) int t6(int x) {
   asm { mov eax, y } // No error.
   asm { ret }
 }
+
+void t7() {
+  __asm {
+    foo: // expected-note {{inline assembly label 'foo' declared here}}
+    mov eax, 0
+  }
+  goto foo; // expected-error {{cannot jump from this goto statement to label 'foo' inside an inline assembly block}}
+}
+
+void t8() {
+  __asm foo: // expected-note {{inline assembly label 'foo' declared here}}
+  __asm mov eax, 0
+  goto foo; // expected-error {{cannot jump from this goto statement to label 'foo' inside an inline assembly block}}
+}
+
+void t9() {
+  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}}
+    mov eax, 0
+  }
+}
+
+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'}}
+}