The attached patch seems to already exist in a number of branches -- it
keeps popping up on Google for me, and is certainly already in Debian --
but is strangely absent from mainstream.
The problem appears to be that the patched file ends up as part of the
target toolchain, but unfortunately the gcc constant folding doesn't
appear to eliminate the __invalid_size_argument_for_IOC value early
enough. Certainly compiling C++ programs which use _IO... macros as
constants fails without this patch.
No doubt this has been pushed upstream before: this problem seems to date
from the very early days of 2.6 ... but here it is again. It makes sense
to do it.
commit 0df6f37b4e4534f219b5e40cb49ffd9311eb6195
Author: Michael Abbott <[email protected]>
Date: Mon Jul 28 07:32:05 2008 +0100
Add long established but strangely absent patch to allow ioctl.h to
work smoothly with userspace program optimisations.
diff --git a/include/asm-generic/ioctl.h b/include/asm-generic/ioctl.h
index 8641813..15828b2 100644
--- a/include/asm-generic/ioctl.h
+++ b/include/asm-generic/ioctl.h
@@ -68,12 +68,16 @@
((nr) << _IOC_NRSHIFT) | \
((size) << _IOC_SIZESHIFT))
+#ifdef __KERNEL__
/* provoke compile error for invalid uses of size argument */
extern unsigned int __invalid_size_argument_for_IOC;
#define _IOC_TYPECHECK(t) \
((sizeof(t) == sizeof(t[1]) && \
sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
sizeof(t) : __invalid_size_argument_for_IOC)
+#else
+#define _IOC_TYPECHECK(t) (sizeof(t))
+#endif
/* used to create numbers */
#define _IO(type,nr) _IOC(_IOC_NONE,(type),(nr),0)
On Mon, 28 Jul 2008 06:48:52 +0000 (GMT) Michael Abbott <[email protected]> wrote:
> The attached patch seems to already exist in a number of branches -- it
> keeps popping up on Google for me, and is certainly already in Debian --
> but is strangely absent from mainstream.
>
> The problem appears to be that the patched file ends up as part of the
> target toolchain, but unfortunately the gcc constant folding doesn't
> appear to eliminate the __invalid_size_argument_for_IOC value early
> enough. Certainly compiling C++ programs which use _IO... macros as
> constants fails without this patch.
Could be that `-O0' is associated with the problems.
Plus compilers other than gcc can legitimately use this header.
> No doubt this has been pushed upstream before: this problem seems to date
> from the very early days of 2.6 ... but here it is again. It makes sense
> to do it.
>
>
> commit 0df6f37b4e4534f219b5e40cb49ffd9311eb6195
> Author: Michael Abbott <[email protected]>
> Date: Mon Jul 28 07:32:05 2008 +0100
>
> Add long established but strangely absent patch to allow ioctl.h to
> work smoothly with userspace program optimisations.
>
> diff --git a/include/asm-generic/ioctl.h b/include/asm-generic/ioctl.h
> index 8641813..15828b2 100644
> --- a/include/asm-generic/ioctl.h
> +++ b/include/asm-generic/ioctl.h
> @@ -68,12 +68,16 @@
> ((nr) << _IOC_NRSHIFT) | \
> ((size) << _IOC_SIZESHIFT))
>
> +#ifdef __KERNEL__
> /* provoke compile error for invalid uses of size argument */
> extern unsigned int __invalid_size_argument_for_IOC;
> #define _IOC_TYPECHECK(t) \
> ((sizeof(t) == sizeof(t[1]) && \
> sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
> sizeof(t) : __invalid_size_argument_for_IOC)
> +#else
> +#define _IOC_TYPECHECK(t) (sizeof(t))
> +#endif
>
> /* used to create numbers */
> #define _IO(type,nr) _IOC(_IOC_NONE,(type),(nr),0)
Gee.
But yes, the patch looks reasonable.
We could also replace that open-coded assertion with the shiny new
BUILD_BUG_ON(), which would a) be cleaner and b) fix the problem which
you describe. I expect that would be quite safe, but obviously doesn't
have all the testing which the above patch has, so shrug.
[PATCH] Make _IOC_TYPECHECK use BUILD_BUG_ON_ZERO
This converts _IOC_TYPECHECK from a link error to a compile-time
error using BUILD_BUG_ON_ZERO. This makes it possible to use
the standard _IOC macros in user space even with non-optizing
compilers.
Signed-off-by: Arnd Bergmann <[email protected]>
---
On Wednesday 30 July 2008, Andrew Morton wrote:
> We could also replace that open-coded assertion with the shiny new
> BUILD_BUG_ON(), which would a) be cleaner and b) fix the problem which
> you describe. ?I expect that would be quite safe, but obviously doesn't
> have all the testing which the above patch has, so shrug.
There is one significant difference: using BUILD_BUG_ON_ZERO will
break user space code that uses broken ioctl number definitions
like _IOC('x', 1, sizeof(int)) that were fixed up in the kernel
but not in external copies of the definitions.
I'm undecided whether such breakage would be a good or a bad thing.
--- a/include/asm-generic/ioctl.h
+++ b/include/asm-generic/ioctl.h
@@ -69,11 +69,9 @@
((size) << _IOC_SIZESHIFT))
/* provoke compile error for invalid uses of size argument */
-extern unsigned int __invalid_size_argument_for_IOC;
-#define _IOC_TYPECHECK(t) \
- ((sizeof(t) == sizeof(t[1]) && \
- sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
- sizeof(t) : __invalid_size_argument_for_IOC)
+#define _IOC_TYPECHECK(t) (sizeof(t) + \
+ BUILD_BUG_ON_ZERO(sizeof(t) != sizeof(t[1]) || \
+ sizeof(t) >= (1 << _IOC_SIZEBITS)))
/* used to create numbers */
#define _IO(type,nr) _IOC(_IOC_NONE,(type),(nr),0)