2017-03-01 16:27:27

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] uapi: fix asm/signal.h userspace compilation errors

On Sun, Feb 26, 2017 at 2:01 AM, Dmitry V. Levin <[email protected]> wrote:
> Include <stddef.h> (guarded by #ifndef __KERNEL__) to fix asm/signal.h
> userspace compilation errors like this:
>
> /usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
> size_t ss_size;
>
> As no uapi header provides a definition of size_t, inclusion
> of <stddef.h> seems to be the most conservative fix available.
>
> On the kernel side size_t is typedef'ed to __kernel_size_t, so
> an alternative fix would be to change the type of sigaltstack.ss_size
> from size_t to __kernel_size_t for all architectures except those where
> sizeof(size_t) < sizeof(__kernel_size_t), namely, x32 and mips n32.
>
> On x32 and mips n32, however, #include <stddef.h> seems to be the most
> straightforward way to obtain the definition for sigaltstack.ss_size's
> type.
>
> Signed-off-by: Dmitry V. Levin <[email protected]>

I'm not sure if this is the best fix. We generally should not include one
standard header from another standard header. Would it be possible
to use __kernel_size_t instead of size_t?

Arnd


2017-03-02 00:19:03

by Dmitry V. Levin

[permalink] [raw]
Subject: [PATCH v2] x86/uapi: fix asm/signal.h userspace compilation error

Replace size_t to fix the following asm/signal.h userspace compilation
error:

/usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
size_t ss_size;

size_t is replaced with __kernel_size_t in all cases except x32 where
unsigned int has to be used instead.

Signed-off-by: Dmitry V. Levin <[email protected]>
---
v2: create a separate patch for x86,
replace size_t instead of including <stddef.h>.

arch/x86/include/uapi/asm/signal.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/signal.h b/arch/x86/include/uapi/asm/signal.h
index 8264f47..f80473f 100644
--- a/arch/x86/include/uapi/asm/signal.h
+++ b/arch/x86/include/uapi/asm/signal.h
@@ -127,7 +127,11 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+#if defined(__x86_64__) && defined(__ILP32__)
+ unsigned int ss_size;
+#else
+ __kernel_size_t ss_size;
+#endif
} stack_t;

#endif /* __ASSEMBLY__ */
--
ldv

2017-03-02 00:20:30

by Dmitry V. Levin

[permalink] [raw]
Subject: [PATCH v2] uapi: fix asm/signal.h userspace compilation errors

Replace size_t with __kernel_size_t to fix asm/signal.h userspace
compilation errors like this:

/usr/include/asm-generic/signal.h:116:2: error: unknown type name 'size_t'
size_t ss_size;

This change is not applicable to x86 port because x32 is the only
architecture where sizeof(size_t) < sizeof(__kernel_size_t).

Signed-off-by: Dmitry V. Levin <[email protected]>
---
v2: create a separate patch for x86,
replace size_t with __kernel_size_t instead of including <stddef.h>.

include/uapi/asm-generic/signal.h | 2 +-
arch/alpha/include/uapi/asm/signal.h | 2 +-
arch/arm/include/uapi/asm/signal.h | 2 +-
arch/avr32/include/uapi/asm/signal.h | 2 +-
arch/cris/include/uapi/asm/signal.h | 2 +-
arch/h8300/include/uapi/asm/signal.h | 2 +-
arch/ia64/include/uapi/asm/signal.h | 2 +-
arch/m32r/include/uapi/asm/signal.h | 2 +-
arch/m68k/include/uapi/asm/signal.h | 2 +-
arch/mips/include/uapi/asm/signal.h | 2 +-
arch/mn10300/include/uapi/asm/signal.h | 2 +-
arch/parisc/include/uapi/asm/signal.h | 2 +-
arch/powerpc/include/uapi/asm/signal.h | 2 +-
arch/s390/include/uapi/asm/signal.h | 2 +-
arch/sparc/include/uapi/asm/signal.h | 2 +-
arch/xtensa/include/uapi/asm/signal.h | 2 +-
16 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/uapi/asm-generic/signal.h b/include/uapi/asm-generic/signal.h
index 3094618..6bbcdfa 100644
--- a/include/uapi/asm-generic/signal.h
+++ b/include/uapi/asm-generic/signal.h
@@ -113,7 +113,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* __ASSEMBLY__ */
diff --git a/arch/alpha/include/uapi/asm/signal.h b/arch/alpha/include/uapi/asm/signal.h
index dd4ca4bc..16a2217 100644
--- a/arch/alpha/include/uapi/asm/signal.h
+++ b/arch/alpha/include/uapi/asm/signal.h
@@ -113,7 +113,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

