Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [rfc v2][0/6] Remote /proc file access
@ 2012-01-13 18:14 Ulrich Weigand
  2012-01-16 16:39 ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2012-01-13 18:14 UTC (permalink / raw)
  To: gdb-patches, palves

Hello,

this is an updated series of patches to implement a generic
"info proc" command that works with remote/gdbserver Linux
targets.

The difference to the earlier version here:
http://sourceware.org/ml/gdb-patches/2011-12/msg00007.html
is that I've implemented Pedro's suggestions to access /proc
via generic file-access primitives instead of using the
target object mechanism.

Some code and ideas from Pedro's prototype here:
http://sourceware.org/ml/gdb-patches/2012-01/msg00353.html
has been integrated into this patch series.


1/6: Distinguish target and "fake" PID values
2/6: Always enable multi-process remote extension if possible
3/6: File I/O target operations
4/6: Readlink as file I/O target operation
5/6: Make "info proc" command generic
6/6: Implement "info proc" on Linux via target file I/O


I've also updated and tested the remote generate-core-file
patch series to use target file I/O instead of TARGET_OBJECT_PROC.
I'll post this once this approach is agreed to.


I've tested this series on i386 native and remote, as well as
on ARM remotely.   No regressions; a couple of additional
test cases (info-proc.exp) are now passing.

Any further comments comments on this approach are appreciated!


Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


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

* Re: [rfc v2][0/6] Remote /proc file access
  2012-01-13 18:14 [rfc v2][0/6] Remote /proc file access Ulrich Weigand
@ 2012-01-16 16:39 ` Pedro Alves
  2012-01-16 18:27   ` Ulrich Weigand
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-01-16 16:39 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

On 01/13/2012 06:13 PM, Ulrich Weigand wrote:

> Any further comments comments on this approach are appreciated!

I've been through the series, and it looks good to me.
Thanks a lot!  If this doesn't work out in the end, I'll certainly help
sort it out.

