Fix dwarf-lineinfo inconsistency of inlined subroutines
diff mbox series

Message ID VI1PR03MB452890F2CC34205AA836CAB2E46E0@VI1PR03MB4528.eurprd03.prod.outlook.com
State New
Headers show
Series
  • Fix dwarf-lineinfo inconsistency of inlined subroutines
Related show

Commit Message

Bernd Edlinger Oct. 20, 2019, 7:58 p.m. UTC
Hi,

this fixes an issue with the gdb step-over aka. "n" command.

It can be seen when you debug an optimized stage-3 cc1
it does not affect -O0 code, though.

This example debug session will explain the effect.

(gdb) b get_alias_set
Breakpoint 5 at 0xa099f0: file ../../gcc-trunk/gcc/alias.c, line 837.
(gdb) r
Breakpoint 5, get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/alias.c:837
837	  if (t == error_mark_node
(gdb) n
839		  && (TREE_TYPE (t) == 0 || TREE_TYPE (t) == error_mark_node)))
(gdb) n
3382	  return __t;  <-- now we have a problem: wrong line info here
(gdb) bt
#0  get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/tree.h:3382
#1  0x0000000000b25dfe in set_mem_attributes_minus_bitpos (ref=0x7ffff746f990, t=0x7ffff7ff7ab0, objectp=1, bitpos=...)
    at ../../gcc-trunk/gcc/emit-rtl.c:1957
#2  0x0000000001137a55 in make_decl_rtl (decl=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/varasm.c:1518
#3  0x000000000113b6e8 in assemble_variable (decl=0x7ffff7ff7ab0, top_level=<optimized out>, at_end=<optimized out>, 
    dont_output_data=0) at ../../gcc-trunk/gcc/varasm.c:2246
#4  0x000000000113f0ea in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:584
#5  0x000000000113fa17 in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:750


There are at least two problems here:

First you did not want to step into the TREE_TYPE, but it happens all
the time, even if you use "n" to step over it.

And secondly, from the call stack, you don't know where you are in get_alias_set.
But the code that is executing at this point is actually the x == 0 || x == error_mark_node
from alias.c, line 839, which contains the inlined body of the TREE_TYPE, but
the rest of the if.  So there is an inconsistency in the  

Contents of the .debug_info section:

 <2><4f686>: Abbrev Number: 12 (DW_TAG_inlined_subroutine)
    <4f687>   DW_AT_abstract_origin: <0x53d4e>
    <4f68b>   DW_AT_entry_pc    : 0x7280
    <4f693>   DW_AT_GNU_entry_view: 1
    <4f695>   DW_AT_ranges      : 0xb480
    <4f699>   DW_AT_call_file   : 8  <- alias.c
    <4f69a>   DW_AT_call_line   : 839
    <4f69c>   DW_AT_call_column : 8
    <4f69d>   DW_AT_sibling     : <0x4f717>

 The File Name Table (offset 0x253):
  8     2       0       0       alias.c
  10    2       0       0       tree.h

Contents of the .debug_ranges section:

    0000b480 0000000000007280 0000000000007291 
    0000b480 0000000000002764 000000000000277e 
    0000b480 <End of list>

The problem is at pc=0x7291 in the Line Number Section:

 Line Number Statements:

  [0x00008826]  Special opcode 61: advance Address by 4 to 0x7284 and Line by 0 to 3380
  [0x00008827]  Set is_stmt to 1
  [0x00008828]  Special opcode 189: advance Address by 13 to 0x7291 and Line by 2 to 3382 (*)
  [0x00008829]  Set is_stmt to 0 (**)
  [0x0000882a]  Copy (view 1)
  [0x0000882b]  Set File Name to entry 8 in the File Name Table <- back to alias.c
  [0x0000882d]  Set column to 8
  [0x0000882f]  Advance Line by -2543 to 839
  [0x00008832]  Copy (view 2)
  [0x00008833]  Set column to 27
  [0x00008835]  Special opcode 61: advance Address by 4 to 0x7295 and Line by 0 to 839
  [0x00008836]  Set column to 3
  [0x00008838]  Set is_stmt to 1 <-- next line info counts: alias.c:847
  [0x00008839]  Special opcode 153: advance Address by 10 to 0x729f and Line by 8 to 847
  [0x0000883a]  Set column to 7

(*) this line is tree.h:3382, but the program counter is *not* within the subroutine,
but exactly at the first instruction *after* the subroutine according to the debug_ranges.

