2013-08-15 12:02:50

by Jan-Simon Möller

[permalink] [raw]
Subject: [RFC] [PATCH] Fix for a warning - crypto/fcrypt,c

Hi all,

please merge the attached patch.

Fix for warning:
linux/crypto/fcrypt.c:143:47: warning: signed shift result (0x598000000)
requires 36 bits to
represent, but 'int' only has 32 bits [-Wshift-overflow]
Z(0xef), Z(0x70), Z(0xcf), Z(0xc2), Z(0x2a), Z(0xb3), Z(0x61),
Z(0xad),
^~~~~~~
linux/crypto/fcrypt.c:113:29: note: expanded from macro 'Z'
#define Z(x) cpu_to_be32((x << 27 ) | (x >> 5))
^ ~~
linux/include/uapi/linux/byteorder/little_endian.h:38:53: note: expanded from
macro
'__cpu_to_be32'
#define __cpu_to_be32(x) ((__force __be32)__swab32((x)))
^
linux/include/uapi/linux/swab.h:116:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
linux/include/uapi/linux/swab.h:18:12: note: expanded from macro
'___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^

Solution - make sure we don't exceed the 32 bit range by adding (x & ~(1U <<
27))
= & (0xF7FFFFFF)_16 or & (1111 0111 1111 1111 1111 1111 1111 1111)_2


Author: PaX Team <pageexec at freemail.hu>
ML-Post: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120507/142707.html
URL: http://llvm.linuxfoundation.org

Merge: Jan-Simon M?ller <dl9pf at gmx.de>



Best,
--

Dipl.-Ing.
Jan-Simon M?ller

[email protected]


Attachments:
0044-Fix-bitoperation-for-compilation-with-clang.patch (2.31 kB)

2013-08-21 10:47:26

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC] [PATCH] Fix for a warning - crypto/fcrypt,c

On Thu, Aug 15, 2013 at 02:01:50PM +0200, Jan-Simon M?ller wrote:
> Hi all,
>
> please merge the attached patch.
>
> Fix for warning:
> linux/crypto/fcrypt.c:143:47: warning: signed shift result (0x598000000)
> requires 36 bits to
> represent, but 'int' only has 32 bits [-Wshift-overflow]
> Z(0xef), Z(0x70), Z(0xcf), Z(0xc2), Z(0x2a), Z(0xb3), Z(0x61),
> Z(0xad),
> ^~~~~~~
> linux/crypto/fcrypt.c:113:29: note: expanded from macro 'Z'
> #define Z(x) cpu_to_be32((x << 27 ) | (x >> 5))
> ^ ~~
> linux/include/uapi/linux/byteorder/little_endian.h:38:53: note: expanded from
> macro
> '__cpu_to_be32'
> #define __cpu_to_be32(x) ((__force __be32)__swab32((x)))
> ^
> linux/include/uapi/linux/swab.h:116:21: note: expanded from macro '__swab32'
> ___constant_swab32(x) : \
> ^
> linux/include/uapi/linux/swab.h:18:12: note: expanded from macro
> '___constant_swab32'
> (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
> ^
>
> Solution - make sure we don't exceed the 32 bit range by adding (x & ~(1U <<
> 27))
> = & (0xF7FFFFFF)_16 or & (1111 0111 1111 1111 1111 1111 1111 1111)_2

Your fix makes no sense. To achieve what you want you'd need to lop
off the first 5 bits, not the fifth bit.

However, which compiler is this? This warning seems to be rather
pointless.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2013-08-21 16:37:50

by PaX Team

[permalink] [raw]
Subject: Re: [RFC] [PATCH] Fix for a warning - crypto/fcrypt,c

On 21 Aug 2013 at 20:47, Herbert Xu wrote:

> On Thu, Aug 15, 2013 at 02:01:50PM +0200, Jan-Simon Möller wrote:
> > Solution - make sure we don't exceed the 32 bit range by adding (x & ~(1U <<
> > 27))
> > = & (0xF7FFFFFF)_16 or & (1111 0111 1111 1111 1111 1111 1111 1111)_2
>
> Your fix makes no sense. To achieve what you want you'd need to lop
> off the first 5 bits, not the fifth bit.

oops, the bitmask expression wanted to be (1U << 27) - 1) instead.
another approach would be to simply cast the macro argument to __be32.

> However, which compiler is this? This warning seems to be rather
> pointless.

it's clang and this warning is about an undefined behaviour because
the hexadecimal constants passed to this macro are treated as signed
ints and for some of the constants the result of the shift cannot be
represented as a signed int.

2013-08-21 20:43:20

by Jan-Simon Möller

[permalink] [raw]
Subject: Re: [RFC] [PATCH] Fix for a warning - crypto/fcrypt,c


Should I resend a fixed version with
(1U << 27) - 1) instead ?

Best,
JS

On Thursday 15 August 2013 14:01:50 Jan-Simon M?ller wrote:
> Hi all,
>
> please merge the attached patch.
>
> Fix for warning:
> linux/crypto/fcrypt.c:143:47: warning: signed shift result (0x598000000)
> requires 36 bits to
> represent, but 'int' only has 32 bits [-Wshift-overflow]
> Z(0xef), Z(0x70), Z(0xcf), Z(0xc2), Z(0x2a), Z(0xb3), Z(0x61),
> Z(0xad),
> ^~~~~~~
> linux/crypto/fcrypt.c:113:29: note: expanded from macro 'Z'
> #define Z(x) cpu_to_be32((x << 27 ) | (x >> 5))
> ^ ~~
> linux/include/uapi/linux/byteorder/little_endian.h:38:53: note: expanded
> from macro
> '__cpu_to_be32'
> #define __cpu_to_be32(x) ((__force __be32)__swab32((x)))
> ^
> linux/include/uapi/linux/swab.h:116:21: note: expanded from macro '__swab32'
> ___constant_swab32(x) : \
> ^
> linux/include/uapi/linux/swab.h:18:12: note: expanded from macro
> '___constant_swab32'
> (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
> ^
>
> Solution - make sure we don't exceed the 32 bit range by adding (x & ~(1U <<
> 27))
> = & (0xF7FFFFFF)_16 or & (1111 0111 1111 1111 1111 1111 1111 1111)_2
>
>
> Author: PaX Team <pageexec at freemail.hu>
> ML-Post:
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120507/142707
> .html URL: http://llvm.linuxfoundation.org
>
> Merge: Jan-Simon M?ller <dl9pf at gmx.de>
>
>
>
> Best,

2013-08-26 06:04:34

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC] [PATCH] Fix for a warning - crypto/fcrypt,c

On Wed, Aug 21, 2013 at 10:42:01PM +0200, Jan-Simon M?ller wrote:
>
> Should I resend a fixed version with
> (1U << 27) - 1) instead ?

Sure.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt