[llvm] r344781 - [MC][DWARF][AsmParser] Ensure nested CFI frames are diagnosed.

Kristina Brooks via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 19 05:14:30 PDT 2018


Author: kristina
Date: Fri Oct 19 05:14:30 2018
New Revision: 344781

URL: http://llvm.org/viewvc/llvm-project?rev=344781&view=rev
Log:
[MC][DWARF][AsmParser] Ensure nested CFI frames are diagnosed.

This avoids a crash (with asserts) or bad codegen (without asserts)
in Dwarf streamer later on. This patch fixes this condition in 
MCStreamer and propogates SMLoc down when it's available with an
added bonus of source locations for those specific types of errors.

Further patches could use similar improvements as currently most
non-Windows CFI directives lack an SMLoc parameter.

Modified an existing test to verify source location propogation and
added an object-file version of it to verify that it does not crash in
addition to a standalone test to only ensure it does not crash.

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


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

Modified: llvm/trunk/include/llvm/MC/MCStreamer.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=344781&r1=344780&r2=344781&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCStreamer.h (original)
+++ llvm/trunk/include/llvm/MC/MCStreamer.h Fri Oct 19 05:14:30 2018
@@ -870,7 +870,7 @@ public:
 
   virtual MCSymbol *getDwarfLineTableSymbol(unsigned CUID);
   virtual void EmitCFISections(bool EH, bool Debug);
-  void EmitCFIStartProc(bool IsSimple);
+  void EmitCFIStartProc(bool IsSimple, SMLoc Loc = SMLoc());
   void EmitCFIEndProc();
   virtual void EmitCFIDefCfa(int64_t Register, int64_t Offset);
   virtual void EmitCFIDefCfaOffset(int64_t Offset);

Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=344781&r1=344780&r2=344781&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
+++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Fri Oct 19 05:14:30 2018
@@ -3919,8 +3919,13 @@ bool AsmParser::parseDirectiveCFIStartPr
         parseToken(AsmToken::EndOfStatement))
       return addErrorSuffix(" in '.cfi_startproc' directive");
   }
-
-  getStreamer().EmitCFIStartProc(!Simple.empty());
+  
+  // TODO(kristina): Deal with a corner case of incorrect diagnostic context
+  // being produced if this directive is emitted as part of preprocessor macro
+  // expansion which can *ONLY* happen if Clang's cc1as is the API consumer.
+  // Tools like llvm-mc on the other hand are not affected by it, and report
+  // correct context information.
+  getStreamer().EmitCFIStartProc(!Simple.empty(), Lexer.getLoc());
   return false;
 }
 

Modified: llvm/trunk/lib/MC/MCStreamer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCStreamer.cpp?rev=344781&r1=344780&r2=344781&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCStreamer.cpp (original)
+++ llvm/trunk/lib/MC/MCStreamer.cpp Fri Oct 19 05:14:30 2018
@@ -347,10 +347,10 @@ void MCStreamer::EmitCFISections(bool EH
   assert(EH || Debug);
 }
 
-void MCStreamer::EmitCFIStartProc(bool IsSimple) {
+void MCStreamer::EmitCFIStartProc(bool IsSimple, SMLoc Loc) {
   if (hasUnfinishedDwarfFrameInfo())
-    getContext().reportError(
-        SMLoc(), "starting new .cfi frame before finishing the previous one");
+    return getContext().reportError(
+        Loc, "starting new .cfi frame before finishing the previous one");
 
   MCDwarfFrameInfo Frame;
   Frame.IsSimple = IsSimple;

Added: llvm/trunk/test/MC/X86/cfi-open-within-another-crash.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/cfi-open-within-another-crash.s?rev=344781&view=auto
==============================================================================
--- llvm/trunk/test/MC/X86/cfi-open-within-another-crash.s (added)
+++ llvm/trunk/test/MC/X86/cfi-open-within-another-crash.s Fri Oct 19 05:14:30 2018
@@ -0,0 +1,18 @@
+# Test for D51695 ensuring there is no crash when two .cfi_startproc are opened
+# without the first one being closed.
+
+# RUN: not llvm-mc %s -filetype=obj -triple=x86_64-unknown-linux -o /dev/null 2>&1 | FileCheck %s
+
+.text
+.globl proc_one
+proc_one:
+ .cfi_startproc
+ 
+.text
+.globl proc_two
+proc_two:
+ .cfi_startproc
+ 
+ .cfi_endproc
+
+# CHECK: error: starting new .cfi frame before finishing the previous one

Modified: llvm/trunk/test/MC/X86/cfi-scope-errors.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/cfi-scope-errors.s?rev=344781&r1=344780&r2=344781&view=diff
==============================================================================
--- llvm/trunk/test/MC/X86/cfi-scope-errors.s (original)
+++ llvm/trunk/test/MC/X86/cfi-scope-errors.s Fri Oct 19 05:14:30 2018
@@ -1,6 +1,5 @@
-# RUN: not llvm-mc %s -triple x86_64-linux -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
-
-# FIXME: Push source locations into diagnostics.
+# RUN: not llvm-mc %s -triple x86_64-linux -o /dev/null 2>&1 | FileCheck %s
+# RUN: not llvm-mc %s -triple x86_64-linux -filetype=obj -o /dev/null 2>&1 | FileCheck %s
 
 .text
 .cfi_def_cfa rsp, 8
@@ -9,8 +8,16 @@
 .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
 .cfi_startproc
-# CHECK: error: starting new .cfi frame before finishing the previous one
+
 nop
 .cfi_endproc
 




More information about the llvm-commits mailing list