| Message ID | 20230713163626.31338-19-jim.cromie@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | None | expand |
Hi Jim On Thu, Jul 13, 2023 at 10:36:23AM -0600, Jim Cromie wrote: > We currently have 3 defns for __UNIQUE_ID(); gcc and clang are using > __COUNTER__ for real uniqueness, 3rd just uses __LINE__, which should > fail on this (and harder to avoid situations): > > DECLARE_FOO(); DECLARE_FOO(); > > Its 2023, can we haz a no-fallback __UNIQUE_ID ? Yeah, I fail to see how this fallback definition can actually be used after commit 95207db8166a ("Remove Intel compiler support"); even before that, it would be pretty unlikely since icc usage has not been visible for a long time. The kernel only officially supports clang or GCC now, so the definitions of __UNIQUE_ID() in include/linux/compiler-clang.h and include/linux/compiler-gcc.h should always be used because of the include in include/linux/compiler_types.h, right? I think the correct clean up is to just hoist the definition of __UNIQUE_ID() out of the individual compiler headers into the common one here but... > NOTE: > > This also changes __UNIQUE_ID_ to _kaUID_. Ive been getting > lkp-reports of collisions on names which should be unique; this > shouldnt happen on gcc & clang, but does on some older ones, on some > platforms, on some allyes & rand-configs. Like this: > > mips64-linux-ld: > drivers/gpu/drm/display/drm_dp_helper.o:(__dyndbg_class_users+0x0): > multiple definition of `__UNIQUE_ID_ddebug_class_user405'; > drivers/gpu/drm/drm_gem_shmem_helper.o:(__dyndbg_class_users+0x0): > first defined here This problem cannot be addressed with this patch given the above information, no? Seems like that might mean that __COUNTER__ has issues in earlier compilers? Cheers, Nathan > Like above, the collision reports appear to always be 3-digit > counters, which look like line-numbers. Changing to _kaUID_ in this > defn should make it more obvious (in *.i file) when a fallback has > happened. To be clear, I havent seen it yet. Nor have I seen the > multiple-defn problem above since adding this patch. > > Lets see what lkp-robot says about this. > > CC: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> (maintainer:SPARSE CHECKER) > CC: Nathan Chancellor <nathan@kernel.org> (supporter:CLANG/LLVM BUILD SUPPORT) > CC: Nick Desaulniers <ndesaulniers@google.com> (supporter:CLANG/LLVM BUILD SUPPORT) > CC: Tom Rix <trix@redhat.com> (reviewer:CLANG/LLVM BUILD SUPPORT) > CC: linux-sparse@vger.kernel.org (open list:SPARSE CHECKER) > CC: linux-kernel@vger.kernel.org (open list) > CC: llvm@lists.linux.dev (open list:CLANG/LLVM BUILD SUPPORT) > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > --- > include/linux/compiler.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index d7779a18b24f..677d6c47cd9e 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -177,9 +177,9 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > __asm__ ("" : "=r" (var) : "0" (var)) > #endif > > -/* Not-quite-unique ID. */ > +/* JFTI: to fix Not-quite-unique ID */ > #ifndef __UNIQUE_ID > -# define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__) > +# define __UNIQUE_ID(prefix) __PASTE(__PASTE(_kaUID_, prefix), __COUNTER__) > #endif > > /** > -- > 2.41.0 >
diff --git a/include/linux/compiler.h b/include/linux/compiler.h index d7779a18b24f..677d6c47cd9e 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -177,9 +177,9 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, __asm__ ("" : "=r" (var) : "0" (var)) #endif -/* Not-quite-unique ID. */ +/* JFTI: to fix Not-quite-unique ID */ #ifndef __UNIQUE_ID -# define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__) +# define __UNIQUE_ID(prefix) __PASTE(__PASTE(_kaUID_, prefix), __COUNTER__) #endif /**
We currently have 3 defns for __UNIQUE_ID(); gcc and clang are using __COUNTER__ for real uniqueness, 3rd just uses __LINE__, which should fail on this (and harder to avoid situations): DECLARE_FOO(); DECLARE_FOO(); Its 2023, can we haz a no-fallback __UNIQUE_ID ? NOTE: This also changes __UNIQUE_ID_ to _kaUID_. Ive been getting lkp-reports of collisions on names which should be unique; this shouldnt happen on gcc & clang, but does on some older ones, on some platforms, on some allyes & rand-configs. Like this: mips64-linux-ld: drivers/gpu/drm/display/drm_dp_helper.o:(__dyndbg_class_users+0x0): multiple definition of `__UNIQUE_ID_ddebug_class_user405'; drivers/gpu/drm/drm_gem_shmem_helper.o:(__dyndbg_class_users+0x0): first defined here Like above, the collision reports appear to always be 3-digit counters, which look like line-numbers. Changing to _kaUID_ in this defn should make it more obvious (in *.i file) when a fallback has happened. To be clear, I havent seen it yet. Nor have I seen the multiple-defn problem above since adding this patch. Lets see what lkp-robot says about this. CC: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> (maintainer:SPARSE CHECKER) CC: Nathan Chancellor <nathan@kernel.org> (supporter:CLANG/LLVM BUILD SUPPORT) CC: Nick Desaulniers <ndesaulniers@google.com> (supporter:CLANG/LLVM BUILD SUPPORT) CC: Tom Rix <trix@redhat.com> (reviewer:CLANG/LLVM BUILD SUPPORT) CC: linux-sparse@vger.kernel.org (open list:SPARSE CHECKER) CC: linux-kernel@vger.kernel.org (open list) CC: llvm@lists.linux.dev (open list:CLANG/LLVM BUILD SUPPORT) Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- include/linux/compiler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)