What makes it worse, is that (**) makes gdb ignore the new location info alias.c:839,
which means, normally the n command would have continued to pc=0x729f, which is at alias.c:847.


The problem happens due to a block with only var
This patch fixes this problem by moving (**) to the first statement with a different line number.

In alias.c.316r.final this looks like that:

(note 2884 2883 1995 31 0x7f903a931ba0 NOTE_INSN_BLOCK_BEG)
(note 1995 2884 2885 31 ../../gcc-trunk/gcc/tree.h:3377 NOTE_INSN_INLINE_ENTRY)
(note 2885 1995 1996 31 0x7f903a931c00 NOTE_INSN_BLOCK_BEG)
[...]
(note 50 39 59 32 [bb 32] NOTE_INSN_BASIC_BLOCK)
(note 59 50 60 32 NOTE_INSN_DELETED)
(note 60 59 1997 32 NOTE_INSN_DELETED)
(note 1997 60 2239 32 ../../gcc-trunk/gcc/tree.h:3382 NOTE_INSN_BEGIN_STMT)
(note 2239 1997 2240 32 (var_location __tD.143911 (nil)) NOTE_INSN_VAR_LOCATION)
(note 2240 2239 2241 32 (var_location __sD.143912 (nil)) NOTE_INSN_VAR_LOCATION)
(note 2241 2240 2242 32 (var_location __fD.143913 (nil)) NOTE_INSN_VAR_LOCATION)
(note 2242 2241 2243 32 (var_location __lD.143914 (nil)) NOTE_INSN_VAR_LOCATION)
(note 2243 2242 2886 32 (var_location __gD.143915 (nil)) NOTE_INSN_VAR_LOCATION)
(note 2886 2243 2887 32 0x7f903a931c00 NOTE_INSN_BLOCK_END)
(note 2887 2886 57 32 0x7f903a931ba0 NOTE_INSN_BLOCK_END)
(insn:TI 57 2887 61 32 (set (reg/f:DI 0 ax [orig:87 _7 ] [87])
        (mem/f/j:DI (plus:DI (reg/f:DI 5 di [orig:83 t.85_2 ] [83])
                (const_int 8 [0x8])) [0 t.85_2->typedD.91322.typeD.90828+0 S8 A64])) "../../gcc-trunk/gcc/alias.c":839:108 66 {*movdi_internal}
     (nil))

So this patch detects the NOTE_INSN_VAR_LOCATION and makes the next location
with a different file&line info a statement location, which is hopefully
a real instruction, thus either part of the subroutine, or the first
instruction after the subroutine, which should have the correct location.
Once the same address has a second statement-type .loc info, gdb will ignore
the first one, and the stepping works as expected.

So this is a bit of a heuristic, but it appears to work quite well.

The test case g++.dg/guality/pr55541.C is the only test where this
change had an effect.  But it is not a regression, since previously
the test case was "unsupported" on any optimization mode, since there
was no breakpoint at line 11, now the breakpoint works, but the variable
value is wrong, but basically this was not working before.

I don't know how to make this test xfail when compiled with optimization,
but the do-skip-if is probably good enough for this kind of test case.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

Comments

Bernd Edlinger Oct. 27, 2019, 8:14 a.m. UTC | #1
Ping...

I'd like to ping for this patch:
https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01459.html


Thanks
Bernd.

