Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Fix internal error with 'set debug infrun 1' under high load
@ 2019-03-24 14:25 Philippe Waroquiers
  2019-03-24 20:50 ` Kevin Buettner
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Waroquiers @ 2019-03-24 14:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

The test  gdb.threads/watchthreads-reorder.exp verifies that the
'set debug infrun 1' debug output does not crash GDB.

Under high load, the test can still cause a GDB internal error (see details
below).

This patch checks that ptid differs from null_ptid to call
pid_to_str so that it does not crash.
The change also outputs event_ptid.

  (top-gdb) bt
  #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x00007f3d54a0642a in __GI_abort () at abort.c:89
  #2  0x0000555c24c60e66 in dump_core () at ../../fixleaks/gdb/utils.c:201
  #3  0x0000555c24c63d49 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=problem@entry=0x555c25338d40 <internal_error_problem>, file=<optimized out>, line=287,
      fmt=<optimized out>, ap=<optimized out>) at ../../fixleaks/gdb/utils.c:411
  #4  0x0000555c24c63eab in internal_verror (file=<optimized out>, line=<optimized out>, fmt=<optimized out>,
      ap=<optimized out>) at ../../fixleaks/gdb/utils.c:436
  #5  0x0000555c249e8c22 in internal_error (file=file@entry=0x555c24e0f2ad "../../fixleaks/gdb/inferior.c",
      line=line@entry=287, fmt=<optimized out>) at ../../fixleaks/gdb/common/errors.c:55
  #6  0x0000555c247d3f5c in find_inferior_pid (pid=<optimized out>) at ../../fixleaks/gdb/inferior.c:287
  #7  0x0000555c24ad2248 in find_inferior_pid (pid=<optimized out>) at ../../fixleaks/gdb/inferior.c:302
  #8  find_inferior_ptid (ptid=...) at ../../fixleaks/gdb/inferior.c:301
  #9  0x0000555c24c35f25 in find_thread_ptid (ptid=...) at ../../fixleaks/gdb/thread.c:522
  #10 0x0000555c24b0ab4d in thread_db_target::pid_to_str[abi:cxx11](ptid_t) (
      this=0x555c2532e3e0 <the_thread_db_target>, ptid=...) at ../../fixleaks/gdb/linux-thread-db.c:1637
  #11 0x0000555c24c2f420 in target_pid_to_str[abi:cxx11](ptid_t) (ptid=...) at ../../fixleaks/gdb/target.c:2083
  #12 0x0000555c24ad9cab in stop_all_threads () at ../../fixleaks/gdb/infrun.c:4373
  #13 0x0000555c24ada00f in stop_waiting (ecs=<optimized out>) at ../../fixleaks/gdb/infrun.c:7464
  #14 0x0000555c24adc401 in process_event_stop_test (ecs=ecs@entry=0x7ffc9402d9d0) at ../../fixleaks/gdb/infrun.c:6181
  ...
  (top-gdb) fr 12
  #12 0x0000555c24ad9cab in stop_all_threads () at ../../fixleaks/gdb/infrun.c:4373
  (top-gdb) p event_ptid
  $5 = {m_pid = 25419, m_lwp = 25427, m_tid = 0}
  (top-gdb) p ptid
  $6 = {m_pid = 0, m_lwp = 0, m_tid = 0}
  (top-gdb) p ws
  $7 = {kind = TARGET_WAITKIND_THREAD_EXITED, value = {integer = 0, sig = GDB_SIGNAL_0, related_pid = {m_pid = 0,
        m_lwp = 0, m_tid = 0}, execd_pathname = 0x0, syscall_number = 0}}
  (top-gdb)

The gdb.log corresponding to the above crash is:
  (gdb) PASS: gdb.threads/watchthreads-reorder.exp: reorder1: set debug infrun 1
  continue
  Continuing.
  infrun: clear_proceed_status_thread (Thread 0x7ffff7fcfb40 (LWP 25419))
  infrun: clear_proceed_status_thread (Thread 0x7ffff7310700 (LWP 25427))
  infrun: clear_proceed_status_thread (Thread 0x7ffff6b0f700 (LWP 25428))
  infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
  infrun: proceed: resuming Thread 0x7ffff7fcfb40 (LWP 25419)
  infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7fcfb40 (LWP 25419)] at 0x7ffff7344317
  infrun: infrun_async(1)
  infrun: prepare_to_wait
  infrun: proceed: resuming Thread 0x7ffff7310700 (LWP 25427)
  infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7310700 (LWP 25427)] at 0x5555555553d7
  infrun: prepare_to_wait
  infrun: proceed: resuming Thread 0x7ffff6b0f700 (LWP 25428)
  infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff6b0f700 (LWP 25428)] at 0x5555555554c8
  infrun: prepare_to_wait
  infrun: target_wait (-1.0.0, status) =
  infrun:   -1.0.0 [process -1],
  infrun:   status->kind = ignore
  infrun: TARGET_WAITKIND_IGNORE
  infrun: prepare_to_wait
  Joining the threads.
  [Thread 0x7ffff6b0f700 (LWP 25428) exited]
  infrun: target_wait (-1.0.0, status) =
  infrun:   -1.0.0 [process -1],
  infrun:   status->kind = ignore
  infrun: TARGET_WAITKIND_IGNORE
  infrun: prepare_to_wait
  infrun: target_wait (-1.0.0, status) =
  infrun:   25419.25419.0 [Thread 0x7ffff7fcfb40 (LWP 25419)],
  infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
  infrun: TARGET_WAITKIND_STOPPED
  infrun: stop_pc = 0x555555555e50
  infrun: context switch
  infrun: Switching context from Thread 0x7ffff6b0f700 (LWP 25428) to Thread 0x7ffff7fcfb40 (LWP 25419)
  infrun: BPSTAT_WHAT_STOP_NOISY
  infrun: stop_waiting
  infrun: stop_all_threads
  infrun: stop_all_threads, pass=0, iterations=0
  infrun:   Thread 0x7ffff7fcfb40 (LWP 25419) not executing
  infrun:   Thread 0x7ffff7310700 (LWP 25427) executing, need stop
  [Thread 0x7ffff7310700 (LWP 25427) exited]
  infrun: target_wait (-1.0.0, status) =
  infrun:   25419.25427.0 [LWP 25427],
  infrun:   status->kind = thread exited, status = 0
  infrun: infrun_async(0)
  ../../fixleaks/gdb/inferior.c:287: internal-error: inferior* find_inferior_pid(int): Assertion `pid != 0' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) FAIL: gdb.threads/watchthreads-reorder.exp: reorder1: continue to breakpoint: break-at-exit (GDB internal error)
  Resyncing due to internal error.
  n
  infrun: infrun_async(1)

  This is a bug, please report it.  For instructions, see:
  <http://www.gnu.org/software/gdb/bugs/>.

  infrun: infrun_async(0)
  ../../fixleaks/gdb/inferior.c:287: internal-error: inferior* find_inferior_pid(int): Assertion `pid != 0' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Create a core file of GDB? (y or n) y
---
 gdb/infrun.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index ad7892105a..111879c318 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4368,9 +4368,13 @@ stop_all_threads (void)
 		  ptid_t ptid = ptid_t (ws.value.integer);
 
 		  fprintf_unfiltered (gdb_stdlog,
-				      "infrun: %s exited while "
+				      "infrun: %s "
+				      "(event_ptid %s) exited while "
 				      "stopping threads\n",
-				      target_pid_to_str (ptid).c_str ());
+				      ptid == null_ptid
+				      ? "<null thread>"
+				      : target_pid_to_str (ptid).c_str (),
+				      target_pid_to_str (event_ptid).c_str ());
 		}
 	    }
 	  else
-- 
2.20.1


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

* Re: [RFA] Fix internal error with 'set debug infrun 1' under high load
  2019-03-24 14:25 [RFA] Fix internal error with 'set debug infrun 1' under high load Philippe Waroquiers
@ 2019-03-24 20:50 ` Kevin Buettner
  2019-03-24 21:09   ` Philippe Waroquiers
  0 siblings, 1 reply; 5+ messages in thread
From: Kevin Buettner @ 2019-03-24 20:50 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

Hi Philippe,

There is definitely a bug in this section of code from infrun.c:

	  else if (ws.kind == TARGET_WAITKIND_THREAD_EXITED
		   || ws.kind == TARGET_WAITKIND_EXITED
		   || ws.kind == TARGET_WAITKIND_SIGNALLED)
	    {
	      if (debug_infrun)
		{
		  ptid_t ptid = ptid_t (ws.value.integer);

		  fprintf_unfiltered (gdb_stdlog,
				      "infrun: %s exited while "
				      "stopping threads\n",
				      target_pid_to_str (ptid).c_str ());
		}
	    }

This line...

		  ptid_t ptid = ptid_t (ws.value.integer);

...doesn't make sense to me since ws.value.integer is supposed to
be the exit status for TARGET_WAITKIND_THREAD_EXITED and
TARGET_WAITKIND_EXITED.

However, for TARGET_WAITKIND_SIGNALLED, the signal number is in
ws.value.sig (which, due to being part of a union occupies some
of the same bytes as ws.value.integer).

So trying to find the ptid in that manner makes no sense at all.

I'm guessing that the ptid values are bogus when it does work.

Does it work when you use 

		  ptid_t ptid = ptid_t (event_pid);

instead?

Kevin


On Sun, 24 Mar 2019 15:25:05 +0100
Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:

> The test  gdb.threads/watchthreads-reorder.exp verifies that the
> 'set debug infrun 1' debug output does not crash GDB.
> 
> Under high load, the test can still cause a GDB internal error (see details
> below).
> 
> This patch checks that ptid differs from null_ptid to call
> pid_to_str so that it does not crash.
> The change also outputs event_ptid.
> 
>   (top-gdb) bt
>   #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
>   #1  0x00007f3d54a0642a in __GI_abort () at abort.c:89
>   #2  0x0000555c24c60e66 in dump_core () at ../../fixleaks/gdb/utils.c:201
>   #3  0x0000555c24c63d49 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=problem@entry=0x555c25338d40 <internal_error_problem>, file=<optimized out>, line=287,
>       fmt=<optimized out>, ap=<optimized out>) at ../../fixleaks/gdb/utils.c:411
>   #4  0x0000555c24c63eab in internal_verror (file=<optimized out>, line=<optimized out>, fmt=<optimized out>,
>       ap=<optimized out>) at ../../fixleaks/gdb/utils.c:436
>   #5  0x0000555c249e8c22 in internal_error (file=file@entry=0x555c24e0f2ad "../../fixleaks/gdb/inferior.c",
>       line=line@entry=287, fmt=<optimized out>) at ../../fixleaks/gdb/common/errors.c:55
>   #6  0x0000555c247d3f5c in find_inferior_pid (pid=<optimized out>) at ../../fixleaks/gdb/inferior.c:287
>   #7  0x0000555c24ad2248 in find_inferior_pid (pid=<optimized out>) at ../../fixleaks/gdb/inferior.c:302
>   #8  find_inferior_ptid (ptid=...) at ../../fixleaks/gdb/inferior.c:301
>   #9  0x0000555c24c35f25 in find_thread_ptid (ptid=...) at ../../fixleaks/gdb/thread.c:522
>   #10 0x0000555c24b0ab4d in thread_db_target::pid_to_str[abi:cxx11](ptid_t) (
>       this=0x555c2532e3e0 <the_thread_db_target>, ptid=...) at ../../fixleaks/gdb/linux-thread-db.c:1637
>   #11 0x0000555c24c2f420 in target_pid_to_str[abi:cxx11](ptid_t) (ptid=...) at ../../fixleaks/gdb/target.c:2083
>   #12 0x0000555c24ad9cab in stop_all_threads () at ../../fixleaks/gdb/infrun.c:4373
>   #13 0x0000555c24ada00f in stop_waiting (ecs=<optimized out>) at ../../fixleaks/gdb/infrun.c:7464
>   #14 0x0000555c24adc401 in process_event_stop_test (ecs=ecs@entry=0x7ffc9402d9d0) at ../../fixleaks/gdb/infrun.c:6181
>   ...
>   (top-gdb) fr 12
>   #12 0x0000555c24ad9cab in stop_all_threads () at ../../fixleaks/gdb/infrun.c:4373
>   (top-gdb) p event_ptid
>   $5 = {m_pid = 25419, m_lwp = 25427, m_tid = 0}
>   (top-gdb) p ptid
>   $6 = {m_pid = 0, m_lwp = 0, m_tid = 0}
>   (top-gdb) p ws
>   $7 = {kind = TARGET_WAITKIND_THREAD_EXITED, value = {integer = 0, sig = GDB_SIGNAL_0, related_pid = {m_pid = 0,
>         m_lwp = 0, m_tid = 0}, execd_pathname = 0x0, syscall_number = 0}}
>   (top-gdb)
> 
> The gdb.log corresponding to the above crash is:
>   (gdb) PASS: gdb.threads/watchthreads-reorder.exp: reorder1: set debug infrun 1
>   continue
>   Continuing.
>   infrun: clear_proceed_status_thread (Thread 0x7ffff7fcfb40 (LWP 25419))
>   infrun: clear_proceed_status_thread (Thread 0x7ffff7310700 (LWP 25427))
>   infrun: clear_proceed_status_thread (Thread 0x7ffff6b0f700 (LWP 25428))
>   infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
>   infrun: proceed: resuming Thread 0x7ffff7fcfb40 (LWP 25419)
>   infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7fcfb40 (LWP 25419)] at 0x7ffff7344317
>   infrun: infrun_async(1)
>   infrun: prepare_to_wait
>   infrun: proceed: resuming Thread 0x7ffff7310700 (LWP 25427)
>   infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7310700 (LWP 25427)] at 0x5555555553d7
>   infrun: prepare_to_wait
>   infrun: proceed: resuming Thread 0x7ffff6b0f700 (LWP 25428)
>   infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff6b0f700 (LWP 25428)] at 0x5555555554c8
>   infrun: prepare_to_wait
>   infrun: target_wait (-1.0.0, status) =
>   infrun:   -1.0.0 [process -1],
>   infrun:   status->kind = ignore
>   infrun: TARGET_WAITKIND_IGNORE
>   infrun: prepare_to_wait
>   Joining the threads.
>   [Thread 0x7ffff6b0f700 (LWP 25428) exited]
>   infrun: target_wait (-1.0.0, status) =
>   infrun:   -1.0.0 [process -1],
>   infrun:   status->kind = ignore
>   infrun: TARGET_WAITKIND_IGNORE
>   infrun: prepare_to_wait
>   infrun: target_wait (-1.0.0, status) =
>   infrun:   25419.25419.0 [Thread 0x7ffff7fcfb40 (LWP 25419)],
>   infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
>   infrun: TARGET_WAITKIND_STOPPED
>   infrun: stop_pc = 0x555555555e50
>   infrun: context switch
>   infrun: Switching context from Thread 0x7ffff6b0f700 (LWP 25428) to Thread 0x7ffff7fcfb40 (LWP 25419)
>   infrun: BPSTAT_WHAT_STOP_NOISY
>   infrun: stop_waiting
>   infrun: stop_all_threads
>   infrun: stop_all_threads, pass=0, iterations=0
>   infrun:   Thread 0x7ffff7fcfb40 (LWP 25419) not executing
>   infrun:   Thread 0x7ffff7310700 (LWP 25427) executing, need stop
>   [Thread 0x7ffff7310700 (LWP 25427) exited]
>   infrun: target_wait (-1.0.0, status) =
>   infrun:   25419.25427.0 [LWP 25427],
>   infrun:   status->kind = thread exited, status = 0
>   infrun: infrun_async(0)
>   ../../fixleaks/gdb/inferior.c:287: internal-error: inferior* find_inferior_pid(int): Assertion `pid != 0' failed.
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Quit this debugging session? (y or n) FAIL: gdb.threads/watchthreads-reorder.exp: reorder1: continue to breakpoint: break-at-exit (GDB internal error)
>   Resyncing due to internal error.
>   n
>   infrun: infrun_async(1)
> 
>   This is a bug, please report it.  For instructions, see:
>   <http://www.gnu.org/software/gdb/bugs/>.
> 
>   infrun: infrun_async(0)
>   ../../fixleaks/gdb/inferior.c:287: internal-error: inferior* find_inferior_pid(int): Assertion `pid != 0' failed.
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Create a core file of GDB? (y or n) y
> ---
>  gdb/infrun.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index ad7892105a..111879c318 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4368,9 +4368,13 @@ stop_all_threads (void)
>  		  ptid_t ptid = ptid_t (ws.value.integer);
>  
>  		  fprintf_unfiltered (gdb_stdlog,
> -				      "infrun: %s exited while "
> +				      "infrun: %s "
> +				      "(event_ptid %s) exited while "
>  				      "stopping threads\n",
> -				      target_pid_to_str (ptid).c_str ());
> +				      ptid == null_ptid
> +				      ? "<null thread>"
> +				      : target_pid_to_str (ptid).c_str (),
> +				      target_pid_to_str (event_ptid).c_str ());
>  		}
>  	    }
>  	  else
> -- 
> 2.20.1
> 


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

* Re: [RFA] Fix internal error with 'set debug infrun 1' under high load
  2019-03-24 20:50 ` Kevin Buettner
