diff mbox

Breakage on builder Solaris11-amd64-m64, revision 080363310650c93ad8e93018bcb6760ba5d32d1c

Message ID 96729b3f-3568-1d41-4bb8-fbe4d037da93@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Nov. 23, 2018, 1:34 p.m. UTC 
  On 11/22/2018 04:49 PM, gdb-buildbot@sergiodj.net wrote:
> Undefined			first referenced
>  symbol  			    in file
> is_exited(ptid_t)                   procfs.o
> ld: fatal: symbol referencing errors
> collect2: error: ld returned 1 exit status
> gmake[2]: *** [Makefile:1912: gdb] Error 1
> gmake[2]: Leaving directory '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build/gdb'
> gmake[1]: *** [Makefile:8792: all-gdb] Error 2
> gmake[1]: Leaving directory '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build'
> gmake: *** [Makefile:849: all] Error 2
> ============================

Whoops, sorry about that.  I assume this works, but I haven't tried it.

From a48ec78677f732cc78eeefecc149742eb4ca0624 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 23 Nov 2018 13:13:35 +0000
Subject: [PATCH] Fix Solaris build

The recent commit 080363310650 ("Per-inferior thread list, thread
ranges/iterators, down with ALL_THREADS, etc.") removed the
definitions of is_running/is_stopped/is_exited but missed updating a
couple uses of is_exited in Solaris-specific code.

gdb/ChangeLog:
2018-11-23  Pedro Alves  <palves@redhat.com>

	* procfs.c (procfs_notice_thread): Replace uses of
	in_thread_list/is_exited with find_thread_ptid/THREAD_EXITED.
	* sol-thread.c (sol_thread_target::wait)
	(sol_update_thread_list_callback): Likewise.
---
 gdb/procfs.c     |  3 ++-
 gdb/sol-thread.c | 14 ++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

Comments

Sergio Durigan Junior Nov. 23, 2018, 2:06 p.m. UTC | #1 
On Friday, November 23 2018, Pedro Alves wrote:

> On 11/22/2018 04:49 PM, gdb-buildbot@sergiodj.net wrote:
>> Undefined			first referenced
>>  symbol  			    in file
>> is_exited(ptid_t)                   procfs.o
>> ld: fatal: symbol referencing errors
>> collect2: error: ld returned 1 exit status
>> gmake[2]: *** [Makefile:1912: gdb] Error 1
>> gmake[2]: Leaving directory '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build/gdb'
>> gmake[1]: *** [Makefile:8792: all-gdb] Error 2
>> gmake[1]: Leaving directory '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build'
>> gmake: *** [Makefile:849: all] Error 2
>> ============================
>
> Whoops, sorry about that.  I assume this works, but I haven't tried it.

I took the liberty of running try builds with your patch on the Solaris
builders:

  https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/410
  https://gdb-build.sergiodj.net/builders/Solaris11-sparcv9-m64/builds/408

Unfortunately the build still fails with:

  ../../binutils-gdb/gdb/sol-thread.c: In member function ‘virtual ptid_t sol_thread_target::wait(ptid_t, target_waitstatus*, int)’:
  ../../binutils-gdb/gdb/sol-thread.c:465:2: error: expected ‘)’ before ‘{’ token
    {
    ^
  ../../binutils-gdb/gdb/sol-thread.c:470:5: error: expected primary-expression before ‘}’ token
       }
       ^

BTW, it's possible to run try builds on specific builders by using the
"-b" option with "buildbot try".  E.g.:

  buildbot try -b Solaris11-amd64-m64 -b Solaris11-sparcv9-m64

Thanks,

