linux-kernel - Re: [PATCH v2] btrfs: make ASSERT no-op in release builds

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: <20251104001800.GM13846@suse.cz>
Date: Tue, 4 Nov 2025 01:18:00 +0100
From: David Sterba <dsterba@...e.cz>
To: Gladyshev Ilya <foxido@...ido.dev>
Cc: Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
	linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] btrfs: make ASSERT no-op in release builds

On Sun, Nov 02, 2025 at 10:38:52AM +0300, Gladyshev Ilya wrote:
> The current definition of `ASSERT(cond)` as `(void)(cond)` is redundant,
> since these checks have no side effects and don't affect code logic.

Have you checked that none of the assert expressions really don't have
side effects other than touching the memory?

> However, some checks contain READ_ONCE or other compiler-unfriendly
> constructs. For example, ASSERT(list_empty) in btrfs_add_dealloc_inode
> was compiled to a redundant mov instruction due to this issue.
> 
> This patch defines ASSERT as BUILD_BUG_ON_INVALID for !CONFIG_BTRFS_ASSERT
> builds. It also marks `full_page_sectors_uptodate` as __maybe_unused to
> suppress "unneeded declaration" warning (it's needed in compile time)
> 
> Signed-off-by: Gladyshev Ilya <foxido@...ido.dev>
> ---
> Changes from v1:
> - Annotate full_page_sectors_uptodate as __maybe_unused to avoid
>   compiler warning
> 
> Link to v1: https://lore.kernel.org/linux-btrfs/20251030182322.4085697-1-foxido@foxido.dev/
> ---
>  fs/btrfs/messages.h | 2 +-
>  fs/btrfs/raid56.c   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/messages.h b/fs/btrfs/messages.h
> index 4416c165644f..f80fe40a2c2b 100644
> --- a/fs/btrfs/messages.h
> +++ b/fs/btrfs/messages.h
> @@ -168,7 +168,7 @@ do {										\
>  #endif
>  
>  #else
> -#define ASSERT(cond, args...)			(void)(cond)
> +#define ASSERT(cond, args...)			BUILD_BUG_ON_INVALID(cond)

I'd rather have the expression open coded rather than using
BUILD_BUG_ON_INVALID, the name is confusing as it's not checking build
time condtitons.