[PATCH] DebugInfo: fix crash with null streamer

[Alp Toker]

On 16/06/2014 20:53, David Blaikie wrote:
> Perhaps the MCNullStreamer should provide a better stub for this
> rather than returning zero?
>
> Given that the null streamer produces no output (I guess that's what
> it's for - I haven't used it personally) I don't suppose there's much
> that can be feature tested other than "this does not crash".
>
> It looks like you can invoke the null streamer in llc using
> "-filetype=null" (I found this with some spelunking, following clang's
> implementation of -emit-codegen-only down to the target layer
> (CGFT_Null), then looking for that in LLVM utilities, etc...
> eventually finding the cl::opt for "filetype" in
> include/llvm/CodeGen/CommandFlags.h)

Thanks for the spelunking David, that was enough to set things in the 
right direction.

Following your suggestion I did a local s/filetype=\w+/filetype=null/ 
throughout all the tests and examined the output for anything 
unexpected. The results were interesting..

It turns out that other bits of machinery like the ASM parser also 
depend on valid file IDs getting generated, not just DwarfUnit.

Removing two no-op overrides in MCNullStreamer kicked the underlying ID 
tracker back into action and fixed everything including the original 
issue, without having to weaken assertions as the original patch did.

New fix attached complete with tests.

Alp.


>
> On Mon, Jun 16, 2014 at 10:13 AM, Alp Toker <alp at nuanti.com> wrote:
>> The attached patch fixes crashes when emitting debug info with a
>> MCNullStreamer.
>>
>> The only test case I could come up with uses clang's EmitBackendOutput in
>> -emit-codegen-only mode to reproduce the crash:
>>
>>    echo 'void f() {}' | clang -cc1 - -gline-tables-only -emit-codegen-only
>>
>> I'm not familiar enough with backend debug testing info to pull together a
>> standalone test. Suggestions welcome!
>>
>> Alp.
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
commit 5013b4cff54e1453d301354a8bd1d11f16ea591f
Author: Alp Toker <alp at nuanti.com>
Date:   Thu Jun 19 03:38:51 2014 +0300

    MCNullStreamer: assign file IDs to resolve crashes and errors
    
    Use the MCStreamer base implementations for file ID tracking instead of
    overriding them as no-ops.
    
    Avoids assertions when streaming Dwarf debug info, and fixes ASM parsing of loc
    and file directives.

diff --git a/lib/MC/MCNullStreamer.cpp b/lib/MC/MCNullStreamer.cpp
index 4f2740e..5ab2829 100644
--- a/lib/MC/MCNullStreamer.cpp
+++ b/lib/MC/MCNullStreamer.cpp
@@ -81,15 +81,7 @@ namespace {
                            unsigned char Value = 0) override { return false; }
 
     void EmitFileDirective(StringRef Filename) override {}
-    unsigned EmitDwarfFileDirective(unsigned FileNo, StringRef Directory,
-                                    StringRef Filename,
-                                    unsigned CUID = 0) override {
-      return 0;
-    }
-    void EmitDwarfLocDirective(unsigned FileNo, unsigned Line,
-                               unsigned Column, unsigned Flags,
-                               unsigned Isa, unsigned Discriminator,
-                               StringRef FileName) override {}
+
     void EmitInstruction(const MCInst &Inst, const MCSubtargetInfo&) override {}
 
     void EmitBundleAlignMode(unsigned AlignPow2) override {}
diff --git a/test/DebugInfo/global.ll b/test/DebugInfo/global.ll
index c515114..3c97f0c 100644
--- a/test/DebugInfo/global.ll
+++ b/test/DebugInfo/global.ll
@@ -3,6 +3,9 @@
 ; RUN: %llc_dwarf -O0 -filetype=obj < %s > %t
 ; RUN: llvm-dwarfdump %t | FileCheck %s
 
+; Also test that the null streamer doesn't crash with debug info.
+; RUN: %llc_dwarf -O0 -filetype=null < %s
+
 ; generated from the following source compiled to bitcode with clang -g -O1
 ; static int i;
 ; int main() {
diff --git a/test/MC/AsmParser/directive_file.s b/test/MC/AsmParser/directive_file.s
index 9b99e0f..d7290eb 100644
--- a/test/MC/AsmParser/directive_file.s
+++ b/test/MC/AsmParser/directive_file.s
@@ -1,4 +1,5 @@
 # RUN: llvm-mc -triple i386-unknown-unknown %s | FileCheck %s
+# RUN: llvm-mc -triple i386-unknown-unknown %s -filetype=null
 
         .file "hello"
         .file 1 "worl\144"   # "\144" is "d"
diff --git a/test/MC/AsmParser/directive_line.s b/test/MC/AsmParser/directive_line.s
index 94ce446..110b68a 100644
--- a/test/MC/AsmParser/directive_line.s
+++ b/test/MC/AsmParser/directive_line.s
@@ -1,4 +1,5 @@
 # RUN: llvm-mc -triple i386-unknown-unknown %s
+# RUN: llvm-mc -triple i386-unknown-unknown %s -filetype=null
 # FIXME: Actually test the output.
 
         .line
diff --git a/test/MC/AsmParser/directive_loc.s b/test/MC/AsmParser/directive_loc.s
index cda9579..404ebce 100644
--- a/test/MC/AsmParser/directive_loc.s
+++ b/test/MC/AsmParser/directive_loc.s
@@ -1,4 +1,5 @@
 # RUN: llvm-mc -triple i386-unknown-unknown %s | FileCheck %s
+# RUN: llvm-mc -triple i386-unknown-unknown %s -filetype=null
 
         .file 1 "hello"
 # CHECK: .file 1 "hello"