public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb: prune inferiors at end of fetch_inferior_event, fix intermittent failure of gdb.threads/fork-plus-threads.exp
@ 2022-04-05 12:00 Simon Marchi
  2022-04-05 13:38 ` Tom de Vries
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2022-04-05 12:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This test sometimes fail like this:

    info threads^M
      Id   Target Id         Frame ^M
      11.12 process 2270719   Couldn't get registers: No such process.^M
    (gdb) FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: no threads left
    [Inferior 11 (process 2270719) exited normally]^M
    info inferiors^M
      Num  Description       Connection           Executable        ^M
    * 1    <null>                                 /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/fork-plus-threads/fork-plus-threads ^M
      11   <null>                                 /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/fork-plus-threads/fork-plus-threads ^M
    (gdb) FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: only inferior 1 left (the program exited)

I can get it to fail quite reliably by pinning it to a core:

  $ taskset -c 5 make check TESTS="gdb.threads/fork-plus-threads.exp"

The previous attempt at fixing this was:

  https://sourceware.org/pipermail/gdb-patches/2021-October/182846.html

What we see is part due to a possible unfortunatle ordering of events
given by the kernel, and what could be considered a bug in GDB.

The test program makes a number of forks, waits them all, then exits.
Most of the time, GDB will get and process the exit event for inferior 1
after the exit events of all the children.  But this is not guaranteed.
After the last child exits and is waited by the parent, the parent can
exit quickly, such that GDB collects from the kernel the exit events for
the parent and that child at the same time.  It then choses one event at
random, which can be the event for the parent.  This will result in the
parent appearing to exit before its child.  There's not much we can do
about it, so I think we have to adjust the test to cope.

After expect has seen the "exited normally" notification for inferior 1,
it immediatly does an "info thread" that it expects to come back empty.
But at this point, GDB might not have processed inferior 11's (the last
child) exit event, so it will look like there is still a thread.  Of
course that thread is dead, we just don't know it yet.  But that makes
the "no thread" test fail.  If the test waited just a bit more for the
"exited normally" notification for inferior 11, then the list of threads
would be empty.

So, first change, make the test collect all the "exited normally"
notifications for all inferiors before proceeding, that should ensure we
see an empty thread list.  That would fix the first FAIL above.

However, we would still have the second FAIL, as we expect inferior 11
to not be there, it should have been deleted automatically.  Inferior 11
is normally deleted when prune_inferiors is called.  That is called by
normal_stop, which is only called by fetch_inferior_event only if the
event thread completed an execution command FSM (thread_fsm).  But the
FSM for the continue command completed when inferior 1 exited.  At that
point inferior 11 was not prunable, as it still had a thread.  When
inferior 11 exits, prune_inferiors is not called.

I think that can be considered a GDB bug.  From the user point of view,
there's no reason why in one case inferior 11 would be deleted and not
in the other case.

This patch makes the somewhat naive change to call prune_inferiors in
fetch_inferior_event, so that it is called in this case.  It is placed
at this particular point in the function so that it is called after the
user inferior / thread selection is restored.  If it was called before
that, inferior 11 wouldn't be pruned, because it would still be the
current inferior.

Change-Id: I48a15d118f30b1c72c528a9f805ed4974170484a
---
 gdb/debug.c                                   |  7 ++++-
 gdb/infrun.c                                  | 10 +++----
 .../gdb.threads/fork-plus-threads.exp         | 30 +++++++++++++++++--
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/gdb/debug.c b/gdb/debug.c
index b29a6620afe..1ab06fd67ab 100644
--- a/gdb/debug.c
+++ b/gdb/debug.c
@@ -27,8 +27,13 @@ int debug_print_depth = 0;
 
 /* See gdbsupport/common-debug.h.  */
 
+static FILE *f;
+
 void
 debug_vprintf (const char *fmt, va_list ap)
 {
-  gdb_vprintf (gdb_stdlog, fmt, ap);
+  if (!f)
+    f = fopen ("/tmp/log", "w");
+
+  vfprintf(f, fmt, ap);
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index ca6666cea1f..927baab947c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4135,6 +4135,7 @@ fetch_inferior_event ()
 		&& cmd_done
 		&& ecs->ws.kind () != TARGET_WAITKIND_NO_RESUMED)
 	      restore_thread.dont_restore ();
+
 	  }
       }
 
@@ -4155,6 +4156,10 @@ fetch_inferior_event ()
      ready for input).  */
   all_uis_check_sync_execution_done ();
 
+  /* Handling this event might have caused some inferiors to become prunable.
+     For example, the exit of an inferior that was automatically added.  */
+  prune_inferiors ();
+
   if (cmd_done
       && exec_done_display_p
       && (inferior_ptid == null_ptid
@@ -8604,11 +8609,6 @@ normal_stop (void)
 	breakpoint_auto_delete (inferior_thread ()->control.stop_bpstat);
     }
 
-  /* Try to get rid of automatically added inferiors that are no
-     longer needed.  Keeping those around slows down things linearly.
-     Note that this never removes the current inferior.  */
-  prune_inferiors ();
-
   return 0;
 }
 \f
diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp
index bc09c89e69b..83bc381f94f 100644
--- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp
+++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp
@@ -49,6 +49,9 @@ proc do_test { detach-on-fork } {
 	return 0
     }
 
+    gdb_test "set debug infrun 1"
+    gdb_test "set debug linux-nat 1"
+
     gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
     set test "continue &"
     gdb_test_multiple $test $test {
@@ -76,6 +79,12 @@ proc do_test { detach-on-fork } {
     set saw_cannot_remove_breakpoints 0
     set saw_thread_stopped 0
 
+    for {set i 1} {$i <= 11} {incr i} {
+	set inferior_exits_seen($i) 0
+    }
+
+    set num_inferior_exits_seen 0
+
     set test "inferior 1 exited"
     gdb_test_multiple "" $test {
 	-re "Cannot remove breakpoints" {
@@ -94,8 +103,25 @@ proc do_test { detach-on-fork } {
 	    # Avoid timeout with check-read1
 	    exp_continue
 	}
-	-re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
-	    pass $test
+	-re "Inferior ($::decimal) \(\[^\r\n\]+\) exited normally" {
+	    set infnum $expect_out(1,string)
+	    incr num_inferior_exits_seen
+	    incr inferior_exits_seen($infnum) 1
+
+	    if { $num_inferior_exits_seen == 11 || ${detach-on-fork} == "on" } {
+		pass $test
+	    } else {
+		exp_continue
+	    }
+	}
+    }
+
+    for {set i 1} {$i <= 11} {incr i} {
+	gdb_assert { $inferior_exits_seen($i) == 1 } \
+	    "seen inferior $i exit"
+
+	if { ${detach-on-fork} == "on" } {
+	    break
 	}
     }
 
-- 
2.35.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gdb: prune inferiors at end of fetch_inferior_event, fix intermittent failure of gdb.threads/fork-plus-threads.exp
  2022-04-05 12:00 [PATCH] gdb: prune inferiors at end of fetch_inferior_event, fix intermittent failure of gdb.threads/fork-plus-threads.exp Simon Marchi
@ 2022-04-05 13:38 ` Tom de Vries
  2022-04-05 14:17   ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2022-04-05 13:38 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 4/5/22 14:00, Simon Marchi via Gdb-patches wrote:
