Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Rikard Falkeborn <rikard.falkeborn@gmail.com>
Date:   Wed, 7 Aug 2019 22:07:53 +0200
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Masahiro Yamada <yamada.masahiro@socionext.com>,
        Rikard Falkeborn <rikard.falkeborn@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Joe Perches <joe@perches.com>,
        Johannes Berg <johannes@sipsolutions.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] linux/bits.h: Add compile time sanity check of
 GENMASK inputs
Message-ID: <20190807200753.GA14779@rikard>
References: <20190731190309.19909-1-rikard.falkeborn@gmail.com>
 <20190801230358.4193-1-rikard.falkeborn@gmail.com>
 <20190801230358.4193-2-rikard.falkeborn@gmail.com>
 <20190807142728.GA16360@roeck-us.net>
 <CAK7LNATGuO0D0a-sTvWw5Pzkn5C7jrLiS=rCwiRsEqaS86KbuQ@mail.gmail.com>
 <d1bd1c05-3e92-1a7f-ebd3-3b26981a6f8e@roeck-us.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d1bd1c05-3e92-1a7f-ebd3-3b26981a6f8e@roeck-us.net>
User-Agent: Mutt/1.12.1 (2019-06-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Aug 07, 2019 at 09:52:33AM -0700, Guenter Roeck wrote:
> On 8/7/19 7:55 AM, Masahiro Yamada wrote:
> > On Wed, Aug 7, 2019 at 11:27 PM Guenter Roeck <linux@roeck-us.net> wrote:
> > > 
> > > On Fri, Aug 02, 2019 at 01:03:58AM +0200, Rikard Falkeborn wrote:
> > > > GENMASK() and GENMASK_ULL() are supposed to be called with the high bit
> > > > as the first argument and the low bit as the second argument. Mixing
> > > > them will return a mask with zero bits set.
> > > > 
> > > > Recent commits show getting this wrong is not uncommon, see e.g.
> > > > commit aa4c0c9091b0 ("net: stmmac: Fix misuses of GENMASK macro") and
> > > > commit 9bdd7bb3a844 ("clocksource/drivers/npcm: Fix misuse of GENMASK
> > > > macro").
> > > > 
> > > > To prevent such mistakes from appearing again, add compile time sanity
> > > > checking to the arguments of GENMASK() and GENMASK_ULL(). If both the
> > > > arguments are known at compile time, and the low bit is higher than the
> > > > high bit, break the build to detect the mistake immediately.
> > > > 
> > > > Since GENMASK() is used in declarations, BUILD_BUG_ON_ZERO() must be
> > > > used instead of BUILD_BUG_ON(), and __is_constexpr() must be used instead
> > > > of __builtin_constant_p().
> > > > 
> > > > If successful, BUILD_BUG_OR_ZERO() returns 0 of type size_t. To avoid
> > > > problems with implicit conversions, cast the result of BUILD_BUG_OR_ZERO
> > > > to unsigned long.
> > > > 
> > > > Since both BUILD_BUG_ON_ZERO() and __is_constexpr() only uses sizeof()
> > > > on the arguments passed to them, neither of them evaluate the expression
> > > > unless it is a VLA. Therefore, GENMASK(1, x++) still behaves as
> > > > expected.
> > > > 
> > > > Commit 95b980d62d52 ("linux/bits.h: make BIT(), GENMASK(), and friends
> > > > available in assembly") made the macros in linux/bits.h available in
> > > > assembly. Since neither BUILD_BUG_OR_ZERO() or __is_constexpr() are asm
> > > > compatible, disable the checks if the file is included in an asm file.
> > > > 
> > > 
> > > Who is going to fix the fallout ? For example, arm64:defconfig no longer
> > > compiles with this patch applied.
> > > 
> > > It seems to me that the benefit of catching misuses of GENMASK is much
> > > less than the fallout from no longer compiling kernels, since those
> > > kernels won't get any test coverage at all anymore.
> > 
> > 
> > We cannot apply this until we fix all errors.
> > 
> > I do not understand why Andrew picked up this so soon.
> > 
> 
> The same was done with the fallthrough warning in mainline, which still results
> in all "sh" builds failing there (and in -next, obviously). I don't understand
> the logic either, but maybe it is the new normal.
> 
> Guenter

Sorry about that. As Masahiro said, the patch was picked up too soon,
I've asked Andrew and Stephen Rothwell to remove the patch in a separate
email thread.

FWIW, there seems to be a patch for the build failure already (all arm
builds seems to have the same build error):

https://lore.kernel.org/lkml/20190807192305.6604-1-natechancellor@gmail.com/T/#u

Rikard