Re: [PATCH 1/2 version 3] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

Bruno Larsen via Gdb-patches
2023-01-24T16:06:56Z

On 24/01/2023 16:06, Pedro Alves wrote:
> On 2023-01-24 2:23 p.m., Bruno Larsen wrote:
>> On 24/01/2023 15:08, Pedro Alves wrote:
>>> On 2023-01-23 9:13 p.m., Carl Love wrote:
>>>> Pedro:
>>>>
>>>> On Mon, 2023-01-23 at 19:17 +0000, Pedro Alves wrote:
>>>>>> Currently on X86, when executing the finish command in reverse, gdb
>>>>>> does a
>>>>>> single step from the first instruction in the callee to get back to
>>>>>> the
>>>>>> caller.  GDB stops on the last instruction in the source code line
>>>>>> where
>>>>>> the call was made.  When stopped at the last instruction of the
>>>>>> source code
>>>>>> line, a reverse next or step command will stop at the first
>>>>>> instruction
>>>>>> of the same source code line thus requiring two step/next commands
>>>>>> to
>>>>>> reach the previous source code line.  It should only require one
>>>>>> step/next
>>>>>> command to reach the previous source code line.
>>>>>>
>>>>>> By contrast, a reverse next or step command from the first line in
>>>>>> a
>>>>>> function stops at the first instruction in the source code line
>>>>>> where the
>>>>>> call was made.
>>>>> I'd think this was on purpose.  Note that next/step/reverse-
>>>>> {next/step} are line-oriented
>>>>> stepping commands, they step/next until the previous/next line.
>>>>> While "finish" is described
>>>>> as undoing the _function call_.
>>>>>
>>>>> The manual says:
>>>>>
>>>>>   reverse-finish
>>>>>   Just as the finish command takes you to the point where the current
>>>>> function returns,
>>>>>   reverse-finish takes you to the point where it was called. Instead
>>>>> of ending up at the end of
>>>>>   the current function invocation, you end up at the beginning.
>>>>>
>>>>> Say you have a line with multiple statements involving multiple
>>>>> function calls.
>>>>> The simplest would be:
>>>>>
>>>>>    func1 ();  func2 ();
>>>>>
>>>>> Say you'd stopped inside 'func2'.  If you do finish there, in current
>>>>> master gdb
>>>>> stops at the call to 'func2', and you can then decide to reverse step
>>>>> into 'func1'.
>>>> I don't think you followed the issue.
>>> Totally possible!
>>>
>>>> So, if you are in func2 and do a reverse-finish, without the patch gdb
>>>> stops on the last instruction for the line that calls func2.
>>> Right.
>>>
>>>> Now if
>>>> you issue a reverse-step, you stop at the first instruction for the
>>>> call to func2, i.e. you are still on the same source code line.
>>> Wait.  That right there sounds bogus.  The source line looks like:
>>>
>>>     func1 ();  func2 ();
>>>
>>> so stepping backwards over that line should always stop at the first
>>> instruction of the line, not in the middle.  Let's simplify this.
>>>
>>> Here's the full source code of my example:
>>>
>>> (gdb) list 1
>>> 1       void func1 ()
>>> 2       {
>>> 3       }
>>> 4
>>> 5       void func2 ()
>>> 6       {
>>> 7       }
>>> 8
>>> 9       int main ()
>>> 10      {
>>> 11        func1 (); func2 ();
>>> 12      }
>>>
>> I think you are describing a different issue to what Carl is trying to solve. Using your example code, I have the following debugging session:
> No, it's the exact same. Please re-read.
>
>> $ ./gdb -q reverse
>> Reading symbols from reverse...
>> (gdb) start
>> Temporary breakpoint 1 at 0x401118: file t.c, line 8.
>> Starting program: /home/blarsen/Documents/build/gdb/reverse
>> [Thread debugging using libthread_db enabled]
>> Using host libthread_db library "/lib64/libthread_db.so.1".
>>
>> Temporary breakpoint 1, main () at t.c:8
>> 8           func1(); func2();
>> (gdb) record
>> (gdb) n
>> 9       }
>> (gdb) rs
>> func2 () at t.c:5
>> 5       }
>> (gdb) reverse-finish
>> Run back to call of #0  func2 () at t.c:5
>> 0x0000000000401127 in main () at t.c:8
>> 8           func1(); func2();
>> (gdb) rs
>> 8           func1(); func2();
>> (gdb)
>> func1 () at t.c:2
>> 2       }
>> (gdb)
>>
>> Notice how there were two "reverse-step" commands needed after the "reverse-finish" used to exit func2.
> Yes. And the reason you need two "reverse-step" is because there are two line ranges for line 8, and reverse-step stops
> at the beginning of the second range instead of at the beginning of the first. It's the exact same as with my simpler
> example of just doing "next" -> "reverse-next", which I used as a simpler example to explain the problem.