> This test sometimes fail like this:
> 
>      info threads^M
>        Id   Target Id         Frame ^M
>        11.12 process 2270719   Couldn't get registers: No such process.^M
>      (gdb) FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: no threads left
>      [Inferior 11 (process 2270719) exited normally]^M
>      info inferiors^M
>        Num  Description       Connection           Executable        ^M
>      * 1    <null>                                 /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/fork-plus-threads/fork-plus-threads ^M
>        11   <null>                                 /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/fork-plus-threads/fork-plus-threads ^M
>      (gdb) FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: only inferior 1 left (the program exited)
> 
> I can get it to fail quite reliably by pinning it to a core:
> 
>    $ taskset -c 5 make check TESTS="gdb.threads/fork-plus-threads.exp"
> 

Nice, didn't know taskset, I'll try to play around with it.

> The previous attempt at fixing this was:
> 
>    https://sourceware.org/pipermail/gdb-patches/2021-October/182846.html
> 
> What we see is part due to a possible unfortunatle ordering of events

unfortunatle -> unfortunate

> given by the kernel, and what could be considered a bug in GDB.
> 
> The test program makes a number of forks, waits them all, then exits.
> Most of the time, GDB will get and process the exit event for inferior 1
> after the exit events of all the children.  But this is not guaranteed.
> After the last child exits and is waited by the parent, the parent can
> exit quickly, such that GDB collects from the kernel the exit events for
> the parent and that child at the same time.  It then choses one event at

