[Assembler] Enable nicer diagnostics for inline assembly.

Fixed test.

Summary:
Enables source location in diagnostic messages from the backend.  This
is after parsing, during finalization.  This requires the SourceMgr, the
inline assembly string buffer, and DiagInfo to still be alive after
EmitInlineAsm returns.

This patch creates a single SourceMgr for inline assembly inside the
AsmPrinter.  MCContext gets a pointer to this SourceMgr.  Using one
SourceMgr per call to EmitInlineAsm would make it difficult for
MCContext to figure out in which SourceMgr the SMLoc is located, while a
single SourceMgr can figure it out if it has multiple buffers.

The Str argument to EmitInlineAsm is copied into a buffer and owned by
the inline asm SourceMgr.  This ensures that DiagHandlers won't print
garbage.  (Clang emits a "note: instantiated into assembly here", which
refers to this string.)

The AsmParser gets destroyed before finalization, which means that the
DiagHandlers the AsmParser installs into the SourceMgr will be stale.
Restore the saved DiagHandlers.

Since now we're using just one SourceMgr for multiple inline asm
strings, we need to tell the AsmParser which buffer it needs to parse
currently.  Hand a buffer id -- returned from SourceMgr::
AddNewSourceBuffer -- to the AsmParser.

Reviewers: rnk, grosbach, compnerd, rengolin, rovka, anemet

