]> granicus.if.org Git - llvm/commitdiff
[DebugInfo] Stop changing labels for register-described parameter DBG_VALUEs
authorDavid Stenberg <david.stenberg@ericsson.com>
Wed, 13 Feb 2019 09:34:07 +0000 (09:34 +0000)
committerDavid Stenberg <david.stenberg@ericsson.com>
Wed, 13 Feb 2019 09:34:07 +0000 (09:34 +0000)
Summary:
This is a follow-up to D57510. This patch stops DebugHandlerBase from
changing the starting label for the first non-overlapping,
register-described parameter DBG_VALUEs to the beginning of the
function. That code did not consider what defined the registers, which
could result in the ranges for the debug values starting before their
defining instructions. We currently do not emit debug values for
constant values directly at the start of the function, so this code is
still useful for such values, but my intention is to remove the code
from DebugHandlerBase completely when we get there. One reason for
removing it is that the code violates the history map's ranges, which I
think can make it quite confusing when troubleshooting.

In D57510, PrologEpilogInserter was amended so that parameter DBG_VALUEs
now are kept at the start of the entry block, even after emission of
prologue code. That was done to reduce the degradation of debug
completeness from this patch. PR40638 is another example, where the
lexical-scope trimming that LDV does, in combination with scheduling,
results in instructions after the prologue being left without locations.
There might be other cases where the DBG_VALUEs are pushed further down,
for which the DebugHandlerBase code may be helpful, but as it now quite
often result in incorrect locations, even after the prologue, it seems
better to remove that code, and try to work our way up with accurate
locations.

In the long run we should maybe not aim to provide accurate locations
inside the prologue. Some single location descriptions, at least those
referring to stack values, generate inaccurate values inside the
epilogue, so we maybe should not aim to achieve accuracy for location
lists. However, it seems that we now emit line number programs that can
result in GDB and LLDB stopping inside the prologue when doing line
number stepping into functions. See PR40188 for more information.

A summary of some of the changed test cases is available in PR40188#c2.

Reviewers: aprantl, dblaikie, rnk, jmorse

Reviewed By: aprantl

Subscribers: jdoerfert, jholewinski, jvesely, javed.absar, llvm-commits

Tags: #debug-info, #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353928 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed:
lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
test/CodeGen/AMDGPU/llvm.dbg.value.ll
test/DebugInfo/AArch64/asan-stack-vars.mir
test/DebugInfo/ARM/partial-subreg.ll
test/DebugInfo/COFF/fp-stack.ll
test/DebugInfo/COFF/fpo-stack-protect.ll
test/DebugInfo/COFF/pieces.ll
test/DebugInfo/MIR/AArch64/clobber-sp.mir
test/DebugInfo/NVPTX/debug-info.ll
test/DebugInfo/X86/debug-loc-asan.mir
test/DebugInfo/X86/debug-loc-offset.mir
test/DebugInfo/X86/dw_op_minus_direct.ll

