public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: Carl Love <cel@us.ibm.com>, Bruno Larsen <blarsen@redhat.com>,
	Ulrich Weigand <Ulrich.Weigand@de.ibm.com>,
	gdb-patches@sourceware.org
Cc: Luis Machado <luis.machado@arm.com>
Subject: Re: [PATCH 1/2 version 3] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp
Date: Tue, 24 Jan 2023 19:21:31 +0000	[thread overview]
Message-ID: <d7ca6642-2318-4b52-2791-8cbf8f89d9a6@palves.net> (raw)
In-Reply-To: <e1e93a4645eda1b74463a039a6f4241892bd4a44.camel@us.ibm.com>

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.  

  reply	other threads:[~2023-01-24 19:21 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <f594ec0070a6c585e83a6d6c8b29481a86778c0f.camel@us.ibm.com>
     [not found] ` <bc6bb459f153c0c5850d4a3e5d80bbf957ec36cc.camel@de.ibm.com>
     [not found]   ` <8bce850fa1e03e798506dc170d9b57f52034a18a.camel@us.ibm.com>
     [not found]     ` <cb5875db4e1ac60475877c685e5f172770314523.camel@de.ibm.com>
     [not found]       ` <adeeeae47c4ca79b32d79aea632ff8b2a24dd93d.camel@us.ibm.com>
     [not found]         ` <86c5e9c47945894f21b1d8bf6089c730a9f0e1a5.camel@de.ibm.com>
     [not found]           ` <b1d7ea600d6bb7af487968d938566fae9d5e1745.camel@us.ibm.com>
     [not found]             ` <5f9047b9582403561d7cce998cab9184167366a1.camel@de.ibm.com>
     [not found]               ` <e7c8093c350ad475277154014a4f0bb9b472b7af.camel@us.ibm.com>
     [not found]                 ` <f8d6379aff7af076d9edcee7d2981d052b2161ee.camel@de.ibm.com>
     [not found]                   ` <5b50668cbe882c57b8c0e9dcf5be0a253713c4c6.camel@us.ibm.com>
     [not found]                     ` <51c4bfc82ac72e475e10577dc60e4d75fa48767e.camel@de.ibm.com>
     [not found]                       ` <3ea97a8aa9cccb39299adde682f92055d1986ab3.camel@us.ibm.com>
     [not found]                         ` <f5ea8da12631f2496ba0e2263e65a0adc7ac56ca.camel@de.ibm.com>
     [not found]                           ` <53878e37c6e57de1d04d9c9960c5d0a74324ee6e.camel@us.ibm.com>
     [not found]                             ` <a5300b64533fdc753c1d50fa0e6efc21b5457547.camel@de.ibm.com>
     [not found]                               ` <50474aa92ba82eff05cdc8f49001eae56be29670.camel@us.ibm.com>
     [not found]                                 ` <f3ef4486c4ba051024602928acdfe5ddf6942b82.camel@de.ibm.com>
     [not found]                                   ` <dae6c9790b23a90d5f1494f5b6798346444f257e.camel@us.ibm.com>
     [not found]                                     ` <89331c26795e3f7743e1e068dce43b3c2dd53008.camel@us.ibm.com>
     [not found]                                       ` <c10a008e441666e4edb0916842d8eefe83f5b2f9.camel@de.ibm.com>
     [not found]                                         ` <071f24ecf9b3a2bbbe8fee7db77492eb55c5f3ff.camel@us.ibm.com>
     [not found]                                           ` <1d9b21914354bef6a290ac30673741e722e11757.camel@de.ibm.com>
2023-01-11 18:27                                             ` [PATCH 0/2] " Carl Love
2023-01-11 18:27                                             ` [PATCH 1/2] " Carl Love
2023-01-12 16:56                                               ` Tom de Vries
2023-01-12 18:54                                                 ` Carl Love
2023-01-13 13:33                                               ` Bruno Larsen
2023-01-13 16:43                                                 ` Carl Love
2023-01-13 17:04                                                   ` Bruno Larsen
2023-01-13 19:10                                                     ` Carl Love
2023-01-14 18:08                                                 ` Carl Love
2023-01-16 12:31                                                   ` Bruno Larsen
2023-01-16 16:37                                                     ` [PATCH 0/2 version 2] " Carl Love
2023-01-16 16:37                                                     ` [PATCH 1/2 " Carl Love
2023-01-17 12:35                                                       ` Bruno Larsen
2023-01-20  0:03                                                         ` [PATCH 1/2 version 3] " Carl Love
2023-01-23 19:17                                                           ` Pedro Alves
2023-01-23 21:13                                                             ` Carl Love
2023-01-24 14:08                                                               ` Pedro Alves
2023-01-24 14:23                                                                 ` Bruno Larsen
2023-01-24 15:06                                                                   ` Pedro Alves
2023-01-24 16:04                                                                     ` Bruno Larsen
2023-01-24 19:12                                                                       ` Pedro Alves
2023-01-25  9:49                                                                         ` Bruno Larsen
2023-01-25 14:11                                                                         ` Ulrich Weigand
2023-01-25 16:42                                                                           ` Pedro Alves
2023-01-25 17:13                                                                             ` Ulrich Weigand
2023-01-25 17:24                                                                               ` Pedro Alves
2023-01-25 19:38                                                                                 ` Carl Love
2023-01-24 15:51                                                                 ` Carl Love
2023-01-24 18:37                                                                   ` Pedro Alves
2023-01-24 18:25                                                                 ` Carl Love
2023-01-24 19:21                                                                   ` Pedro Alves [this message]
2023-01-24 19:26                                                                     ` Pedro Alves
2023-01-31  0:17                                                                 ` Reverse-next bug test case Carl Love
2023-02-01 14:37                                                                   ` Pedro Alves
2023-02-01 18:40                                                                     ` Carl Love
2023-01-24 15:53                                                             ` [PATCH 1/2 version 3] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp Tom de Vries
2023-01-24 18:48                                                               ` Pedro Alves
2023-01-16 16:37                                                     ` [PATCH 2/2 version 2] " Carl Love
2023-01-17 14:29                                                       ` Bruno Larsen
2023-01-17 16:36                                                         ` Carl Love
2023-01-17 16:55                                                           ` Tom de Vries
2023-01-17 17:03                                                             ` Carl Love
2023-01-17 17:14                                                               ` Tom de Vries
2023-01-17 19:31                                                                 ` Carl Love
2023-01-18 10:55                                                                   ` Tom de Vries
2023-01-18 16:16                                                                     ` Carl Love
2023-01-18 22:26                                                                     ` Carl Love
2023-01-19  8:04                                                                       ` Bruno Larsen
2023-01-19 16:56                                                                         ` Carl Love
2023-01-19 23:57                                                                           ` Carl Love
2023-01-20 20:04                                                                             ` Carl Love
2023-01-23 16:42                                                                               ` [PATCH 1/2 version 3] " Carl Love
2023-01-23 16:42                                                                               ` [PATCH 2/2 " Carl Love
2023-02-10 20:55                                                                               ` [PATCH ] PowerPC: " Carl Love
2023-02-17 12:24                                                                                 ` Ulrich Weigand
2023-02-20 16:34                                                                                   ` Carl Love
2023-02-20 16:48                                                                                     ` Bruno Larsen
2023-02-20 20:24                                                                                   ` Carl Love
2023-02-27 16:09                                                                                     ` [PING] " Carl Love
2023-02-28 13:39                                                                                     ` Bruno Larsen
2023-02-28 16:19                                                                                       ` Carl Love
2023-03-01 13:43                                                                                     ` Tom de Vries
2023-03-01 16:26                                                                                       ` Carl Love
2023-03-01 14:03                                                                                     ` Tom de Vries
2023-03-01 16:43                                                                                       ` Carl Love
2023-03-01 14:34                                                                                     ` Tom de Vries
2023-03-01 20:39                                                                                       ` Carl Love
2023-03-01 20:59                                                                                         ` [PATCH 0/2 " Carl Love
2023-03-01 20:59                                                                                         ` [PATCH 1/2] " Carl Love
2023-03-03 11:56                                                                                           ` Bruno Larsen
2023-03-08 16:19                                                                                             ` [PING] " Carl Love
2023-03-09 16:09                                                                                               ` Carl Love
2023-03-09 19:03                                                                                           ` Tom Tromey
2023-03-09 21:42                                                                                             ` Carl Love
2023-03-09 21:54                                                                                             ` [PATCH 1/2 ver 2] " Carl Love
2023-03-10  3:53                                                                                               ` Tom Tromey
2023-03-01 20:59                                                                                         ` [PATCH 2/2 ] " Carl Love
2023-03-08 16:19                                                                                           ` [PING] " Carl Love
2023-03-09 16:09                                                                                             ` Carl Love
2023-03-13 14:16                                                                                           ` Ulrich Weigand
2023-03-13 17:31                                                                                             ` Carl Love
2023-03-13 17:38                                                                                             ` [PATCH 2/2 ver2] " Carl Love
2023-03-17 17:19                                                                                               ` Ulrich Weigand
2023-03-17 23:05                                                                                                 ` Tom de Vries
2023-03-20 15:04                                                                                                   ` Carl Love
2023-03-20 23:21                                                                                                   ` Carl Love
2023-03-21  3:17                                                                                                     ` Carl Love
2023-03-21  6:52                                                                                                       ` Ulrich Weigand
2023-03-24 17:23                                                                                           ` [PATCH 2/2 ] " Simon Marchi
2023-03-24 22:16                                                                                             ` Carl Love
2023-03-25 12:39                                                                                               ` Simon Marchi
2023-03-27 23:59                                                                                                 ` Carl Love
2023-03-28  1:19                                                                                                   ` Simon Marchi
2023-03-28 15:17                                                                                                     ` Carl Love
2023-03-28 15:38                                                                                                       ` Simon Marchi
2023-07-20 12:01                                                                                                         ` Bruno Larsen
2023-07-20 14:45                                                                                                           ` Carl Love
2023-07-21  7:24                                                                                                             ` Bruno Larsen
2023-07-31 22:59                                                                                                               ` Carl Love
2023-08-02  9:29                                                                                                                 ` Bruno Larsen
2023-08-02 15:11                                                                                                                   ` Carl Love
2023-01-13 15:42                                               ` [PATCH 1/2] " Bruno Larsen
2023-01-11 18:27                                             ` [PATCH 2/2] " Carl Love
2023-01-13 15:55                                               ` Bruno Larsen
2023-01-14 18:08                                                 ` Carl Love

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d7ca6642-2318-4b52-2791-8cbf8f89d9a6@palves.net \
    --to=pedro@palves.net \
    --cc=Ulrich.Weigand@de.ibm.com \
    --cc=blarsen@redhat.com \
    --cc=cel@us.ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).