Reviewed By: rnk

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D29441

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294458 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/CodeGen/AsmPrinter.h b/include/llvm/CodeGen/AsmPrinter.h
index 4daca0347b77bdaed937e16214e6a674e7a08d13..db3a6fc082851e5e6b655ce187cf6de52cb523b4 100644 (file)
--- a/include/llvm/CodeGen/AsmPrinter.h
+++ b/include/llvm/CodeGen/AsmPrinter.h
@@ -23,6 +23,7 @@
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/SourceMgr.h"
 
 namespace llvm {
 class AsmPrinterHandler;
@@ -137,6 +138,19 @@ private:
   /// maintains ownership of the emitters.
   SmallVector<HandlerInfo, 1> Handlers;
 
+public:
+  struct SrcMgrDiagInfo {
+    SourceMgr SrcMgr;
+    const MDNode *LocInfo;
+    LLVMContext::InlineAsmDiagHandlerTy DiagHandler;
+    void *DiagContext;
+  };
+
+private:
+  /// Structure for generating diagnostics for inline assembly. Only initialised
+  /// when necessary.
+  mutable std::unique_ptr<SrcMgrDiagInfo> DiagInfo;
+
   /// If the target supports dwarf debug info, this pointer is non-null.
   DwarfDebug *DD;
 

diff --git a/include/llvm/MC/MCContext.h b/include/llvm/MC/MCContext.h
index 5c6c179c3050ebda0669b9caa653a9afab369d1b..b73c74db5d7d9940734614be9fe70063e6779c18 100644 (file)
--- a/include/llvm/MC/MCContext.h
+++ b/include/llvm/MC/MCContext.h
@@ -59,6 +59,9 @@ namespace llvm {
     /// The SourceMgr for this object, if any.
     const SourceMgr *SrcMgr;
 
+    /// The SourceMgr for inline assembly, if any.
+    SourceMgr *InlineSrcMgr;
+
     /// The MCAsmInfo for this target.
     const MCAsmInfo *MAI;
 
@@ -243,6 +246,8 @@ namespace llvm {
 
     const SourceMgr *getSourceManager() const { return SrcMgr; }
 
+    void setInlineSourceManager(SourceMgr *SM) { InlineSrcMgr = SM; }
+
     const MCAsmInfo *getAsmInfo() const { return MAI; }
 
     const MCRegisterInfo *getRegisterInfo() const { return MRI; }

diff --git a/include/llvm/MC/MCParser/MCAsmParser.h b/include/llvm/MC/MCParser/MCAsmParser.h
index 84e1c8f531900c99040104563a7ae2a7621e1665..6763374185ec169ea73c0829b7df9151cffdccf3 100644 (file)
--- a/include/llvm/MC/MCParser/MCAsmParser.h
+++ b/include/llvm/MC/MCParser/MCAsmParser.h
@@ -259,7 +259,7 @@ public:
 
 /// \brief Create an MCAsmParser instance.
 MCAsmParser *createMCAsmParser(SourceMgr &, MCContext &, MCStreamer &,
-                               const MCAsmInfo &);
+                               const MCAsmInfo &, unsigned CB = 0);
 
 } // end namespace llvm
 

diff --git a/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index 57864e4e4d4f206c7747d1de677c4c567cef8f63..165b8eea0943e65a99329f7ccde664f7f8a813e4 100644 (file)
--- a/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -40,19 +40,12 @@ using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
 
-namespace {
-  struct SrcMgrDiagInfo {
-    const MDNode *LocInfo;
-    LLVMContext::InlineAsmDiagHandlerTy DiagHandler;
-    void *DiagContext;
-  };
-}
-
 /// srcMgrDiagHandler - This callback is invoked when the SourceMgr for an
 /// inline asm has an error in it.  diagInfo is a pointer to the SrcMgrDiagInfo
 /// struct above.
 static void srcMgrDiagHandler(const SMDiagnostic &Diag, void *diagInfo) {
-  SrcMgrDiagInfo *DiagInfo = static_cast<SrcMgrDiagInfo *>(diagInfo);
+  AsmPrinter::SrcMgrDiagInfo *DiagInfo =
+      static_cast<AsmPrinter::SrcMgrDiagInfo *>(diagInfo);
   assert(DiagInfo && "Diagnostic context not passed down?");
 
   // If the inline asm had metadata associated with it, pull out a location
@@ -99,35 +92,34 @@ void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
     return;
   }
 
-  SourceMgr SrcMgr;
-  SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
+  if (!DiagInfo) {
+    DiagInfo = make_unique<SrcMgrDiagInfo>();
 
-  SrcMgrDiagInfo DiagInfo;
-
-  // If the current LLVMContext has an inline asm handler, set it in SourceMgr.
-  LLVMContext &LLVMCtx = MMI->getModule()->getContext();
-  bool HasDiagHandler = false;
-  if (LLVMCtx.getInlineAsmDiagnosticHandler() != nullptr) {
-    // If the source manager has an issue, we arrange for srcMgrDiagHandler
-    // to be invoked, getting DiagInfo passed into it.
-    DiagInfo.LocInfo = LocMDNode;
-    DiagInfo.DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
-    DiagInfo.DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
-    SrcMgr.setDiagHandler(srcMgrDiagHandler, &DiagInfo);
-    HasDiagHandler = true;
+    MCContext &Context = MMI->getContext();
+    Context.setInlineSourceManager(&DiagInfo->SrcMgr);
+
+    LLVMContext &LLVMCtx = MMI->getModule()->getContext();
+    if (LLVMCtx.getInlineAsmDiagnosticHandler()) {
+      DiagInfo->DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
+      DiagInfo->DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
+      DiagInfo->SrcMgr.setDiagHandler(srcMgrDiagHandler, DiagInfo.get());
+    }
   }
 
+  SourceMgr &SrcMgr = DiagInfo->SrcMgr;
+  SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
+  DiagInfo->LocInfo = LocMDNode;
+
   std::unique_ptr<MemoryBuffer> Buffer;
-  if (isNullTerminated)
-    Buffer = MemoryBuffer::getMemBuffer(Str, "<inline asm>");
-  else
-    Buffer = MemoryBuffer::getMemBufferCopy(Str, "<inline asm>");
+  // The inline asm source manager will outlive Str, so make a copy of the
+  // string for SourceMgr to own.
+  Buffer = MemoryBuffer::getMemBufferCopy(Str, "<inline asm>");
 
   // Tell SrcMgr about this buffer, it takes ownership of the buffer.
-  SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
+  unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 
   std::unique_ptr<MCAsmParser> Parser(
-      createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI));
+      createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
 
   // We create a new MCInstrInfo here since we might be at the module level
   // and not have a MachineFunction to initialize the TargetInstrInfo from and
@@ -151,7 +143,13 @@ void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
   int Res = Parser->Run(/*NoInitialTextSection*/ true,
                         /*NoFinalize*/ true);
   emitInlineAsmEnd(STI, &TAP->getSTI());
-  if (Res && !HasDiagHandler)
+
+  // LocInfo cannot be used for error generation from the backend.
+  // FIXME: associate LocInfo with the SourceBuffer to improve backend
+  // messages.
+  DiagInfo->LocInfo = nullptr;
+
+  if (Res && !DiagInfo->DiagHandler)
     report_fatal_error("Error parsing inline asm\n");
 }
 