index 9676086fb1506a5f7237e6c3e86f1a5e135cc5e9..0680d2040fb4d0faaf6f05760fe821de188bcbca 100644 (file)
@@ -215,24 +215,36 @@ void DebugHandlerBase::beginFunction(const MachineFunction *MF) {
     if (Ranges.empty())
       continue;
 
-    // The first mention of a function argument gets the CurrentFnBegin
-    // label, so arguments are visible when breaking at function entry.
+    auto IsDescribedByReg = [](const MachineInstr *MI) {
+      return MI->getOperand(0).isReg() && MI->getOperand(0).getReg();
+    };
+
+    // The first mention of a function argument gets the CurrentFnBegin label,
+    // so arguments are visible when breaking at function entry.
+    //
+    // We do not change the label for values that are described by registers,
+    // as that could place them above their defining instructions. We should
+    // ideally not change the labels for constant debug values either, since
+    // doing that violates the ranges that are calculated in the history map.
+    // However, we currently do not emit debug values for constant arguments
+    // directly at the start of the function, so this code is still useful.
     const DILocalVariable *DIVar = Ranges.front().first->getDebugVariable();
     if (DIVar->isParameter() &&
         getDISubprogram(DIVar->getScope())->describes(&MF->getFunction())) {
-      LabelsBeforeInsn[Ranges.front().first] = Asm->getFunctionBegin();
+      if (!IsDescribedByReg(Ranges.front().first))
+        LabelsBeforeInsn[Ranges.front().first] = Asm->getFunctionBegin();
       if (Ranges.front().first->getDebugExpression()->isFragment()) {
         // Mark all non-overlapping initial fragments.
         for (auto I = Ranges.begin(); I != Ranges.end(); ++I) {
           const DIExpression *Fragment = I->first->getDebugExpression();
-          if (std::all_of(Ranges.begin(), I,
+          if (std::any_of(Ranges.begin(), I,
                           [&](DbgValueHistoryMap::InstrRange Pred) {
-                            return !Fragment->fragmentsOverlap(
+                            return Fragment->fragmentsOverlap(
                                 Pred.first->getDebugExpression());
                           }))
-            LabelsBeforeInsn[I->first] = Asm->getFunctionBegin();
-          else
             break;
+          if (!IsDescribedByReg(I->first))
+            LabelsBeforeInsn[I->first] = Asm->getFunctionBegin();
         }
       }
     }
index be1920f3ca4cc84c9d73affe0f81e3fe164df3fb..697fdfb3695ec99db21bb56ab5928a7893d667fa 100644 (file)
@@ -4,6 +4,7 @@
 ; GCN-LABEL: {{^}}test_debug_value:
 ; NOOPT: .loc  1 1 42 prologue_end     ; /tmp/test_debug_value.cl:1:42
 ; NOOPT-NEXT: s_load_dwordx2 s[4:5], s[4:5], 0x0
+; NOOPT-NEXT: .Ltmp
 ; NOOPT-NEXT: ;DEBUG_VALUE: test_debug_value:globalptr_arg <- $sgpr4_sgpr5
 
 ; GCN: flat_store_dword
index 428cef6272266b288a962c986d21ee212a72a3e7..4bbacc1ae0d4973a3ff585d7a29432308744d5fd 100644 (file)
@@ -28,7 +28,7 @@
 # CHECK: "_cmd"
 # CHECK: DW_TAG_formal_parameter
 # CHECK-NEXT: DW_AT_location
-# CHECK-NEXT:   [0x{{0*}}, 0x{{.*}}):
+# CHECK-NEXT:   [0x{{.*}}, 0x{{.*}}):
 # CHECK-NOT:    DW_AT_
 # CHECK:        [0x{{.*}}, [[FN_END]]):
 # CHECK-NEXT: DW_AT_name {{.*}}"imageSize"
index a76725d9c2498dfe36e7820186613b2c7ea89582..1d9efc8aa4aba0cd053fb3443e21447418b79aa1 100644 (file)
@@ -9,6 +9,7 @@
 ; CHECK:   DW_AT_name {{.*}}"subscript.get"
 ; CHECK:  DW_TAG_formal_parameter
 ; CHECK-NEXT: DW_AT_location [DW_FORM_sec_offset]      ({{.*}}
+; CHECK-NEXT:  [0x{{.*}}, 0x{{.*}}): DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4
 ; CHECK-NEXT:  [0x{{.*}}, 0x{{.*}}): DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4, DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4
 
 source_filename = "simd.ll"
index 2d57e5aa3f7b73cd9c7db8afcdf8c64add8ba1a9..f0d80b3e9b26a22cb93e7ea618e274dc90542136 100644 (file)
@@ -10,14 +10,14 @@ entry:
   ret double %sub
 }
 
-; ASM:         .cv_def_range    Lfunc_begin0 Lfunc_end0, "A\021\200\000\000\000"
+; ASM:         .cv_def_range    Ltmp1 Lfunc_end0, "A\021\200\000\000\000"
 ; OBJ:    DefRangeRegisterSym {
 ; OBJ:      Register: ST0 (0x80)
 ; OBJ:      MayHaveNoName: 0
 ; OBJ:      LocalVariableAddrRange {
-; OBJ:        OffsetStart: .text+0x0
+; OBJ:        OffsetStart: .text+0x6
 ; OBJ:        ISectStart: 0x0
-; OBJ:        Range: 0x7
+; OBJ:        Range: 0x1
 ; OBJ:      }
 ; OBJ:    }
 
index 643cebae969e3ad0a6398fae57e99ad355cc3f21..c9ea2c695b928422b2af850885863db33d26e05a 100644 (file)
@@ -30,7 +30,7 @@
 ; CHECK:         addl    $20, %esp
 ; CHECK:         popl    %esi
 ; CHECK:         retl
-; CHECK: Ltmp2:
+; CHECK: Ltmp3:
 ; CHECK:         .cv_fpo_endproc
 
 ; ModuleID = 't.c'
index 10a81e17d924834ad7932fdcfed90f142450ecff..e04255d4940ffc7622327dbbf4464c3b9185dd25 100644 (file)
 ; ASM:        .asciz  "pad_right"             # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    .Lfunc_begin1 .Ltmp8, "C\021\021\000\000\000\004\000\000\000"
+; ASM:        .cv_def_range    .Ltmp8 .Ltmp8, "C\021\021\000\000\000\004\000\000\000"
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; ASM:        .asciz  "pad_left"              # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    .Lfunc_begin2 .Ltmp10, "C\021\021\000\000\000\000\000\000\000"
+; ASM:        .cv_def_range    .Ltmp10 .Ltmp10, "C\021\021\000\000\000\000\000\000\000"
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
index 4594065cc2969c8cd081b7d88376876214c90891..13c5935d0cf3e92c38982550ea6298a966a8aba9 100644 (file)
@@ -4,7 +4,7 @@
 # CHECK: DW_TAG_formal_parameter
 # CHECK: DW_TAG_formal_parameter
 # CHECK-NEXT: DW_AT_location
-# CHECK-NEXT:   [0x0000000000000000, 0x0000000000000014): DW_OP_reg1 W1
+# CHECK-NEXT:   [0x0000000000000010, 0x0000000000000014): DW_OP_reg1 W1
 # CHECK-NEXT:   [0x0000000000000014, 0x0000000000000038): DW_OP_breg31 WSP+8
 # CHECK-NEXT: DW_AT_name {{.*}}"y"
 
index 653c13f64715c27fa4f5040c3dbfa88094a3c6b2..04cd621a5b9f726653192361b4eb972948b9f527 100644 (file)
@@ -4781,8 +4781,8 @@ if.end:                                           ; preds = %if.then, %entry
 ; CHECK-NEXT: .b8 6                                // DW_AT_call_line
 ; CHECK-NEXT: .b8 43                               // Abbrev [43] 0x270e:0x22 DW_TAG_inlined_subroutine
 ; CHECK-NEXT: .b32 9791                            // DW_AT_abstract_origin
-; CHECK-NEXT: .b64 Ltmp                          // DW_AT_low_pc
-; CHECK-NEXT: .b64 Ltmp10                          // DW_AT_high_pc
+; CHECK-NEXT: .b64 Ltmp10                          // DW_AT_low_pc
+; CHECK-NEXT: .b64 Ltmp11                          // DW_AT_high_pc
 ; CHECK-NEXT: .b8 12                               // DW_AT_call_file
 ; CHECK-NEXT: .b8 8                                // DW_AT_call_line
 ; CHECK-NEXT: .b8 44                               // Abbrev [44] 0x2725:0x5 DW_TAG_formal_parameter
index e4a6057deefbe05a69dd0c072a86536fbf982535..901c6e28d767768b2fe7e9c1ef5c7aaab8677b22 100644 (file)
@@ -27,7 +27,7 @@
 # We expect two location ranges for the variable.
 #
 # First, its address is stored in %rcx:
-# CHECK:      .quad .Lfunc_begin0-.Lfunc_begin0
+# CHECK:      .quad .Ltmp0-.Lfunc_begin0
 # CHECK-NEXT: .quad [[START_LABEL]]-.Lfunc_begin0
 # CHECK: DW_OP_breg2
 # DWARF:       DW_TAG_formal_parameter
index 07b7972ec1a240297bb643ad1cdde1ad6dad3a7a..728f9041b36f662de18a5c19a95e786f619a42be 100644 (file)
@@ -42,7 +42,7 @@
 # CHECK: DW_TAG_formal_parameter
 # CHECK-NOT: DW_TAG
 # CHECK:       DW_AT_location [DW_FORM_sec_offset]   ({{.*}}
-# CHECK-NEXT:    [0x00000020, 0x00000037): DW_OP_breg0 EAX+0, DW_OP_deref
+# CHECK-NEXT:    [0x00000029, 0x00000037): DW_OP_breg0 EAX+0, DW_OP_deref
 # CHECK-NEXT:    [0x00000037, 0x00000063): DW_OP_breg5 EBP-8, DW_OP_deref, DW_OP_deref
 # CHECK-NEXT:  DW_AT_name [DW_FORM_strp]{{.*}}"a"
 #
@@ -70,7 +70,7 @@
 # CHECK-NEXT:    [0x00000000, 0x0000000a): DW_OP_consts +0, DW_OP_stack_value
 # CHECK-NEXT:    [0x0000000a, 0x00000017): DW_OP_consts +1, DW_OP_stack_value
 # CHECK:       0x00000022:
-# CHECK-NEXT:    [0x00000000, 0x00000017): DW_OP_breg0 EAX+0, DW_OP_deref
+# CHECK-NEXT:    [0x00000009, 0x00000017): DW_OP_breg0 EAX+0, DW_OP_deref
 # CHECK-NEXT:    [0x00000017, 0x00000043): DW_OP_breg5 EBP-8, DW_OP_deref, DW_OP_deref
 --- |
   target triple = "i386-unknown-linux-gnu"
index bc4241c1cc1be81286d876f8f51470c7abcd45f0..efd640019017f8dff10303865f9ad66cb2762a1f 100644 (file)
@@ -17,7 +17,7 @@
 
 ; CHECK: .debug_loc contents:
 ; CHECK: 0x00000000:
-; CHECK-NEXT:   [0x0000000000000000, 0x0000000000000004): DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_lit1, DW_OP_minus, DW_OP_stack_value
+; CHECK-NEXT:   [0x0000000000000003, 0x0000000000000004): DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_lit1, DW_OP_minus, DW_OP_stack_value
 ;        rax+0, constu 0xffffffff, and, constu 0x00000001, minus, stack-value
 
 source_filename = "minus.c"