Fix mixed disassembly showing source lines for "line 0"

[ted]

Summary

"line 0" in a DWARF linetable means something that doesn't have associated
source. The code for mixed disassembly has a comment indicating that
"line 0" should be skipped, but the wrong value was returned. Fix the return
value and add a test to check that we don't incorrectly show source lines
from the beginning of the file.

[ted]

Fix test to exit lldb

[clayborg]

LGTM. This is in Jason's function so I will let him give the final ok.

lldb/test/Shell/Commands/command-disassemble-mixed.c

12–19

are we guaranteed to get some debug info with line zero in it with this example?

[ted]

lldb/test/Shell/Commands/command-disassemble-mixed.c

12–19

I tried this with x86 Linux clang 12.0 and our internal Hexagon clang; both gave me a line zero.

I ran this test on Ubuntu 18 using x86 Linux clang 12.0 and the test failed without the change and passed with it.

[labath]

lldb/test/Shell/Commands/command-disassemble-mixed.c

12–19

Nonetheless, it would be better to test this via a .s file with some explicit .line directives. That way you could check for the actual command output instead of the absence of some string.

(Negative tests like this are very brittle, as they can be rendered ineffective by a change in the capitalization of the error message.)

[ted]

lldb/test/Shell/Commands/command-disassemble-mixed.c

12–19

The test is specifically "the first line of the source file is not displayed when we do dis -m". The only way I can think of testing this is a negative test. If anyone can think of a better way, I'm happy to do that.
Fortunately, the message we're testing against is contained in the test, so it avoids the brittleness of changes to text in the tool.

[labath]

lldb/test/Shell/Commands/command-disassemble-mixed.c

12–19

I missed the fact that this is a string coming from the test. That makes it better indeed, though I'd still recommend a .s file, as that also removes the dependence on the compiler producing a particular debug_line sequence.

Regarding the negative test, that's easy to avoid with a .s file, as you can assert exact command output. Something like this ought to do it (the test input should be alright, the assertions are made up).

# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
# RUN: lldb %t -o "disassemble -m -n main" | FileCheck %s

# CHECK: disassemble -m -n main
# CHECK-NEXT: first line of disassembly
# CHECK-NEXT: second line of disassembly
# CHECK-NEXT: etc.

        .text
        .globl  main
        .type   main,@function
main:
        .file   1 "" "mytest.s"
        .loc    1 10
        nop
        .loc    1 0
        nop
        .loc    1 20
        nop
.Lfunc_end0:
        .size   main, .Lfunc_end0-main
                                        # -- End function
        .section        .debug_abbrev,"",@progbits
        .byte   1                               # Abbreviation Code
        .byte   17                              # DW_TAG_compile_unit
        .byte   0                               # DW_CHILDREN_no
        .byte   37                              # DW_AT_producer
        .byte   8                               # DW_FORM_string
        .byte   19                              # DW_AT_language
        .byte   5                               # DW_FORM_data2
        .byte   3                               # DW_AT_name
        .byte   8                               # DW_FORM_string
        .byte   16                              # DW_AT_stmt_list
        .byte   23                              # DW_FORM_sec_offset
        .byte   17                              # DW_AT_low_pc
        .byte   1                               # DW_FORM_addr
        .byte   18                              # DW_AT_high_pc
        .byte   6                               # DW_FORM_data4
        .byte   0                               # EOM(1)
        .byte   0                               # EOM(2)
        .byte   0                               # EOM(3)
        .section        .debug_info,"",@progbits
.Lcu_begin0:
        .long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
.Ldebug_info_start0:
        .short  4                               # DWARF version number
        .long   .debug_abbrev                   # Offset Into Abbrev. Section
        .byte   8                               # Address Size (in bytes)
        .byte   1                               # Abbrev [1] 0xb:0x1f DW_TAG_compile_unit
        .asciz  "Hand-written DWARF"            # DW_AT_producer
        .short  12                              # DW_AT_language
        .asciz  "mytest.s"                      # DW_AT_name
        .long   .Lline_table_start0             # DW_AT_stmt_list
        .quad   main                            # DW_AT_low_pc
        .long   .Lfunc_end0-main                # DW_AT_high_pc
.Ldebug_info_end0:
        .section        .debug_line,"",@progbits
.Lline_table_start0:

[ted]

lldb/test/Shell/Commands/command-disassemble-mixed.c

12–19

My issues with the above:

• I want to test a c source file, not an assembly file.
• This assumes the user's toolchain supports x86-64, which many downstream toolchains do not. I'd rather the test be core-agnostic, which is why I didn't check any actual disassembly.

[ted]

@jasonmolenda friendly ping

[jasonmolenda]

Hi Ted, sorry for not getting back to you earlier on this, yes this looks unintentional to me and I think this change is good. Nice test case. Thanks for the ping.

lldb/source/Core/Disassembler.cpp

 //===-- Disassembler.cpp --------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
bool Disassembler::ElideMixedSourceAndDisassemblyLine(
     SourceLine &line) {
 
   // TODO: should we also check target.process.thread.step-avoid-libraries ?
 
   const RegularExpression *avoid_regex = nullptr;
 
   // Skip any line #0 entries - they are implementation details
   if (line.line == 0)
-    return false;
+    return true;
 
   ThreadSP thread_sp = exe_ctx.GetThreadSP();
   if (thread_sp) {
     avoid_regex = thread_sp->GetSymbolsToAvoidRegexp();
   } else {
     TargetSP target_sp = exe_ctx.GetTargetSP();
     if (target_sp) {
       Status error;

lldb/test/Shell/Commands/command-disassemble-mixed.c

+// invalid mixed disassembly line
+
+// RUN: %clang -g %s -o %t
+// RUN: %lldb %t -o "dis -m -n main" | FileCheck %s
+
+// CHECK: int main
+// CHECK: int i
+// CHECK-NOT: invalid mixed disassembly line
+// CHECK: return 0;
+
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;
+}