linux-kernel - Re: [PATCH v4 18/21] compiler.h: RFC - s/__LINE__/__COUNTER__/ in __UNIQUE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230713171901.GA4036397@dev-arch.thelio-3990X>
Date:   Thu, 13 Jul 2023 10:19:01 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Jim Cromie <jim.cromie@...il.com>
Cc:     jbaron@...mai.com, daniel.vetter@...ll.ch,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        amd-gfx@...ts.freedesktop.org, intel-gvt-dev@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org, jani.nikula@...el.com,
        ville.syrjala@...ux.intel.com, seanpaul@...omium.org,
        robdclark@...il.com, gregkh@...uxfoundation.org,
        Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>, linux-sparse@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH v4 18/21] compiler.h: RFC - s/__LINE__/__COUNTER__/ in
 __UNIQUE_ID fallback

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@...il.com> (maintainer:SPARSE CHECKER)
> CC: Nathan Chancellor <nathan@...nel.org> (supporter:CLANG/LLVM BUILD SUPPORT)
> CC: Nick Desaulniers <ndesaulniers@...gle.com> (supporter:CLANG/LLVM BUILD SUPPORT)
> CC: Tom Rix <trix@...hat.com> (reviewer:CLANG/LLVM BUILD SUPPORT)
> CC: linux-sparse@...r.kernel.org (open list:SPARSE CHECKER)
> CC: linux-kernel@...r.kernel.org (open list)
> CC: llvm@...ts.linux.dev (open list:CLANG/LLVM BUILD SUPPORT)
> Signed-off-by: Jim Cromie <jim.cromie@...il.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
>