Re: PATCH: Add x32 arch_prctl support

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, May 21, 2012 at 2:33 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> > This is probably going to get a "might be used uninitialized" warning.
>>
>> Yes. ?Should I silence it?
>
> Whenever you can reorganize the code so as not to get such a warning,
> without actually producing less optimal compiled code, then do.

I have to add "addr_saved = NULL" even when it will never
be used.  I will leave  it ASIS.

>> > Anyway, I think it's cleaner to pass an 'unsigned long long int' (or
>> > uint64_t) to the syscall, since that's what it is in the kernel.
>> > Might as well just make it:
>>
>> I thought about it and decided against it since it will require
>> special treatment for x32 when it is used.
>
> I don't understand. ?Please elaborate.

With my current approach, this works:

[hjl@gnu-6 tmp]$ cat p.c
#include <stdio.h>
#include <asm/prctl.h>
#include <sys/prctl.h>

int
main ()
{
  void *base;

  if (arch_prctl (ARCH_GET_FS, &base) == 0)
    printf ("FS base: %p\n", base);
  return 0;
}
[hjl@gnu-6 tmp]$ gcc -mx32 p.c
[hjl@gnu-6 tmp]$ ./a.out
FS base: 0xf7fda720
[hjl@gnu-6 tmp]$ gcc -m64 p.c
[hjl@gnu-6 tmp]$ ./a.out
FS base: 0x7ffff7fdb720
[hjl@gnu-6 tmp]$

Change it to unsigned long long int makes it impossible
to get address as pointer since it will override memory on stack.

>> PTRACE_ARCH_PRCTL is x86 specific and is only used by GDB,
>> GDB has #ifdef to define it, which will never be removed. ?If we add
>> it to ptrace.h, I have to add it a new ptrace for x86-64. ?I don't think
>> we should do it.
>
> Well, it's unclean to have parts of the API that we don't properly
> declare in our headers. ?So generically I would want to clean this up.
> But since it so happens that PTRACE_ARCH_PRCTL is obsolete with modern
> kernels anyway, I don't think it's worth the trouble.
>
>> >> + ?switch ((int) request)
>> >> + ? ?{
>> >
>> > There's no need to cast to int when you have a default case anyway.
>>
>> I need it if PTRACE_ARCH_PRCTL isn't in x86-64 ptrace.h.
>
> You mean the compiler complains about a case that is not part of the enum?

Yes,

>> That is true. However, since it is used in GDB today, I need it
>> to support it for x32.
>
> There is no legacy of a GDB compiled with -mx32 that must continue to work.
> For an -m64 GDB, there is no problem and won't be one.
>

I am trying to limit my x32 GDB change as small as possible:

hjl@gnu-6 gdb]$ grep PTRACE_ARCH_PRCTL  amd64-linux-nat.c
#ifndef PTRACE_ARCH_PRCTL
#define PTRACE_ARCH_PRCTL      30
	  if (ptrace (PTRACE_ARCH_PRCTL, lwpid, base, ARCH_GET_FS) == 0)
	  if (ptrace (PTRACE_ARCH_PRCTL, lwpid, base, ARCH_GET_GS) == 0)
[hjl@gnu-6 gdb]$

Without PTRACE_ARCH_PRCTL means I have to write a whole
new patch to use the new interface.   It will make it harder to enable
x32 in GDB.

Thanks.

-- 
H.J.