I've given a couple comments in reply to the patches directly.  I'm leaving
some general-ish comments here:

 - We could consider making "info proc" work with the default run
   target if the current target can't handle it, so that
   "info proc PID" works even when not debugging a process yet,
   like today.

 - It could be argued that the pid parsing should be kept at
   the target/gdbarch callbacks side (pass down `char *args'),
   so that we didn't have:

+  if (args && *args == '/')
+    tid = strtoul (args + 1, &args, 10);
+  else
+    tid = 0;

   in infcmd.c:info_proc_cmd_1 which is only needed by procfs.c.

But please, don't consider these comments blocking in any way.

Thanks again.
-- 
Pedro Alves


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

* Re: [rfc v2][0/6] Remote /proc file access
  2012-01-16 16:39 ` Pedro Alves
@ 2012-01-16 18:27   ` Ulrich Weigand
  2012-01-16 19:38     ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2012-01-16 18:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> I've been through the series, and it looks good to me.
> Thanks a lot!  If this doesn't work out in the end, I'll certainly help
> sort it out.

Thanks for the review!

> I've given a couple comments in reply to the patches directly.  I'm leaving
> some general-ish comments here:
> 
>  - We could consider making "info proc" work with the default run
>    target if the current target can't handle it, so that
>    "info proc PID" works even when not debugging a process yet,
>    like today.

Maybe.  On the other hand, once we've switched to the gdbarch based
implementation, it would automatically work when not yet debugging
a process anyway, so I'm not sure this is really necessary ...

>  - It could be argued that the pid parsing should be kept at
>    the target/gdbarch callbacks side (pass down `char *args'),
>    so that we didn't have:
> 
> +  if (args && *args == '/')
> +    tid = strtoul (args + 1, &args, 10);
> +  else
> +    tid = 0;
> 
>    in infcmd.c:info_proc_cmd_1 which is only needed by procfs.c.

Yes, good point.  In fact, that makes more sense; the format of a
process ID ought to be target-specific (like with attach).  Also,
this will make the diffs a bit smaller ...

I'll provide an updated patch set.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


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

* Re: [rfc v2][0/6] Remote /proc file access
  2012-01-16 18:27   ` Ulrich Weigand
@ 2012-01-16 19:38     ` Pedro Alves
  2012-01-17 19:41       ` Ulrich Weigand
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-01-16 19:38 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

On 01/16/2012 05:28 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
>> I've been through the series, and it looks good to me.
>> Thanks a lot!  If this doesn't work out in the end, I'll certainly help
>> sort it out.
> 
> Thanks for the review!
> 
>> I've given a couple comments in reply to the patches directly.  I'm leaving
>> some general-ish comments here:
>>
>>  - We could consider making "info proc" work with the default run
>>    target if the current target can't handle it, so that
>>    "info proc PID" works even when not debugging a process yet,
>>    like today.
> 
> Maybe.  On the other hand, once we've switched to the gdbarch based
> implementation, it would automatically work when not yet debugging
> a process anyway, so I'm not sure this is really necessary ...

Hmm.  I though the target_file_xxx routines would all fail, hitting the default
when no target implement the methods that way.  IOW, I was thinking that we'd
need to pass the struct target_ops pointer down to the gdbarch callback so
the callback could work with the correct target.

-- 
Pedro Alves


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

* Re: [rfc v2][0/6] Remote /proc file access
  2012-01-16 19:38     ` Pedro Alves
@ 2012-01-17 19:41       ` Ulrich Weigand
  2012-01-17 19:46         ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2012-01-17 19:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> On 01/16/2012 05:28 PM, Ulrich Weigand wrote:
> > Maybe.  On the other hand, once we've switched to the gdbarch based
> > implementation, it would automatically work when not yet debugging
> > a process anyway, so I'm not sure this is really necessary ...
> 
> Hmm.  I though the target_file_xxx routines would all fail, hitting the default
> when no target implement the methods that way.  IOW, I was thinking that we'd
> need to pass the struct target_ops pointer down to the gdbarch callback so
> the callback could work with the correct target.

I'd really prefer the gdbarch callback to stand alone; longer term I think
we ought to get rid of target_info_proc completely and have it just always
use the gdbarch callback.

However, you're right that the target_fileio_ routines fail.  I've now fixed
this by making those fall back to the native target if we're not yet connected
to any actual target (>= process_stratum), just like it is already done for
target_get_osdata ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


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

* Re: [rfc v2][0/6] Remote /proc file access
  2012-01-17 19:41       ` Ulrich Weigand
@ 2012-01-17 19:46         ` Pedro Alves
  0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2012-01-17 19:46 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

On 01/17/2012 07:36 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
>> On 01/16/2012 05:28 PM, Ulrich Weigand wrote:
>>> Maybe.  On the other hand, once we've switched to the gdbarch based
>>> implementation, it would automatically work when not yet debugging
>>> a process anyway, so I'm not sure this is really necessary ...
>>
>> Hmm.  I though the target_file_xxx routines would all fail, hitting the default
>> when no target implement the methods that way.  IOW, I was thinking that we'd
>> need to pass the struct target_ops pointer down to the gdbarch callback so
>> the callback could work with the correct target.
> 
> I'd really prefer the gdbarch callback to stand alone; longer term I think
> we ought to get rid of target_info_proc completely and have it just always
> use the gdbarch callback.

Ah.  Yes, agreed.

> 
> However, you're right that the target_fileio_ routines fail.  I've now fixed
> this by making those fall back to the native target if we're not yet connected
> to any actual target (>= process_stratum), just like it is already done for
> target_get_osdata ...

Great, thanks!

-- 
Pedro Alves


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

end of thread, other threads:[~2012-01-17 19:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-13 18:14 [rfc v2][0/6] Remote /proc file access Ulrich Weigand
2012-01-16 16:39 ` Pedro Alves
2012-01-16 18:27   ` Ulrich Weigand
2012-01-16 19:38     ` Pedro Alves
2012-01-17 19:41       ` Ulrich Weigand
2012-01-17 19:46         ` Pedro Alves

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