Re: [PATCH/RFC 01/02 v2] Refactor PRPSINFO handling on Binutils

[Pedro Alves <palves@redhat.com>]

On 01/10/2013 07:47 PM, Sergio Durigan Junior wrote:

>> On 01/04/2013 04:39 AM, Sergio Durigan Junior wrote:> +++ b/bfd/elf-psinfo.h
>>> @@ -0,0 +1,124 @@
>>> +/* Definitions for PRPSINFO structures under ELF on GNU/Linux.
>>
>> What about other OSs, like e.g., Solaris?
> 
> Since I took the definitions from the Linux kernel, I thought it'd be
> better to explicitly mention that.  I'm afraid I don't know how Solaris
> handle corefiles, do you think this comment should be expanded?

It's not that different on other Unixen, but the structures
may be different.  But I'm actually pondering about is whether we're
tying the whole new interface to Linux, causing problems for converting
other non-Linux ports.

E.g., a quick websearch for prpsinfo finds:

 http://bintree.net/freebsd/d0/ddc/sys_2procfs_8h_source.html

00075 typedef struct prpsinfo {
00076     int         pr_version;     /* Version number of struct (1) */
00077     size_t      pr_psinfosz;    /* sizeof(prpsinfo_t) (1) */
00078     char        pr_fname[PRFNAMESZ+1];  /* Command name, null terminated (1) */
00079     char        pr_psargs[PRARGSZ+1];   /* Arguments, null terminated (1) */
00080 } prpsinfo_t;

Other BSDs are probably similar, Solaris, IRIX, etc.,
will probably have a different structure in their own ways.

So indeed it looks like there's a problem here, and I think these
differences need to be contemplated.  How is the question.

One way would be for the hook to still take a "struct elf_internal_prpsinfo"
pointer, but make it so that "struct elf_internal_prpsinfo" itself is
opaque to the common code.  Both GDB's core generator code, and then
bfd's elf_backend_write_core_note hook would then have to agree on what
the type of "struct elf_internal_prpsinfo" really is, and either cast
appropriately (e.g., to/from struct elf_linux_internal_prpsinfo),
or including a file that has the proper definition of
"struct elf_internal_prpsinfo" for the target (I think that violates
C++'s ODR, so I tend to dislike it, although gdb/binutils do this
for other similar cases).

This is largely what the patch does already, except it takes
no consideration, either in naming or in comments that
"struct elf_internal_prpsinfo" is Linux specific, giving the
impression that it'd work for all ports.  But it doesn't
look like it would.  E.g., elf64-x86-64.c:elf_x86_64_write_core_note
with the patch is always using the Linux specific structure.
We need to instead have a Linux-specific version of that hook;
comments at the top of the file show it's used with FreeBSD and
Solaris too as well, and those will need their own versions
(could be the current pre-patch version).  Etc.

-- 
Pedro Alves