[COMMITTED,PR,gdb/25939] Move push_target call earlier in procfs.c

Message ID yddk100pe85.fsf_-_@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • [COMMITTED,PR,gdb/25939] Move push_target call earlier in procfs.c
Related show

Commit Message

Rainer Orth June 21, 2020, 4:37 p.m.
Hi Pedro,

> On 6/19/20 1:36 PM, Rainer Orth wrote:

>>> On 6/18/20 3:55 PM, Pedro Alves via Gdb-patches wrote:

>

>>> Your push_target fix is still necessary, FAOD.

>> 

>> Should I push it as is (with an appropriate description, of course) or

>> would the code change need a comment, too?

>

> It's fine without a comment.  I think you can remove the 

> push_target call from procfs_init_inferior at the same time,

> too, as that one becomes unnecessary.  Basically make the fix be

> about moving the push_target call earlier.


that works just fine.  Here's what I've committed after testing both
pathes together on amd64-pc-solaris2.11.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


[PR gdb/25939] Move push_target call earlier in procfs.c

Since the multi-target patch, the run command fails on Solaris with an
assertion failure even for a trivial program:

$ ./gdb -D ./data-directory ./hello
GNU gdb (GDB) 10.0.50.20200106-git
[...]
Reading symbols from ./hello...
(gdb) run
Starting program: /vol/obj/gnu/gdb/gdb/reghunt/no-resync/122448/gdb/hello 
/vol/src/gnu/gdb/hg/master/reghunt/gdb/thread.c:336: internal-error:
thread_info::thread_info(inferior*, ptid_t): Assertion `inf_ != NULL'
failed.

Here's the start of the corresponding stack trace:

#0  internal_error (
    file=file@entry=0x966150
"/vol/src/gnu/gdb/hg/master/reghunt/gdb/thread.c", line=line@entry=336,
fmt=0x9ddb94 "%s: Assertion `%s' failed.")
    at /vol/src/gnu/gdb/hg/master/reghunt/gdb/gdbsupport/errors.c:51
#1  0x0000000000ef81f4 in thread_info::thread_info (this=0x1212020, 
    inf_=<optimized out>, ptid_=...)
    at /vol/src/gnu/gdb/hg/master/reghunt/gdb/thread.c:344
#2  0x0000000000ef82cd in new_thread (inf=inf@entry=0x0, ptid=...)
    at /vol/src/gnu/gdb/hg/master/reghunt/gdb/thread.c:239
#3  0x0000000000efac3c in add_thread_silent (
    targ=targ@entry=0x11b0940 <the_procfs_target>, ptid=...)
    at /vol/src/gnu/gdb/hg/master/reghunt/gdb/thread.c:304
#4  0x0000000000d90692 in procfs_target::create_inferior (
    this=0x11b0940 <the_procfs_target>, 
    exec_file=0x13dbef0
"/vol/obj/gnu/gdb/gdb/reghunt/no-resync/122448/gdb/hello", allargs="",
env=0x13c48f0, from_tty=<optimized out>)
    at /vol/src/gnu/gdb/hg/master/reghunt/gdb/gdbsupport/ptid.h:47
#5  0x0000000000c84e64 in run_command_1 (args=<optimized out>, from_tty=1, 
    run_how=run_how@entry=RUN_NORMAL)
    at /vol/gcc-9/include/c++/9.1.0/bits/basic_string.h:263
#6  0x0000000000c85007 in run_command (args=<optimized out>, 
    from_tty=<optimized out>)
    at /vol/src/gnu/gdb/hg/master/reghunt/gdb/infcmd.c:687

Looking closer, I found that in add_thread_silent as called from
procfs.c (procfs_target::create_inferior) find_inferior_ptid returns
NULL.  The all_inferiors (targ) iterator comes up empty.

Going from there, I see that in add_thread_silent

m_target_stack = {m_top = file_stratum, m_stack = {0x20190e0
<the_dummy_target>, 0x200b8c0 <exec_ops>, 0x0, 0x0, 0x0, 0x0, 0x0}}}

i.e. the_procfs_target is missing compared to the_amd64_linux_nat_target
on Linux/x86_64.

Moving the push_target call earlier allows debugging to get over the
initial assertion failure.  I run instead into

procfs: couldn't find pid 0 in procinfo list.

which is fixed by

	https://sourceware.org/pipermail/gdb-patches/2020-June/169674.html

Both patches tested together on amd64-pc-solaris2.11.

	PR gdb/25939
	* procfs.c (procfs_target::procfs_init_inferior): Move push_target
	call ...
	(procfs_target::create_inferior): ... here.

Comments

Jose E. Marchesi via Gdb-patches June 22, 2020, 10:19 a.m. | #1
On 6/21/20 5:37 PM, Rainer Orth wrote:
> Hi Pedro,

> 

>> On 6/19/20 1:36 PM, Rainer Orth wrote:

>>>> On 6/18/20 3:55 PM, Pedro Alves via Gdb-patches wrote:

>>

>>>> Your push_target fix is still necessary, FAOD.

>>>

>>> Should I push it as is (with an appropriate description, of course) or

>>> would the code change need a comment, too?

>>

>> It's fine without a comment.  I think you can remove the 

>> push_target call from procfs_init_inferior at the same time,

>> too, as that one becomes unnecessary.  Basically make the fix be

>> about moving the push_target call earlier.

> 

> that works just fine.  Here's what I've committed after testing both

> pathes together on amd64-pc-solaris2.11.


I've committed mine now.

Thanks,
Pedro Alves

Patch

# HG changeset patch
# Parent  34403039561e7d3e10eb37561e6b574370a182d5
Move push_target call earlier in procfs.c [PR25939]

diff --git a/gdb/procfs.c b/gdb/procfs.c
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2781,11 +2781,6 @@  procfs_target::procfs_init_inferior (int
   int fail;
   int lwpid;
 
-  /* This routine called on the parent side (GDB side)
-     after GDB forks the inferior.  */
-  if (!target_is_pushed (this))
-    push_target (this);
-
   pi = create_procinfo (pid, 0);
   if (pi == NULL)
     perror (_("procfs: out of memory in 'init_inferior'"));
@@ -3006,6 +3001,9 @@  procfs_target::create_inferior (const ch
       shell_file = tryname;
     }
 
+  if (!target_is_pushed (this))
+    push_target (this);
+
   pid = fork_inferior (exec_file, allargs, env, procfs_set_exec_trap,
 		       NULL, NULL, shell_file, NULL);