diff --git a/lib/MC/MCContext.cpp b/lib/MC/MCContext.cpp
index 16c2c99be670674bed97c7bfbbbea4c5e2d11286..2bfd8e55cadd22377b777ae5f56c5f4b2e2b5c3f 100644 (file)
--- a/lib/MC/MCContext.cpp
+++ b/lib/MC/MCContext.cpp
@@ -521,13 +521,15 @@ CodeViewContext &MCContext::getCVContext() {
 void MCContext::reportError(SMLoc Loc, const Twine &Msg) {
   HadError = true;
 
-  // If we have a source manager use it. Otherwise just use the generic
-  // report_fatal_error().
-  if (!SrcMgr)
+  // If we have a source manager use it. Otherwise, try using the inline source
+  // manager.
+  // If that fails, use the generic report_fatal_error().
+  if (SrcMgr)
+    SrcMgr->PrintMessage(Loc, SourceMgr::DK_Error, Msg);
+  else if (InlineSrcMgr)
+    InlineSrcMgr->PrintMessage(Loc, SourceMgr::DK_Error, Msg);
+  else
     report_fatal_error(Msg, false);
-
-  // Use the source manager to print the message.
-  SrcMgr->PrintMessage(Loc, SourceMgr::DK_Error, Msg);
 }
 
 void MCContext::reportFatalError(SMLoc Loc, const Twine &Msg) {

diff --git a/lib/MC/MCParser/AsmParser.cpp b/lib/MC/MCParser/AsmParser.cpp
index f714aeef054428e0a62c1e8399ad53c9d596f368..f71e05107d7ac1e9fd5d27fb108972819e35b4d8 100644 (file)
--- a/lib/MC/MCParser/AsmParser.cpp
+++ b/lib/MC/MCParser/AsmParser.cpp
@@ -209,7 +209,7 @@ private:
 
 public:
   AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
-            const MCAsmInfo &MAI);
+            const MCAsmInfo &MAI, unsigned CB);
   ~AsmParser() override;
 
   bool Run(bool NoInitialTextSection, bool NoFinalize = false) override;
@@ -572,9 +572,9 @@ extern MCAsmParserExtension *createCOFFAsmParser();
 enum { DEFAULT_ADDRSPACE = 0 };
 
 AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
-                     const MCAsmInfo &MAI)
+                     const MCAsmInfo &MAI, unsigned CB = 0)
     : Lexer(MAI), Ctx(Ctx), Out(Out), MAI(MAI), SrcMgr(SM),
-      PlatformParser(nullptr), CurBuffer(SM.getMainFileID()),
+      PlatformParser(nullptr), CurBuffer(CB ? CB : SM.getMainFileID()),
       MacrosEnabledFlag(true), CppHashInfo(), AssemblerDialect(~0U),
       IsDarwin(false), ParsingInlineAsm(false) {
   HadError = false;
@@ -608,6 +608,10 @@ AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
 AsmParser::~AsmParser() {
   assert((HadError || ActiveMacros.empty()) &&
          "Unexpected active macro instantiation!");
+
+  // Restore the saved diagnostics handler and context for use during
+  // finalization.
+  SrcMgr.setDiagHandler(SavedDiagHandler, SavedDiagContext);
 }
 
 void AsmParser::printMacroInstantiations() {
@@ -5520,6 +5524,7 @@ bool parseAssignmentExpression(StringRef Name, bool allow_redef,
 
 /// \brief Create an MCAsmParser instance.
 MCAsmParser *llvm::createMCAsmParser(SourceMgr &SM, MCContext &C,
-                                     MCStreamer &Out, const MCAsmInfo &MAI) {
-  return new AsmParser(SM, C, Out, MAI);
+                                     MCStreamer &Out, const MCAsmInfo &MAI,
+                                     unsigned CB) {
+  return new AsmParser(SM, C, Out, MAI, CB);
 }

diff --git a/test/Assembler/inline-asm-diags.ll b/test/Assembler/inline-asm-diags.ll
new file mode 100644 (file)
index 0000000..d5d7035
--- /dev/null
+++ b/test/Assembler/inline-asm-diags.ll
@@ -0,0 +1,9 @@
+; RUN: not llc -march=x86 -filetype=obj < %s 2>&1 -o /dev/null | FileCheck %s
+
+module asm ".word 0x10"
+module asm ".word -bar"
+
+; CHECK: <inline asm>:2:7: error: expected relocatable expression
+
+module asm ".word -foo"
+; CHECK: <inline asm>:3:7: error: expected relocatable expression