@ 2019-03-24 21:09   ` Philippe Waroquiers
  2019-03-24 21:35     ` Kevin Buettner
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Waroquiers @ 2019-03-24 21:09 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches

On Sun, 2019-03-24 at 13:50 -0700, Kevin Buettner wrote:
> Hi Philippe,
> 
> There is definitely a bug in this section of code from infrun.c:
> 
> 	  else if (ws.kind == TARGET_WAITKIND_THREAD_EXITED
> 		   || ws.kind == TARGET_WAITKIND_EXITED
> 		   || ws.kind == TARGET_WAITKIND_SIGNALLED)
> 	    {
> 	      if (debug_infrun)
> 		{
> 		  ptid_t ptid = ptid_t (ws.value.integer);
> 
> 		  fprintf_unfiltered (gdb_stdlog,
> 				      "infrun: %s exited while "
> 				      "stopping threads\n",
> 				      target_pid_to_str (ptid).c_str ());
> 		}
> 	    }
> 
> This line...
> 
> 		  ptid_t ptid = ptid_t (ws.value.integer);
> 
> ...doesn't make sense to me since ws.value.integer is supposed to
> be the exit status for TARGET_WAITKIND_THREAD_EXITED and
> TARGET_WAITKIND_EXITED.
> 
> However, for TARGET_WAITKIND_SIGNALLED, the signal number is in
> ws.value.sig (which, due to being part of a union occupies some
> of the same bytes as ws.value.integer).
> 
> So trying to find the ptid in that manner makes no sense at all.
> 
> I'm guessing that the ptid values are bogus when it does work.
> 
> Does it work when you use 
> 
> 		  ptid_t ptid = ptid_t (event_pid);
> 
> instead?
I guess you mean to only print event_ptid.

