lkml.org 
[lkml]   [2017]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] kernel.h: handle pointers to arrays better in container_of()
Date
On Mon, May 22 2017, Ian Abbott wrote:
> If the first parameter of container_of() is a pointer to a
> non-const-qualified array type (and the third parameter names a
> non-const-qualified array member), the local variable __mptr will be
> defined with a const-qualified array type. In ISO C, these types are
> incompatible. They work as expected in GNU C, but some versions will
> issue warnings. For example, GCC 4.9 produces the warning
> "initialization from incompatible pointer type".
>
> Here is an example of where the problem occurs:
>
> -------------------------------------------------------
> #include <linux/kernel.h>
> #include <linux/module.h>
>
> MODULE_LICENSE("GPL");
>
> struct st {
> int a;
> char b[16];
> };
>
> static int __init example_init(void) {
> struct st t = { .a = 101, .b = "hello" };
> char (*p)[16] = &t.b;
> struct st *x = container_of(p, struct st, b);
> printk(KERN_DEBUG "%p %p\n", (void *)&t, (void *)x);
> return 0;
> }
>
> static void __exit example_exit(void) {
> }
>
> module_init(example_init);
> module_exit(example_exit);
> -------------------------------------------------------
>
> Building the module with gcc-4.9 results in these warnings (where '{m}'
> is the module source and '{k}' is the kernel source):
>
> -------------------------------------------------------
> In file included from {m}/example.c:1:0:
> {m}/example.c: In function ‘example_init’:
> {k}/include/linux/kernel.h:854:48: warning: initialization from
> incompatible pointer type
> const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> ^
> {m}/example.c:14:17: note: in expansion of macro ‘container_of’
> struct st *x = container_of(p, struct st, b);
> ^
> {k}/include/linux/kernel.h:854:48: warning: (near initialization for
> ‘x’)
> const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> ^
> {m}/example.c:14:17: note: in expansion of macro ‘container_of’
> struct st *x = container_of(p, struct st, b);
> ^
> -------------------------------------------------------
>
> Fix it by avoiding defining the __mptr variable. This also avoids other
> GCC extensions.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Alexander Potapenko <glider@google.com>
> ---
> v2: Rebased and altered description to provide an example of when the
> compiler warnings occur. v1 (from 2016-10-10) also modified a
> 'container_of_safe()' macro that never made it out of "linux-next".
> ---
> include/linux/kernel.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 13bc08aba704..169fe6f51b7b 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -850,9 +850,8 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
> * @member: the name of the member within the struct.
> *
> */
> -#define container_of(ptr, type, member) ({ \
> - const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> - (type *)( (char *)__mptr - offsetof(type,member) );})
> +#define container_of(ptr, type, member) \
> + ((type *)((char *)(ptr) - offsetof(type, member)))

Now the type of ptr is not checked though. Using your example, I can
now write:

struct st t = { .a = 101, .b = "hello" };
int *p = &t.a;
struct st *x = container_of(p, struct st, b);

and it will compile with no warnings. Previously it would fail. The
best I can think of would be (not tested):

#define container_of(ptr, type, member) ( \
_Static_assert(__builtin_types_compatible_p( \
typeof(ptr), typeof( ((type *)0)->member )*), "WUT"), \
((type *)((char *)(ptr) - offsetof(type, member))); \
)

or maybe:

#define container_of(ptr, type, member) ( \
_Static_assert(__builtin_types_compatible_p( \
typeof(*ptr), typeof( ((type *)0)->member )), "WUT"), \
((type *)((char *)(ptr) - offsetof(type, member))); \
)

>
> /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
> #ifdef CONFIG_FTRACE_MCOUNT_RECORD

--
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

\
 
 \ /
  Last update: 2017-05-22 19:59    [W:0.375 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site