On 10/20/19 9:58 PM, Bernd Edlinger wrote:
> Hi,
> 
> this fixes an issue with the gdb step-over aka. "n" command.
> 
> It can be seen when you debug an optimized stage-3 cc1
> it does not affect -O0 code, though.
> 
> This example debug session will explain the effect.
> 
> (gdb) b get_alias_set
> Breakpoint 5 at 0xa099f0: file ../../gcc-trunk/gcc/alias.c, line 837.
> (gdb) r
> Breakpoint 5, get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/alias.c:837
> 837	  if (t == error_mark_node
> (gdb) n
> 839		  && (TREE_TYPE (t) == 0 || TREE_TYPE (t) == error_mark_node)))
> (gdb) n
> 3382	  return __t;  <-- now we have a problem: wrong line info here
> (gdb) bt
> #0  get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/tree.h:3382
> #1  0x0000000000b25dfe in set_mem_attributes_minus_bitpos (ref=0x7ffff746f990, t=0x7ffff7ff7ab0, objectp=1, bitpos=...)
>     at ../../gcc-trunk/gcc/emit-rtl.c:1957
> #2  0x0000000001137a55 in make_decl_rtl (decl=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/varasm.c:1518
> #3  0x000000000113b6e8 in assemble_variable (decl=0x7ffff7ff7ab0, top_level=<optimized out>, at_end=<optimized out>, 
>     dont_output_data=0) at ../../gcc-trunk/gcc/varasm.c:2246
> #4  0x000000000113f0ea in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:584
> #5  0x000000000113fa17 in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:750
> 
> 
> There are at least two problems here:
> 
> First you did not want to step into the TREE_TYPE, but it happens all
> the time, even if you use "n" to step over it.
> 
> And secondly, from the call stack, you don't know where you are in get_alias_set.
> But the code that is executing at this point is actually the x == 0 || x == error_mark_node
> from alias.c, line 839, which contains the inlined body of the TREE_TYPE, but
> the rest of the if.  So there is an inconsistency in the  
> 
> Contents of the .debug_info section:
> 
>  <2><4f686>: Abbrev Number: 12 (DW_TAG_inlined_subroutine)
>     <4f687>   DW_AT_abstract_origin: <0x53d4e>
>     <4f68b>   DW_AT_entry_pc    : 0x7280
>     <4f693>   DW_AT_GNU_entry_view: 1
>     <4f695>   DW_AT_ranges      : 0xb480
>     <4f699>   DW_AT_call_file   : 8  <- alias.c
>     <4f69a>   DW_AT_call_line   : 839
>     <4f69c>   DW_AT_call_column : 8
>     <4f69d>   DW_AT_sibling     : <0x4f717>
> 
>  The File Name Table (offset 0x253):
>   8     2       0       0       alias.c
>   10    2       0       0       tree.h
> 
> Contents of the .debug_ranges section:
> 
>     0000b480 0000000000007280 0000000000007291 
>     0000b480 0000000000002764 000000000000277e 
>     0000b480 <End of list>
> 
> The problem is at pc=0x7291 in the Line Number Section:
> 
>  Line Number Statements:
> 
>   [0x00008826]  Special opcode 61: advance Address by 4 to 0x7284 and Line by 0 to 3380
>   [0x00008827]  Set is_stmt to 1
>   [0x00008828]  Special opcode 189: advance Address by 13 to 0x7291 and Line by 2 to 3382 (*)
>   [0x00008829]  Set is_stmt to 0 (**)
>   [0x0000882a]  Copy (view 1)
>   [0x0000882b]  Set File Name to entry 8 in the File Name Table <- back to alias.c
>   [0x0000882d]  Set column to 8
>   [0x0000882f]  Advance Line by -2543 to 839
>   [0x00008832]  Copy (view 2)
>   [0x00008833]  Set column to 27
>   [0x00008835]  Special opcode 61: advance Address by 4 to 0x7295 and Line by 0 to 839
>   [0x00008836]  Set column to 3
>   [0x00008838]  Set is_stmt to 1 <-- next line info counts: alias.c:847
>   [0x00008839]  Special opcode 153: advance Address by 10 to 0x729f and Line by 8 to 847
>   [0x0000883a]  Set column to 7
> 
> (*) this line is tree.h:3382, but the program counter is *not* within the subroutine,
> but exactly at the first instruction *after* the subroutine according to the debug_ranges.
> 
> What makes it worse, is that (**) makes gdb ignore the new location info alias.c:839,
> which means, normally the n command would have continued to pc=0x729f, which is at alias.c:847.
> 
> 
> The problem happens due to a block with only var
> This patch fixes this problem by moving (**) to the first statement with a different line number.
> 
> In alias.c.316r.final this looks like that:
> 
> (note 2884 2883 1995 31 0x7f903a931ba0 NOTE_INSN_BLOCK_BEG)
> (note 1995 2884 2885 31 ../../gcc-trunk/gcc/tree.h:3377 NOTE_INSN_INLINE_ENTRY)
> (note 2885 1995 1996 31 0x7f903a931c00 NOTE_INSN_BLOCK_BEG)
> [...]
> (note 50 39 59 32 [bb 32] NOTE_INSN_BASIC_BLOCK)
> (note 59 50 60 32 NOTE_INSN_DELETED)
> (note 60 59 1997 32 NOTE_INSN_DELETED)
> (note 1997 60 2239 32 ../../gcc-trunk/gcc/tree.h:3382 NOTE_INSN_BEGIN_STMT)
> (note 2239 1997 2240 32 (var_location __tD.143911 (nil)) NOTE_INSN_VAR_LOCATION)
> (note 2240 2239 2241 32 (var_location __sD.143912 (nil)) NOTE_INSN_VAR_LOCATION)
> (note 2241 2240 2242 32 (var_location __fD.143913 (nil)) NOTE_INSN_VAR_LOCATION)
> (note 2242 2241 2243 32 (var_location __lD.143914 (nil)) NOTE_INSN_VAR_LOCATION)
> (note 2243 2242 2886 32 (var_location __gD.143915 (nil)) NOTE_INSN_VAR_LOCATION)
> (note 2886 2243 2887 32 0x7f903a931c00 NOTE_INSN_BLOCK_END)
> (note 2887 2886 57 32 0x7f903a931ba0 NOTE_INSN_BLOCK_END)
> (insn:TI 57 2887 61 32 (set (reg/f:DI 0 ax [orig:87 _7 ] [87])
>         (mem/f/j:DI (plus:DI (reg/f:DI 5 di [orig:83 t.85_2 ] [83])
>                 (const_int 8 [0x8])) [0 t.85_2->typedD.91322.typeD.90828+0 S8 A64])) "../../gcc-trunk/gcc/alias.c":839:108 66 {*movdi_internal}
>      (nil))
> 
> So this patch detects the NOTE_INSN_VAR_LOCATION and makes the next location
> with a different file&line info a statement location, which is hopefully
> a real instruction, thus either part of the subroutine, or the first
> instruction after the subroutine, which should have the correct location.
> Once the same address has a second statement-type .loc info, gdb will ignore
> the first one, and the stepping works as expected.
> 
> So this is a bit of a heuristic, but it appears to work quite well.
> 
> The test case g++.dg/guality/pr55541.C is the only test where this
> change had an effect.  But it is not a regression, since previously
> the test case was "unsupported" on any optimization mode, since there
> was no breakpoint at line 11, now the breakpoint works, but the variable
> value is wrong, but basically this was not working before.
> 
> I don't know how to make this test xfail when compiled with optimization,
> but the do-skip-if is probably good enough for this kind of test case.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
>
Bernd Edlinger Nov. 2, 2019, 6:49 a.m. UTC | #2
Ping...

