-fsanitize-coverage produces incorrect debug info at function entry points

[khuey]

Consider the following function:

int square(int i) {
    return i * i;
}

Compiling with

Debian clang version 15.0.0-++20220411071831+e995526e661f-1~exp1~20220411071915.217
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/local/bin

The assembly looks like:

0000000000001130 <square>:
    1130:       55                      push   %rbp
    1131:       48 89 e5                mov    %rsp,%rbp
    1134:       89 7d fc                mov    %edi,-0x4(%rbp)
    1137:       8b 45 fc                mov    -0x4(%rbp),%eax
    113a:       0f af 45 fc             imul   -0x4(%rbp),%eax
    113e:       5d                      pop    %rbp
    113f:       c3                      retq   

The DWARF output from dwarfdump -i is:

<snip>
< 1><0x00000023>    DW_TAG_subprogram
                      DW_AT_low_pc                (indirect address, index 0x0): 0x00001130
                      DW_AT_high_pc               <offset-from-lowpc>16
                      DW_AT_frame_base            len 0x0001: 56: DW_OP_reg6
                      DW_AT_name                  (indirect string, index 0x3): square
                      DW_AT_decl_file             0x00000000
                      DW_AT_decl_line             0x00000001
                      DW_AT_prototyped            yes
                      DW_AT_type                  0x00000064<.debug_info+0x00000064>
                      DW_AT_external              yes
< 2><0x00000032>      DW_TAG_formal_parameter
                        DW_AT_location              len 0x0002: 917c: DW_OP_fbreg -4
                        DW_AT_name                  (indirect string, index 0x6): i
                        DW_AT_decl_file             0x00000000
                        DW_AT_decl_line             0x00000001
                        DW_AT_type                  0x00000064<.debug_info+0x00000064>

<snip>

And from dwarfdump -l

.debug_line: line number info for a single cu
Source lines (from CU-DIE at .debug_info offset 0x0000000c):

            NS new statement, BB new basic block, ET end of text sequence
            PE prologue end, EB epilogue begin
            IS=val ISA number, DI=val discriminator value
<pc>        [lno,col] NS BB ET PE EB IS= DI= uri: "filepath"
0x00001130  [   1, 0] NS uri: "/build/coverage-test.c"
0x00001137  [   2,10] NS PE
<snip>

Note that the debug_info declares the parameter is on the stack at -0x4(%rbp), not in the register specified in the ABI (this is normal for debug builds). The debug_line section specifies the prologue end is at 0x1137 (the PE marker) so that's where a debugger will place a breakpoint at function entry. Because that's after the value of %edi at function entry has been moved into the stack slot, everything will work as expected.

Now add -fsanitize-coverage=trace-pc-guard to the compiler arguments:

The assembly looks like:

000000000002d8c0 <square>:
   2d8c0:       55                      push   %rbp
   2d8c1:       48 89 e5                mov    %rsp,%rbp
   2d8c4:       48 83 ec 10             sub    $0x10,%rsp
   2d8c8:       89 7d f8                mov    %edi,-0x8(%rbp)
   2d8cb:       48 8d 3d be 72 01 00    lea    0x172be(%rip),%rdi        # 44b90 <__TMC_END__>
   2d8d2:       e8 19 02 ff ff          callq  1daf0 <__sanitizer_cov_trace_pc_guard>
   2d8d7:       8b 7d f8                mov    -0x8(%rbp),%edi
   2d8da:       89 7d fc                mov    %edi,-0x4(%rbp)
   2d8dd:       8b 45 fc                mov    -0x4(%rbp),%eax
   2d8e0:       0f af 45 fc             imul   -0x4(%rbp),%eax
   2d8e4:       48 83 c4 10             add    $0x10,%rsp
   2d8e8:       5d                      pop    %rbp
   2d8e9:       c3                      retq   
   2d8ea:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)

Note that the compiler has inserted a call to __sanitizer_cov_trace_pc_guard, and a second stack slot for the original value of %edi at -0x8(%rbp).

The DWARF output from dwarfdump -i is:

<snip>
< 1><0x0000002b>    DW_TAG_subprogram
                      DW_AT_low_pc                (indirect address, index 0x0): 0x0002d8c0
                      DW_AT_high_pc               <offset-from-lowpc>42
                      DW_AT_frame_base            len 0x0001: 56: DW_OP_reg6
                      DW_AT_name                  (indirect string, index 0x3): square
                      DW_AT_decl_file             0x00000000
                      DW_AT_decl_line             0x00000001
                      DW_AT_prototyped            yes
                      DW_AT_type                  0x0000006c<.debug_info+0x0000006c>
                      DW_AT_external              yes
< 2><0x0000003a>      DW_TAG_formal_parameter
                        DW_AT_location              len 0x0002: 917c: DW_OP_fbreg -4
                        DW_AT_name                  (indirect string, index 0x6): i
                        DW_AT_decl_file             0x00000000
                        DW_AT_decl_line             0x00000001
                        DW_AT_type                  0x0000006c<.debug_info+0x0000006c>
