From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) by sourceware.org (Postfix) with ESMTPS id 84C0B3858D28 for ; Tue, 24 Jan 2023 19:21:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 84C0B3858D28 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-f47.google.com with SMTP id d4-20020a05600c3ac400b003db1de2aef0so11729307wms.2 for ; Tue, 24 Jan 2023 11:21:34 -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:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=n7ZXhyOD2b7pjERRFGWDHXgy5m6M2cNoiTFeLIfiBFc=; b=4pgAPLyNz2H23daNZCu52TW683sfjRtL5XGNJiRqN42pBJ1VblePNshnqIoOMTlPdf ru/eqB2xNtsWelgbWE+om0CdwmuvpzlXKK7fvubo2F9e1uJ7zT/pxQEgpGRFScCHTH/K TDGYdb3mz5Wx+HDyPStUgO3rqS8HD4RDJsD+ReegswMkeMO2X+qPCFlx7yuOP6oU9ego CznKk9w0StRv8yZi7ZKp0iwSfU78fhwhEmcQhr0KFnofTKGoXIklBd/7+4D0CpmF1Heg Zfa0MXE/Hu5fCHc444N726APV9UC9RIxwRPmyv1Tr6kD0SlHfEK5DUTmhHQbB4jbx/A0 V2wg== X-Gm-Message-State: AFqh2krAjNnbJnCOJNrcZ+diDjSDrhE2/pPQDWhY+RODE28ikHMmdmM4 VFJ2Zyv4VWX2Q9aDku5/lTlZI1x3fs9vsg== X-Google-Smtp-Source: AMrXdXtB90B5ShjAzSr9U091M7wpjNKzKp4+KkAk/xHpopferBaCa5aCC/UC9Bplum7WGzvmtShGtg== X-Received: by 2002:a05:600c:3b02:b0:3da:1bb0:4d71 with SMTP id m2-20020a05600c3b0200b003da1bb04d71mr36983253wms.11.1674588093340; Tue, 24 Jan 2023 11:21:33 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id p7-20020a05600c468700b003a3442f1229sm15089317wmo.29.2023.01.24.11.21.32 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 Jan 2023 11:21:32 -0800 (PST) Subject: Re: [PATCH 1/2 version 3] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp To: Carl Love , Bruno Larsen , Ulrich Weigand , gdb-patches@sourceware.org Cc: Luis Machado 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> From: Pedro Alves Message-ID: Date: Tue, 24 Jan 2023 19:21:31 +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: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,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 6:25 p.m., Carl Love wrote: > I have not tested with clang. Actually I have never used clang so this > is another thing to look at and test. > > > Let me see if I can summarize the current situation. > > 1) The goal of the current X86 reverse patch is to fix the case where > we have called function foo () and are trying to do a reverse-finish > from inside the function. Wrong goal. There's nothing to fix there. The command is working as designed. > The mainline gdb code stops at the entry to > foo () then does a single instruction in the reverse direction to get > to the caller. Specifically, the code that this patch removes: > > - if (ecs->event_thread->control.proceed_to_finish > - && execution_direction == EXEC_REVERSE) > - { > - struct thread_info *tp = ecs->event_thread; > - > - /* We are finishing a function in reverse, and just hit the > - step-resume breakpoint at the start address of the > - function, and we're almost there -- just need to back up > - by one more single-step, which should take us back to the > - function call. */ > - tp->control.step_range_start = tp->control.step_range_end = 1; > - keep_going (ecs); > - return; > > The single-step in gdb results in stopping at the last instruction in > the line where function foo is called. The result here is that you now > need to do two reverse-next instructions to get to the previous line. And that is the bug. reverse-next/reverse-step should _never_ stop at the same line. Same as forward step/next. For instance, this "next" command will just continue stepping forever: Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc98) at reverse.c:11 11 while (1); (gdb) next Same for "step" instead of "next". It turns out that reverse-next/step do stop immediately! (gdb) reverse-next 11 while (1); (gdb) reverse-next 11 while (1); (gdb) That's probably caused by the same bug. > The command sequence is: reverse-finish; reverse-next; reverst-next to > go from inside the function to the previous line in the caller. > > Note, in this case you are in the function and trying to return to the > caller with the reverse-finish command. That is a different scenario > from what Pedro is talking about in 2) below. > > 2) The scenario that Pedro brings up is a reverse-next over a line with > two function calls on the same source line, i.e. > >> 9 int main () >> 10 { >> 11 func1 (); func2 (); >> 12 } > > In this case you are in the caller and are trying to do a reverse-next > over the two function callers. This is a different scenario from 1). > I know it's a different scenario, but my scenario is just a way to show up that the root of the problem can be seen with a simpler scenario. The problem is in the reverse-step command, and you are focusing on the reverse-finish command, incorrectly. Please read my responses to Bruno as well. > 3) The bug that I mentioned earlier for the case of > multiple executable statements on the same line. > > https://sourceware.org/bugzilla/show_bug.cgi?id=28426 > > is for a case like: > > int main () > { > int a, b, c, d; > a = 1; > b = 2; > c = 3; d = 4; > a = a + b + c + d; > } > > In this case the reverse-next from the last line a = a + b + c + d; is > not stepping all the way back over the line c = 3; d = 4;. This is a > simpler version of 2). Specifically the command sequence to step over > line c = 3; d = 4; is reverse-next, reverse-next. Only one reverse- > next should be required. > Exactly. It's all the same bug. > The patch that Luis and I have worked on fixes this issue, however it > does not fix the case of the multiple statements being function calls, > i.e. 2) above that Pedro is bringing up. I have said it numerous times now, but it's worth repeating. It's all the same bug. :-) Fix reverse-step, and then the reverse-finish scenario fixes itself, because you will no longer need to issue two reverse-step commands after reverse-finish. > > > From a subsequent message from Pedro in this thread: > > On Tue, 2023-01-24 at 15:06 +0000, Pedro Alves wrote: > > 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... > > I don't agree that I am "papering over the actual problem" rather I > think at this point that Pedro's test case is an additional case of the > reverse-next command being broken by my patch to fix the reverse-finish > command. The problem doesn't exist without my patch. > > > So at this point, I need to go see if I can figure out how the patch to > fix the reverse-finish command causes the regression in 2) for the > reverse-next command. At this point. NAK on the reverse-finish fix. > > Looks like we also need an to add an additional test case for 2). > Also, will need to look at how any new fix for 2) behaves with clang. > > Thanks for all the input on this issue. It seems that there is still > work to do.