On 10/27/19 9:14 AM, Bernd Edlinger wrote:
> Ping...
> 
> I'd like to ping for this patch:
> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01459.html
> 
> 
> Thanks
> Bernd.
> 
> On 10/20/19 9:58 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> this fixes an issue with the gdb step-over aka. "n" command.
>>
>> It can be seen when you debug an optimized stage-3 cc1
>> it does not affect -O0 code, though.
>>
>> This example debug session will explain the effect.
>>
>> (gdb) b get_alias_set
>> Breakpoint 5 at 0xa099f0: file ../../gcc-trunk/gcc/alias.c, line 837.
>> (gdb) r
>> Breakpoint 5, get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/alias.c:837
>> 837	  if (t == error_mark_node
>> (gdb) n
>> 839		  && (TREE_TYPE (t) == 0 || TREE_TYPE (t) == error_mark_node)))
>> (gdb) n
>> 3382	  return __t;  <-- now we have a problem: wrong line info here
>> (gdb) bt
>> #0  get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/tree.h:3382
>> #1  0x0000000000b25dfe in set_mem_attributes_minus_bitpos (ref=0x7ffff746f990, t=0x7ffff7ff7ab0, objectp=1, bitpos=...)
>>     at ../../gcc-trunk/gcc/emit-rtl.c:1957
>> #2  0x0000000001137a55 in make_decl_rtl (decl=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/varasm.c:1518
>> #3  0x000000000113b6e8 in assemble_variable (decl=0x7ffff7ff7ab0, top_level=<optimized out>, at_end=<optimized out>, 
>>     dont_output_data=0) at ../../gcc-trunk/gcc/varasm.c:2246
>> #4  0x000000000113f0ea in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:584
>> #5  0x000000000113fa17 in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:750
>>
>>
>> There are at least two problems here:
>>
>> First you did not want to step into the TREE_TYPE, but it happens all
>> the time, even if you use "n" to step over it.
>>
>> And secondly, from the call stack, you don't know where you are in get_alias_set.
>> But the code that is executing at this point is actually the x == 0 || x == error_mark_node
>> from alias.c, line 839, which contains the inlined body of the TREE_TYPE, but
>> the rest of the if.  So there is an inconsistency in the  
>>
>> Contents of the .debug_info section:
>>
>>  <2><4f686>: Abbrev Number: 12 (DW_TAG_inlined_subroutine)
>>     <4f687>   DW_AT_abstract_origin: <0x53d4e>
>>     <4f68b>   DW_AT_entry_pc    : 0x7280
>>     <4f693>   DW_AT_GNU_entry_view: 1
>>     <4f695>   DW_AT_ranges      : 0xb480
>>     <4f699>   DW_AT_call_file   : 8  <- alias.c
>>     <4f69a>   DW_AT_call_line   : 839
>>     <4f69c>   DW_AT_call_column : 8
>>     <4f69d>   DW_AT_sibling     : <0x4f717>
>>
>>  The File Name Table (offset 0x253):
>>   8     2       0       0       alias.c
>>   10    2       0       0       tree.h
>>
>> Contents of the .debug_ranges section:
>>
>>     0000b480 0000000000007280 0000000000007291 
>>     0000b480 0000000000002764 000000000000277e 
>>     0000b480 <End of list>
>>
>> The problem is at pc=0x7291 in the Line Number Section:
>>
>>  Line Number Statements:
>>
>>   [0x00008826]  Special opcode 61: advance Address by 4 to 0x7284 and Line by 0 to 3380
>>   [0x00008827]  Set is_stmt to 1
>>   [0x00008828]  Special opcode 189: advance Address by 13 to 0x7291 and Line by 2 to 3382 (*)
>>   [0x00008829]  Set is_stmt to 0 (**)
>>   [0x0000882a]  Copy (view 1)
>>   [0x0000882b]  Set File Name to entry 8 in the File Name Table <- back to alias.c
>>   [0x0000882d]  Set column to 8
>>   [0x0000882f]  Advance Line by -2543 to 839
>>   [0x00008832]  Copy (view 2)
>>   [0x00008833]  Set column to 27
>>   [0x00008835]  Special opcode 61: advance Address by 4 to 0x7295 and Line by 0 to 839
>>   [0x00008836]  Set column to 3
>>   [0x00008838]  Set is_stmt to 1 <-- next line info counts: alias.c:847
>>   [0x00008839]  Special opcode 153: advance Address by 10 to 0x729f and Line by 8 to 847
>>   [0x0000883a]  Set column to 7
>>
>> (*) this line is tree.h:3382, but the program counter is *not* within the subroutine,
>> but exactly at the first instruction *after* the subroutine according to the debug_ranges.
>>
>> What makes it worse, is that (**) makes gdb ignore the new location info alias.c:839,
>> which means, normally the n command would have continued to pc=0x729f, which is at alias.c:847.
>>
>>
>> The problem happens due to a block with only var
>> This patch fixes this problem by moving (**) to the first statement with a different line number.
>>
>> In alias.c.316r.final this looks like that:
>>
>> (note 2884 2883 1995 31 0x7f903a931ba0 NOTE_INSN_BLOCK_BEG)
>> (note 1995 2884 2885 31 ../../gcc-trunk/gcc/tree.h:3377 NOTE_INSN_INLINE_ENTRY)
>> (note 2885 1995 1996 31 0x7f903a931c00 NOTE_INSN_BLOCK_BEG)
>> [...]
>> (note 50 39 59 32 [bb 32] NOTE_INSN_BASIC_BLOCK)
>> (note 59 50 60 32 NOTE_INSN_DELETED)
>> (note 60 59 1997 32 NOTE_INSN_DELETED)
>> (note 1997 60 2239 32 ../../gcc-trunk/gcc/tree.h:3382 NOTE_INSN_BEGIN_STMT)
>> (note 2239 1997 2240 32 (var_location __tD.143911 (nil)) NOTE_INSN_VAR_LOCATION)
>> (note 2240 2239 2241 32 (var_location __sD.143912 (nil)) NOTE_INSN_VAR_LOCATION)
>> (note 2241 2240 2242 32 (var_location __fD.143913 (nil)) NOTE_INSN_VAR_LOCATION)
>> (note 2242 2241 2243 32 (var_location __lD.143914 (nil)) NOTE_INSN_VAR_LOCATION)
>> (note 2243 2242 2886 32 (var_location __gD.143915 (nil)) NOTE_INSN_VAR_LOCATION)
>> (note 2886 2243 2887 32 0x7f903a931c00 NOTE_INSN_BLOCK_END)
>> (note 2887 2886 57 32 0x7f903a931ba0 NOTE_INSN_BLOCK_END)
>> (insn:TI 57 2887 61 32 (set (reg/f:DI 0 ax [orig:87 _7 ] [87])
>>         (mem/f/j:DI (plus:DI (reg/f:DI 5 di [orig:83 t.85_2 ] [83])
>>                 (const_int 8 [0x8])) [0 t.85_2->typedD.91322.typeD.90828+0 S8 A64])) "../../gcc-trunk/gcc/alias.c":839:108 66 {*movdi_internal}
>>      (nil))
>>
>> So this patch detects the NOTE_INSN_VAR_LOCATION and makes the next location
>> with a different file&line info a statement location, which is hopefully
>> a real instruction, thus either part of the subroutine, or the first
>> instruction after the subroutine, which should have the correct location.
>> Once the same address has a second statement-type .loc info, gdb will ignore
>> the first one, and the stepping works as expected.
>>
>> So this is a bit of a heuristic, but it appears to work quite well.
>>
>> The test case g++.dg/guality/pr55541.C is the only test where this
>> change had an effect.  But it is not a regression, since previously
>> the test case was "unsupported" on any optimization mode, since there
>> was no breakpoint at line 11, now the breakpoint works, but the variable
>> value is wrong, but basically this was not working before.
>>
>> I don't know how to make this test xfail when compiled with optimization,
>> but the do-skip-if is probably good enough for this kind of test case.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
Bernd Edlinger Nov. 9, 2019, 10:20 a.m. UTC | #3
Ping...