<snip>

i.e. essentially unchanged, note that the DWARF still claims the variable lives at -0x4(%rbp).

The output from dwarfdump -l, however, is:

.debug_line: line number info for a single cu
Source lines (from CU-DIE at .debug_info offset 0x0000000c):

            NS new statement, BB new basic block, ET end of text sequence
            PE prologue end, EB epilogue begin
            IS=val ISA number, DI=val discriminator value
<pc>        [lno,col] NS BB ET PE EB IS= DI= uri: "filepath"
0x0002d8c0  [   1, 0] NS uri: "/build/coverage-test.c"
0x0002d8cb  [   1, 0] NS PE
0x0002d8dd  [   2,10] NS
<snip>

The prologue end marker is now placed at 0x2d8cb, which is after the value of %rdi has been moved to the newly added stack slot -0x8(%rbp). But the DWARF still claims that value of i is in -0x4(%rbp), which is not initialized until 0x2d8dd. So a debugger will break at 0x2d8cb, read the value of i from -0x4(%rbp), and get a garbage value because that slot is not yet initialized.

This affects displaying the values of variables on function entry, including setting conditional breakpoints on them

(This issue was originally reported to me by @hsivonen in a Rust binary produced by cargo-fuzz.)

[llvmbot]

@llvm/issue-subscribers-debuginfo

[dwblaikie]

@kcc perhaps you can throw this over to someone working on sanitizer coverage? I'm happy to discuss/help out

[MaskRay]

In gdb/lldb, b square; r; p i, we can observe an incorrect value with several instrumentations that call a function at the function start.

• clang -g -fsanitize-coverage=trace-pc-guard a.c
• clang -g -pg a.c
• clang -g -finstrument-functions a.c

I do not know how SanitizerCoverag can tell MCDwarf to place DW_LNS_set_prologue_end after the function call.

For gcc -g -pg a.c, the parameter value is correct at the function entry.
It appears to achieve this by ensuring the breakpoint is set after the function call (i.e. as if the prologue includes the function call)

Address            Line   Column File   ISA Discriminator Flags
------------------ ------ ------ ------ --- ------------- -------------
0x0000000000001979      1     19      1   0             0  is_stmt
0x000000000000198a      2     14      1   0             0  is_stmt  ## gdb/lldb set a breakpoint here
0x0000000000001990      3      1      1   0             0  is_stmt
0x0000000000001992      4     11      1   0             0  is_stmt
0x000000000000199c      5     10      1   0             0  is_stmt
0x00000000000019a6      6      1      1   0             0  is_stmt
0x00000000000019a8      6      1      1   0             0  is_stmt end_sequence

0000000000001979 <square>:
    1979: 55                            pushq   %rbp
    197a: 48 89 e5                      movq    %rsp, %rbp
    197d: 48 83 ec 08                   subq    $8, %rsp
    1981: ff 15 11 13 00 00             callq   *4881(%rip)             # 0x2c98 <mcount+0x2c98>
    1987: 89 7d fc                      movl    %edi, -4(%rbp)    ## DW_AT_frame_base (DW_OP_call_frame_cfa); DW_AT_location (DW_OP_fbreg -20) is now correct describing the variable
    198a: 8b 45 fc                      movl    -4(%rbp), %eax    ######## gdb/lldb set a breakpoint here
    198d: 0f af c0                      imull   %eax, %eax
    1990: c9                            leave
    1991: c3                            retq

[khuey]

It appears to achieve this by ensuring the breakpoint is set after the function call (i.e. as if the prologue includes the function call)

Yes, I think that would be the desired behavior.

[serge-sans-paille]

The following (clumsy) patch fixes the issue (in the case of -pg)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 6b5ad62e083e..df1fee0301e7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -2147,6 +2147,12 @@ static std::pair<DebugLoc, bool> findPrologueEndLoc(const MachineFunction *MF) {
     for (const auto &MI : MBB) {
       if (!MI.isMetaInstruction()) {
         if (!MI.getFlag(MachineInstr::FrameSetup) && MI.getDebugLoc()) {
+          if(MI.isCall() && MI.getOperand(0).isGlobal()) {
+            const GlobalValue* GV =  MI.getOperand(0).getGlobal();
+            if(GV->getName() == "mcount") {
+              continue;
+            }
+          }
           // Scan forward to try to find a non-zero line number. The
           // prologue_end marks the first breakpoint in the function after the
           // frame setup, and a compiler-generated line 0 location is not a

I'm not familiar with that part of LLVM, but maybe we could add the MachineInstr::FrameSetup flag to these instrument functions call?

[tysmith]

@kcc are you able to help with this?

[tysmith]

@vitalybuka it was mentioned that you may be able to help?

[vitalybuka]

Assigning to my self but can't promise quick results.
I will try to bundle this work with other fsanitize-coverage work I'd like to do this quarter.