[llvm] fb9942f - [AsmParser] Add source location to all errors related to .cfi directives

Alex Richardson via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 11 09:00:57 PST 2020


Author: Alex Richardson
Date: 2020-11-11T17:00:07Z
New Revision: fb9942f87628101e1d72da1942416189b8f43825

URL: https://github.com/llvm/llvm-project/commit/fb9942f87628101e1d72da1942416189b8f43825
DIFF: https://github.com/llvm/llvm-project/commit/fb9942f87628101e1d72da1942416189b8f43825.diff

LOG: [AsmParser] Add source location to all errors related to .cfi directives

I was trying to add .cfi_ annotations to assembly code in the FreeBSD
kernel and changed a macro that then resulted in incorrectly nested
directives. However, clang's diagnostics said the error was happening at
<unknown>:0. This addresses one of the TODOs added in D51695.

Reviewed By: MaskRay

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

Added: 
    

Modified: 
    llvm/include/llvm/MC/MCStreamer.h
    llvm/lib/MC/MCParser/AsmParser.cpp
    llvm/lib/MC/MCParser/MasmParser.cpp
    llvm/lib/MC/MCStreamer.cpp
    llvm/test/MC/X86/cfi-open-within-another-crash.s
    llvm/test/MC/X86/cfi-scope-errors.s

Removed: 
    llvm/test/MC/X86/cfi-scope-unclosed.s


################################################################################
diff  --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index b670dee02d8c..d90241d89535 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -1064,7 +1064,7 @@ class MCStreamer {
   /// Streamer specific finalization.
   virtual void finishImpl();
   /// Finish emission of machine code.
-  void Finish();
+  void Finish(SMLoc EndLoc = SMLoc());
 
   virtual bool mayHaveInstructions(MCSection &Sec) const { return true; }
 };

diff  --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index cdaba5a3fbd5..936d2b004225 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -994,7 +994,7 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
   // Finalize the output stream if there are no errors and if the client wants
   // us to.
   if (!HadError && !NoFinalize)
-    Out.Finish();
+    Out.Finish(Lexer.getLoc());
 
   return HadError || getContext().hadError();
 }

diff  --git a/llvm/lib/MC/MCParser/MasmParser.cpp b/llvm/lib/MC/MCParser/MasmParser.cpp
index 036551ea5f4d..33e550773545 100644
--- a/llvm/lib/MC/MCParser/MasmParser.cpp
+++ b/llvm/lib/MC/MCParser/MasmParser.cpp
@@ -1280,7 +1280,7 @@ bool MasmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
   // Finalize the output stream if there are no errors and if the client wants
   // us to.
   if (!HadError && !NoFinalize)
-    Out.Finish();
+    Out.Finish(Lexer.getLoc());
 
   return HadError || getContext().hadError();
 }

diff  --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index 46aa0b89842c..e4216c429d8a 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -273,9 +273,9 @@ bool MCStreamer::hasUnfinishedDwarfFrameInfo() {
 
 MCDwarfFrameInfo *MCStreamer::getCurrentDwarfFrameInfo() {
   if (!hasUnfinishedDwarfFrameInfo()) {
-    getContext().reportError(SMLoc(), "this directive must appear between "
-                                      ".cfi_startproc and .cfi_endproc "
-                                      "directives");
+    getContext().reportError(getStartTokLoc(),
+                             "this directive must appear between "
+                             ".cfi_startproc and .cfi_endproc directives");
     return nullptr;
   }
   return &DwarfFrameInfos.back();
@@ -967,10 +967,10 @@ void MCStreamer::emitRawText(const Twine &T) {
 void MCStreamer::EmitWindowsUnwindTables() {
 }
 
-void MCStreamer::Finish() {
+void MCStreamer::Finish(SMLoc EndLoc) {
   if ((!DwarfFrameInfos.empty() && !DwarfFrameInfos.back().End) ||
       (!WinFrameInfos.empty() && !WinFrameInfos.back()->End)) {
-    getContext().reportError(SMLoc(), "Unfinished frame!");
+    getContext().reportError(EndLoc, "Unfinished frame!");
     return;
   }
 

diff  --git a/llvm/test/MC/X86/cfi-open-within-another-crash.s b/llvm/test/MC/X86/cfi-open-within-another-crash.s
index 81627f4459c0..6ec5338f54b4 100644
--- a/llvm/test/MC/X86/cfi-open-within-another-crash.s
+++ b/llvm/test/MC/X86/cfi-open-within-another-crash.s
@@ -12,7 +12,7 @@ proc_one:
 .globl proc_two
 proc_two:
  .cfi_startproc
- 
+# CHECK: [[#@LINE]]:1: error: starting new .cfi frame before finishing the previous one
+
  .cfi_endproc
 
-# CHECK: error: starting new .cfi frame before finishing the previous one

diff  --git a/llvm/test/MC/X86/cfi-scope-errors.s b/llvm/test/MC/X86/cfi-scope-errors.s
index a7d6a8a157a5..c17eadd9ddf6 100644
--- a/llvm/test/MC/X86/cfi-scope-errors.s
+++ b/llvm/test/MC/X86/cfi-scope-errors.s
@@ -3,23 +3,23 @@
 
 .text
 .cfi_def_cfa rsp, 8
-# CHECK: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
+# CHECK: [[#@LINE-1]]:1: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
 
 .cfi_startproc
 nop
 
-# TODO(kristina): As Reid suggested, this now supports source locations as a side effect
-# of another patch aimed at fixing the crash that would occur here, however the other
-# ones do not unfortunately. Will address it in a further patch propogating SMLoc down to
-# other CFI directives at which point more LINE checks can be added to ensure proper source
-# location reporting.
-
-# This tests source location correctness as well as the error and it not crashing.
-# CHECK: [[@LINE+2]]:1: error: starting new .cfi frame before finishing the previous one
+## This tests source location correctness as well as the error and it not crashing.
+# CHECK: [[#@LINE+2]]:1: error: starting new .cfi frame before finishing the previous one
 .cfi_startproc
 
 nop
 .cfi_endproc
 
 .cfi_def_cfa rsp, 8
-# CHECK: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
+# CHECK: [[#@LINE-1]]:1: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
+
+## Check we don't crash on unclosed frame scope.
+.globl foo
+foo:
+ .cfi_startproc
+# CHECK: [[#@LINE+1]]:1: error: Unfinished frame!

diff  --git a/llvm/test/MC/X86/cfi-scope-unclosed.s b/llvm/test/MC/X86/cfi-scope-unclosed.s
deleted file mode 100644
index 0574a97b69bb..000000000000
--- a/llvm/test/MC/X86/cfi-scope-unclosed.s
+++ /dev/null
@@ -1,10 +0,0 @@
-# RUN: not llvm-mc %s -filetype=obj -triple=x86_64-unknown-linux \
-# RUN:   -o /dev/null 2>&1 | FileCheck %s
-
-## Check we don't crash on unclosed frame scope.
-# CHECK: error: Unfinished frame!
-
-.text
-.globl foo
-foo:
- .cfi_startproc


        


More information about the llvm-commits mailing list