> From a48ec78677f732cc78eeefecc149742eb4ca0624 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 23 Nov 2018 13:13:35 +0000
> Subject: [PATCH] Fix Solaris build
>
> The recent commit 080363310650 ("Per-inferior thread list, thread
> ranges/iterators, down with ALL_THREADS, etc.") removed the
> definitions of is_running/is_stopped/is_exited but missed updating a
> couple uses of is_exited in Solaris-specific code.
>
> gdb/ChangeLog:
> 2018-11-23  Pedro Alves  <palves@redhat.com>
>
> 	* procfs.c (procfs_notice_thread): Replace uses of
> 	in_thread_list/is_exited with find_thread_ptid/THREAD_EXITED.
> 	* sol-thread.c (sol_thread_target::wait)
> 	(sol_update_thread_list_callback): Likewise.
> ---
>  gdb/procfs.c     |  3 ++-
>  gdb/sol-thread.c | 14 ++++++++------
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/procfs.c b/gdb/procfs.c
> index 0d8f9df966..f82fc83987 100644
> --- a/gdb/procfs.c
> +++ b/gdb/procfs.c
> @@ -3133,7 +3133,8 @@ procfs_notice_thread (procinfo *pi, procinfo *thread, void *ptr)
>  {
>    ptid_t gdb_threadid = ptid_t (pi->pid, thread->tid, 0);
>  
> -  if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
> +  thread_info *thr = find_thread_ptid (gdb_threadid);
> +  if (thr == NULL || thr->state == THREAD_EXITED)
>      add_thread (gdb_threadid);
>  
>    return 0;
> diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
> index 682302a40b..0a57c4d1a6 100644
> --- a/gdb/sol-thread.c
> +++ b/gdb/sol-thread.c
> @@ -462,9 +462,11 @@ sol_thread_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>        /* See if we have a new thread.  */
>        if (rtnval.tid_p ()
>  	  && rtnval != save_ptid
> -	  && (!in_thread_list (rtnval)
> -	      || is_exited (rtnval)))
> -	add_thread (rtnval);
> +	{
> +	  thread_info *thr = find_thread_ptid (rtnval);
> +	  if (thr == NULL || thr->state == THREAD_EXITED)
> +	    add_thread (rtnval);
> +	}
>      }
>  
>    /* During process initialization, we may get here without the thread
> @@ -1035,14 +1037,14 @@ sol_update_thread_list_callback (const td_thrhandle_t *th, void *ignored)
>  {
>    td_err_e retval;
>    td_thrinfo_t ti;
> -  ptid_t ptid;
>  
>    retval = p_td_thr_get_info (th, &ti);
>    if (retval != TD_OK)
>      return -1;
>  
> -  ptid = ptid_t (inferior_ptid.pid (), 0, ti.ti_tid);
> -  if (!in_thread_list (ptid) || is_exited (ptid))
> +  ptid_t ptid = ptid_t (inferior_ptid.pid (), 0, ti.ti_tid);
> +  thread_info *thr = find_thread_ptid (ptid);
> +  if (thr == NULL || thr->state == THREAD_EXITED)
>      add_thread (ptid);
>  
>    return 0;
> -- 
> 2.14.4
Sergio Durigan Junior Nov. 23, 2018, 2:11 p.m. UTC | #2 
On Friday, November 23 2018, I wrote:

> On Friday, November 23 2018, Pedro Alves wrote:
>
>> On 11/22/2018 04:49 PM, gdb-buildbot@sergiodj.net wrote:
>>> Undefined			first referenced
>>>  symbol  			    in file
>>> is_exited(ptid_t)                   procfs.o
>>> ld: fatal: symbol referencing errors
>>> collect2: error: ld returned 1 exit status
>>> gmake[2]: *** [Makefile:1912: gdb] Error 1
>>> gmake[2]: Leaving directory '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build/gdb'
>>> gmake[1]: *** [Makefile:8792: all-gdb] Error 2
>>> gmake[1]: Leaving directory '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build'
>>> gmake: *** [Makefile:849: all] Error 2
>>> ============================
>>
>> Whoops, sorry about that.  I assume this works, but I haven't tried it.
>
> I took the liberty of running try builds with your patch on the Solaris
> builders:
>
>   https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/410
>   https://gdb-build.sergiodj.net/builders/Solaris11-sparcv9-m64/builds/408
>
> Unfortunately the build still fails with:
>
>   ../../binutils-gdb/gdb/sol-thread.c: In member function ‘virtual ptid_t sol_thread_target::wait(ptid_t, target_waitstatus*, int)’:
>   ../../binutils-gdb/gdb/sol-thread.c:465:2: error: expected ‘)’ before ‘{’ token
>     {
>     ^
>   ../../binutils-gdb/gdb/sol-thread.c:470:5: error: expected primary-expression before ‘}’ token
>        }
>        ^
>
> BTW, it's possible to run try builds on specific builders by using the
> "-b" option with "buildbot try".  E.g.:
>
>   buildbot try -b Solaris11-amd64-m64 -b Solaris11-sparcv9-m64

And the actual commit failed with another reason:

https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/411/steps/compile%20gdb/logs/stdio

../../binutils-gdb/gdb/procfs.c: In function ‘int procfs_notice_thread(procinfo*, procinfo*, void*)’:
../../binutils-gdb/gdb/procfs.c:3136:41: error: ‘is_exited’ was not declared in this scope
   if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
                                         ^~~~~~~~~
../../binutils-gdb/gdb/procfs.c:3136:41: note: suggested alternative: ‘inf_exited’
   if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
                                         ^~~~~~~~~
                                         inf_exited
Rainer Orth Nov. 23, 2018, 2:16 p.m. UTC | #3 
Hi Sergio,

> On Friday, November 23 2018, I wrote:
>
>> On Friday, November 23 2018, Pedro Alves wrote:
>>
>>> On 11/22/2018 04:49 PM, gdb-buildbot@sergiodj.net wrote:
>>>> Undefined			first referenced
>>>>  symbol  			    in file
>>>> is_exited(ptid_t)                   procfs.o
>>>> ld: fatal: symbol referencing errors
>>>> collect2: error: ld returned 1 exit status
>>>> gmake[2]: *** [Makefile:1912: gdb] Error 1
>>>> gmake[2]: Leaving directory
>>>> '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build/gdb'
>>>> gmake[1]: *** [Makefile:8792: all-gdb] Error 2
>>>> gmake[1]: Leaving directory
>>>> '/opt/gdb-buildbot/home/solaris11-amd64/solaris11-amd64-m64/build'
>>>> gmake: *** [Makefile:849: all] Error 2
>>>> ============================
>>>
>>> Whoops, sorry about that.  I assume this works, but I haven't tried it.
>>
>> I took the liberty of running try builds with your patch on the Solaris
>> builders:
>>
>>   https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/410
>>   https://gdb-build.sergiodj.net/builders/Solaris11-sparcv9-m64/builds/408
>>
>> Unfortunately the build still fails with:
>>
>>   ../../binutils-gdb/gdb/sol-thread.c: In member function ‘virtual ptid_t sol_thread_target::wait(ptid_t, target_waitstatus*, int)’:
>>   ../../binutils-gdb/gdb/sol-thread.c:465:2: error: expected ‘)’ before ‘{’ token
>>     {
>>     ^
>>   ../../binutils-gdb/gdb/sol-thread.c:470:5: error: expected primary-expression before ‘}’ token
>>        }
>>        ^
>>
>> BTW, it's possible to run try builds on specific builders by using the
>> "-b" option with "buildbot try".  E.g.:
>>
>>   buildbot try -b Solaris11-amd64-m64 -b Solaris11-sparcv9-m64
>
> And the actual commit failed with another reason:
>
> https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/411/steps/compile%20gdb/logs/stdio
>
> ../../binutils-gdb/gdb/procfs.c: In function ‘int procfs_notice_thread(procinfo*, procinfo*, void*)’:
> ../../binutils-gdb/gdb/procfs.c:3136:41: error: ‘is_exited’ was not declared in this scope
>    if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
>                                          ^~~~~~~~~
> ../../binutils-gdb/gdb/procfs.c:3136:41: note: suggested alternative: ‘inf_exited’
>    if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
>                                          ^~~~~~~~~
>                                          inf_exited

this is weird: I still see a declaration in gdbthread.h (and also
is_running and is_stopped), which should go as well.

With just the typo above fixed, a local build on amd64-pc-solaris2.11
has just finished; make check running...

	Rainer
Pedro Alves Nov. 23, 2018, 4:39 p.m. UTC | #4
Rainer Orth Nov. 25, 2018, 2:55 p.m. UTC | #5 
Hi Pedro,

>>> And the actual commit failed with another reason:
>>>
>>> https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/411/steps/compile%20gdb/logs/stdio
>>>
>>> ../../binutils-gdb/gdb/procfs.c: In function ‘int procfs_notice_thread(procinfo*, procinfo*, void*)’:
>>> ../../binutils-gdb/gdb/procfs.c:3136:41: error: ‘is_exited’ was not declared in this scope
>>>    if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
>>>                                          ^~~~~~~~~
>>> ../../binutils-gdb/gdb/procfs.c:3136:41: note: suggested alternative: ‘inf_exited’
>>>    if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
>>>                                          ^~~~~~~~~
>>>                                          inf_exited
>> 
>> this is weird: I still see a declaration in gdbthread.h (and also
>> is_running and is_stopped), which should go as well.
>
>That's because I meanwhile pushed this:
>  https://sourceware.org/ml/gdb-patches/2018-11/msg00380.html

I only saw that commit after I'd sent my mail.

>So before that we got a link error, and now we get a compile
>error, which is expected.
>
>> 
>> With just the typo above fixed, a local build on amd64-pc-solaris2.11
>> has just finished; make check running...

It took me a bit to double-check, but the differences in testsuite
results are within the (usual, mostly thread-related) variations.

So the patch is ok with the missing parenthesis added.

Thanks.
	Rainer
Pedro Alves Nov. 26, 2018, 1:26 p.m. UTC | #6 
On 11/25/2018 02:55 PM, Rainer Orth wrote:
> Hi Pedro,
> 
>>>> And the actual commit failed with another reason:
>>>>
>>>> https://gdb-build.sergiodj.net/builders/Solaris11-amd64-m64/builds/411/steps/compile%20gdb/logs/stdio
>>>>
>>>> ../../binutils-gdb/gdb/procfs.c: In function ‘int procfs_notice_thread(procinfo*, procinfo*, void*)’:
>>>> ../../binutils-gdb/gdb/procfs.c:3136:41: error: ‘is_exited’ was not declared in this scope
>>>>    if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
>>>>                                          ^~~~~~~~~
>>>> ../../binutils-gdb/gdb/procfs.c:3136:41: note: suggested alternative: ‘inf_exited’
>>>>    if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
>>>>                                          ^~~~~~~~~
>>>>                                          inf_exited
>>>
>>> this is weird: I still see a declaration in gdbthread.h (and also
>>> is_running and is_stopped), which should go as well.
>>
>> That's because I meanwhile pushed this:
>>  https://sourceware.org/ml/gdb-patches/2018-11/msg00380.html
> 
> I only saw that commit after I'd sent my mail.
> 
>> So before that we got a link error, and now we get a compile
>> error, which is expected.
>>
>>>
>>> With just the typo above fixed, a local build on amd64-pc-solaris2.11
>>> has just finished; make check running...
> 
> It took me a bit to double-check, but the differences in testsuite
> results are within the (usual, mostly thread-related) variations.
> 
> So the patch is ok with the missing parenthesis added.

Thanks much.  I've merged it now.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/procfs.c b/gdb/procfs.c
index 0d8f9df966..f82fc83987 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -3133,7 +3133,8 @@  procfs_notice_thread (procinfo *pi, procinfo *thread, void *ptr)
 {
   ptid_t gdb_threadid = ptid_t (pi->pid, thread->tid, 0);
 
-  if (!in_thread_list (gdb_threadid) || is_exited (gdb_threadid))
+  thread_info *thr = find_thread_ptid (gdb_threadid);
+  if (thr == NULL || thr->state == THREAD_EXITED)
     add_thread (gdb_threadid);
 
   return 0;
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 682302a40b..0a57c4d1a6 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -462,9 +462,11 @@  sol_thread_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       /* See if we have a new thread.  */
       if (rtnval.tid_p ()
 	  && rtnval != save_ptid
-	  && (!in_thread_list (rtnval)
-	      || is_exited (rtnval)))
-	add_thread (rtnval);
+	{
+	  thread_info *thr = find_thread_ptid (rtnval);
+	  if (thr == NULL || thr->state == THREAD_EXITED)
+	    add_thread (rtnval);
+	}
     }
 
   /* During process initialization, we may get here without the thread
@@ -1035,14 +1037,14 @@  sol_update_thread_list_callback (const td_thrhandle_t *th, void *ignored)
 {
   td_err_e retval;
   td_thrinfo_t ti;
-  ptid_t ptid;
 
   retval = p_td_thr_get_info (th, &ti);
   if (retval != TD_OK)
     return -1;
 
-  ptid = ptid_t (inferior_ptid.pid (), 0, ti.ti_tid);
-  if (!in_thread_list (ptid) || is_exited (ptid))
+  ptid_t ptid = ptid_t (inferior_ptid.pid (), 0, ti.ti_tid);
+  thread_info *thr = find_thread_ptid (ptid);
+  if (thr == NULL || thr->state == THREAD_EXITED)
     add_thread (ptid);
 
   return 0;