Re: [PATCH v4] Add support for NetBSD threads in sparc-nat.c

[Simon Marchi <simark@simark.ca>]

On 2020-03-17 12:51 p.m., Simon Marchi wrote:
> On 2020-03-17 12:46 p.m., Kamil Rytarowski wrote:
>> NetBSD ptrace(2) accepts thread id (LWP) as the 4th argument for threads.
>>
>> This file is still shared with other targets that use different 4th argument
>> type, that is always unused.
>> ---
>>  gdb/sparc-nat.c | 50 ++++++++++++++++++++++---------------------------
>>  1 file changed, 22 insertions(+), 28 deletions(-)
>>
>> diff --git a/gdb/sparc-nat.c b/gdb/sparc-nat.c
>> index dff0f521565..10cc01375ed 100644
>> --- a/gdb/sparc-nat.c
>> +++ b/gdb/sparc-nat.c
>> @@ -78,6 +78,19 @@ typedef struct fp_status fpregset_t;
>>  #define PTRACE_SETFPREGS PT_SETFPREGS
>>  #endif
>>
>> +static int
>> +gdb_ptrace (int request, ptid_t pid, void *addr)
>> +{
>> +#ifdef __NetBSD__
>> +  /* Support for NetBSD threads: unlike other ptrace implementations in this
>> +     file, NetBSD requires that we pass both the pid and lwp.  */
>> +  return ptrace (request, ptid (). pid (), addr, ptid ().lwp ());
> 
> Does that even build?  ptid isn't the name of a parameter, and the parenthesis after
> "ptid" should not be there.  Rename the pid parameter to ptid, and replace the above
> with "ptid.pid ()" and "ptid.lwp ()".
> 
> Simon
> 

I tried to build this using a sparc-linux cross-compiler, and I think we need
to adjust gdb_ptrace to account for the various possible arguments types.  The
following version of the function builds fine with sparc64-linux-gnu-gcc, can you
try it with NetBSD?

static PTRACE_TYPE_RET
gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG4 addr)
{
#ifdef __NetBSD__
  /* Support for NetBSD threads: unlike other ptrace implementations in this
     file, NetBSD requires that we pass both the pid and lwp.  */
  return ptrace (request, ptid.pid (), addr, ptid.lwp ());
#else
  pid_t pid = get_ptrace_pid (ptid);
  return ptrace (request, pid, addr, 0);
#endif
}

Alternatively, we could keep address as a "void *", and cast the value inside gdb_ptrace:

  return ptrace (request, ptid.pid (), (PTRACE_TYPE_ARG4) addr, ptid.lwp ());

All the callers of gdb_ptrace wouldn't have to cast that themselves, which is perhaps
a bit nicer.

Simon