linux-kernel - Re: [PATCH v2 29/59] dyndbg: change __dynamic_func_call

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 for Android: free password hash cracker in your pocket

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

Message-ID: <05dff3c7-91e8-4d42-b75a-90c410d6470d@bootlin.com>
Date: Mon, 24 Mar 2025 16:19:54 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Jim Cromie <jim.cromie@...il.com>, 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,
 intel-gfx-trybot@...ts.freedesktop.org
Cc: jbaron@...mai.com, gregkh@...uxfoundation.org, ukaszb@...omium.org,
 daniel.vetter@...ll.ch, tvrtko.ursulin@...ux.intel.com,
 jani.nikula@...el.com, ville.syrjala@...ux.intel.com
Subject: Re: [PATCH v2 29/59] dyndbg: change __dynamic_func_call_cls* macros
 into expressions



Le 20/03/2025 à 19:52, Jim Cromie a écrit :
> The Xe driver's XE_IOCTL_DBG macro calls drm_dbg() from inside an if
> (expression).  This breaks when CONFIG_DRM_USE_DYNAMIC_DEBUG=y because
> the invoked macro has a do-while-0 wrapper.
> 
>     if (cond && (drm_dbg("expr-form"),1)) {
>        ... do some more stuff
>     }
> 
> Fix for this usage by changing __dynamic_func_call_cls{,_no_desc}
> macros into expressions, by replacing the do-while-0s with a ({ })
> wrapper.  In the common usage, the trailing ';' converts the
> expression into a statement.
> 
>     drm_dbg("statement form");
> 
> Signed-off-by: Jim Cromie <jim.cromie@...il.com>
> ---
> ---
>   include/linux/dynamic_debug.h | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 8043966a0fd6..80bcaad03400 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -339,20 +339,20 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
>    * (|_cls):	adds in _DPRINT_CLASS_DFLT as needed
>    * (|_no_desc):	former gets callsite descriptor as 1st arg (for prdbgs)
>    */
> -#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do {	\
> -	DEFINE_DYNAMIC_DEBUG_METADATA_CLS((id), cls, fmt);	\
> +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) ({	\
> +	DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);	\

Is it normal to remove the parenthesis around id? Or the other way 
around, why did you add parenthesis in PATCH 17?

>   	if (DYNAMIC_DEBUG_BRANCH(id))				\
> -		func(&id, ##__VA_ARGS__);			\
> -} while (0)
> +		func(&(id), ##__VA_ARGS__);			\
> +})
>   #define __dynamic_func_call(id, fmt, func, ...)				\
>   	__dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt,		\
>   				func, ##__VA_ARGS__)
>   
> -#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do {	\
> +#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) ({	\
>   	DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);		\

I expect the same constraints around id here, both with parenthesis, or 
no parenthesis at all.

>   	if (DYNAMIC_DEBUG_BRANCH(id))					\
>   		func(__VA_ARGS__);					\
> -} while (0)
> +})
>   #define __dynamic_func_call_no_desc(id, fmt, func, ...)			\
>   	__dynamic_func_call_cls_no_desc(id, _DPRINTK_CLASS_DFLT,	\
>   					fmt, func, ##__VA_ARGS__)

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com