From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by sourceware.org (Postfix) with ESMTPS id BF64B3858D28 for ; Tue, 24 Jan 2023 15:06:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BF64B3858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f54.google.com with SMTP id iv8-20020a05600c548800b003db04a0a46bso1257010wmb.0 for ; Tue, 24 Jan 2023 07:06:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=E2Uq7zh19skB6jehRaWjKEWSrdT+DQzhGy0NOAc1zO0=; b=zLsaALyqJQhm8AQAMpCJcYP28kkanYPGhWQBJ0gT2xBh8J8OWr7KHk1fR7qQY4ABpF XorD4o8b5SYrm7ztxklsF10h0lGJCtVB+4Chl0etcKJUwbLrQLeDDw1xR0ArjedUCrKn MJP+zQrjwEhSrdzP+rN51peDPyJUyMCvJ5QV7PILcxdkLur9di8wGdOJgBUs3FAiHFuk zXWqlAUgi/1MVl6h0u5+OySfEzhAOjuCvwbQ5n+1lZnUFNoBpqPk0yHGGs6Xym6IAh/v b3/Cghe3Az5vRcmkzm5A17hUB9mU2LwO4R1hFhDZ12YThH/UaXMsd3eqt0+Q6s4xnVzm VJEA== X-Gm-Message-State: AFqh2kp8MmivG6XEAn0Jb5c88jIlCktjml1OAhzGxKFw5lpT1yPn0cJn IQFEFb/OShUlmSpBhsaKA8/GHafRp7uRlQ== X-Google-Smtp-Source: AMrXdXvYfafFbiplrhmMjkHOskE5jCwwMHBj0wd9+K92hLYgP14fxgZIrhZYie1XzjVhIm65sRrSMw== X-Received: by 2002:a05:600c:1c23:b0:3db:b9f:f2e with SMTP id j35-20020a05600c1c2300b003db0b9f0f2emr26394486wms.14.1674572776952; Tue, 24 Jan 2023 07:06:16 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id c3-20020a1c3503000000b003dc0b0e1e47sm3028425wma.48.2023.01.24.07.06.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 Jan 2023 07:06:16 -0800 (PST) Subject: Re: [PATCH 1/2 version 3] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp To: Bruno Larsen , 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: Pedro Alves Message-ID: Date: Tue, 24 Jan 2023 15:06:15 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <8c4ebc33-f7b5-14a3-3bb0-155fe03e92d8@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00,BODY_8BITS,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham 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 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. > 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.