Yes, that is working (the proposed patch was printing both
event_ptid and the ptid derived from ws.value.integer, assuming
that sometimes ws.value.integer was something relevant).

Here is the trace I obtain after a few trials under high load:
infrun: stop_all_threads, pass=0, iterations=0
infrun:   Thread 0x7ffff7fcfb40 (LWP 3587) not executing
infrun:   Thread 0x7ffff7310700 (LWP 3632) executing, need stop
[Thread 0x7ffff7310700 (LWP 3632) exited]
infrun: target_wait (-1.0.0, status) =
infrun:   3587.3632.0 [LWP 3632],
infrun:   status->kind = thread exited, status = 0
infrun: LWP 3632 exited while stopping threads
infrun:   Thread 0x7ffff7fcfb40 (LWP 3587) not executing
infrun: stop_all_threads, pass=1, iterations=1
infrun:   Thread 0x7ffff7fcfb40 (LWP 3587) not executing
infrun: stop_all_threads done

The above is obtained with the patch:

diff --git a/gdb/infrun.c b/gdb/infrun.c
index ad7892105a..7f1339a917 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4365,12 +4365,10 @@ stop_all_threads (void)
            {
              if (debug_infrun)
                {
-                 ptid_t ptid = ptid_t (ws.value.integer);
-
                  fprintf_unfiltered (gdb_stdlog,
                                      "infrun: %s exited while "
                                      "stopping threads\n",
-                                     target_pid_to_str (ptid).c_str ());
+                                     target_pid_to_str (event_ptid).c_str ());
                }
            }
          else


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