/* sigstack(2) is deprecated, and will be withdrawn in a future version
diff --git a/arch/arm/include/uapi/asm/signal.h b/arch/arm/include/uapi/asm/signal.h
index 33073bd..859f2de 100644
--- a/arch/arm/include/uapi/asm/signal.h
+++ b/arch/arm/include/uapi/asm/signal.h
@@ -113,7 +113,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/avr32/include/uapi/asm/signal.h b/arch/avr32/include/uapi/asm/signal.h
index ffe8c77..46af348 100644
--- a/arch/avr32/include/uapi/asm/signal.h
+++ b/arch/avr32/include/uapi/asm/signal.h
@@ -115,7 +115,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* _UAPI__ASM_AVR32_SIGNAL_H */
diff --git a/arch/cris/include/uapi/asm/signal.h b/arch/cris/include/uapi/asm/signal.h
index ce42fa7..02149d2 100644
--- a/arch/cris/include/uapi/asm/signal.h
+++ b/arch/cris/include/uapi/asm/signal.h
@@ -109,7 +109,7 @@ struct sigaction {
typedef struct sigaltstack {
void *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/h8300/include/uapi/asm/signal.h b/arch/h8300/include/uapi/asm/signal.h
index af3a6c3..0b1825d 100644
--- a/arch/h8300/include/uapi/asm/signal.h
+++ b/arch/h8300/include/uapi/asm/signal.h
@@ -108,7 +108,7 @@ struct sigaction {
typedef struct sigaltstack {
void *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/ia64/include/uapi/asm/signal.h b/arch/ia64/include/uapi/asm/signal.h
index c0ea285..04604da 100644
--- a/arch/ia64/include/uapi/asm/signal.h
+++ b/arch/ia64/include/uapi/asm/signal.h
@@ -113,7 +113,7 @@ struct siginfo;
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/m32r/include/uapi/asm/signal.h b/arch/m32r/include/uapi/asm/signal.h
index 54acacb..a7f5c0b 100644
--- a/arch/m32r/include/uapi/asm/signal.h
+++ b/arch/m32r/include/uapi/asm/signal.h
@@ -110,7 +110,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/m68k/include/uapi/asm/signal.h b/arch/m68k/include/uapi/asm/signal.h
index cba6f85..387fddc 100644
--- a/arch/m68k/include/uapi/asm/signal.h
+++ b/arch/m68k/include/uapi/asm/signal.h
@@ -106,7 +106,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* _UAPI_M68K_SIGNAL_H */
diff --git a/arch/mips/include/uapi/asm/signal.h b/arch/mips/include/uapi/asm/signal.h
index addb9f5..6ec6885 100644
--- a/arch/mips/include/uapi/asm/signal.h
+++ b/arch/mips/include/uapi/asm/signal.h
@@ -111,7 +111,7 @@ struct sigaction {
/* IRIX compatible stack_t */
typedef struct sigaltstack {
void __user *ss_sp;
- size_t ss_size;
+ __kernel_size_t ss_size;
int ss_flags;
} stack_t;

diff --git a/arch/mn10300/include/uapi/asm/signal.h b/arch/mn10300/include/uapi/asm/signal.h
index f423a08..c17d363 100644
--- a/arch/mn10300/include/uapi/asm/signal.h
+++ b/arch/mn10300/include/uapi/asm/signal.h
@@ -118,7 +118,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/parisc/include/uapi/asm/signal.h b/arch/parisc/include/uapi/asm/signal.h
index e26043b..403e2d8 100644
--- a/arch/parisc/include/uapi/asm/signal.h
+++ b/arch/parisc/include/uapi/asm/signal.h
@@ -99,7 +99,7 @@ typedef __signalfn_t __user *__sighandler_t;
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* !__ASSEMBLY */
diff --git a/arch/powerpc/include/uapi/asm/signal.h b/arch/powerpc/include/uapi/asm/signal.h
index 6c69ee9..c1c2a0b 100644
--- a/arch/powerpc/include/uapi/asm/signal.h
+++ b/arch/powerpc/include/uapi/asm/signal.h
@@ -109,7 +109,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/s390/include/uapi/asm/signal.h b/arch/s390/include/uapi/asm/signal.h
index 2f43cfb..b8c2db6 100644
--- a/arch/s390/include/uapi/asm/signal.h
+++ b/arch/s390/include/uapi/asm/signal.h
@@ -122,7 +122,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/sparc/include/uapi/asm/signal.h b/arch/sparc/include/uapi/asm/signal.h
index f387400..cd6c036 100644
--- a/arch/sparc/include/uapi/asm/signal.h
+++ b/arch/sparc/include/uapi/asm/signal.h
@@ -172,7 +172,7 @@ struct __old_sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/xtensa/include/uapi/asm/signal.h b/arch/xtensa/include/uapi/asm/signal.h
index 586756e..d835627 100644
--- a/arch/xtensa/include/uapi/asm/signal.h
+++ b/arch/xtensa/include/uapi/asm/signal.h
@@ -126,7 +126,7 @@ struct sigaction {
typedef struct sigaltstack {
void *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* __ASSEMBLY__ */
--
ldv

2017-03-02 00:27:45

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2] x86/uapi: fix asm/signal.h userspace compilation error

On March 1, 2017 4:18:54 PM PST, "Dmitry V. Levin" <[email protected]> wrote:
>Replace size_t to fix the following asm/signal.h userspace compilation
>error:
>
>/usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
> size_t ss_size;
>
>size_t is replaced with __kernel_size_t in all cases except x32 where
>unsigned int has to be used instead.
>
>Signed-off-by: Dmitry V. Levin <[email protected]>
>---
>v2: create a separate patch for x86,
> replace size_t instead of including <stddef.h>.
>
> arch/x86/include/uapi/asm/signal.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/include/uapi/asm/signal.h
>b/arch/x86/include/uapi/asm/signal.h
>index 8264f47..f80473f 100644
>--- a/arch/x86/include/uapi/asm/signal.h
>+++ b/arch/x86/include/uapi/asm/signal.h
>@@ -127,7 +127,11 @@ struct sigaction {
> typedef struct sigaltstack {
> void __user *ss_sp;
> int ss_flags;
>- size_t ss_size;
>+#if defined(__x86_64__) && defined(__ILP32__)
>+ unsigned int ss_size;
>+#else
>+ __kernel_size_t ss_size;
>+#endif
> } stack_t;
>
> #endif /* __ASSEMBLY__ */

Sounds like we still ought to make this a type by itself.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2017-03-02 01:28:28

by Dmitry V. Levin

[permalink] [raw]
Subject: Re: [PATCH v2] x86/uapi: fix asm/signal.h userspace compilation error

On Wed, Mar 01, 2017 at 04:26:29PM -0800, [email protected] wrote:
> On March 1, 2017 4:18:54 PM PST, "Dmitry V. Levin" <[email protected]> wrote:
> >Replace size_t to fix the following asm/signal.h userspace compilation
> >error:
> >
> >/usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
> > size_t ss_size;
> >
> >size_t is replaced with __kernel_size_t in all cases except x32 where
> >unsigned int has to be used instead.
> >
> >Signed-off-by: Dmitry V. Levin <[email protected]>
> >---
> >v2: create a separate patch for x86,
> > replace size_t instead of including <stddef.h>.
> >
> > arch/x86/include/uapi/asm/signal.h | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/arch/x86/include/uapi/asm/signal.h
> >b/arch/x86/include/uapi/asm/signal.h
> >index 8264f47..f80473f 100644
> >--- a/arch/x86/include/uapi/asm/signal.h
> >+++ b/arch/x86/include/uapi/asm/signal.h
> >@@ -127,7 +127,11 @@ struct sigaction {
> > typedef struct sigaltstack {
> > void __user *ss_sp;
> > int ss_flags;
> >- size_t ss_size;
> >+#if defined(__x86_64__) && defined(__ILP32__)
> >+ unsigned int ss_size;
> >+#else
> >+ __kernel_size_t ss_size;
> >+#endif
> > } stack_t;
> >
> > #endif /* __ASSEMBLY__ */
>
> Sounds like we still ought to make this a type by itself.

A new type would need a name, like __kernel_uapi_size_t.
Do you have any preferences?


--
ldv

2017-03-02 15:29:49

by Carlos O'Donell

[permalink] [raw]
Subject: Re: [PATCH] uapi: fix asm/signal.h userspace compilation errors

On Wed, Mar 1, 2017 at 11:20 AM, Arnd Bergmann <[email protected]> wrote:
> On Sun, Feb 26, 2017 at 2:01 AM, Dmitry V. Levin <[email protected]> wrote:
>> Include <stddef.h> (guarded by #ifndef __KERNEL__) to fix asm/signal.h
>> userspace compilation errors like this:
>>
>> /usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
>> size_t ss_size;
>>
>> As no uapi header provides a definition of size_t, inclusion
>> of <stddef.h> seems to be the most conservative fix available.
>>
>> On the kernel side size_t is typedef'ed to __kernel_size_t, so
>> an alternative fix would be to change the type of sigaltstack.ss_size
>> from size_t to __kernel_size_t for all architectures except those where
>> sizeof(size_t) < sizeof(__kernel_size_t), namely, x32 and mips n32.
>>
>> On x32 and mips n32, however, #include <stddef.h> seems to be the most
>> straightforward way to obtain the definition for sigaltstack.ss_size's
>> type.
>>
>> Signed-off-by: Dmitry V. Levin <[email protected]>
>
> I'm not sure if this is the best fix. We generally should not include one
> standard header from another standard header. Would it be possible
> to use __kernel_size_t instead of size_t?

In glibc we handle this with special use of __need_size_t with GCC's
provided stddef.h.

For example glibc's signal.h does this:

# define __need_size_t
# include <stddef.h>

And...

/* Any one of these symbols __need_* means that GNU libc
wants us just to define one data type. So don't define
the symbols that indicate this file's entire job has been done. */
#if (!defined(__need_wchar_t) && !defined(__need_size_t) \
&& !defined(__need_ptrdiff_t) && !defined(__need_NULL) \
&& !defined(__need_wint_t))

The idea being that the type you want is really defined by stddef.h,
but you just one the one type.

Changing the fundamental type causes the issues you see in patch v2
where sizeof(size_t) < sizeof(__kernel_size_t).

It will only lead to problem substituting the wrong type.

Cheers,
Carlos.

2017-03-02 16:00:19

by Dmitry V. Levin

[permalink] [raw]
Subject: Re: [PATCH] uapi: fix asm/signal.h userspace compilation errors

On Thu, Mar 02, 2017 at 10:22:18AM -0500, Carlos O'Donell wrote:
> On Wed, Mar 1, 2017 at 11:20 AM, Arnd Bergmann <[email protected]> wrote:
> > On Sun, Feb 26, 2017 at 2:01 AM, Dmitry V. Levin <[email protected]> wrote:
> >> Include <stddef.h> (guarded by #ifndef __KERNEL__) to fix asm/signal.h
> >> userspace compilation errors like this:
> >>
> >> /usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
> >> size_t ss_size;
> >>
> >> As no uapi header provides a definition of size_t, inclusion
> >> of <stddef.h> seems to be the most conservative fix available.
[...]
> > I'm not sure if this is the best fix. We generally should not include one
> > standard header from another standard header. Would it be possible
> > to use __kernel_size_t instead of size_t?
>
> In glibc we handle this with special use of __need_size_t with GCC's
> provided stddef.h.
>
> For example glibc's signal.h does this:
>
> # define __need_size_t
> # include <stddef.h>

Just to make it clear, do you suggest this approach for asm/signal.h as well?

[...]
> Changing the fundamental type causes the issues you see in patch v2
> where sizeof(size_t) < sizeof(__kernel_size_t).
>
> It will only lead to problem substituting the wrong type.

I don't see any appetite for creating more ABIs like x32 with
sizeof(size_t) < sizeof(__kernel_size_t), so v2 approach
is not going to be any different from v1 in maintenance.


--
ldv


Attachments:
(No filename) (1.39 kB)
(No filename) (801.00 B)
Download all attachments

2017-03-03 01:48:25

by Dmitry V. Levin

[permalink] [raw]
Subject: [PATCH v3 2/2] x86/uapi: fix asm/signal.h userspace compilation error

Replace size_t with __kernel_uapi_size_t to fix the following
asm/signal.h userspace compilation error:

/usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
size_t ss_size;

Signed-off-by: Dmitry V. Levin <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
---
arch/x86/include/uapi/asm/signal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/signal.h b/arch/x86/include/uapi/asm/signal.h
index 8264f47..c8b2c95 100644
--- a/arch/x86/include/uapi/asm/signal.h
+++ b/arch/x86/include/uapi/asm/signal.h
@@ -127,7 +127,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_uapi_size_t ss_size;
} stack_t;

#endif /* __ASSEMBLY__ */
--
ldv

2017-03-03 01:49:16

by Dmitry V. Levin

[permalink] [raw]
Subject: [PATCH 1/2] uapi: introduce __kernel_uapi_size_t

__kernel_uapi_size_t is a pure UAPI type, in defined(__KERNEL__) code
it's always the same as __kernel_size_t.

It's also the same as __kernel_size_t on all architectures except x32
where sizeof(size_t) < sizeof(__kernel_size_t).

__kernel_uapi_size_t can be used as a size_t replacement in UAPI
headers, e.g. in cases when size_t might differ from __kernel_size_t.

Signed-off-by: Dmitry V. Levin <[email protected]>
---
include/uapi/asm-generic/posix_types.h | 10 ++++++++++
arch/x86/include/uapi/asm/posix_types_x32.h | 3 +++
2 files changed, 13 insertions(+)

diff --git a/include/uapi/asm-generic/posix_types.h b/include/uapi/asm-generic/posix_types.h
index fe74fcc..e93c324 100644
--- a/include/uapi/asm-generic/posix_types.h
+++ b/include/uapi/asm-generic/posix_types.h
@@ -74,6 +74,16 @@ typedef __kernel_long_t __kernel_ptrdiff_t;
#endif
#endif

+/*
+ * __kernel_uapi_size_t is a pure UAPI type, in defined(__KERNEL__) code
+ * it's always the same as __kernel_size_t.
+ * __kernel_uapi_size_t can be used as a size_t replacement in UAPI headers,
+ * e.g. in cases when size_t might differ from __kernel_size_t.
+ */
+#ifndef __kernel_uapi_size_t
+typedef __kernel_size_t __kernel_uapi_size_t;
+#endif
+
#ifndef __kernel_fsid_t
typedef struct {
int val[2];
diff --git a/arch/x86/include/uapi/asm/posix_types_x32.h b/arch/x86/include/uapi/asm/posix_types_x32.h
index 85f9bda..0e36e67 100644
--- a/arch/x86/include/uapi/asm/posix_types_x32.h
+++ b/arch/x86/include/uapi/asm/posix_types_x32.h
@@ -14,6 +14,9 @@ typedef long long __kernel_long_t;
typedef unsigned long long __kernel_ulong_t;
#define __kernel_long_t __kernel_long_t

+typedef unsigned int __kernel_uapi_size_t;
+#define __kernel_uapi_size_t __kernel_uapi_size_t
+
#include <asm/posix_types_64.h>

#endif /* _ASM_X86_POSIX_TYPES_X32_H */
--
ldv

2017-03-04 01:24:25

by Carlos O'Donell

[permalink] [raw]
Subject: Re: [PATCH] uapi: fix asm/signal.h userspace compilation errors

On Thu, Mar 2, 2017 at 10:48 AM, Dmitry V. Levin <[email protected]> wrote:
> On Thu, Mar 02, 2017 at 10:22:18AM -0500, Carlos O'Donell wrote:
>> On Wed, Mar 1, 2017 at 11:20 AM, Arnd Bergmann <[email protected]> wrote:
>> > On Sun, Feb 26, 2017 at 2:01 AM, Dmitry V. Levin <[email protected]> wrote:
>> >> Include <stddef.h> (guarded by #ifndef __KERNEL__) to fix asm/signal.h
>> >> userspace compilation errors like this:
>> >>
>> >> /usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
>> >> size_t ss_size;
>> >>
>> >> As no uapi header provides a definition of size_t, inclusion
>> >> of <stddef.h> seems to be the most conservative fix available.
> [...]
>> > I'm not sure if this is the best fix. We generally should not include one
>> > standard header from another standard header. Would it be possible
>> > to use __kernel_size_t instead of size_t?
>>
>> In glibc we handle this with special use of __need_size_t with GCC's
>> provided stddef.h.
>>
>> For example glibc's signal.h does this:
>>
>> # define __need_size_t
>> # include <stddef.h>
>
> Just to make it clear, do you suggest this approach for asm/signal.h as well?

The kernel is duplicating userspace headers in the UAPI implementation
and running into exactly the same problems we have already solved in
userspace. We currently have no better solution than the "__need_*"
interface for avoiding the duplication of the type definitions and the
problems that come with that.

I am taking this up with senior glibc developers on libc-alpha to see
if we have a better suggestion. If you give me 72 hours I'll either
have a better suggestion or the acknowledgement that my suggestion is
the best practical solution we have.

Note that in a GNU userspace stddef.h here comes from gcc, which
completes the implementation of the C development environment that
provides the types you need. The use of "__need_size_t" is a collusion
between the components of the implementation and use by the Linux
kernel would mean expecting something specific to a GNU
implementation.

I might suggest you use include/uapi/linux/libc-compat.h in an attempt
to abstract away the GNU-specific magic for getting just size_t from
stddef.h. That way you have documented the places that other runtime
authors need to fill out for things to work.

Cheers,
Carlos.

2017-03-06 15:10:33

by Carlos O'Donell

[permalink] [raw]
Subject: Re: [PATCH] uapi: fix asm/signal.h userspace compilation errors

On Fri, Mar 3, 2017 at 8:23 PM, Carlos O'Donell <[email protected]> wrote:
> On Thu, Mar 2, 2017 at 10:48 AM, Dmitry V. Levin <[email protected]> wrote:
>> On Thu, Mar 02, 2017 at 10:22:18AM -0500, Carlos O'Donell wrote:
>>> On Wed, Mar 1, 2017 at 11:20 AM, Arnd Bergmann <[email protected]> wrote:
>>> > On Sun, Feb 26, 2017 at 2:01 AM, Dmitry V. Levin <[email protected]> wrote:
>>> >> Include <stddef.h> (guarded by #ifndef __KERNEL__) to fix asm/signal.h
>>> >> userspace compilation errors like this:
>>> >>
>>> >> /usr/include/asm/signal.h:126:2: error: unknown type name 'size_t'
>>> >> size_t ss_size;
>>> >>
>>> >> As no uapi header provides a definition of size_t, inclusion
>>> >> of <stddef.h> seems to be the most conservative fix available.
>> [...]
>>> > I'm not sure if this is the best fix. We generally should not include one
>>> > standard header from another standard header. Would it be possible
>>> > to use __kernel_size_t instead of size_t?
>>>
>>> In glibc we handle this with special use of __need_size_t with GCC's
>>> provided stddef.h.
>>>
>>> For example glibc's signal.h does this:
>>>
>>> # define __need_size_t
>>> # include <stddef.h>
>>
>> Just to make it clear, do you suggest this approach for asm/signal.h as well?

The best practice from the glibc community looks like this:

(a) Create a bits/types/*.h header for the type you need.

e.g.
./time/bits/types/struct_timeval.h
./time/bits/types/struct_itimerspec.h
./time/bits/types/time_t.h
./time/bits/types/struct_timespec.h
./time/bits/types/struct_tm.h
./time/bits/types/clockid_t.h
./time/bits/types/clock_t.h
./time/bits/types/timer_t.h

(b) If neccessary the bits/types/*.h header is a wrapper:
~~~
#define __need_size_t
#include <stddef.h>
~~~
to get access to the compiler provided type.

This way all of the code you need simplifies to includes for the types you need.

e.g.

time/sys/time.h:
...
#include <bits/types.h>
#include <bits/types/time_t.h>
#include <bits/types/struct_timeval.h>
...

This is what we've been doing in glibc starting last September as we
cleaned up all the convoluted conditional logic to get the types we
needed in the headers that needed them.

Cheers,
Carlos.

2021-12-28 15:54:34

by Dmitry V. Levin

[permalink] [raw]
Subject: [PATCH v3] uapi: fix asm/signal.h userspace compilation errors

Userspace cannot compile <asm/signal.h> due to missing type definitions.
For example, compiling it for x86_64 fails with the following diagnostics
when the exception for asm/signal.h is removed from usr/include/Makefile:

$ make usr CONFIG_HEADERS_INSTALL=y CONFIG_UAPI_HEADER_TEST=y
...
HDRTEST usr/include/asm/signal.h
In file included from <command-line>:
./usr/include/asm/signal.h:103:9: error: unknown type name 'size_t'
103 | size_t ss_size;
| ^~~~~~

Replace size_t with __kernel_size_t to make asm/signal.h self-contained,
also add it to the compile-test coverage.

Signed-off-by: Dmitry V. Levin <[email protected]>
---

This was submitted almost 5 years ago, so I was under impression that
it was applied among others of this kind, but, apparently, it's still
relevant.

v3: update commit message, remove usr/include/Makefile exception for
asm/signal.h.

arch/alpha/include/uapi/asm/signal.h | 2 +-
arch/arm/include/uapi/asm/signal.h | 2 +-
arch/h8300/include/uapi/asm/signal.h | 2 +-
arch/ia64/include/uapi/asm/signal.h | 2 +-
arch/m68k/include/uapi/asm/signal.h | 2 +-
arch/mips/include/uapi/asm/signal.h | 2 +-
arch/parisc/include/uapi/asm/signal.h | 2 +-
arch/powerpc/include/uapi/asm/signal.h | 2 +-
arch/s390/include/uapi/asm/signal.h | 2 +-
arch/sparc/include/uapi/asm/signal.h | 2 +-
arch/x86/include/uapi/asm/signal.h | 2 +-
arch/xtensa/include/uapi/asm/signal.h | 2 +-
include/uapi/asm-generic/signal.h | 2 +-
usr/include/Makefile | 1 -
14 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/signal.h b/arch/alpha/include/uapi/asm/signal.h
index a69dd8d080a8..1413075f7616 100644
--- a/arch/alpha/include/uapi/asm/signal.h
+++ b/arch/alpha/include/uapi/asm/signal.h
@@ -100,7 +100,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

/* sigstack(2) is deprecated, and will be withdrawn in a future version
diff --git a/arch/arm/include/uapi/asm/signal.h b/arch/arm/include/uapi/asm/signal.h
index c9a3ea1d8d41..9e2178420db2 100644
--- a/arch/arm/include/uapi/asm/signal.h
+++ b/arch/arm/include/uapi/asm/signal.h
@@ -93,7 +93,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/h8300/include/uapi/asm/signal.h b/arch/h8300/include/uapi/asm/signal.h
index 2cd0dce2b6a6..1165481f80f6 100644
--- a/arch/h8300/include/uapi/asm/signal.h
+++ b/arch/h8300/include/uapi/asm/signal.h
@@ -85,7 +85,7 @@ struct sigaction {
typedef struct sigaltstack {
void *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/ia64/include/uapi/asm/signal.h b/arch/ia64/include/uapi/asm/signal.h
index 38166a88e4c9..63d574e802a2 100644
--- a/arch/ia64/include/uapi/asm/signal.h
+++ b/arch/ia64/include/uapi/asm/signal.h
@@ -90,7 +90,7 @@ struct siginfo;
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/m68k/include/uapi/asm/signal.h b/arch/m68k/include/uapi/asm/signal.h
index 4619291df601..80f520b9b10b 100644
--- a/arch/m68k/include/uapi/asm/signal.h
+++ b/arch/m68k/include/uapi/asm/signal.h
@@ -83,7 +83,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* _UAPI_M68K_SIGNAL_H */
diff --git a/arch/mips/include/uapi/asm/signal.h b/arch/mips/include/uapi/asm/signal.h
index e6c78a15cb2f..94a00f82e373 100644
--- a/arch/mips/include/uapi/asm/signal.h
+++ b/arch/mips/include/uapi/asm/signal.h
@@ -100,7 +100,7 @@ struct sigaction {
/* IRIX compatible stack_t */
typedef struct sigaltstack {
void __user *ss_sp;
- size_t ss_size;
+ __kernel_size_t ss_size;
int ss_flags;
} stack_t;

diff --git a/arch/parisc/include/uapi/asm/signal.h b/arch/parisc/include/uapi/asm/signal.h
index e5a2657477ac..8e4895c5ea5d 100644
--- a/arch/parisc/include/uapi/asm/signal.h
+++ b/arch/parisc/include/uapi/asm/signal.h
@@ -67,7 +67,7 @@ struct siginfo;
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* !__ASSEMBLY */
diff --git a/arch/powerpc/include/uapi/asm/signal.h b/arch/powerpc/include/uapi/asm/signal.h
index 04873dd311c2..37d41d87c45b 100644
--- a/arch/powerpc/include/uapi/asm/signal.h
+++ b/arch/powerpc/include/uapi/asm/signal.h
@@ -86,7 +86,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/s390/include/uapi/asm/signal.h b/arch/s390/include/uapi/asm/signal.h
index 0189f326aac5..5c776e1f2cbd 100644
--- a/arch/s390/include/uapi/asm/signal.h
+++ b/arch/s390/include/uapi/asm/signal.h
@@ -108,7 +108,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/sparc/include/uapi/asm/signal.h b/arch/sparc/include/uapi/asm/signal.h
index 53758d53ac0e..d3faa3bfce3d 100644
--- a/arch/sparc/include/uapi/asm/signal.h
+++ b/arch/sparc/include/uapi/asm/signal.h
@@ -171,7 +171,7 @@ struct __old_sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;


diff --git a/arch/x86/include/uapi/asm/signal.h b/arch/x86/include/uapi/asm/signal.h
index 164a22a72984..777c3a0f4e23 100644
--- a/arch/x86/include/uapi/asm/signal.h
+++ b/arch/x86/include/uapi/asm/signal.h
@@ -104,7 +104,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* __ASSEMBLY__ */
diff --git a/arch/xtensa/include/uapi/asm/signal.h b/arch/xtensa/include/uapi/asm/signal.h
index 79ddabaa4e5d..b8c824dd4b74 100644
--- a/arch/xtensa/include/uapi/asm/signal.h
+++ b/arch/xtensa/include/uapi/asm/signal.h
@@ -103,7 +103,7 @@ struct sigaction {
typedef struct sigaltstack {
void *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* __ASSEMBLY__ */
diff --git a/include/uapi/asm-generic/signal.h b/include/uapi/asm-generic/signal.h
index f634822906e4..0eb69dc8e572 100644
--- a/include/uapi/asm-generic/signal.h
+++ b/include/uapi/asm-generic/signal.h
@@ -85,7 +85,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;

#endif /* __ASSEMBLY__ */
diff --git a/usr/include/Makefile b/usr/include/Makefile
index 129d13e71691..8ac2c33a59e7 100644
--- a/usr/include/Makefile
+++ b/usr/include/Makefile
@@ -20,7 +20,6 @@ override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include
# Please consider to fix the header first.
#
# Sorted alphabetically.
-no-header-test += asm/signal.h
no-header-test += asm/ucontext.h
no-header-test += drm/vmwgfx_drm.h
no-header-test += linux/am437x-vpfe.h


--
ldv