The simplified example is not the exact same use case. Looking at how
the program executes forward, if we next over this line we skip
everything, but if we step into func1 and use finish we will still stop
in line 11 and the user can decide to step into func2 or not. That is
why I assumed you were thinking of a different issue when I saw you
using next and reverse-next as examples.

Looking at where GDB lands when finishing from func1 we get (code
compiled with gcc -gno-column-info):

(gdb) start
Temporary breakpoint 1 at 0x401118: file t.c, line 8.
Starting program: /home/blarsen/Documents/downstream_build/gdb/reverse
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Temporary breakpoint 1, main () at t.c:8
8           f1(); f2();
(gdb) s
f1 () at t.c:2
2       }
(gdb) finish
Run till exit from #0  f1 () at t.c:2
0x0000000000401122 in main () at t.c:8
8           f1(); f2();
(gdb) disas /s
Dump of assembler code for function main:
t.c:
7       int main(){
0x0000000000401114 <+0>:     push   %rbp
0x0000000000401115 <+1>:     mov    %rsp,%rbp

8           f1(); f2();
0x0000000000401118 <+4>:     mov    $0x0,%eax
0x000000000040111d <+9>:     call   0x401106 <f1>
=> 0x0000000000401122 <+14>:    mov    $0x0,%eax
0x0000000000401127 <+19>:    call   0x40110d <f2>
0x000000000040112c <+24>:    mov    $0x0,%eax

9       }
0x0000000000401131 <+29>:    pop    %rbp
0x0000000000401132 <+30>:    ret
End of assembler dump.

The behavior that we should emulate when going backwards is that
finishing from func2 stops us on the 'mov' instruction between the
calls. Looking at the clang session that I cut off from the previous email:

Alright, here's the gdb session, with clang, no gdb patch:

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc48) at reverse.c:11
11 func1 (argc); func2 (argc);
(gdb) disassemble /s
Dump of assembler code for function main:
reverse.c:
10 {
0x0000555555555150 <+0>: push %rbp
0x0000555555555151 <+1>: mov %rsp,%rbp
0x0000555555555154 <+4>: sub $0x10,%rsp
0x0000555555555158 <+8>: mov %edi,-0x4(%rbp)
0x000055555555515b <+11>: mov %rsi,-0x10(%rbp)

11 func1 (argc); func2 (argc);
=> 0x000055555555515f <+15>: mov -0x4(%rbp),%edi
0x0000555555555162 <+18>: call 0x555555555130 <func1>
0x0000555555555167 <+23>: mov -0x4(%rbp),%edi
0x000055555555516a <+26>: call 0x555555555140 <func2>

12 }
0x000055555555516f <+31>: xor %eax,%eax
0x0000555555555171 <+33>: add $0x10,%rsp
0x0000555555555175 <+37>: pop %rbp
0x0000555555555176 <+38>: ret
End of assembler dump.
(gdb) record
(gdb) b func2
Breakpoint 2 at 0x555555555147: file reverse.c, line 7.
(gdb) c
Continuing.

