AARCH64: remove asm/prtace.h inclusion in sys/user.h and sys/procfs.h

Message ID CAD57uCe1eQ-womj1WW2jc6Y6mv+w6pY85dDkLGDcd0Of_6RE5A@mail.gmail.com
State Committed
Headers

Commit Message

Yvan Roux May 14, 2014, 11:46 a.m. UTC
  Hi,

this patch fixes an issue observed by the Xen project, where including
signal.h exposes various PSR_MODE #defines.  This is due to the usage
in sys/user.h and sys/procfs.h of the struct user_pt_regs and
user_fpsimd_state included via asm/ptrace.h.  The namespace pollution
this inclusion introduce is already partially fixed with some #undef
of the PTRACE_* symbols, but other symbols lile the PSR_MODE ones are
still present, and undefining them is not safe since a user can
include ptrace.h before user.h.

My proposition is to define the 2 structures we need in user.h and get
ride of the asm/ptrace.h inclusion.

Build + Make check are clean on AArch64.

Thanks,
Yvan

2014-05-14  Yvan Roux  <yvan.roux@linaro.org>

        * sysdeps/unix/sysv/linux/aarch64/sys/user.h: Remove unused #include of
        asm/ptrace.h.
        (PTRACE_GET_THREAD_AREA): Remove #undef.
        (PTRACE_GETHBPREGS): Likewise.
        (PTRACE_SETHBPREGS): Likewise.
        (struct user_regs_struct): New structure.
        (struct user_fpsimd_struct): New structure
        * sysdeps/unix/sysv/linux/aarch64/sys/procfs.h: Remove unused #include
        of asm/ptrace.h and second #include of sys/user.h.
        (PTRACE_GET_THREAD_AREA): Remove #undef.
        (PTRACE_GETHBPREGS): Likewise.
        (PTRACE_SETHBPREGS): Likewise.
        (ELF_NGREG): Use new struct user_regs_struct.
        (elf_fpregset_t): Use new struct user_fpsimd_struct.
  

Comments

Yvan Roux May 20, 2014, 8:19 a.m. UTC | #1
Ping.

On 14 May 2014 13:46, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> this patch fixes an issue observed by the Xen project, where including
> signal.h exposes various PSR_MODE #defines.  This is due to the usage
> in sys/user.h and sys/procfs.h of the struct user_pt_regs and
> user_fpsimd_state included via asm/ptrace.h.  The namespace pollution
> this inclusion introduce is already partially fixed with some #undef
> of the PTRACE_* symbols, but other symbols lile the PSR_MODE ones are
> still present, and undefining them is not safe since a user can
> include ptrace.h before user.h.
>
> My proposition is to define the 2 structures we need in user.h and get
> ride of the asm/ptrace.h inclusion.
>
> Build + Make check are clean on AArch64.
>
> Thanks,
> Yvan
>
> 2014-05-14  Yvan Roux  <yvan.roux@linaro.org>
>
>         * sysdeps/unix/sysv/linux/aarch64/sys/user.h: Remove unused #include of
>         asm/ptrace.h.
>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>         (PTRACE_GETHBPREGS): Likewise.
>         (PTRACE_SETHBPREGS): Likewise.
>         (struct user_regs_struct): New structure.
>         (struct user_fpsimd_struct): New structure
>         * sysdeps/unix/sysv/linux/aarch64/sys/procfs.h: Remove unused #include
>         of asm/ptrace.h and second #include of sys/user.h.
>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>         (PTRACE_GETHBPREGS): Likewise.
>         (PTRACE_SETHBPREGS): Likewise.
>         (ELF_NGREG): Use new struct user_regs_struct.
>         (elf_fpregset_t): Use new struct user_fpsimd_struct.
  
Will Newton May 20, 2014, 8:29 a.m. UTC | #2
On 14 May 2014 12:46, Yvan Roux <yvan.roux@linaro.org> wrote:

Hi Yvan,

There is a typo in the subject line (ptrace.h).

> this patch fixes an issue observed by the Xen project, where including
> signal.h exposes various PSR_MODE #defines.  This is due to the usage
> in sys/user.h and sys/procfs.h of the struct user_pt_regs and
> user_fpsimd_state included via asm/ptrace.h.  The namespace pollution
> this inclusion introduce is already partially fixed with some #undef
> of the PTRACE_* symbols, but other symbols lile the PSR_MODE ones are
> still present, and undefining them is not safe since a user can
> include ptrace.h before user.h.
>
> My proposition is to define the 2 structures we need in user.h and get
> ride of the asm/ptrace.h inclusion.
>
> Build + Make check are clean on AArch64.
>
> Thanks,
> Yvan
>
> 2014-05-14  Yvan Roux  <yvan.roux@linaro.org>
>
>         * sysdeps/unix/sysv/linux/aarch64/sys/user.h: Remove unused #include of
>         asm/ptrace.h.
>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>         (PTRACE_GETHBPREGS): Likewise.
>         (PTRACE_SETHBPREGS): Likewise.
>         (struct user_regs_struct): New structure.
>         (struct user_fpsimd_struct): New structure

Missing full stop at the end of the sentence.

>         * sysdeps/unix/sysv/linux/aarch64/sys/procfs.h: Remove unused #include
>         of asm/ptrace.h and second #include of sys/user.h.
>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>         (PTRACE_GETHBPREGS): Likewise.
>         (PTRACE_SETHBPREGS): Likewise.
>         (ELF_NGREG): Use new struct user_regs_struct.
>         (elf_fpregset_t): Use new struct user_fpsimd_struct.

Otherwise this looks ok to me.
  
Marcus Shawcroft May 20, 2014, 10:31 a.m. UTC | #3
On 14 May 2014 12:46, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> this patch fixes an issue observed by the Xen project, where including
> signal.h exposes various PSR_MODE #defines.  This is due to the usage
> in sys/user.h and sys/procfs.h of the struct user_pt_regs and
> user_fpsimd_state included via asm/ptrace.h.  The namespace pollution
> this inclusion introduce is already partially fixed with some #undef
> of the PTRACE_* symbols, but other symbols lile the PSR_MODE ones are
> still present, and undefining them is not safe since a user can
> include ptrace.h before user.h.
>
> My proposition is to define the 2 structures we need in user.h and get
> ride of the asm/ptrace.h inclusion.
>
> Build + Make check are clean on AArch64.
>
> Thanks,
> Yvan
>
> 2014-05-14  Yvan Roux  <yvan.roux@linaro.org>
>
>         * sysdeps/unix/sysv/linux/aarch64/sys/user.h: Remove unused #include of
>         asm/ptrace.h.
>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>         (PTRACE_GETHBPREGS): Likewise.
>         (PTRACE_SETHBPREGS): Likewise.
>         (struct user_regs_struct): New structure.
>         (struct user_fpsimd_struct): New structure
>         * sysdeps/unix/sysv/linux/aarch64/sys/procfs.h: Remove unused #include
>         of asm/ptrace.h and second #include of sys/user.h.
>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>         (PTRACE_GETHBPREGS): Likewise.
>         (PTRACE_SETHBPREGS): Likewise.
>         (ELF_NGREG): Use new struct user_regs_struct.
>         (elf_fpregset_t): Use new struct user_fpsimd_struct.

I think this re-structure is OK and can go in with the corrections
Will called out.

Thanks
/Marcus
  
Will Newton May 20, 2014, 12:50 p.m. UTC | #4
On 20 May 2014 11:31, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
> On 14 May 2014 12:46, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Hi,
>>
>> this patch fixes an issue observed by the Xen project, where including
>> signal.h exposes various PSR_MODE #defines.  This is due to the usage
>> in sys/user.h and sys/procfs.h of the struct user_pt_regs and
>> user_fpsimd_state included via asm/ptrace.h.  The namespace pollution
>> this inclusion introduce is already partially fixed with some #undef
>> of the PTRACE_* symbols, but other symbols lile the PSR_MODE ones are
>> still present, and undefining them is not safe since a user can
>> include ptrace.h before user.h.
>>
>> My proposition is to define the 2 structures we need in user.h and get
>> ride of the asm/ptrace.h inclusion.
>>
>> Build + Make check are clean on AArch64.
>>
>> Thanks,
>> Yvan
>>
>> 2014-05-14  Yvan Roux  <yvan.roux@linaro.org>
>>
>>         * sysdeps/unix/sysv/linux/aarch64/sys/user.h: Remove unused #include of
>>         asm/ptrace.h.
>>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>>         (PTRACE_GETHBPREGS): Likewise.
>>         (PTRACE_SETHBPREGS): Likewise.
>>         (struct user_regs_struct): New structure.
>>         (struct user_fpsimd_struct): New structure
>>         * sysdeps/unix/sysv/linux/aarch64/sys/procfs.h: Remove unused #include
>>         of asm/ptrace.h and second #include of sys/user.h.
>>         (PTRACE_GET_THREAD_AREA): Remove #undef.
>>         (PTRACE_GETHBPREGS): Likewise.
>>         (PTRACE_SETHBPREGS): Likewise.
>>         (ELF_NGREG): Use new struct user_regs_struct.
>>         (elf_fpregset_t): Use new struct user_fpsimd_struct.
>
> I think this re-structure is OK and can go in with the corrections
> Will called out.

I committed this patch on Yvan's behalf.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/aarch64/sys/procfs.h b/sysdeps/unix/sysv/linux/aarch64/sys/procfs.h
index b02af8a..211227c 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sys/procfs.h
@@ -33,17 +33,6 @@ 
 #include <sys/types.h>
 #include <sys/user.h>
 
-/* We need to see the definition of struct pt_regs but do not want the
-   linux PTRACE_* defines since they conflict with the generic eglibc
-   definitions in sys/ptrace.h Hence the undef's below.  */
-#include <asm/ptrace.h>
-
-#undef PTRACE_GET_THREAD_AREA
-#undef PTRACE_GETHBPREGS
-#undef PTRACE_SETHBPREGS
-
-#include <sys/user.h>
-
 __BEGIN_DECLS
 
 /* Type for a general-purpose register.  */
@@ -53,11 +42,11 @@  typedef unsigned long elf_greg_t;
    pt_regs' directly in the typedef, but tradition says that
    the register set is an array, which does have some peculiar
    semantics, so leave it that way.  */
-#define ELF_NGREG (sizeof (struct user_pt_regs) / sizeof(elf_greg_t))
+#define ELF_NGREG (sizeof (struct user_regs_struct) / sizeof(elf_greg_t))
 typedef elf_greg_t elf_gregset_t[ELF_NGREG];
 
 /* Register set for the floating-point registers.  */
-typedef struct user_fpsimd_state elf_fpregset_t;
+typedef struct user_fpsimd_struct elf_fpregset_t;
 
 /* Signal info.  */
 struct elf_siginfo
diff --git a/sysdeps/unix/sysv/linux/aarch64/sys/user.h b/sysdeps/unix/sysv/linux/aarch64/sys/user.h
index eceeb38..0ca2715 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sys/user.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sys/user.h
@@ -19,13 +19,19 @@ 
 #ifndef _SYS_USER_H
 #define _SYS_USER_H	1
 
-/* We need to see the definition of struct pt_regs but do not want the
-   linux PTRACE_* defines since they conflict with the generic glibc
-   definitions in sys/ptrace.h Hence the undef's below.  */
-#include <asm/ptrace.h>
-
-#undef PTRACE_GET_THREAD_AREA
-#undef PTRACE_GETHBPREGS
-#undef PTRACE_SETHBPREGS
+struct user_regs_struct
+{
+  unsigned long long regs[31];
+  unsigned long long sp;
+  unsigned long long pc;
+  unsigned long long pstate;
+};
+
+struct user_fpsimd_struct
+{
+  __uint128_t  vregs[32];
+  unsigned int fpsr;
+  unsigned int fpcr;
+};
 
 #endif