From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 24DAB3858C00 for ; Tue, 24 Jan 2023 16:06:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 24DAB3858C00 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674576359; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lNLEYEoVOEI7e+EQr2CADYKDVRlziL8u+cL4FsgxEWQ=; b=Cw8vt/vjsj++hp1LvCXpPEOK6UjhrJ84onfATarwWHJNlJjIuTa8LZVWvUCGoluyBtoBA7 EUKABtoeu8vTleggMuj/DWDzJVE8cs4i15bL+CqFp17/LQ1zb2wGbsw15D9Z2oS2KuAj2C /56kN6bV9Gw0OFhfoUyhZxQO3Z7diQc= Received: from mail-ot1-f71.google.com (mail-ot1-f71.google.com [209.85.210.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-191-A8-ytWKuMa-Zgbhyq7Nf8A-1; Tue, 24 Jan 2023 11:04:50 -0500 X-MC-Unique: A8-ytWKuMa-Zgbhyq7Nf8A-1 Received: by mail-ot1-f71.google.com with SMTP id bx22-20020a056830601600b00684958cb0b9so7808617otb.1 for ; Tue, 24 Jan 2023 08:04:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=lNLEYEoVOEI7e+EQr2CADYKDVRlziL8u+cL4FsgxEWQ=; b=aFQDZFKwELLhfWJ1qFwg4bUej51RUbzRKxhv9Rhl4njyKpqKvSH3VrJkFT8FiISZZH H+HPefIOMO1UwbH22J3GTq536n4TxKjvwstCKeaY0KkO+OQ7vIj/Sx/MO9Xkl5zifuxQ xNw1OboqwB8fEyMnmb8JURCBZhfJlL7Bp006J2XvADOFZ/k/wJKZcJHVcUAA+IZCDsXg 798kHnsy7T36VBQ8v+pXqIm8R3UNo26q/esVjx0ien9LXJgwCt8PQiuZBIrt/Iu5rQQP +KEyDCd/QeiYwc/Bi8DHxGRDhn/0Pl+GkWjXOAQGc4H9nOEhoyZmO9foV1hK1WFFLdYg PteA== X-Gm-Message-State: AO0yUKVuD+imTVpDKckFTtKPtTz7XoPwHKynLnUPfLXpitfjV6u23fRI P+JhF2Q8zKU6smuhjmYv2/IyA3/9/8FpgULQ09RJ8dxm3DIkqgGL+C6VEUmL+t5DqsDAFWOFyyU F1PvJSM6zjFjlgoIsM5G8Fw== X-Received: by 2002:a05:6358:4308:b0:ef:6350:1f57 with SMTP id r8-20020a056358430800b000ef63501f57mr153969rwc.6.1674576284525; Tue, 24 Jan 2023 08:04:44 -0800 (PST) X-Google-Smtp-Source: AK7set9Xc/2MffUuV2sAnK6175rzcxFuXGjXiu8L0mkQY7T+7IlCFcYJGyyhOrm1LTS9V1AsyQpnSA== X-Received: by 2002:a05:6358:4308:b0:ef:6350:1f57 with SMTP id r8-20020a056358430800b000ef63501f57mr153951rwc.6.1674576284118; Tue, 24 Jan 2023 08:04:44 -0800 (PST) Received: from [192.168.0.45] (ip-62-245-66-121.bb.vodafone.cz. [62.245.66.121]) by smtp.gmail.com with ESMTPSA id 188-20020a3703c5000000b006fb112f512csm1636182qkd.74.2023.01.24.08.04.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 Jan 2023 08:04:43 -0800 (PST) Message-ID: <837e302b-3ac3-67bd-5314-55194ab35421@redhat.com> Date: Tue, 24 Jan 2023 17:04:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH 1/2 version 3] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp To: Pedro Alves , Carl Love , Ulrich Weigand , "will_schmidt@vnet.ibm.com" , gdb-patches@sourceware.org References: <89331c26795e3f7743e1e068dce43b3c2dd53008.camel@us.ibm.com> <071f24ecf9b3a2bbbe8fee7db77492eb55c5f3ff.camel@us.ibm.com> <1d9b21914354bef6a290ac30673741e722e11757.camel@de.ibm.com> <3e3c9c40f07ab01c79fe10915e76ffa187c42ad9.camel@us.ibm.com> <122f5d2d3db9ef1979b0f8da927d005f32bba82c.camel@us.ibm.com> <011768e8-2b76-f8ed-1174-fbaa020b15e7@redhat.com> <58cebd1a-7883-fbc6-ac94-c67293f8fc8d@redhat.com> <5e5dc4a49aa8feb370419a1efecf277673b7dfc7.camel@us.ibm.com> <610d5f171d5f4baeb94887217e69d0e6d70e9d66.camel@us.ibm.com> <873eb58a-a6ab-08b2-0827-ca6e0c8088ae@palves.net> <8c4ebc33-f7b5-14a3-3bb0-155fe03e92d8@redhat.com> From: Bruno Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,TXREP,T_SPF_TEMPERROR,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 => 0x0000000000401122 <+14>:    mov    $0x0,%eax    0x0000000000401127 <+19>:    call   0x40110d    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 0x0000555555555167 <+23>: mov -0x4(%rbp),%edi 0x000055555555516a <+26>: call 0x555555555140 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 0x0000555555555167 <+23>: mov -0x4(%rbp),%edi => 0x000055555555516a <+26>: call 0x555555555140 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 >> (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 > ... 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