* Re: [RFA] Fix internal error with 'set debug infrun 1' under high load
  2019-03-24 21:09   ` Philippe Waroquiers
@ 2019-03-24 21:35     ` Kevin Buettner
  2019-03-24 21:40       ` Philippe Waroquiers
  0 siblings, 1 reply; 5+ messages in thread
From: Kevin Buettner @ 2019-03-24 21:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

On Sun, 24 Mar 2019 22:09:43 +0100
Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:

> On Sun, 2019-03-24 at 13:50 -0700, Kevin Buettner wrote:
> > Hi Philippe,
> > 
> > There is definitely a bug in this section of code from infrun.c:
> > 
> > 	  else if (ws.kind == TARGET_WAITKIND_THREAD_EXITED
> > 		   || ws.kind == TARGET_WAITKIND_EXITED
> > 		   || ws.kind == TARGET_WAITKIND_SIGNALLED)
> > 	    {
> > 	      if (debug_infrun)
> > 		{
> > 		  ptid_t ptid = ptid_t (ws.value.integer);
> > 
> > 		  fprintf_unfiltered (gdb_stdlog,
> > 				      "infrun: %s exited while "
> > 				      "stopping threads\n",
> > 				      target_pid_to_str (ptid).c_str ());
> > 		}
> > 	    }
> > 
> > This line...
> > 
> > 		  ptid_t ptid = ptid_t (ws.value.integer);
> > 
> > ...doesn't make sense to me since ws.value.integer is supposed to
> > be the exit status for TARGET_WAITKIND_THREAD_EXITED and
> > TARGET_WAITKIND_EXITED.
> > 
> > However, for TARGET_WAITKIND_SIGNALLED, the signal number is in
> > ws.value.sig (which, due to being part of a union occupies some
> > of the same bytes as ws.value.integer).
> > 
> > So trying to find the ptid in that manner makes no sense at all.
> > 
> > I'm guessing that the ptid values are bogus when it does work.
> > 
> > Does it work when you use 
> > 
> > 		  ptid_t ptid = ptid_t (event_pid);
> > 
> > instead?  
> I guess you mean to only print event_ptid.
> 
> Yes, that is working (the proposed patch was printing both
> event_ptid and the ptid derived from ws.value.integer, assuming
> that sometimes ws.value.integer was something relevant).
> 
> Here is the trace I obtain after a few trials under high load:
> infrun: stop_all_threads, pass=0, iterations=0
> infrun:   Thread 0x7ffff7fcfb40 (LWP 3587) not executing
> infrun:   Thread 0x7ffff7310700 (LWP 3632) executing, need stop
> [Thread 0x7ffff7310700 (LWP 3632) exited]
> infrun: target_wait (-1.0.0, status) =
> infrun:   3587.3632.0 [LWP 3632],
> infrun:   status->kind = thread exited, status = 0
> infrun: LWP 3632 exited while stopping threads
> infrun:   Thread 0x7ffff7fcfb40 (LWP 3587) not executing
> infrun: stop_all_threads, pass=1, iterations=1
> infrun:   Thread 0x7ffff7fcfb40 (LWP 3587) not executing
> infrun: stop_all_threads done
> 
> The above is obtained with the patch:
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index ad7892105a..7f1339a917 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4365,12 +4365,10 @@ stop_all_threads (void)
>             {
>               if (debug_infrun)
>                 {
> -                 ptid_t ptid = ptid_t (ws.value.integer);
> -
>                   fprintf_unfiltered (gdb_stdlog,
>                                       "infrun: %s exited while "
>                                       "stopping threads\n",
> -                                     target_pid_to_str (ptid).c_str ());
> +                                     target_pid_to_str (event_ptid).c_str ());
>                 }
>             }
>           else
> 

You make a good point about trying to make use of ws.value.integer.

So, here are my suggestions:

1) Move TARGET_WAITKIND_SIGNALLED into another "else if" clause.  It
doesn't make sense for the debug message to indicate that the process
has exited when it's actually been signalled.

2) Make the TARGET_WAITKIND_THREAD_EXITED / TARGET_WAITKIND_EXITED
case print the exit status and make the TARGET_WAITKIND_SIGNALLED case
print the signal.  These are available (respectively) in ws.value.integer and
ws.value.sig.

Kevin


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

* Re: [RFA] Fix internal error with 'set debug infrun 1' under high load
  2019-03-24 21:35     ` Kevin Buettner