choses -> chooses

> random, which can be the event for the parent.  This will result in the
> parent appearing to exit before its child.  There's not much we can do
> about it, so I think we have to adjust the test to cope.
> 
> After expect has seen the "exited normally" notification for inferior 1,
> it immediatly does an "info thread" that it expects to come back empty.

immediatly -> immediately

> But at this point, GDB might not have processed inferior 11's (the last
> child) exit event, so it will look like there is still a thread.  Of
> course that thread is dead, we just don't know it yet.  But that makes
> the "no thread" test fail.  If the test waited just a bit more for the
> "exited normally" notification for inferior 11, then the list of threads
> would be empty.
> 
> So, first change, make the test collect all the "exited normally"
> notifications for all inferiors before proceeding, that should ensure we
> see an empty thread list.  That would fix the first FAIL above.
> 
> However, we would still have the second FAIL, as we expect inferior 11
> to not be there, it should have been deleted automatically.  Inferior 11
> is normally deleted when prune_inferiors is called.  That is called by
> normal_stop, which is only called by fetch_inferior_event only if the
> event thread completed an execution command FSM (thread_fsm).  But the
> FSM for the continue command completed when inferior 1 exited.  At that
> point inferior 11 was not prunable, as it still had a thread.  When
> inferior 11 exits, prune_inferiors is not called.
> 
> I think that can be considered a GDB bug.  From the user point of view,
> there's no reason why in one case inferior 11 would be deleted and not
> in the other case.
> 
> This patch makes the somewhat naive change to call prune_inferiors in
> fetch_inferior_event, so that it is called in this case.  It is placed
> at this particular point in the function so that it is called after the
> user inferior / thread selection is restored.  If it was called before
> that, inferior 11 wouldn't be pruned, because it would still be the
> current inferior.
> 

This sounds plausible to me, but I don't know this part of gdb very 
well, so I can't really comment, unfortunately.

> Change-Id: I48a15d118f30b1c72c528a9f805ed4974170484a
> ---
>   gdb/debug.c                                   |  7 ++++-
>   gdb/infrun.c                                  | 10 +++----
>   .../gdb.threads/fork-plus-threads.exp         | 30 +++++++++++++++++--
>   3 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/debug.c b/gdb/debug.c
> index b29a6620afe..1ab06fd67ab 100644
> --- a/gdb/debug.c
> +++ b/gdb/debug.c
> @@ -27,8 +27,13 @@ int debug_print_depth = 0;
>   
>   /* See gdbsupport/common-debug.h.  */
>   
> +static FILE *f;
> +
>   void
>   debug_vprintf (const char *fmt, va_list ap)
>   {
> -  gdb_vprintf (gdb_stdlog, fmt, ap);
> +  if (!f)
> +    f = fopen ("/tmp/log", "w");
> +
> +  vfprintf(f, fmt, ap);
>   }

Leftover from debugging?

> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index ca6666cea1f..927baab947c 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4135,6 +4135,7 @@ fetch_inferior_event ()
>   		&& cmd_done
>   		&& ecs->ws.kind () != TARGET_WAITKIND_NO_RESUMED)
>   	      restore_thread.dont_restore ();
> +

Stray added newline ?

