[rfa] Assume thread-db loaded over a live process

[rfa] Assume thread-db loaded over a live process

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

Hello,

This patch changes:

-      /* We can only poke around if there actually is a child process.
-         If there is no child process alive, postpone the steps below
-         until one has been created.  */
-      if (proc_handle.pid != 0)
-       {
-         enable_thread_event_reporting ();
-         thread_db_find_new_threads ();
-       }
+      enable_thread_event_reporting ();
+      thread_db_find_new_threads ();

this code is only executed when there is a child process so the guard 
isn't needed.    Tested on GNU/Linux, no change in test results.

ok?
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 935 bytes --]

2004-09-11  Andrew Cagney  <cagney@gnu.org>

	* thread-db.c (thread_db_new_objfile): Assume that there is a
	child process.

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.44
diff -p -u -r1.44 thread-db.c
--- thread-db.c	8 Sep 2004 19:44:22 -0000	1.44
+++ thread-db.c	13 Sep 2004 13:51:23 -0000
@@ -697,14 +697,8 @@ thread_db_new_objfile (struct objfile *o
       push_target (&thread_db_ops);
       using_thread_db = 1;
 
-      /* We can only poke around if there actually is a child process.
-         If there is no child process alive, postpone the steps below
-         until one has been created.  */
-      if (proc_handle.pid != 0)
-	{
-	  enable_thread_event_reporting ();
-	  thread_db_find_new_threads ();
-	}
+      enable_thread_event_reporting ();
+      thread_db_find_new_threads ();
       break;
 
     default:

Re: [rfa] Assume thread-db loaded over a live process

[Daniel Jacobowitz]

On Mon, Sep 13, 2004 at 09:57:25AM -0400, Andrew Cagney wrote:
> Hello,
> 
> This patch changes:
> 
> -      /* We can only poke around if there actually is a child process.
> -         If there is no child process alive, postpone the steps below
> -         until one has been created.  */
> -      if (proc_handle.pid != 0)
> -       {
> -         enable_thread_event_reporting ();
> -         thread_db_find_new_threads ();
> -       }
> +      enable_thread_event_reporting ();
> +      thread_db_find_new_threads ();
> 
> this code is only executed when there is a child process so the guard 
> isn't needed.    Tested on GNU/Linux, no change in test results.

Historical note: this code is older than the !target_has_execution
check above.  It used to be reachable, and was for the static
executable case.

Seems a reasonable change to me.

-- 
Daniel Jacobowitz

Re: [rfa] Assume thread-db loaded over a live process

[Michael Snyder]

Andrew Cagney wrote:
> Hello,
> 
> This patch changes:
> 
> -      /* We can only poke around if there actually is a child process.
> -         If there is no child process alive, postpone the steps below
> -         until one has been created.  */
> -      if (proc_handle.pid != 0)
> -       {
> -         enable_thread_event_reporting ();
> -         thread_db_find_new_threads ();
> -       }
> +      enable_thread_event_reporting ();
> +      thread_db_find_new_threads ();
> 
> this code is only executed when there is a child process so the guard 
> isn't needed.    Tested on GNU/Linux, no change in test results.
> 
> ok?

 From memory, I think this code was to guard against the corefile case.
When you load a corefile, you may call thread_db_new_objfile, but
you won't have a child process.  Is that no longer the case?  Does
loading a corefile no longer cause this function to be called?

Re: [rfa] Assume thread-db loaded over a live process

[Andrew Cagney]

> Andrew Cagney wrote:
> 
>> Hello,
>>
>> This patch changes:
>>
>> -      /* We can only poke around if there actually is a child process.
>> -         If there is no child process alive, postpone the steps below
>> -         until one has been created.  */
>> -      if (proc_handle.pid != 0)
>> -       {
>> -         enable_thread_event_reporting ();
>> -         thread_db_find_new_threads ();
>> -       }
>> +      enable_thread_event_reporting ();
>> +      thread_db_find_new_threads ();
>>
>> this code is only executed when there is a child process so the guard isn't needed.    Tested on GNU/Linux, no change in test results.
>>
>> ok?
> 
> 
> From memory, I think this code was to guard against the corefile case.
> When you load a corefile, you may call thread_db_new_objfile, but
> you won't have a child process.  Is that no longer the case?  Does
> loading a corefile no longer cause this function to be called?

This code is only executed when there is a child process.  As you note, 
when loading a core file there is no child process (and as daniel 
pointed out, !target_has_execution holds) so this code is not executed.

Andrew

Re: [rfa] Assume thread-db loaded over a live process

[Michael Snyder]

Andrew Cagney wrote:
>> Andrew Cagney wrote:
>>
>>> Hello,
>>>
>>> This patch changes:
>>>
>>> -      /* We can only poke around if there actually is a child process.
>>> -         If there is no child process alive, postpone the steps below
>>> -         until one has been created.  */
>>> -      if (proc_handle.pid != 0)
>>> -       {
>>> -         enable_thread_event_reporting ();
>>> -         thread_db_find_new_threads ();
>>> -       }
>>> +      enable_thread_event_reporting ();
>>> +      thread_db_find_new_threads ();
>>>
>>> this code is only executed when there is a child process so the guard 
>>> isn't needed.    Tested on GNU/Linux, no change in test results.
>>>
>>> ok?
>>
>>
>>
>> From memory, I think this code was to guard against the corefile case.
>> When you load a corefile, you may call thread_db_new_objfile, but
>> you won't have a child process.  Is that no longer the case?  Does
>> loading a corefile no longer cause this function to be called?
> 
> 
> This code is only executed when there is a child process.  As you note, 
> when loading a core file there is no child process (and as daniel 
> pointed out, !target_has_execution holds) so this code is not executed.

That's what I don't understand, Andrew.  This code *used to* get
called for a corefile.  What's changed?  As far as I can see,
it will get called from symbol_file_add.  In my existing July
build, it does.

Re: [rfa] Assume thread-db loaded over a live process

[Daniel Jacobowitz]

On Mon, Sep 13, 2004 at 10:51:39PM +0000, Michael Snyder wrote:
> Andrew Cagney wrote:
> >>Andrew Cagney wrote:
> >>
> >>>Hello,
> >>>
> >>>This patch changes:
> >>>
> >>>-      /* We can only poke around if there actually is a child process.
> >>>-         If there is no child process alive, postpone the steps below
> >>>-         until one has been created.  */
> >>>-      if (proc_handle.pid != 0)
> >>>-       {
> >>>-         enable_thread_event_reporting ();
> >>>-         thread_db_find_new_threads ();
> >>>-       }
> >>>+      enable_thread_event_reporting ();
> >>>+      thread_db_find_new_threads ();
> >>>
> >>>this code is only executed when there is a child process so the guard 
> >>>isn't needed.    Tested on GNU/Linux, no change in test results.
> >>>
> >>>ok?
> >>
> >>
> >>
> >>From memory, I think this code was to guard against the corefile case.
> >>When you load a corefile, you may call thread_db_new_objfile, but
> >>you won't have a child process.  Is that no longer the case?  Does
> >>loading a corefile no longer cause this function to be called?
> >
> >
> >This code is only executed when there is a child process.  As you note, 
> >when loading a core file there is no child process (and as daniel 
> >pointed out, !target_has_execution holds) so this code is not executed.
> 
> That's what I don't understand, Andrew.  This code *used to* get
> called for a corefile.  What's changed?  As far as I can see,
> it will get called from symbol_file_add.  In my existing July
> build, it does.

Um, is that July of _this_ year?  There's a !target_has_execution check
up above it.

-- 
Daniel Jacobowitz

Re: [rfa] Assume thread-db loaded over a live process

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Mon, Sep 13, 2004 at 10:51:39PM +0000, Michael Snyder wrote:
> 
>>Andrew Cagney wrote:
>>
>>>>Andrew Cagney wrote:
>>>>
>>>>
>>>>>Hello,
>>>>>
>>>>>This patch changes:
>>>>>
>>>>>-      /* We can only poke around if there actually is a child process.
>>>>>-         If there is no child process alive, postpone the steps below
>>>>>-         until one has been created.  */
>>>>>-      if (proc_handle.pid != 0)
>>>>>-       {
>>>>>-         enable_thread_event_reporting ();
>>>>>-         thread_db_find_new_threads ();
>>>>>-       }
>>>>>+      enable_thread_event_reporting ();
>>>>>+      thread_db_find_new_threads ();
>>>>>
>>>>>this code is only executed when there is a child process so the guard 
>>>>>isn't needed.    Tested on GNU/Linux, no change in test results.
>>>>>
>>>>>ok?
>>>>
>>>>
>>>>
>>>>From memory, I think this code was to guard against the corefile case.
>>>
>>>>When you load a corefile, you may call thread_db_new_objfile, but
>>>>you won't have a child process.  Is that no longer the case?  Does
>>>>loading a corefile no longer cause this function to be called?
>>>
>>>
>>>This code is only executed when there is a child process.  As you note, 
>>>when loading a core file there is no child process (and as daniel 
>>>pointed out, !target_has_execution holds) so this code is not executed.
>>
>>That's what I don't understand, Andrew.  This code *used to* get
>>called for a corefile.  What's changed?  As far as I can see,
>>it will get called from symbol_file_add.  In my existing July
>>build, it does.
> 
> 
> Um, is that July of _this_ year?  There's a !target_has_execution check
> up above it.

Ah.  OK.  I only checked to see if the function was called.
Now I see that this code doesn't get reached.

OK, sorry for the confusion -- the change looks fine to me.

Re: [rfa] Assume thread-db loaded over a live process

[Andrew Cagney]

> OK, sorry for the confusion -- the change looks fine to me.

I've checked this in.

Andrew

RE: [rfa] Assume thread-db loaded over a live process

[Howell, David P]

As I recall from 5.3 this was necessary due to libthread_db 
using the /proc access libraries to get info from the inferior; 
it would require an alternate implementation for similar core 
file access, it wasn't there then and so the guard was necessary.
It would be nice if this were fixed. 

Thanks,
Dave Howell

These are my opinions and not official opinions of Intel Corp.
 
David Howell
Intel Corporation
Telco Server Development
Server Products Division
Voice: (803) 216-2359  Fax: (803) 216-2178
 
Intel Corporation
Columbia Design Center, CBA-1
100 Center Point Circle, Suite 210
Columbia, SC 29210
 
david.p.howell@intel.com
 

-----Original Message-----
From: gdb-patches-owner@sources.redhat.com
[mailto:gdb-patches-owner@sources.redhat.com] On Behalf Of Michael
Snyder
Sent: Monday, September 13, 2004 2:44 PM
To: Andrew Cagney
Cc: gdb-patches@sources.redhat.com
Subject: Re: [rfa] Assume thread-db loaded over a live process

Andrew Cagney wrote:
> Hello,
> 
> This patch changes:
> 
> -      /* We can only poke around if there actually is a child
process.
> -         If there is no child process alive, postpone the steps below
> -         until one has been created.  */
> -      if (proc_handle.pid != 0)
> -       {
> -         enable_thread_event_reporting ();
> -         thread_db_find_new_threads ();
> -       }
> +      enable_thread_event_reporting ();
> +      thread_db_find_new_threads ();
> 
> this code is only executed when there is a child process so the guard 
> isn't needed.    Tested on GNU/Linux, no change in test results.
> 
> ok?

 From memory, I think this code was to guard against the corefile case.
When you load a corefile, you may call thread_db_new_objfile, but
you won't have a child process.  Is that no longer the case?  Does
loading a corefile no longer cause this function to be called?