From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760879AbdEVR66 (ORCPT ); Mon, 22 May 2017 13:58:58 -0400 Received: from mail-wr0-f172.google.com ([209.85.128.172]:35971 "EHLO mail-wr0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760846AbdEVR6w (ORCPT ); Mon, 22 May 2017 13:58:52 -0400 From: Michal Nazarewicz To: Ian Abbott , linux-kernel@vger.kernel.org Cc: Ian Abbott , Andrew Morton , Hidehiro Kawai , Borislav Petkov , Rasmus Villemoes , Johannes Berg , Peter Zijlstra , Alexander Potapenko Subject: Re: [PATCH v2] kernel.h: handle pointers to arrays better in container_of() In-Reply-To: <20170522150340.15461-1-abbotti@mev.co.uk> Organization: http://mina86.com/ References: <20170522150340.15461-1-abbotti@mev.co.uk> Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACP0lEQVQ4T23Sv2vbQBQHcBk1xE6WyALX107VUEgmn6+ouUwpEQQ6uRjttkWP4CkBg2M0BQLBdPFZYPsyFYo7qEtKDQ7on+t7+nF2Ux8ahD587717OmNYrOvycHsZ+o2r051wHTHysAvGb8ygvgu4QWT0sCmkgZCIEnlV2X8BtyraazFGDuxhmKSQJMlwHQ7v5MHSNxmz78rfElwAa3ieVD9e+hBhjaPDDG6NgFo2f4wBMNIo5YmRtF0RyDgFjJjlMIWbnuM4x9MMfABGTlN4qgIQB4A1DEyA1BHWtfeWNUMwiVJKoqh97KrkOO+qzgluVYLvFCUKAX73nONeBr7BGMdM6Sg0kuep03VywLaIzRiVr+GAzKlpQIsAFnWAG2e6DT5WmWDiudZMIc6hYrMOmeMQK9WX0B+/RfjzL9DI7Y9/Iayn29Ci0r2i4f9gMimMSZLCDMalgQGU5hnUtqAN0OGvEmO1Wnl0C0wWSCEHnuHBqmygxdxA8oWXwbipoc1EoNR9DqOpBpOJrnr0criQab9ZT4LL+wI+K7GBQH30CrhUruilgP9DRTrhVWZCiAyILP+wiuLeCKGTD6r/nc8LOJcAwR6IBTUs+7CASw3QFZ0MdA2PI3zNziH4ZKVhXCRMBjeZ1DWMekKwDCASwExy+NQ86TaykaDAFHO4aP48y4fIcDM5yOG8GcTLbOyp8A8azjJI93JFd1EA6yN8sSxMQJWoABqniRZVykYgRXErzrdqExAoUrRb0xfRp8p2A/4XmfilTtkDZ4cAAAAASUVORK5CYII= X-Face: -TR8(rDTHy/(xl?SfWd1|3:TTgDIatE^t'vop%*gVg[kn$t{EpK(P"VQ=~T2#ysNmJKN$"yTRLB4YQs$4{[.]Fc1)*O]3+XO^oXM>Q#b^ix,O)Zbn)q[y06$`e3?C)`CwR9y5riE=fv^X@x$y?D:XO6L&x4f-}}I4=VRNwiA^t1-ZrVK^07.Pi/57c_du'& OpenPGP: id=AC1F5F5CD41888F8CC8458582060401250751FF4; url=http://mina86.com/mina86.pub X-Hashcash: 1:20:170522:linux@rasmusvillemoes.dk::00CNks1DfIIbNwaq:00000000000000000000000000000000000001jvv X-Hashcash: 1:20:170522:bp@suse.de::u1KOLj59Smejy/xG:00000001Fzw X-Hashcash: 1:20:170522:hidehiro.kawai.ez@hitachi.com::Uwt2FHPKDADlqZ1C:000000000000000000000000000000000ED1 X-Hashcash: 1:20:170522:abbotti@mev.co.uk::kws8mNARwpfjaXht:000000000000000000000000000000000000000000000ugh X-Hashcash: 1:20:170522:glider@google.com::sQqyjUGwD94vAN7M:000000000000000000000000000000000000000000002Qn/ X-Hashcash: 1:20:170522:peterz@infradead.org::4Ke0xiGkNd73SNeQ:000000000000000000000000000000000000000003c5P X-Hashcash: 1:20:170522:johannes.berg@intel.com::9xXFaW2Nfi09UFH6:000000000000000000000000000000000000003Fpy X-Hashcash: 1:20:170522:akpm@linux-foundation.org::zDDtT5Aj7ieOMrBN:00000000000000000000000000000000000041pH X-Hashcash: 1:20:170522:linux-kernel@vger.kernel.org::7cjm0KOc988VjSNQ:00000000000000000000000000000000075la X-Hashcash: 1:20:170522:abbotti@mev.co.uk::HYH3IO+VB7V9hyfe:00000000000000000000000000000000000000000000FS0+ Date: Mon, 22 May 2017 19:58:36 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v4MHx7pa014956 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 > #include > > 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 > Cc: Andrew Morton > Cc: Michal Nazarewicz > Cc: Hidehiro Kawai > Cc: Borislav Petkov > Cc: Rasmus Villemoes > Cc: Johannes Berg > Cc: Peter Zijlstra > Cc: Alexander Potapenko > --- > 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»