linux-next: build failure while building Linus' tree

linux-next: build failure while building Linus' tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

Hi all,

Building Linus' tree, today's linux-next build (powerpc
ppc6xx_defconfig gcc4.9) failed like this:

ERROR: modpost: "__divdi3" [drivers/block/nbd.ko] undefined!

Caused by commit

  fad7cd3310db ("nbd: add the check to prevent overflow in __nbd_ioctl()")

The added check_mul_overflow() call is being passed 64 bit values.
COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW is not set for this build (see
include/linux/overflow.h).

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure while building Linus' tree

[Nick Desaulniers]

+ Rasmus

This was introduced in
commit f0907827a8a91 ("compiler.h: enable builtin overflow checkers and add
fallback code")
which added division using the `/` operator, which is problematic when checking
for overflows of 64b operands on 32b targets.

We'll probably need helpers from linux/math64.h and some combination of
__builtin_choose_expr/__builtin_types_compatible_p.

That will help us fix another compiler bug for older clang releases, too.
https://github.com/ClangBuiltLinux/linux/issues/1438.

Re: linux-next: build failure while building Linus' tree

[Nick Desaulniers]

On Thu, Sep 9, 2021 at 3:50 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> + Rasmus
>
> This was introduced in
> commit f0907827a8a91 ("compiler.h: enable builtin overflow checkers and add
> fallback code")
> which added division using the `/` operator, which is problematic when checking
> for overflows of 64b operands on 32b targets.
>
> We'll probably need helpers from linux/math64.h and some combination of
> __builtin_choose_expr/__builtin_types_compatible_p.
>
> That will help us fix another compiler bug for older clang releases, too.
> https://github.com/ClangBuiltLinux/linux/issues/1438.

Ok, I have something hacked up that I think will work:
https://gist.github.com/nickdesaulniers/2479818f4983bbf2d688cebbab435863
This incomplete diff is a little hacked up to reproduce the issue with
a known-bad revision of clang that demonstrates a similar issue to GCC
4.9.  You can ignore the movement of check_mul_overflow and friends in
include/linux/overflow.h.

I think I'm going to break that up into 2 or 3 patches:
1. move is_signed_type from include/linux/overflow.h to perhaps
include/linux/typecheck.h.
2. add div64_x64, div_x64, and div_64 to include/linux/math64.h, use
them in include/linux/overflow.h to fix GCC 4.9
3. move multiply fallbacks out of
COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW for clang < 14.
-- 
Thanks,
~Nick Desaulniers

Re: linux-next: build failure while building Linus' tree

[Nick Desaulniers]

On Fri, Sep 10, 2021 at 3:02 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Sep 9, 2021 at 3:50 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > + Rasmus
> >
> > This was introduced in
> > commit f0907827a8a91 ("compiler.h: enable builtin overflow checkers and add
> > fallback code")
> > which added division using the `/` operator, which is problematic when checking
> > for overflows of 64b operands on 32b targets.
> >
> > We'll probably need helpers from linux/math64.h and some combination of
> > __builtin_choose_expr/__builtin_types_compatible_p.
> >
> > That will help us fix another compiler bug for older clang releases, too.
> > https://github.com/ClangBuiltLinux/linux/issues/1438.
>
> Ok, I have something hacked up that I think will work:
> https://gist.github.com/nickdesaulniers/2479818f4983bbf2d688cebbab435863

hmm...playing around with adding some static asserts to the above, I
don't think it's quite right, specifically:
#define div_64(dividend, divisor) ({            \
  typeof(dividend) z;

might declare z with the expected sign, but not necessarily the
correct width when the dividend is 32b but the divisor is 64b.  Feels
a bit like trying to encode the C type promotion rules in a macro...

> This incomplete diff is a little hacked up to reproduce the issue with
> a known-bad revision of clang that demonstrates a similar issue to GCC
> 4.9.  You can ignore the movement of check_mul_overflow and friends in
> include/linux/overflow.h.
>
> I think I'm going to break that up into 2 or 3 patches:
> 1. move is_signed_type from include/linux/overflow.h to perhaps
> include/linux/typecheck.h.
> 2. add div64_x64, div_x64, and div_64 to include/linux/math64.h, use
> them in include/linux/overflow.h to fix GCC 4.9
> 3. move multiply fallbacks out of
> COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW for clang < 14.
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

Re: linux-next: build failure while building Linus' tree

[Nick Desaulniers]

On Fri, Sep 10, 2021 at 3:17 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Sep 10, 2021 at 3:02 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Thu, Sep 9, 2021 at 3:50 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > + Rasmus
> > >
> > > This was introduced in
> > > commit f0907827a8a91 ("compiler.h: enable builtin overflow checkers and add
> > > fallback code")
> > > which added division using the `/` operator, which is problematic when checking
> > > for overflows of 64b operands on 32b targets.
> > >
> > > We'll probably need helpers from linux/math64.h and some combination of
> > > __builtin_choose_expr/__builtin_types_compatible_p.
> > >
> > > That will help us fix another compiler bug for older clang releases, too.
> > > https://github.com/ClangBuiltLinux/linux/issues/1438.
> >
> > Ok, I have something hacked up that I think will work:
> > https://gist.github.com/nickdesaulniers/2479818f4983bbf2d688cebbab435863
>
> hmm...playing around with adding some static asserts to the above, I

ah! static_assert can't be used for the type agnostic macros,
BUILD_BUG_ON needs to be used in its place. Ok, let me add a few and
see if that can help instill some confidence here.

> don't think it's quite right, specifically:
> #define div_64(dividend, divisor) ({            \
>   typeof(dividend) z;
>
> might declare z with the expected sign, but not necessarily the
> correct width when the dividend is 32b but the divisor is 64b.  Feels
> a bit like trying to encode the C type promotion rules in a macro...
>
> > This incomplete diff is a little hacked up to reproduce the issue with
> > a known-bad revision of clang that demonstrates a similar issue to GCC
> > 4.9.  You can ignore the movement of check_mul_overflow and friends in
> > include/linux/overflow.h.
> >
> > I think I'm going to break that up into 2 or 3 patches:
> > 1. move is_signed_type from include/linux/overflow.h to perhaps
> > include/linux/typecheck.h.
> > 2. add div64_x64, div_x64, and div_64 to include/linux/math64.h, use
> > them in include/linux/overflow.h to fix GCC 4.9
> > 3. move multiply fallbacks out of
> > COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW for clang < 14.
> > --
> > Thanks,
> > ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

Re: linux-next: build failure while building Linus' tree

[Nick Desaulniers]

On Fri, Sep 10, 2021 at 3:26 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Sep 10, 2021 at 3:17 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Fri, Sep 10, 2021 at 3:02 PM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > On Thu, Sep 9, 2021 at 3:50 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > >
> > > > + Rasmus
> > > >
> > > > This was introduced in
> > > > commit f0907827a8a91 ("compiler.h: enable builtin overflow checkers and add
> > > > fallback code")
> > > > which added division using the `/` operator, which is problematic when checking
> > > > for overflows of 64b operands on 32b targets.
> > > >
> > > > We'll probably need helpers from linux/math64.h and some combination of
> > > > __builtin_choose_expr/__builtin_types_compatible_p.
> > > >
> > > > That will help us fix another compiler bug for older clang releases, too.
> > > > https://github.com/ClangBuiltLinux/linux/issues/1438.
> > >
> > > Ok, I have something hacked up that I think will work:
> > > https://gist.github.com/nickdesaulniers/2479818f4983bbf2d688cebbab435863
> >
> > hmm...playing around with adding some static asserts to the above, I
>
> ah! static_assert can't be used for the type agnostic macros,
> BUILD_BUG_ON needs to be used in its place. Ok, let me add a few and
> see if that can help instill some confidence here.

Ah, I just saw your comment now about raising the minimum required
version of gcc to 5.1.
https://github.com/ClangBuiltLinux/linux/issues/1438#issuecomment-916745801

> > > This incomplete diff is a little hacked up to reproduce the issue with
> > > a known-bad revision of clang that demonstrates a similar issue to GCC
> > > 4.9.  You can ignore the movement of check_mul_overflow and friends in
> > > include/linux/overflow.h.
> > >
> > > I think I'm going to break that up into 2 or 3 patches:
> > > 1. move is_signed_type from include/linux/overflow.h to perhaps
> > > include/linux/typecheck.h.
> > > 2. add div64_x64, div_x64, and div_64 to include/linux/math64.h, use
> > > them in include/linux/overflow.h to fix GCC 4.9
> > > 3. move multiply fallbacks out of
> > > COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW for clang < 14.

In that case, I do have __mulodi4() rewritten from compiler-rt to be
usable in the kernel.
-- 
Thanks,
~Nick Desaulniers