>   	  }
>         }
>   
> @@ -4155,6 +4156,10 @@ fetch_inferior_event ()
>        ready for input).  */
>     all_uis_check_sync_execution_done ();
>   
> +  /* Handling this event might have caused some inferiors to become prunable.
> +     For example, the exit of an inferior that was automatically added.  */
> +  prune_inferiors ();
> +
>     if (cmd_done
>         && exec_done_display_p
>         && (inferior_ptid == null_ptid
> @@ -8604,11 +8609,6 @@ normal_stop (void)
>   	breakpoint_auto_delete (inferior_thread ()->control.stop_bpstat);
>       }
>   
> -  /* Try to get rid of automatically added inferiors that are no
> -     longer needed.  Keeping those around slows down things linearly.
> -     Note that this never removes the current inferior.  */
> -  prune_inferiors ();
> -
>     return 0;
>   }
>   \f
> diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp
> index bc09c89e69b..83bc381f94f 100644
> --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp
> +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp
> @@ -49,6 +49,9 @@ proc do_test { detach-on-fork } {
>   	return 0
>       }
>   
> +    gdb_test "set debug infrun 1"
> +    gdb_test "set debug linux-nat 1"
> +

Why are these on, are you parsing some of the debug output?

>       gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
>       set test "continue &"
>       gdb_test_multiple $test $test {
> @@ -76,6 +79,12 @@ proc do_test { detach-on-fork } {
>       set saw_cannot_remove_breakpoints 0
>       set saw_thread_stopped 0
>   
> +    for {set i 1} {$i <= 11} {incr i} {
> +	set inferior_exits_seen($i) 0
> +    }
> +
> +    set num_inferior_exits_seen 0
> +
>       set test "inferior 1 exited"
>       gdb_test_multiple "" $test {
>   	-re "Cannot remove breakpoints" {
> @@ -94,8 +103,25 @@ proc do_test { detach-on-fork } {
>   	    # Avoid timeout with check-read1
>   	    exp_continue
>   	}
> -	-re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
> -	    pass $test
> +	-re "Inferior ($::decimal) \(\[^\r\n\]+\) exited normally" {
> +	    set infnum $expect_out(1,string)
> +	    incr num_inferior_exits_seen
> +	    incr inferior_exits_seen($infnum) 1
> +
> +	    if { $num_inferior_exits_seen == 11 || ${detach-on-fork} == "on" } {
> +		pass $test
> +	    } else {
> +		exp_continue
> +	    }
> +	}
> +    }
> +
> +    for {set i 1} {$i <= 11} {incr i} {
> +	gdb_assert { $inferior_exits_seen($i) == 1 } \
> +	    "seen inferior $i exit"
> +

Nit: I realize it's easier to write like this, but I think one FAIL 
would do.

> +	if { ${detach-on-fork} == "on" } {
> +	    break
>   	}
>       }
>   

Test-case part LGTM, as mentioned before I cannot really comment on the 
fetch_inferior_event part.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gdb: prune inferiors at end of fetch_inferior_event, fix intermittent failure of gdb.threads/fork-plus-threads.exp
  2022-04-05 13:38 ` Tom de Vries
@ 2022-04-05 14:17   ` Simon Marchi
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2022-04-05 14:17 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 2022-04-05 09:38, Tom de Vries wrote:
> On 4/5/22 14:00, Simon Marchi via Gdb-patches wrote:
>> This test sometimes fail like this:
>>
>>      info threads^M
>>        Id   Target Id         Frame ^M
>>        11.12 process 2270719   Couldn't get registers: No such process.^M
>>      (gdb) FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: no threads left
>>      [Inferior 11 (process 2270719) exited normally]^M
>>      info inferiors^M
>>        Num  Description       Connection           Executable        ^M
>>      * 1    <null>                                 /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/fork-plus-threads/fork-plus-threads ^M
>>        11   <null>                                 /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/fork-plus-threads/fork-plus-threads ^M
>>      (gdb) FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: only inferior 1 left (the program exited)
>>
>> I can get it to fail quite reliably by pinning it to a core:
>>
>>    $ taskset -c 5 make check TESTS="gdb.threads/fork-plus-threads.exp"
>>
>
> Nice, didn't know taskset, I'll try to play around with it.
>
>> The previous attempt at fixing this was:
>>
>>    https://sourceware.org/pipermail/gdb-patches/2021-October/182846.html
>>
>> What we see is part due to a possible unfortunatle ordering of events
>
> unfortunatle -> unfortunate

Fixed.

>> given by the kernel, and what could be considered a bug in GDB.
>>
>> The test program makes a number of forks, waits them all, then exits.
>> Most of the time, GDB will get and process the exit event for inferior 1
>> after the exit events of all the children.  But this is not guaranteed.
>> After the last child exits and is waited by the parent, the parent can
>> exit quickly, such that GDB collects from the kernel the exit events for
>> the parent and that child at the same time.  It then choses one event at
>
> choses -> chooses

Fixed.

>> random, which can be the event for the parent.  This will result in the
>> parent appearing to exit before its child.  There's not much we can do
>> about it, so I think we have to adjust the test to cope.
>>
>> After expect has seen the "exited normally" notification for inferior 1,
>> it immediatly does an "info thread" that it expects to come back empty.
>
> immediatly -> immediately

Fixed.  I always get this one wrong, because other words in english do
end with "tly".

>> But at this point, GDB might not have processed inferior 11's (the last
>> child) exit event, so it will look like there is still a thread.  Of
>> course that thread is dead, we just don't know it yet.  But that makes
>> the "no thread" test fail.  If the test waited just a bit more for the
>> "exited normally" notification for inferior 11, then the list of threads
>> would be empty.
>>
>> So, first change, make the test collect all the "exited normally"
>> notifications for all inferiors before proceeding, that should ensure we
>> see an empty thread list.  That would fix the first FAIL above.
>>
>> However, we would still have the second FAIL, as we expect inferior 11
>> to not be there, it should have been deleted automatically.  Inferior 11
>> is normally deleted when prune_inferiors is called.  That is called by
>> normal_stop, which is only called by fetch_inferior_event only if the
>> event thread completed an execution command FSM (thread_fsm).  But the
>> FSM for the continue command completed when inferior 1 exited.  At that
>> point inferior 11 was not prunable, as it still had a thread.  When
>> inferior 11 exits, prune_inferiors is not called.
>>
>> I think that can be considered a GDB bug.  From the user point of view,
>> there's no reason why in one case inferior 11 would be deleted and not
>> in the other case.
>>
>> This patch makes the somewhat naive change to call prune_inferiors in
>> fetch_inferior_event, so that it is called in this case.  It is placed
>> at this particular point in the function so that it is called after the
>> user inferior / thread selection is restored.  If it was called before
>> that, inferior 11 wouldn't be pruned, because it would still be the
>> current inferior.
>>
>
> This sounds plausible to me, but I don't know this part of gdb very well, so I can't really comment, unfortunately.

I tried to place it where the comment

    /* This scope is used to ensure that readline callbacks are
       reinstalled here.  */

is and this is what I observed.  There is
"scoped_restore_current_thread" in that scope.

>> Change-Id: I48a15d118f30b1c72c528a9f805ed4974170484a
>> ---
>>   gdb/debug.c                                   |  7 ++++-
>>   gdb/infrun.c                                  | 10 +++----
>>   .../gdb.threads/fork-plus-threads.exp         | 30 +++++++++++++++++--
>>   3 files changed, 39 insertions(+), 8 deletions(-)
>>
>> diff --git a/gdb/debug.c b/gdb/debug.c
>> index b29a6620afe..1ab06fd67ab 100644
>> --- a/gdb/debug.c
>> +++ b/gdb/debug.c
>> @@ -27,8 +27,13 @@ int debug_print_depth = 0;
>>     /* See gdbsupport/common-debug.h.  */
>>   +static FILE *f;
>> +
>>   void
>>   debug_vprintf (const char *fmt, va_list ap)
>>   {
>> -  gdb_vprintf (gdb_stdlog, fmt, ap);
>> +  if (!f)
>> +    f = fopen ("/tmp/log", "w");
>> +
>> +  vfprintf(f, fmt, ap);
>>   }
>
> Leftover from debugging?

Ahh, yes, sorry, it was late at night.  Removed

>
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index ca6666cea1f..927baab947c 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -4135,6 +4135,7 @@ fetch_inferior_event ()
>>           && cmd_done
>>           && ecs->ws.kind () != TARGET_WAITKIND_NO_RESUMED)
>>             restore_thread.dont_restore ();
>> +
>
> Stray added newline ?

Removed.

>>         }
>>         }
>>   @@ -4155,6 +4156,10 @@ fetch_inferior_event ()
>>        ready for input).  */
>>     all_uis_check_sync_execution_done ();
>>   +  /* Handling this event might have caused some inferiors to become prunable.
>> +     For example, the exit of an inferior that was automatically added.  */
>> +  prune_inferiors ();
>> +
>>     if (cmd_done
>>         && exec_done_display_p
>>         && (inferior_ptid == null_ptid
>> @@ -8604,11 +8609,6 @@ normal_stop (void)
>>       breakpoint_auto_delete (inferior_thread ()->control.stop_bpstat);
>>       }
>>   -  /* Try to get rid of automatically added inferiors that are no
>> -     longer needed.  Keeping those around slows down things linearly.
>> -     Note that this never removes the current inferior.  */
>> -  prune_inferiors ();
>> -
>>     return 0;
>>   }
>>   \f
>> diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp
>> index bc09c89e69b..83bc381f94f 100644
>> --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp
>> +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp
>> @@ -49,6 +49,9 @@ proc do_test { detach-on-fork } {
>>       return 0
>>       }
>>   +    gdb_test "set debug infrun 1"
>> +    gdb_test "set debug linux-nat 1"
>> +
>
> Why are these on, are you parsing some of the debug output?