@ 2019-03-24 21:40       ` Philippe Waroquiers
  0 siblings, 0 replies; 5+ messages in thread
From: Philippe Waroquiers @ 2019-03-24 21:40 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches

On Sun, 2019-03-24 at 14:35 -0700, Kevin Buettner wrote:
> You make a good point about trying to make use of ws.value.integer.
> 
> So, here are my suggestions:
> 
> 1) Move TARGET_WAITKIND_SIGNALLED into another "else if" clause.  It
> doesn't make sense for the debug message to indicate that the process
> has exited when it's actually been signalled.
> 
> 2) Make the TARGET_WAITKIND_THREAD_EXITED / TARGET_WAITKIND_EXITED
> case print the exit status and make the TARGET_WAITKIND_SIGNALLED case
> print the signal.  These are available (respectively) in ws.value.integer and
> ws.value.sig.
Thanks for the feedback, I will (some time this week) prepare a patch
based on these suggestions.

Philippe


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

end of thread, other threads:[~2019-03-24 21:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-24 14:25 [RFA] Fix internal error with 'set debug infrun 1' under high load Philippe Waroquiers
2019-03-24 20:50 ` Kevin Buettner
2019-03-24 21:09   ` Philippe Waroquiers
2019-03-24 21:35     ` Kevin Buettner
2019-03-24 21:40       ` Philippe Waroquiers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox