[llvm] fix wrong inline assembly line/col info in the error message with ThinLTO (PR #102211)

via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 7 17:05:27 PDT 2024


https://github.com/HighW4y2H3ll updated https://github.com/llvm/llvm-project/pull/102211

>From 5735e32fc185b277273842c862b7b4e363957f89 Mon Sep 17 00:00:00 2001
From: h2h <h2h at meta.com>
Date: Fri, 2 Aug 2024 17:26:47 -0700
Subject: [PATCH 1/4] fix wrong inline assembly line/col info in the error
 message with ThinLTO

Differential Revision: https://phabricator.intern.facebook.com/D60703275
---
 llvm/include/llvm/CodeGen/AsmPrinter.h            |  6 ++++--
 llvm/include/llvm/Support/SourceMgr.h             | 10 ++++++++++
 .../CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp    | 15 ++++++++++-----
 llvm/lib/Support/SourceMgr.cpp                    | 13 +++++++++++--
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 36d1b47973870..88f437b1efaa7 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -877,7 +877,8 @@ class AsmPrinter : public MachineFunctionPass {
   emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
                 const MCTargetOptions &MCOptions,
                 const MDNode *LocMDNode = nullptr,
-                InlineAsm::AsmDialect AsmDialect = InlineAsm::AD_ATT) const;
+                InlineAsm::AsmDialect AsmDialect = InlineAsm::AD_ATT,
+                const DebugLoc *DbgLoc = nullptr) const;
 
   /// This method formats and emits the specified machine instruction that is an
   /// inline asm.
@@ -886,7 +887,8 @@ class AsmPrinter : public MachineFunctionPass {
   /// Add inline assembly info to the diagnostics machinery, so we can
   /// emit file and position info. Returns SrcMgr memory buffer position.
   unsigned addInlineAsmDiagBuffer(StringRef AsmStr,
-                                  const MDNode *LocMDNode) const;
+                                  const MDNode *LocMDNode,
+                                  const DebugLoc *DbgLoc) const;
 
   //===------------------------------------------------------------------===//
   // Internal Implementation Details
diff --git a/llvm/include/llvm/Support/SourceMgr.h b/llvm/include/llvm/Support/SourceMgr.h
index 7a4b6de1162da..167ccb8a4810d 100644
--- a/llvm/include/llvm/Support/SourceMgr.h
+++ b/llvm/include/llvm/Support/SourceMgr.h
@@ -59,6 +59,9 @@ class SourceMgr {
     /// dynamically based on the size of Buffer.
     mutable void *OffsetCache = nullptr;
 
+    // Base Source LineNo and Column where this Buffer starts
+    unsigned BaseLine, BaseCol;
+
     /// Look up a given \p Ptr in the buffer, determining which line it came
     /// from.
     unsigned getLineNumber(const char *Ptr) const;
@@ -143,9 +146,16 @@ class SourceMgr {
   /// the memory buffer.
   unsigned AddNewSourceBuffer(std::unique_ptr<MemoryBuffer> F,
                               SMLoc IncludeLoc) {
+    return AddNewSourceBuffer(std::move(F), 0, 0, IncludeLoc);
+  }
+  unsigned AddNewSourceBuffer(std::unique_ptr<MemoryBuffer> F,
+                              unsigned Line, unsigned Col,
+                              SMLoc IncludeLoc) {
     SrcBuffer NB;
     NB.Buffer = std::move(F);
     NB.IncludeLoc = IncludeLoc;
+    NB.BaseLine = Line;
+    NB.BaseCol = Col;
     Buffers.push_back(std::move(NB));
     return Buffers.size();
   }
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index 6fe8d0e0af995..0f62269aeb334 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -43,7 +43,8 @@ using namespace llvm;
 #define DEBUG_TYPE "asm-printer"
 
 unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
-                                            const MDNode *LocMDNode) const {
+                                            const MDNode *LocMDNode,
+                                            const DebugLoc *DbgLoc) const {
   MCContext &Context = MMI->getContext();
   Context.initInlineSourceManager();
   SourceMgr &SrcMgr = *Context.getInlineSourceManager();
@@ -55,7 +56,9 @@ unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
   Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, "<inline asm>");
 
   // Tell SrcMgr about this buffer, it takes ownership of the buffer.
-  unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
+  unsigned BufNum = DbgLoc ? \
+                    SrcMgr.AddNewSourceBuffer(std::move(Buffer), DbgLoc->getLine()-1, DbgLoc->getCol()-1, SMLoc()) : \
+                    SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 
   // Store LocMDNode in DiagInfo, using BufNum as an identifier.
   if (LocMDNode) {
@@ -71,7 +74,8 @@ unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
 void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
                                const MCTargetOptions &MCOptions,
                                const MDNode *LocMDNode,
-                               InlineAsm::AsmDialect Dialect) const {
+                               InlineAsm::AsmDialect Dialect,
+                               const DebugLoc *DbgLoc) const {
   assert(!Str.empty() && "Can't emit empty inline asm block");
 
   // Remember if the buffer is nul terminated or not so we can avoid a copy.
@@ -95,7 +99,7 @@ void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
     return;
   }
 
-  unsigned BufNum = addInlineAsmDiagBuffer(Str, LocMDNode);
+  unsigned BufNum = addInlineAsmDiagBuffer(Str, LocMDNode, DbgLoc);
   SourceMgr &SrcMgr = *MMI->getContext().getInlineSourceManager();
   SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
 
@@ -415,8 +419,9 @@ void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
     }
   }
 
+  const DebugLoc &DbgLoc = MI->getDebugLoc();
   emitInlineAsm(StringData, getSubtargetInfo(), TM.Options.MCOptions, LocMD,
-                MI->getInlineAsmDialect());
+                MI->getInlineAsmDialect(), DbgLoc ? &DbgLoc : nullptr);
 
   // Emit the #NOAPP end marker.  This has to happen even if verbose-asm isn't
   // enabled, so we use emitRawComment.
diff --git a/llvm/lib/Support/SourceMgr.cpp b/llvm/lib/Support/SourceMgr.cpp
index 3f97213d86c05..4a8355bd8e934 100644
--- a/llvm/lib/Support/SourceMgr.cpp
+++ b/llvm/lib/Support/SourceMgr.cpp
@@ -114,7 +114,7 @@ unsigned SourceMgr::SrcBuffer::getLineNumberSpecialized(const char *Ptr) const {
 
   // llvm::lower_bound gives the number of EOL before PtrOffset. Add 1 to get
   // the line number.
-  return llvm::lower_bound(Offsets, PtrOffset) - Offsets.begin() + 1;
+  return BaseLine + llvm::lower_bound(Offsets, PtrOffset) - Offsets.begin() + 1;
 }
 
 /// Look up a given \p Ptr in the buffer, determining which line it came
@@ -137,6 +137,10 @@ const char *SourceMgr::SrcBuffer::getPointerForLineNumberSpecialized(
   std::vector<T> &Offsets =
       GetOrCreateOffsetCache<T>(OffsetCache, Buffer.get());
 
+  // Offset the LineNo where the Buffer starts
+  if (LineNo >= BaseLine)
+    LineNo -= BaseLine;
+
   // We start counting line and column numbers from 1.
   if (LineNo != 0)
     --LineNo;
@@ -169,6 +173,7 @@ SourceMgr::SrcBuffer::getPointerForLineNumber(unsigned LineNo) const {
 
 SourceMgr::SrcBuffer::SrcBuffer(SourceMgr::SrcBuffer &&Other)
     : Buffer(std::move(Other.Buffer)), OffsetCache(Other.OffsetCache),
+      BaseLine(Other.BaseLine), BaseCol(Other.BaseCol),
       IncludeLoc(Other.IncludeLoc) {
   Other.OffsetCache = nullptr;
 }
@@ -202,7 +207,7 @@ SourceMgr::getLineAndColumn(SMLoc Loc, unsigned BufferID) const {
   size_t NewlineOffs = StringRef(BufStart, Ptr - BufStart).find_last_of("\n\r");
   if (NewlineOffs == StringRef::npos)
     NewlineOffs = ~(size_t)0;
-  return std::make_pair(LineNo, Ptr - BufStart - NewlineOffs);
+  return std::make_pair(LineNo, Ptr - BufStart - NewlineOffs + SB.BaseCol);
 }
 
 // FIXME: Note that the formatting of source locations is spread between
@@ -238,6 +243,10 @@ SMLoc SourceMgr::FindLocForLineAndColumn(unsigned BufferID, unsigned LineNo,
   if (!Ptr)
     return SMLoc();
 
+  // Offset the ColNo where Buffer starts
+  if (ColNo >= SB.BaseCol)
+    ColNo -= SB.BaseCol;
+
   // We start counting line and column numbers from 1.
   if (ColNo != 0)
     --ColNo;

>From 12a12141d4f063355cd0b88671119f5f56f287cc Mon Sep 17 00:00:00 2001
From: h2h <h2h at meta.com>
Date: Wed, 7 Aug 2024 16:35:44 -0700
Subject: [PATCH 2/4] include source filename

---
 llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index 0f62269aeb334..b784856e6d246 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -22,6 +22,7 @@
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/LLVMContext.h"
@@ -50,10 +51,11 @@ unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
   SourceMgr &SrcMgr = *Context.getInlineSourceManager();
   std::vector<const MDNode *> &LocInfos = Context.getLocInfos();
 
+  DIScope *Scope = DbgLoc ? dyn_cast<DIScope>(DbgLoc->getScope()) : nullptr;
   std::unique_ptr<MemoryBuffer> Buffer;
   // The inline asm source manager will outlive AsmStr, so make a copy of the
   // string for SourceMgr to own.
-  Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, "<inline asm>");
+  Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, Scope ? Scope->getFilename() : "<inline asm>");
 
   // Tell SrcMgr about this buffer, it takes ownership of the buffer.
   unsigned BufNum = DbgLoc ? \

>From 47204e6f3e2751ca0863de43010883a807125c2f Mon Sep 17 00:00:00 2001
From: h2h <h2h at meta.com>
Date: Wed, 7 Aug 2024 16:36:02 -0700
Subject: [PATCH 3/4] add testcase

---
 .../CodeGen/Generic/inline-asm-debuginfo.c    | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 llvm/test/CodeGen/Generic/inline-asm-debuginfo.c

diff --git a/llvm/test/CodeGen/Generic/inline-asm-debuginfo.c b/llvm/test/CodeGen/Generic/inline-asm-debuginfo.c
new file mode 100644
index 0000000000000..e6856344c0275
--- /dev/null
+++ b/llvm/test/CodeGen/Generic/inline-asm-debuginfo.c
@@ -0,0 +1,33 @@
+// RUN: set +o pipefail; clang -emit-llvm -c %s -g -o /dev/stdout | llc -o /dev/null 2>&1 | FileCheck %s
+void bad_asm() {
+  asm volatile ("BAD SYNTAX$%"); // CHECK: inline-asm-debuginfo.c:3:16: error: unknown token in expression
+}
+
+void good_asm() {
+  asm volatile ("movq $0xdeadbeef, %rax");
+}
+
+void bad_multi_asm() {
+  asm ( "movl $10, %eax;"
+        "BAD SYNTAX;"   // CHECK: inline-asm-debuginfo.c:11:19: error: invalid instruction mnemonic 'bad'
+        "subl %ebx, %eax;" );
+}
+
+void bad_multi_asm_linechg() {
+  asm ( "movl $10, %eax;\n"
+        "BAD SYNTAX;\n" // CHECK: inline-asm-debuginfo.c:18:3: error: invalid instruction mnemonic 'bad'
+        "subl %ebx, %eax;\n" );
+}
+
+void good_multi_asm_linechg() {
+  asm ( "movl $10, %eax;\n"
+        "test %rax, %rax;\n"
+        "subl %ebx, %eax;\n" );
+}
+
+void bad_multi_asm_op() {
+  unsigned val=1, i=0;
+  asm ( "movl %1, %%eax;\n"
+        "BAD SYNTAX;\n" // CHECK: inline-asm-debuginfo.c:31:3: error: invalid instruction mnemonic 'bad'
+        "subl %0, %%eax;\n" : "=r" (val) : "r" (i) : );
+}

>From 2f3c04c84c1f4c2b5ffbda60a769978d09673c6e Mon Sep 17 00:00:00 2001
From: h2h <h2h at meta.com>
Date: Wed, 7 Aug 2024 17:05:09 -0700
Subject: [PATCH 4/4] make clang-format happy

---
 llvm/include/llvm/CodeGen/AsmPrinter.h             | 14 ++++++--------
 llvm/include/llvm/Support/SourceMgr.h              |  5 ++---
 .../lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp | 12 +++++++-----
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 88f437b1efaa7..7897bcabed02d 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -873,12 +873,11 @@ class AsmPrinter : public MachineFunctionPass {
   void emitFunctionPrefix(ArrayRef<const Constant *> Prefix);
 
   /// Emit a blob of inline asm to the output streamer.
-  void
-  emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
-                const MCTargetOptions &MCOptions,
-                const MDNode *LocMDNode = nullptr,
-                InlineAsm::AsmDialect AsmDialect = InlineAsm::AD_ATT,
-                const DebugLoc *DbgLoc = nullptr) const;
+  void emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
+                     const MCTargetOptions &MCOptions,
+                     const MDNode *LocMDNode = nullptr,
+                     InlineAsm::AsmDialect AsmDialect = InlineAsm::AD_ATT,
+                     const DebugLoc *DbgLoc = nullptr) const;
 
   /// This method formats and emits the specified machine instruction that is an
   /// inline asm.
@@ -886,8 +885,7 @@ class AsmPrinter : public MachineFunctionPass {
 
   /// Add inline assembly info to the diagnostics machinery, so we can
   /// emit file and position info. Returns SrcMgr memory buffer position.
-  unsigned addInlineAsmDiagBuffer(StringRef AsmStr,
-                                  const MDNode *LocMDNode,
+  unsigned addInlineAsmDiagBuffer(StringRef AsmStr, const MDNode *LocMDNode,
                                   const DebugLoc *DbgLoc) const;
 
   //===------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/Support/SourceMgr.h b/llvm/include/llvm/Support/SourceMgr.h
index 167ccb8a4810d..6a0ef62c252c5 100644
--- a/llvm/include/llvm/Support/SourceMgr.h
+++ b/llvm/include/llvm/Support/SourceMgr.h
@@ -148,9 +148,8 @@ class SourceMgr {
                               SMLoc IncludeLoc) {
     return AddNewSourceBuffer(std::move(F), 0, 0, IncludeLoc);
   }
-  unsigned AddNewSourceBuffer(std::unique_ptr<MemoryBuffer> F,
-                              unsigned Line, unsigned Col,
-                              SMLoc IncludeLoc) {
+  unsigned AddNewSourceBuffer(std::unique_ptr<MemoryBuffer> F, unsigned Line,
+                              unsigned Col, SMLoc IncludeLoc) {
     SrcBuffer NB;
     NB.Buffer = std::move(F);
     NB.IncludeLoc = IncludeLoc;
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index b784856e6d246..67dec6678c4ce 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -55,12 +55,15 @@ unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
   std::unique_ptr<MemoryBuffer> Buffer;
   // The inline asm source manager will outlive AsmStr, so make a copy of the
   // string for SourceMgr to own.
-  Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, Scope ? Scope->getFilename() : "<inline asm>");
+  Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, Scope ? Scope->getFilename()
+                                                        : "<inline asm>");
 
   // Tell SrcMgr about this buffer, it takes ownership of the buffer.
-  unsigned BufNum = DbgLoc ? \
-                    SrcMgr.AddNewSourceBuffer(std::move(Buffer), DbgLoc->getLine()-1, DbgLoc->getCol()-1, SMLoc()) : \
-                    SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
+  unsigned BufNum =
+      DbgLoc
+          ? SrcMgr.AddNewSourceBuffer(std::move(Buffer), DbgLoc->getLine() - 1,
+                                      DbgLoc->getCol() - 1, SMLoc())
+          : SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 
   // Store LocMDNode in DiagInfo, using BufNum as an identifier.
   if (LocMDNode) {
@@ -71,7 +74,6 @@ unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
   return BufNum;
 }
 
-
 /// EmitInlineAsm - Emit a blob of inline asm to the output streamer.
 void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
                                const MCTargetOptions &MCOptions,



More information about the llvm-commits mailing list