Breakpoint 2, func2 (i=1) at reverse.c:7
7 }
(gdb) reverse-finish
Run back to call of #0 func2 (i=1) at reverse.c:7
0x000055555555516a in main (argc=1, argv=0x7fffffffdc48) at reverse.c:11
11 func1 (argc); func2 (argc);
(gdb) disassemble /s
Dump of assembler code for function main:
reverse.c:
10 {
0x0000555555555150 <+0>: push %rbp
0x0000555555555151 <+1>: mov %rsp,%rbp
0x0000555555555154 <+4>: sub $0x10,%rsp
0x0000555555555158 <+8>: mov %edi,-0x4(%rbp)
0x000055555555515b <+11>: mov %rsi,-0x10(%rbp)

11 func1 (argc); func2 (argc);
0x000055555555515f <+15>: mov -0x4(%rbp),%edi
0x0000555555555162 <+18>: call 0x555555555130 <func1>
0x0000555555555167 <+23>: mov -0x4(%rbp),%edi
=> 0x000055555555516a <+26>: call 0x555555555140 <func2>

12 }
0x000055555555516f <+31>: xor %eax,%eax
0x0000555555555171 <+33>: add $0x10,%rsp
0x0000555555555175 <+37>: pop %rbp
0x0000555555555176 <+38>: ret
End of assembler dump.
(gdb) reverse-step
func1 (i=1) at reverse.c:3
3 }
(gdb)

We can see that GDB stopped on the call instruction instead. So a user
that finished from func1 or reverse-finished from func2 may see
different inferior states.

>
>> What Carl is proposing we do is make it so GDB only needs one command.
> I understand. However, I am saying that that is papering over the actual problem, _and_ it only works in the situation
> where you ended up with two line entries with is-stmt for the same line. Note how the patch breaks clang, and gcc with
> -gno-column-info...
>
>> If I compare the $pc of where GDB is stopped and the linetable, we get:
>>
>> (gdb) print $pc
>> $2 = (void (*)()) 0x401127 <main+19>
>> (gdb) maint info line-table
>> objfile: /home/blarsen/Documents/downstream_build/gdb/reverse ((struct objfile *) 0x10f7750)
>> compunit_symtab: t.c ((struct compunit_symtab *) 0x116c330)
>> symtab: /home/blarsen/Documents/downstream_build/gdb/t.c ((struct symtab *) 0x116c3b0)
>> linetable: ((struct linetable *) 0x11aec80):
>> INDEX  LINE   ADDRESS            IS-STMT PROLOGUE-END
>> 0      1      0x0000000000401106 Y
>> 1      2      0x000000000040110a Y
>> 2      4      0x000000000040110d Y
>> 3      5      0x0000000000401111 Y
>> 4      7      0x0000000000401114 Y
>> 5      8      0x0000000000401118 Y
>> 6      8      0x0000000000401122 Y
>> 7      9      0x0000000000401131 Y
>> 8      END    0x0000000000401133 Y
>>
>> We can see that GDB shouldn't even be able to stop at that $pc, it isn't an is_stmt.
> reverse-finish is not supposed to step backwards until it reaches is_stmt. Doing so makes it
> step backwards too much, as I've shown in my previous example.
>
>> We should have stopped at 0x401122, which is where the first reverse-step stops:
> No...
>
>> (gdb) rs
>> 8           f1(); f2();
>> (gdb) p $pc
>> $4 = (void (*)()) 0x401122 <main+14>
> ... no because in the case where you don't have column debug info (or with clang), there won't be
> an is-stmt entry for the f2 call/column, there will only be one single line entry for the whole of
> line 8, so gdb would step back too much.
>
>> So not only are we needing an extra command to re-sync with user expectations, we are in an instruction where the inferior state might be all over the place.
>>
> What does that mean, the state might be all over the place? The DWARF should describe the locations of all variables accurately at all instructions.
>
Unrelated to this specific issue, but I was looking at record/30025
(https://sourceware.org/bugzilla/show_bug.cgi?id=30025) and it confused
me because because GDB sometimes reported the incorrect parameter, but
if we continue single-stepping through instructions, GDB eventually
finds the correct value when we stop at an is_stmt instruction.

If I misunderstood something about 30025 and is_stmt, please let me
know! but as far as I can see, this is what happened.

--
Cheers,
Bruno