linux-kernel - Re: [PATCH] btrfs: use BUG() instead of BUG

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] [day] [month] [year] [list]

Message-ID: <20190325131305.GD10640@suse.cz>
Date:   Mon, 25 Mar 2019 14:13:05 +0100
From:   David Sterba <dsterba@...e.cz>
To:     Nikolay Borisov <nborisov@...e.com>
Cc:     Arnd Bergmann <arnd@...db.de>, Chris Mason <clm@...com>,
        Josef Bacik <josef@...icpanda.com>,
        Nathan Chancellor <natechancellor@...il.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        clang-built-linux@...glegroups.com,
        Liu Bo <bo.liu@...ux.alibaba.com>,
        Anand Jain <Anand.Jain@...cle.com>,
        Filipe Manana <FdManana@...e.com>, WenRuo Qu <wqu@...e.com>,
        Johannes Thumshirn <jthumshirn@...e.de>,
        linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: use BUG() instead of BUG_ON(1)

On Mon, Mar 25, 2019 at 03:06:12PM +0200, Nikolay Borisov wrote:
> 
> 
> On 25.03.19 г. 15:02 ч., Arnd Bergmann wrote:
> > BUG_ON(1) leads to bogus warnings from clang when
> > CONFIG_PROFILE_ANNOTATED_BRANCHES is set:
> > 
> > fs/btrfs/volumes.c:5041:3: error: variable 'max_chunk_size' is used uninitialized whenever 'if' condition is false
> >       [-Werror,-Wsometimes-uninitialized]
> >                 BUG_ON(1);
> >                 ^~~~~~~~~
> > include/asm-generic/bug.h:61:36: note: expanded from macro 'BUG_ON'
> >  #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
> >                                    ^~~~~~~~~~~~~~~~~~~
> > include/linux/compiler.h:48:23: note: expanded from macro 'unlikely'
> >  #  define unlikely(x)   (__branch_check__(x, 0, __builtin_constant_p(x)))
> >                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > fs/btrfs/volumes.c:5046:9: note: uninitialized use occurs here
> >                              max_chunk_size);
> >                              ^~~~~~~~~~~~~~
> > include/linux/kernel.h:860:36: note: expanded from macro 'min'
> >  #define min(x, y)       __careful_cmp(x, y, <)
> >                                          ^
> > include/linux/kernel.h:853:17: note: expanded from macro '__careful_cmp'
> >                 __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
> >                               ^
> > include/linux/kernel.h:847:25: note: expanded from macro '__cmp_once'
> >                 typeof(y) unique_y = (y);               \
> >                                       ^
> > fs/btrfs/volumes.c:5041:3: note: remove the 'if' if its condition is always true
> >                 BUG_ON(1);
> >                 ^
> > include/asm-generic/bug.h:61:32: note: expanded from macro 'BUG_ON'
> >  #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
> >                                ^
> > fs/btrfs/volumes.c:4993:20: note: initialize the variable 'max_chunk_size' to silence this warning
> >         u64 max_chunk_size;
> >                           ^
> >                            = 0
> > 
> > Change it to BUG() so clang can see that this code path can never
> > continue.
> 
> Long-term we have to eliminate to bug_on's but until then:

I think in those places it's "this must never happen", so it's not the
the typical example of BUG_ON misuse we want to eliminate. Fixing the
compiler waring is ok though.