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-25T09:49:56Z

On 24/01/2023 20:12, Pedro Alves wrote:
> On 2023-01-24 4:04 p.m., Bruno Larsen wrote:
>> On 24/01/2023 16:06, Pedro Alves wrote:
>>>> 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.
> Correct, but it's the same root cause. Forget at reverse-finish, that is doing what it says it should do, that is,
> to stop at the call to the function. You can just put a breakpoint at that same instruction, and once there, do a
> reverse-step. GDB should then reverse step until it reaches a different line. But it does not. _That_ is the bug.
Ok, you convinced me. I played around some more with the no-column-info
example and what you said and I agree that fixing the column info bug
would fix Carl's bug for this patch. I'll update the relevant bug
(https://sourceware.org/bugzilla/show_bug.cgi?id=28426) and now we have
a way to reproduce that!
>
> Vis (with gcc and column info):
>
> ...
> (gdb) reverse-finish
> Run back to call of #0 func2 (i=1) at reverse.c:7
> 0x0000555555555167 in main (argc=1, argv=0x7fffffffdc58) at reverse.c:11
> ^^^^^^^^^^^^^^^^^^
> 11 func1 (argc); func2 (argc);
> (gdb) start
> The program being debugged has been started already.
> Start it from the beginning? (y or n) y
> Temporary breakpoint 2 at 0x555555555158: file reverse.c, line 11.
> Starting program: /home/pedro/reverse
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>
> Temporary breakpoint 2, main (argc=1, argv=0x7fffffffdc58) at reverse.c:11
> 11 func1 (argc); func2 (argc);
> (gdb) record
> (gdb) b *0x0000555555555167 << manually set the breakpoint where the reverse-finish would stop
> Breakpoint 4 at 0x555555555167: file reverse.c, line 11.
> (gdb) c
> Continuing.
>
> Breakpoint 4, 0x0000555555555167 in main (argc=1, argv=0x7fffffffdc58) at reverse.c:11
> 11 func1 (argc); func2 (argc); << started at line 11
> (gdb) reverse-step
> 11 func1 (argc); func2 (argc); << stopped at the same line, bug!
>
> See, reverse-step stopped in the same line you started with. That is a bug, it should never
> happen. Just like forward "step" never steps in the same line. Because that's how the
> commands are supposed to work -- step until the line number changes!
>
>> 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:
>>
> What is there are more instructions in between the calls? How do you know where to stop?
>
>> 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.
>>
> Seeing different inferior states is expected, as finish and reverse-finish are not the exact mirror of one another, like step
> and reverse-step are. The exact reversal of finish would be the equivalent of being stopped at a function call return insn after
> a "finish" (and the command could only be used while stopped there), and stepping back into the function up until the point
> where you had typed "finish" and stopping there. Obviously impossible to implement. So we made "reverse-finish" do something
> sensible, such as stepping backwards up until the call site.
>
>>>> 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.
>>
> I can't tell what is going on in that bug without debugging it myself, but seeing SIGTRAP makes me suspect that
> one thing that could be going on is that GDB didn't apply the decr-pc-after-break adjustment properly, so
> then the PC is pointing into the middle of an instruction and then all hell breaks loose.
>
oh! This is very helpful, thank you!

--
Cheers,
Bruno