No, leftover from debugging (combined with the hunk that wrote debug
output to /tmp/log.

>>       gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
>>       set test "continue &"
>>       gdb_test_multiple $test $test {
>> @@ -76,6 +79,12 @@ proc do_test { detach-on-fork } {
>>       set saw_cannot_remove_breakpoints 0
>>       set saw_thread_stopped 0
>>   +    for {set i 1} {$i <= 11} {incr i} {
>> +    set inferior_exits_seen($i) 0
>> +    }
>> +
>> +    set num_inferior_exits_seen 0
>> +
>>       set test "inferior 1 exited"
>>       gdb_test_multiple "" $test {
>>       -re "Cannot remove breakpoints" {
>> @@ -94,8 +103,25 @@ proc do_test { detach-on-fork } {
>>           # Avoid timeout with check-read1
>>           exp_continue
>>       }
>> -    -re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
>> -        pass $test
>> +    -re "Inferior ($::decimal) \(\[^\r\n\]+\) exited normally" {
>> +        set infnum $expect_out(1,string)
>> +        incr num_inferior_exits_seen
>> +        incr inferior_exits_seen($infnum) 1
>> +
>> +        if { $num_inferior_exits_seen == 11 || ${detach-on-fork} == "on" } {
>> +        pass $test
>> +        } else {
>> +        exp_continue
>> +        }
>> +    }
>> +    }
>> +
>> +    for {set i 1} {$i <= 11} {incr i} {
>> +    gdb_assert { $inferior_exits_seen($i) == 1 } \
>> +        "seen inferior $i exit"
>> +
>
> Nit: I realize it's easier to write like this, but I think one FAIL would do.

Yeah well... tests are cheap.  But I'll change it in v2.  And I'll
factor out that "11" magic number, it will actually simplify things a
bit.

>
>> +    if { ${detach-on-fork} == "on" } {
>> +        break
>>       }
>>       }
>>
>
> Test-case part LGTM, as mentioned before I cannot really comment on the fetch_inferior_event part.

Thanks for the review anyway.  I'll post a v2 and wait a bit.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-04-05 14:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 12:00 [PATCH] gdb: prune inferiors at end of fetch_inferior_event, fix intermittent failure of gdb.threads/fork-plus-threads.exp Simon Marchi
2022-04-05 13:38 ` Tom de Vries
2022-04-05 14:17   ` Simon Marchi

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).