[RFC 05/31] x86/compiler: Tweak __UNIQUE_ID naming

Josh Poimboeuf posted 31 patches 1 year, 5 months ago
[RFC 05/31] x86/compiler: Tweak __UNIQUE_ID naming
Posted by Josh Poimboeuf 1 year, 5 months ago
Add an underscore between the "name" and the counter so tooling can
distinguish between the non-unique and unique portions of the symbol
name.

This will come in handy for "objtool klp diff".

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 include/linux/compiler.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 8c252e073bd8..d3f100821d45 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -186,7 +186,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 	__asm__ ("" : "=r" (var) : "0" (var))
 #endif
 
-#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
+/* Format: __UNIQUE_ID_<name>_<__COUNTER__> */
+#define __UNIQUE_ID(name)					\
+	__PASTE(__UNIQUE_ID_,					\
+	__PASTE(name,						\
+	__PASTE(_, __COUNTER__)))
 
 /**
  * data_race - mark an expression as containing intentional data races
@@ -218,7 +222,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
  */
 #define ___ADDRESSABLE(sym, __attrs) \
 	static void * __used __attrs \
-	__UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
+	__UNIQUE_ID(__PASTE(addressable_, sym)) = (void *)(uintptr_t)&sym;
 #define __ADDRESSABLE(sym) \
 	___ADDRESSABLE(sym, __section(".discard.addressable"))
 
-- 
2.45.2
Re: [RFC 05/31] x86/compiler: Tweak __UNIQUE_ID naming
Posted by Peter Zijlstra 1 year, 5 months ago
On Mon, Sep 02, 2024 at 08:59:48PM -0700, Josh Poimboeuf wrote:
> Add an underscore between the "name" and the counter so tooling can
> distinguish between the non-unique and unique portions of the symbol
> name.
> 
> This will come in handy for "objtool klp diff".
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  include/linux/compiler.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 8c252e073bd8..d3f100821d45 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -186,7 +186,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>  	__asm__ ("" : "=r" (var) : "0" (var))
>  #endif
>  
> -#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> +/* Format: __UNIQUE_ID_<name>_<__COUNTER__> */
> +#define __UNIQUE_ID(name)					\
> +	__PASTE(__UNIQUE_ID_,					\
> +	__PASTE(name,						\
> +	__PASTE(_, __COUNTER__)))

OK, that's just painful to read; how about so?

	__PASTE(__UNIQUE_ID_,					\
	        __PASTE(name,					\
		        __PASTE(_, __COUNTER)))

>  
>  /**
>   * data_race - mark an expression as containing intentional data races
> @@ -218,7 +222,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>   */
>  #define ___ADDRESSABLE(sym, __attrs) \
>  	static void * __used __attrs \
> -	__UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
> +	__UNIQUE_ID(__PASTE(addressable_, sym)) = (void *)(uintptr_t)&sym;

This change doesn't get mention ?

>  #define __ADDRESSABLE(sym) \
>  	___ADDRESSABLE(sym, __section(".discard.addressable"))
>  
> -- 
> 2.45.2
>
RE: [RFC 05/31] x86/compiler: Tweak __UNIQUE_ID naming
Posted by David Laight 1 year, 5 months ago
From: Peter Zijlstra
> Sent: 03 September 2024 08:57
> 
> On Mon, Sep 02, 2024 at 08:59:48PM -0700, Josh Poimboeuf wrote:
> > Add an underscore between the "name" and the counter so tooling can
> > distinguish between the non-unique and unique portions of the symbol
> > name.
> >
> > This will come in handy for "objtool klp diff".
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > ---
> >  include/linux/compiler.h | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index 8c252e073bd8..d3f100821d45 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -186,7 +186,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> >  	__asm__ ("" : "=r" (var) : "0" (var))
> >  #endif
> >
> > -#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> > +/* Format: __UNIQUE_ID_<name>_<__COUNTER__> */
> > +#define __UNIQUE_ID(name)					\
> > +	__PASTE(__UNIQUE_ID_,					\
> > +	__PASTE(name,						\
> > +	__PASTE(_, __COUNTER__)))
> 
> OK, that's just painful to read; how about so?
> 
> 	__PASTE(__UNIQUE_ID_,					\
> 	        __PASTE(name,					\
> 		        __PASTE(_, __COUNTER)))

Why not just generate name_nnnnn?
I believe it that the 'prefix' was added to allow multiple unique names be
generated on a single line on versions of gcc that didn't support __COUNTER__
and __LINE__ was used instead.
So prior to that the result would have been __UNIQUE_ID_nnnn with no indication
of the variable name.

In one of the min/max changes I suggested just passing __COUNTER__ through
the required 2 #defines and then just appending it to the variable name.
IIRC Linus liked that idea :-)
Need to find time to write the patch...

After all __PASTE() itself is only really necessary for a K&R C cpp.
Possibly still relevant for .S files - not sure.
(You had to generate a/**/b to get ab when the comment was removed.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Re: [RFC 05/31] x86/compiler: Tweak __UNIQUE_ID naming
Posted by Josh Poimboeuf 1 year, 5 months ago
On Tue, Sep 03, 2024 at 09:56:34AM +0200, Peter Zijlstra wrote:
> > -#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> > +/* Format: __UNIQUE_ID_<name>_<__COUNTER__> */
> > +#define __UNIQUE_ID(name)					\
> > +	__PASTE(__UNIQUE_ID_,					\
> > +	__PASTE(name,						\
> > +	__PASTE(_, __COUNTER__)))
> 
> OK, that's just painful to read; how about so?
> 
> 	__PASTE(__UNIQUE_ID_,					\
> 	        __PASTE(name,					\
> 		        __PASTE(_, __COUNTER)))

Sure.

> >  /**
> >   * data_race - mark an expression as containing intentional data races
> > @@ -218,7 +222,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> >   */
> >  #define ___ADDRESSABLE(sym, __attrs) \
> >  	static void * __used __attrs \
> > -	__UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
> > +	__UNIQUE_ID(__PASTE(addressable_, sym)) = (void *)(uintptr_t)&sym;
> 
> This change doesn't get mention ?

Hm, I have no idea why I did that...  I'll drop it.

-- 
Josh