Re: [PATCH 4/7] minmax: Simplify signedness check

Re: [PATCH 4/7] minmax: Simplify signedness check

[kernel test robot]

Hi David,

kernel test robot noticed the following build errors:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v6.10 next-20240725]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-Put-all-the-clamp-definitions-together/20240724-224832
base:   linux/master
patch link:    https://lore.kernel.org/r/03601661326c4efba4e618ead15fa0e2%40AcuMS.aculab.com
patch subject: [PATCH 4/7] minmax: Simplify signedness check
config: mips-loongson1b_defconfig (https://download.01.org/0day-ci/archive/20240725/202407252100.fDFchC5O-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240725/202407252100.fDFchC5O-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407252100.fDFchC5O-lkp@intel.com/

All errors (new ones prefixed by >>):

>> crypto/skcipher.c:83:9: error: static assertion expression is not an integral constant expression
           return max(start, end_page);
                  ^~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:70:19: note: expanded from macro 'max'
   #define max(x, y)       __careful_cmp(max, x, y)
                           ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:56:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:49:17: note: expanded from macro '__cmp_once'
           _Static_assert(__types_ok(x, y),                        \
                          ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:38:2: note: expanded from macro '__types_ok'
           ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   crypto/skcipher.c:83:9: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression
   include/linux/minmax.h:70:19: note: expanded from macro 'max'
   #define max(x, y)       __careful_cmp(max, x, y)
                           ^
   include/linux/minmax.h:56:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
                   ^
   include/linux/minmax.h:49:17: note: expanded from macro '__cmp_once'
           _Static_assert(__types_ok(x, y),                        \
                          ^
   include/linux/minmax.h:38:4: note: expanded from macro '__types_ok'
           ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
             ^
   include/linux/minmax.h:34:27: note: expanded from macro '__is_ok_signed'
   #define __is_ok_signed(x) is_signed_type(typeof((x) + 0))
                             ^
   include/linux/compiler.h:273:32: note: expanded from macro 'is_signed_type'
   #define is_signed_type(type) (((type)(-1)) < (__force type)1)
                                  ^
   1 error generated.
--
>> lib/lzo/lzo1x_compress.c:53:33: error: static assertion expression is not an integral constant expression
                           const unsigned char *limit = min(ip_end, ip + MAX_ZERO_RUN_LENGTH + 1);
                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:63:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(min, x, y)
                           ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:56:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:49:17: note: expanded from macro '__cmp_once'
           _Static_assert(__types_ok(x, y),                        \
                          ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:38:2: note: expanded from macro '__types_ok'
           ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   lib/lzo/lzo1x_compress.c:53:33: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression
   include/linux/minmax.h:63:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(min, x, y)
                           ^
   include/linux/minmax.h:56:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
                   ^
   include/linux/minmax.h:49:17: note: expanded from macro '__cmp_once'
           _Static_assert(__types_ok(x, y),                        \
                          ^
   include/linux/minmax.h:38:4: note: expanded from macro '__types_ok'
           ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
             ^
   include/linux/minmax.h:34:27: note: expanded from macro '__is_ok_signed'
   #define __is_ok_signed(x) is_signed_type(typeof((x) + 0))
                             ^
   include/linux/compiler.h:273:32: note: expanded from macro 'is_signed_type'
   #define is_signed_type(type) (((type)(-1)) < (__force type)1)
                                  ^
   1 error generated.


vim +83 crypto/skcipher.c

b286d8b1a690667 Herbert Xu 2016-11-22  75  
b286d8b1a690667 Herbert Xu 2016-11-22  76  /* Get a spot of the specified length that does not straddle a page.
b286d8b1a690667 Herbert Xu 2016-11-22  77   * The caller needs to ensure that there is enough space for this operation.
b286d8b1a690667 Herbert Xu 2016-11-22  78   */
b286d8b1a690667 Herbert Xu 2016-11-22  79  static inline u8 *skcipher_get_spot(u8 *start, unsigned int len)
b286d8b1a690667 Herbert Xu 2016-11-22  80  {
b286d8b1a690667 Herbert Xu 2016-11-22  81  	u8 *end_page = (u8 *)(((unsigned long)(start + len - 1)) & PAGE_MASK);
b286d8b1a690667 Herbert Xu 2016-11-22  82  
b286d8b1a690667 Herbert Xu 2016-11-22 @83  	return max(start, end_page);
b286d8b1a690667 Herbert Xu 2016-11-22  84  }
b286d8b1a690667 Herbert Xu 2016-11-22  85  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH 4/7] minmax: Simplify signedness check

[David Laight]

From: kernel test robot <lkp@intel.com>
> Sent: 25 July 2024 14:24
> 
> Hi David,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on linux/master]
> [also build test ERROR on linus/master v6.10 next-20240725]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-Put-all-the-clamp-
> definitions-together/20240724-224832
> base:   linux/master
> patch link:    https://lore.kernel.org/r/03601661326c4efba4e618ead15fa0e2%40AcuMS.aculab.com
> patch subject: [PATCH 4/7] minmax: Simplify signedness check
> config: mips-loongson1b_defconfig (https://download.01.org/0day-
> ci/archive/20240725/202407252100.fDFchC5O-lkp@intel.com/config)
> compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project
> 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> reproduce (this is a W=1 build): (https://download.01.org/0day-
> ci/archive/20240725/202407252100.fDFchC5O-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202407252100.fDFchC5O-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
...
> b286d8b1a690667 Herbert Xu 2016-11-22  75
> b286d8b1a690667 Herbert Xu 2016-11-22  76  /* Get a spot of the specified length that does not straddle a page.
> b286d8b1a690667 Herbert Xu 2016-11-22  77   * The caller needs to ensure that there is enough space for this operation.
> b286d8b1a690667 Herbert Xu 2016-11-22  78   */
> b286d8b1a690667 Herbert Xu 2016-11-22  79  static inline u8 *skcipher_get_spot(u8 *start, unsigned int len)
> b286d8b1a690667 Herbert Xu 2016-11-22  80  {
> b286d8b1a690667 Herbert Xu 2016-11-22  81  	u8 *end_page = (u8 *)(((unsigned long)(start + len - 1)) & PAGE_MASK);
> b286d8b1a690667 Herbert Xu 2016-11-22  82
> b286d8b1a690667 Herbert Xu 2016-11-22 @83  	return max(start, end_page);
> b286d8b1a690667 Herbert Xu 2016-11-22  84  }
> b286d8b1a690667 Herbert Xu 2016-11-22  85

I thought this version supported that :-(
Certainly I've hit and fixed this before.
Using min/max() on pointers just makes it all more horrid.
The problem is that is_signed_type() isn't 'constant enough' for pointers.

Supporting pointers may require yet another (bloating) _is_constexpr() call.
Or add a simpler min_ptr() that includes a 'pointer' check.

But this might only be reported by clang with W=1.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)