On 11/2/19 7:49 AM, Bernd Edlinger wrote:
> Ping...
> 
> On 10/27/19 9:14 AM, Bernd Edlinger wrote:
>> Ping...
>>
>> I'd like to ping for this patch:
>> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01459.html
>>
>>
>> Thanks
>> Bernd.
>>
>> On 10/20/19 9:58 PM, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> this fixes an issue with the gdb step-over aka. "n" command.
>>>
>>> It can be seen when you debug an optimized stage-3 cc1
>>> it does not affect -O0 code, though.
>>>
>>> This example debug session will explain the effect.
>>>
>>> (gdb) b get_alias_set
>>> Breakpoint 5 at 0xa099f0: file ../../gcc-trunk/gcc/alias.c, line 837.
>>> (gdb) r
>>> Breakpoint 5, get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/alias.c:837
>>> 837	  if (t == error_mark_node
>>> (gdb) n
>>> 839		  && (TREE_TYPE (t) == 0 || TREE_TYPE (t) == error_mark_node)))
>>> (gdb) n
>>> 3382	  return __t;  <-- now we have a problem: wrong line info here
>>> (gdb) bt
>>> #0  get_alias_set (t=t@entry=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/tree.h:3382
>>> #1  0x0000000000b25dfe in set_mem_attributes_minus_bitpos (ref=0x7ffff746f990, t=0x7ffff7ff7ab0, objectp=1, bitpos=...)
>>>     at ../../gcc-trunk/gcc/emit-rtl.c:1957
>>> #2  0x0000000001137a55 in make_decl_rtl (decl=0x7ffff7ff7ab0) at ../../gcc-trunk/gcc/varasm.c:1518
>>> #3  0x000000000113b6e8 in assemble_variable (decl=0x7ffff7ff7ab0, top_level=<optimized out>, at_end=<optimized out>, 
>>>     dont_output_data=0) at ../../gcc-trunk/gcc/varasm.c:2246
>>> #4  0x000000000113f0ea in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:584
>>> #5  0x000000000113fa17 in varpool_node::assemble_decl (this=0x7ffff745b000) at ../../gcc-trunk/gcc/varpool.c:750
>>>
>>>
>>> There are at least two problems here:
>>>
>>> First you did not want to step into the TREE_TYPE, but it happens all
>>> the time, even if you use "n" to step over it.
>>>
>>> And secondly, from the call stack, you don't know where you are in get_alias_set.
>>> But the code that is executing at this point is actually the x == 0 || x == error_mark_node
>>> from alias.c, line 839, which contains the inlined body of the TREE_TYPE, but
>>> the rest of the if.  So there is an inconsistency in the  
>>>
>>> Contents of the .debug_info section:
>>>
>>>  <2><4f686>: Abbrev Number: 12 (DW_TAG_inlined_subroutine)
>>>     <4f687>   DW_AT_abstract_origin: <0x53d4e>
>>>     <4f68b>   DW_AT_entry_pc    : 0x7280
>>>     <4f693>   DW_AT_GNU_entry_view: 1
>>>     <4f695>   DW_AT_ranges      : 0xb480
>>>     <4f699>   DW_AT_call_file   : 8  <- alias.c
>>>     <4f69a>   DW_AT_call_line   : 839
>>>     <4f69c>   DW_AT_call_column : 8
>>>     <4f69d>   DW_AT_sibling     : <0x4f717>
>>>
>>>  The File Name Table (offset 0x253):
>>>   8     2       0       0       alias.c
>>>   10    2       0       0       tree.h
>>>
>>> Contents of the .debug_ranges section:
>>>
>>>     0000b480 0000000000007280 0000000000007291 
>>>     0000b480 0000000000002764 000000000000277e 
>>>     0000b480 <End of list>
>>>
>>> The problem is at pc=0x7291 in the Line Number Section:
>>>
>>>  Line Number Statements:
>>>
>>>   [0x00008826]  Special opcode 61: advance Address by 4 to 0x7284 and Line by 0 to 3380
>>>   [0x00008827]  Set is_stmt to 1
>>>   [0x00008828]  Special opcode 189: advance Address by 13 to 0x7291 and Line by 2 to 3382 (*)
>>>   [0x00008829]  Set is_stmt to 0 (**)
>>>   [0x0000882a]  Copy (view 1)
>>>   [0x0000882b]  Set File Name to entry 8 in the File Name Table <- back to alias.c
>>>   [0x0000882d]  Set column to 8
>>>   [0x0000882f]  Advance Line by -2543 to 839
>>>   [0x00008832]  Copy (view 2)
>>>   [0x00008833]  Set column to 27
>>>   [0x00008835]  Special opcode 61: advance Address by 4 to 0x7295 and Line by 0 to 839
>>>   [0x00008836]  Set column to 3
>>>   [0x00008838]  Set is_stmt to 1 <-- next line info counts: alias.c:847
>>>   [0x00008839]  Special opcode 153: advance Address by 10 to 0x729f and Line by 8 to 847
>>>   [0x0000883a]  Set column to 7
>>>
>>> (*) this line is tree.h:3382, but the program counter is *not* within the subroutine,
>>> but exactly at the first instruction *after* the subroutine according to the debug_ranges.
>>>
>>> What makes it worse, is that (**) makes gdb ignore the new location info alias.c:839,
>>> which means, normally the n command would have continued to pc=0x729f, which is at alias.c:847.
>>>
>>>
>>> The problem happens due to a block with only var
>>> This patch fixes this problem by moving (**) to the first statement with a different line number.
>>>
>>> In alias.c.316r.final this looks like that:
>>>
>>> (note 2884 2883 1995 31 0x7f903a931ba0 NOTE_INSN_BLOCK_BEG)
>>> (note 1995 2884 2885 31 ../../gcc-trunk/gcc/tree.h:3377 NOTE_INSN_INLINE_ENTRY)
>>> (note 2885 1995 1996 31 0x7f903a931c00 NOTE_INSN_BLOCK_BEG)
>>> [...]
>>> (note 50 39 59 32 [bb 32] NOTE_INSN_BASIC_BLOCK)
>>> (note 59 50 60 32 NOTE_INSN_DELETED)
>>> (note 60 59 1997 32 NOTE_INSN_DELETED)
>>> (note 1997 60 2239 32 ../../gcc-trunk/gcc/tree.h:3382 NOTE_INSN_BEGIN_STMT)
>>> (note 2239 1997 2240 32 (var_location __tD.143911 (nil)) NOTE_INSN_VAR_LOCATION)
>>> (note 2240 2239 2241 32 (var_location __sD.143912 (nil)) NOTE_INSN_VAR_LOCATION)
>>> (note 2241 2240 2242 32 (var_location __fD.143913 (nil)) NOTE_INSN_VAR_LOCATION)
>>> (note 2242 2241 2243 32 (var_location __lD.143914 (nil)) NOTE_INSN_VAR_LOCATION)
>>> (note 2243 2242 2886 32 (var_location __gD.143915 (nil)) NOTE_INSN_VAR_LOCATION)
>>> (note 2886 2243 2887 32 0x7f903a931c00 NOTE_INSN_BLOCK_END)
>>> (note 2887 2886 57 32 0x7f903a931ba0 NOTE_INSN_BLOCK_END)
>>> (insn:TI 57 2887 61 32 (set (reg/f:DI 0 ax [orig:87 _7 ] [87])
>>>         (mem/f/j:DI (plus:DI (reg/f:DI 5 di [orig:83 t.85_2 ] [83])
>>>                 (const_int 8 [0x8])) [0 t.85_2->typedD.91322.typeD.90828+0 S8 A64])) "../../gcc-trunk/gcc/alias.c":839:108 66 {*movdi_internal}
>>>      (nil))
>>>
>>> So this patch detects the NOTE_INSN_VAR_LOCATION and makes the next location
>>> with a different file&line info a statement location, which is hopefully
>>> a real instruction, thus either part of the subroutine, or the first
>>> instruction after the subroutine, which should have the correct location.
>>> Once the same address has a second statement-type .loc info, gdb will ignore
>>> the first one, and the stepping works as expected.
>>>
>>> So this is a bit of a heuristic, but it appears to work quite well.
>>>
>>> The test case g++.dg/guality/pr55541.C is the only test where this
>>> change had an effect.  But it is not a regression, since previously
>>> the test case was "unsupported" on any optimization mode, since there
>>> was no breakpoint at line 11, now the breakpoint works, but the variable
>>> value is wrong, but basically this was not working before.
>>>
>>> I don't know how to make this test xfail when compiled with optimization,
>>> but the do-skip-if is probably good enough for this kind of test case.
>>>
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>

