| Message ID | 1365779399-16654-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ian, On Fri, Apr 12, 2013 at 04:09:59PM +0100, Ian Campbell wrote: > Exposing these in ptrace.h ends up polluting the namespace for user > applications which include headers such as <signal.h>, e.g. when building Xen > userspace tools on arm64: > > tools/include/xen/arch-arm.h:229:0: error: "PSR_MODE_EL0t" redefined [-Werror] > In file included from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/user.h:25:0, > from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/procfs.h:34, > from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/ucontext.h:26, > from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/signal.h:360, > from xentrace.c:21: > /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/asm/ptrace.h:30:0: note: this is the location of the previous definition Strange... I don't see the PSR definitions in any headers other than ptrace.h for my aarch64 toolchain. Which tools are you using? Will
On Fri, 2013-04-12 at 16:18 +0100, Will Deacon wrote: > Hi Ian, > > On Fri, Apr 12, 2013 at 04:09:59PM +0100, Ian Campbell wrote: > > Exposing these in ptrace.h ends up polluting the namespace for user > > applications which include headers such as <signal.h>, e.g. when building Xen > > userspace tools on arm64: > > > > tools/include/xen/arch-arm.h:229:0: error: "PSR_MODE_EL0t" redefined [-Werror] > > In file included from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/user.h:25:0, > > from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/procfs.h:34, > > from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/ucontext.h:26, > > from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/signal.h:360, > > from xentrace.c:21: > > /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/asm/ptrace.h:30:0: note: this is the location of the previous definition > > Strange... I don't see the PSR definitions in any headers other than > ptrace.h for my aarch64 toolchain. It's the definition in ptrace.h which is the problem, since it conflicts with the application's own definitions (Xen userspace in this case). The names are in the "applications" namespace (i.e. not prefixed with _) and it hasn't included any headers which are defined to cause those names to be used. I'm not sure which (if any) spec includes ptrace.h but signal.h isn't defined to #define PSR_FOO. Maybe the real bug is the signal.h ends up including ptrace.h at all? > Which tools are you using? Ubuntu Raring multiarch cross thing, which is gcc 4.7.2-22ubuntu3 and glibc 2.17-0ubuntu4. Ian.
On Fri, 2013-04-12 at 16:50 +0100, Ian Campbell wrote: > I'm not sure which (if any) spec includes ptrace.h but signal.h isn't > defined to #define PSR_FOO. Maybe the real bug is the signal.h ends up > including ptrace.h at all? Having spoken to someone who understands this stuff better than I (although I still reserve the right to be talking out my a**e) it appears this is the case. ptrace.h is allowed to define whatever it likes because it's not defined by a standard but signal.h is specified by POSIX and is not allowed to define anything which isn't in the POSIX reserved namespace or which isn't explicitly mentions, which PSR_* is not. So it seems like the bug is on the libc side for including this particular #include chain? On the flip side is there any reason for the Linux uapi headers to be including architectural constants? e.g. the x86 uapi/ptrace.h doesn't define the RFLAGS bits etc... Ian.
On Fri, 12 Apr 2013 17:06:29 +0100, Ian Campbell <Ian.Campbell@citrix.com> wrote: [...] > On the flip side is there any reason for the Linux uapi headers to be > including architectural constants? e.g. the x86 uapi/ptrace.h doesn't > define the RFLAGS bits etc... My favourite userspace program (aka the other platform emulation that shall not ever be merged) needs it. QEMU probably will as well. Of course, we could define them locally, but that seems like a massive waste of perfectly innocent bytes. or convince the power that be to merge the damned thing so we can include kernel headers with any remorse... ;-) M.
On Fri, 2013-04-12 at 17:17 +0100, Marc Zyngier wrote: > On Fri, 12 Apr 2013 17:06:29 +0100, Ian Campbell <Ian.Campbell@citrix.com> > wrote: > > [...] > > > On the flip side is there any reason for the Linux uapi headers to be > > including architectural constants? e.g. the x86 uapi/ptrace.h doesn't > > define the RFLAGS bits etc... > > My favourite userspace program (aka the other platform emulation that > shall not ever be merged) needs it. QEMU probably will as well. > > Of course, we could define them locally, but that seems like a massive > waste of perfectly innocent bytes. I appreciate it's convenient but I'm not sure there is a shortage of bytes in the world, especially for well known unchanging constants like this. Your favourite userspace program is a bit of a special case since it doesn't need to be portable to non-Linux (or I assume not). Anyway, ignoring the red-herring I introduced the main point isn't that uapi/ptrace.h includes these definitions (that seems to be OK, standards wise and is useful to you etc) but rather that they end up infecting signal.h too. I'll file a bug against glibc/ubuntu/linaro as soon as I figure out which is the best target... > or convince the power that be to merge > the damned thing so we can include kernel headers with any remorse... ;-) /me steps back several paces and puts away his barge pole. ;-) Ian.
On Mon, 2013-04-15 at 12:48 +0100, Ian Campbell wrote: > I'll file a bug against glibc/ubuntu/linaro as soon as I > figure out which is the best target... https://bugs.launchpad.net/linaro-aarch64/+bug/1169164 Ian.
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index 4ce845f..758e41e 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -21,6 +21,40 @@ #include <uapi/asm/ptrace.h> +/* + * PSR bits + */ +#define PSR_MODE_EL0t 0x00000000 +#define PSR_MODE_EL1t 0x00000004 +#define PSR_MODE_EL1h 0x00000005 +#define PSR_MODE_EL2t 0x00000008 +#define PSR_MODE_EL2h 0x00000009 +#define PSR_MODE_EL3t 0x0000000c +#define PSR_MODE_EL3h 0x0000000d +#define PSR_MODE_MASK 0x0000000f + +/* AArch32 CPSR bits */ +#define PSR_MODE32_BIT 0x00000010 + +/* AArch64 SPSR bits */ +#define PSR_F_BIT 0x00000040 +#define PSR_I_BIT 0x00000080 +#define PSR_A_BIT 0x00000100 +#define PSR_D_BIT 0x00000200 +#define PSR_Q_BIT 0x08000000 +#define PSR_V_BIT 0x10000000 +#define PSR_C_BIT 0x20000000 +#define PSR_Z_BIT 0x40000000 +#define PSR_N_BIT 0x80000000 + +/* + * Groups of PSR bits + */ +#define PSR_f 0xff000000 /* Flags */ +#define PSR_s 0x00ff0000 /* Status */ +#define PSR_x 0x0000ff00 /* Extension */ +#define PSR_c 0x000000ff /* Control */ + /* AArch32-specific ptrace requests */ #define COMPAT_PTRACE_GETREGS 12 #define COMPAT_PTRACE_SETREGS 13 diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h index 6913643..fd3ed98 100644 --- a/arch/arm64/include/uapi/asm/ptrace.h +++ b/arch/arm64/include/uapi/asm/ptrace.h @@ -23,42 +23,6 @@ #include <asm/hwcap.h> - -/* - * PSR bits - */ -#define PSR_MODE_EL0t 0x00000000 -#define PSR_MODE_EL1t 0x00000004 -#define PSR_MODE_EL1h 0x00000005 -#define PSR_MODE_EL2t 0x00000008 -#define PSR_MODE_EL2h 0x00000009 -#define PSR_MODE_EL3t 0x0000000c -#define PSR_MODE_EL3h 0x0000000d -#define PSR_MODE_MASK 0x0000000f - -/* AArch32 CPSR bits */ -#define PSR_MODE32_BIT 0x00000010 - -/* AArch64 SPSR bits */ -#define PSR_F_BIT 0x00000040 -#define PSR_I_BIT 0x00000080 -#define PSR_A_BIT 0x00000100 -#define PSR_D_BIT 0x00000200 -#define PSR_Q_BIT 0x08000000 -#define PSR_V_BIT 0x10000000 -#define PSR_C_BIT 0x20000000 -#define PSR_Z_BIT 0x40000000 -#define PSR_N_BIT 0x80000000 - -/* - * Groups of PSR bits - */ -#define PSR_f 0xff000000 /* Flags */ -#define PSR_s 0x00ff0000 /* Status */ -#define PSR_x 0x0000ff00 /* Extension */ -#define PSR_c 0x000000ff /* Control */ - - #ifndef __ASSEMBLY__ /*
Exposing these in ptrace.h ends up polluting the namespace for user applications which include headers such as <signal.h>, e.g. when building Xen userspace tools on arm64: tools/include/xen/arch-arm.h:229:0: error: "PSR_MODE_EL0t" redefined [-Werror] In file included from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/user.h:25:0, from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/procfs.h:34, from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/sys/ucontext.h:26, from /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/signal.h:360, from xentrace.c:21: /usr/lib/gcc-cross/aarch64-linux-gnu/4.7/../../../../aarch64-linux-gnu/include/asm/ptrace.h:30:0: note: this is the location of the previous definition I think these were brought over as part of the uapi transition in error because they were not protected by the __KERNEL__ define. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: David Howells <dhowells@redhat.com> Cc: linux-arm-kernel@lists.infradead.org Cc: xen-devel@lists.xen.org --- arch/arm64/include/asm/ptrace.h | 34 ++++++++++++++++++++++++++++++++ arch/arm64/include/uapi/asm/ptrace.h | 36 ---------------------------------- 2 files changed, 34 insertions(+), 36 deletions(-)