| Message ID | 1542856462-18836-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/3] kernel.h: disable type-checks in container_of() for Sparse | expand |
On Thu, 22 Nov 2018 12:14:20 +0900 Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot > reported lots of "unknown expression" warnings from container_of(), > which seemed false positive. > > I addressed this in [1], but fixing Sparse is the right thing to do. > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of > function designator"), but it will take time until the fixed version > of Sparse is widely available. > > Disable the container_of() type checks for Sparse for now. > > [1] https://lore.kernel.org/lkml/1542623503-3755-1-git-send-email-yamada.masahiro@socionext.com/ > > ... > > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -985,6 +985,21 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } > #define __CONCAT(a, b) a ## b > #define CONCATENATE(a, b) __CONCAT(a, b) > > +/* > + * TODO: > + * Sparse emits "unknown expression" warnings. > + * It was fixed by commit 0eb8175d3e9c0d20354763d07ce3d4c0e543d988 in Sparse. > + * Remove the following workaround when the fixed Sparse is widely available. > + */ > +#ifdef __CHECKER__ > +#define TYPE_CHECK_CONTAINER_OF(ptr, type, member) do {} while (0) > +#else > +#define TYPE_CHECK_CONTAINER_OF(ptr, type, member) \ > + BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ > + !__same_type(*(ptr), void), \ > + "pointer type mismatch in container_of()") > +#endif I think that's OK. A few years hence, someone will happen upon this comment and will perform the obvious cleanup.
On Thu, Nov 22, 2018 at 12:14:20PM +0900, Masahiro Yamada wrote: > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot > reported lots of "unknown expression" warnings from container_of(), > which seemed false positive. > > I addressed this in [1], but fixing Sparse is the right thing to do. > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of > function designator"), but it will take time until the fixed version > of Sparse is widely available. > > Disable the container_of() type checks for Sparse for now. I would prefer that developers upgrade their version of sparse but ... Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
On Fri, Nov 23, 2018 at 10:14 PM Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > > On Thu, Nov 22, 2018 at 12:14:20PM +0900, Masahiro Yamada wrote: > > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot > > reported lots of "unknown expression" warnings from container_of(), > > which seemed false positive. > > > > I addressed this in [1], but fixing Sparse is the right thing to do. > > > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of > > function designator"), but it will take time until the fixed version > > of Sparse is widely available. > > > > Disable the container_of() type checks for Sparse for now. > > I would prefer that developers upgrade their version of sparse but ... > > Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> Indeed. If someone is writing code for the latest kernels, I think it is reasonable to assume they are able to use the latest sparse too, since it is not required for compilation anyway. Cheers, Miguel
On Sat, Nov 24, 2018 at 6:06 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Fri, Nov 23, 2018 at 10:14 PM Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: > > > > On Thu, Nov 22, 2018 at 12:14:20PM +0900, Masahiro Yamada wrote: > > > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot > > > reported lots of "unknown expression" warnings from container_of(), > > > which seemed false positive. > > > > > > I addressed this in [1], but fixing Sparse is the right thing to do. > > > > > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of > > > function designator"), but it will take time until the fixed version > > > of Sparse is widely available. > > > > > > Disable the container_of() type checks for Sparse for now. > > > > I would prefer that developers upgrade their version of sparse but ... > > > > Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > > Indeed. If someone is writing code for the latest kernels, I think it > is reasonable to assume they are able to use the latest sparse too, > since it is not required for compilation anyway. I am OK with either way. I leave this to Andrew. -- Best Regards Masahiro Yamada
Hi Andrew, On Sat, Nov 24, 2018 at 6:06 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Fri, Nov 23, 2018 at 10:14 PM Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: > > > > On Thu, Nov 22, 2018 at 12:14:20PM +0900, Masahiro Yamada wrote: > > > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot > > > reported lots of "unknown expression" warnings from container_of(), > > > which seemed false positive. > > > > > > I addressed this in [1], but fixing Sparse is the right thing to do. > > > > > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of > > > function designator"), but it will take time until the fixed version > > > of Sparse is widely available. > > > > > > Disable the container_of() type checks for Sparse for now. > > > > I would prefer that developers upgrade their version of sparse but ... > > > > Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > > Indeed. If someone is writing code for the latest kernels, I think it > is reasonable to assume they are able to use the latest sparse too, > since it is not required for compilation anyway. I was reconsidering about this. I saw other Sparse warnings anyway unless I use the state-of-the-art version of Sparse. So, now I think Luc and Miguel were right. Requiring the latest Sparse is the right solution. We do not need to take care of old Sparse. Andrew, Could you drop this patch please? -- Best Regards Masahiro Yamada
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index d6aac75..d8c4adb 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -985,6 +985,21 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } #define __CONCAT(a, b) a ## b #define CONCATENATE(a, b) __CONCAT(a, b) +/* + * TODO: + * Sparse emits "unknown expression" warnings. + * It was fixed by commit 0eb8175d3e9c0d20354763d07ce3d4c0e543d988 in Sparse. + * Remove the following workaround when the fixed Sparse is widely available. + */ +#ifdef __CHECKER__ +#define TYPE_CHECK_CONTAINER_OF(ptr, type, member) do {} while (0) +#else +#define TYPE_CHECK_CONTAINER_OF(ptr, type, member) \ + BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ + !__same_type(*(ptr), void), \ + "pointer type mismatch in container_of()") +#endif + /** * container_of - cast a member of a structure out to the containing structure * @ptr: the pointer to the member. @@ -994,9 +1009,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } */ #define container_of(ptr, type, member) ({ \ void *__mptr = (void *)(ptr); \ - BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ - !__same_type(*(ptr), void), \ - "pointer type mismatch in container_of()"); \ + TYPE_CHECK_CONTAINER_OF(ptr, type, member); \ ((type *)(__mptr - offsetof(type, member))); }) /** @@ -1009,9 +1022,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } */ #define container_of_safe(ptr, type, member) ({ \ void *__mptr = (void *)(ptr); \ - BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ - !__same_type(*(ptr), void), \ - "pointer type mismatch in container_of()"); \ + TYPE_CHECK_CONTAINER_OF(ptr, type, member); \ IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \ ((type *)(__mptr - offsetof(type, member))); })
When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot reported lots of "unknown expression" warnings from container_of(), which seemed false positive. I addressed this in [1], but fixing Sparse is the right thing to do. The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of function designator"), but it will take time until the fixed version of Sparse is widely available. Disable the container_of() type checks for Sparse for now. [1] https://lore.kernel.org/lkml/1542623503-3755-1-git-send-email-yamada.masahiro@socionext.com/ Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v4: - New patch Changes in v3: None Changes in v2: None include/linux/kernel.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) -- 2.7.4