typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com ---
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
Thanks, Kevin
include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x))
-#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
-----Original Message----- From: Kevin Brodsky kevin.brodsky@arm.com Sent: 09 November 2022 10:48 To: linux-morello@op-lists.linaro.org Cc: Kevin Brodsky Kevin.Brodsky@arm.com; Ruben Ayrapetyan Ruben.Ayrapetyan@arm.com Subject: [PATCH] uapi/linux/const.h: Prefer ISO-friendly __typeof__
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
I've tried this patch - the related user-space build issue is fixed. Tested-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
Thanks, Kevin
include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
2.34.1
On 09/11/2022 13:10, Ruben Ayrapetyan wrote:
-----Original Message----- From: Kevin Brodsky kevin.brodsky@arm.com Sent: 09 November 2022 10:48 To: linux-morello@op-lists.linaro.org Cc: Kevin Brodsky Kevin.Brodsky@arm.com; Ruben Ayrapetyan Ruben.Ayrapetyan@arm.com Subject: [PATCH] uapi/linux/const.h: Prefer ISO-friendly __typeof__
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
I've tried this patch - the related user-space build issue is fixed. Tested-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com
Thanks for this. Now applied on next.
Kevin
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
Thanks, Kevin
include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
2.34.1
On Wed, Nov 09, 2022 at 10:47:44AM +0000, Kevin Brodsky wrote:
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including
I'm not entirely sure I understand what you mean by "in a way that is always expanded" .....
siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
The change looks reasonable and as there were already attempts to make the uapi headers unconstrained by gnu* modes [1] it might be a good idea to upstream it
[1] Starting from: https://github.com/torvalds/linux/commit/d6fc9fcbaa655cff2d2be05e16867d1918f...
- Might be worth to double check UAPI_HEADER_TEST and why it is not picking up typeof in either netlink nor netfilter (both are not blacklisted)
--- BR B.
Thanks, Kevin
include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
2.34.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 09/11/2022 21:33, Beata Michalska wrote:
On Wed, Nov 09, 2022 at 10:47:44AM +0000, Kevin Brodsky wrote:
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including
I'm not entirely sure I understand what you mean by "in a way that is always expanded" .....
That's the key difference between this new use of __ALIGN_KERNEL() and the ones in mainline: in siginfo.h, we _actually_ expand the __ALIGN_KERNEL() macro in the header itself, so if the macro makes use of typeof, merely including the header will result in a build failure. Contrast with netfilter, where __ALIGN_KERNEL() is _always_ used to define other macros, which are not used in the headers. To get a build failure, you would therefore have to use the provided macros, not simply include the header. I think that's the reason why no one has noticed so far (these macros are very specific and it's completely possible no one has ever written ISO code using them).
siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
The change looks reasonable and as there were already attempts to make the uapi headers unconstrained by gnu* modes [1] it might be a good idea to upstream it
[1] Starting from: https://github.com/torvalds/linux/commit/d6fc9fcbaa655cff2d2be05e16867d1918f...
- Might be worth to double check UAPI_HEADER_TEST and why it is not picking up
typeof in either netlink nor netfilter (both are not blacklisted)
Thanks for pointing this out, it's good to see there are efforts in that direction! Unfortunately this approach couldn't pick up the uses of typeof in mainline for the reason I mentioned above - simply expanding the headers does not result in typeof appearing, so no error either.
What I'm planning to do then is to merge this patch in next as this is a real issue for us, and then post it on LKML to test the waters. If the patch gets accepted in a modified version, I'll simply replace the one in next during the next rebase (if it hasn't already reached mainline).
Kevin
BR B.
Thanks, Kevin
include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
2.34.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On Mon, Nov 14, 2022 at 11:50:47AM +0000, Kevin Brodsky wrote:
On 09/11/2022 21:33, Beata Michalska wrote:
On Wed, Nov 09, 2022 at 10:47:44AM +0000, Kevin Brodsky wrote:
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including
I'm not entirely sure I understand what you mean by "in a way that is always expanded" .....
That's the key difference between this new use of __ALIGN_KERNEL() and the ones in mainline: in siginfo.h, we _actually_ expand the __ALIGN_KERNEL() macro in the header itself, so if the macro makes use of typeof, merely including the header will result in a build failure. Contrast with netfilter, where __ALIGN_KERNEL() is _always_ used to define other macros, which are not used in the headers. To get a build failure, you would therefore have to use the provided macros, not simply include the header. I think that's the reason why no one has noticed so far (these macros are very specific and it's completely possible no one has ever written ISO code using them).
Right, I have completely missed the fact that netfiler/netlink macros are not being used in the uapi headers.
siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
The change looks reasonable and as there were already attempts to make the uapi headers unconstrained by gnu* modes [1] it might be a good idea to upstream it
[1] Starting from: https://github.com/torvalds/linux/commit/d6fc9fcbaa655cff2d2be05e16867d1918f...
- Might be worth to double check UAPI_HEADER_TEST and why it is not picking up
typeof in either netlink nor netfilter (both are not blacklisted)
Thanks for pointing this out, it's good to see there are efforts in that direction! Unfortunately this approach couldn't pick up the uses of typeof in mainline for the reason I mentioned above - simply expanding the headers does not result in typeof appearing, so no error either.
What I'm planning to do then is to merge this patch in next as this is a real issue for us, and then post it on LKML to test the waters. If the patch gets accepted in a modified version, I'll simply replace the one in next during the next rebase (if it hasn't already reached mainline).
Sounds like a plan.
--- BR B.
Kevin
BR B.
Thanks, Kevin
include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
2.34.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 09/11/2022 10:47, Kevin Brodsky wrote:
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
It's a bit late now but FWIW the patch looks good to me too and should be easy to upstream (you may be asked to fix netfilter as well).
Thanks, Kristina
On 17/11/2022 18:14, Kristina Martsenko wrote:
On 09/11/2022 10:47, Kevin Brodsky wrote:
typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__.
Reported-by: Ruben Ayrapetyan ruben.ayrapetyan@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including siginfo.h gets a build failure.
Thinking about posting this on LKML, any opinion for/against or suggestion?
It's a bit late now but FWIW the patch looks good to me too and should be easy to upstream (you may be asked to fix netfilter as well).
Thanks, Kristina
Haven't got to the upstreaming part yet so not too late, thanks for the feedback! I do suspect I might be asked to fix the other occurrences too, we'll see what happens.
Kevin
linux-morello@op-lists.linaro.org
-
Beata Michalska -
Kevin Brodsky -
Kristina Martsenko -
Ruben Ayrapetyan