Patch
diff mbox series

2019-10-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* final.c (force_stmt_source_line): New variable.
	(final_scan_insn_1): Force stmt-type location info
	after a NOTE_INSN_VAR_LOCATION.

testsuite:
2019-10-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* g++.dg/guality/pr55541.C: Add dg-skip-if.

Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 277155)
+++ gcc/final.c	(working copy)
@@ -156,6 +156,9 @@  static int override_discriminator;
 /* Whether to force emission of a line note before the next insn.  */
 static bool force_source_line = false;
 
+/* Force emission of a stmt line note on next line info change.  */
+static bool force_stmt_source_line = false;
+
 extern const int length_unit_log; /* This is defined in insn-attrtab.c.  */
 
 /* Nonzero while outputting an `asm' with operands.
@@ -2412,6 +2415,7 @@  final_scan_insn_1 (rtx_insn *insn, FILE *file, int
 	    {
 	      debug_hooks->var_location (insn);
 	      set_next_view_needed (seen);
+	      force_stmt_source_line = true;
 	    }
 	  break;
 
@@ -2425,6 +2429,7 @@  final_scan_insn_1 (rtx_insn *insn, FILE *file, int
 					   last_filename, last_discriminator,
 					   true);
 	      clear_next_view_needed (seen);
+	      force_stmt_source_line = false;
 	    }
 	  break;
 
@@ -2529,7 +2534,7 @@  final_scan_insn_1 (rtx_insn *insn, FILE *file, int
 
 	if (MAY_HAVE_DEBUG_MARKER_INSNS && cfun->debug_nonbind_markers)
 	  {
-	    is_stmt = false;
+	    is_stmt = force_stmt_source_line;
 	    is_stmt_p = NULL;
 	  }
 	else
@@ -2648,6 +2653,7 @@  final_scan_insn_1 (rtx_insn *insn, FILE *file, int
 					 last_filename, last_discriminator,
 					 is_stmt);
 	    clear_next_view_needed (seen);
+	    force_stmt_source_line = false;
 	  }
 	else
 	  maybe_output_next_view (seen);
Index: gcc/testsuite/g++.dg/guality/pr55541.C
===================================================================
--- gcc/testsuite/g++.dg/guality/pr55541.C	(revision 277155)
+++ gcc/testsuite/g++.dg/guality/pr55541.C	(working copy)
@@ -1,7 +1,7 @@ 
 // PR debug/55541
 // { dg-do run }
 // { dg-options "-g" }
-
+// { dg-skip-if "" { *-*-* } { "*" } { "-O0" } }
 int
 main ()
 {