Re: [gpio:ib-for-each-clump 4/4] include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long

[Syed Nayyar Waris <syednwaris@gmail.com>]

On Fri, Jul 17, 2020 at 2:55 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Jul 16, 2020 at 11:13 PM kernel test robot <lkp@intel.com> wrote:
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-for-each-clump
> > head:   3358c938236d6a1be51124fbbb2698e50689d382
> > commit: 3358c938236d6a1be51124fbbb2698e50689d382 [4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set_value.
> > config: alpha-randconfig-s031-20200716 (attached as .config)
> > compiler: alpha-linux-gcc (GCC) 9.3.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # apt-get install sparse
> >         # sparse version: v0.6.2-49-g707c5017-dirty
> >         git checkout 3358c938236d6a1be51124fbbb2698e50689d382
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=alpha
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> >
> > sparse warnings: (new ones prefixed by >>)
> >
> > >> include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long
> > >> include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long
> >    include/linux/bitmap.h:594:63: sparse: sparse: shift too big (64) for type unsigned long
> > >> include/linux/bitmap.h:639:45: sparse: sparse: shift too big (64) for type unsigned long
> > >> include/linux/bitmap.h:638:17: sparse: sparse: invalid access past the end of 'old' (8 8)
> >
> > vim +639 include/linux/bitmap.h
> >
> > 169c474fb22d8a5 William Breathitt Gray 2019-12-04  613
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  614  /**
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  615   * bitmap_set_value - set n-bit value within a memory region
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  616   * @map: address to the bitmap memory region
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  617   * @value: value of nbits
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  618   * @start: bit offset of the n-bit value
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  619   * @nbits: size of value in bits
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  620   */
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  621  static inline void bitmap_set_value(unsigned long *map,
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  622                                      unsigned long value,
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  623                                      unsigned long start, unsigned long nbits)
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  624  {
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  625          const size_t index = BIT_WORD(start);
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  626          const unsigned long offset = start % BITS_PER_LONG;
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  627          const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  628          const unsigned long space = ceiling - start;
>
> If start == 0:
>   index = 0, offset = 0, ceiling = 64, space = 64
>
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  629
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  630          value &= GENMASK(nbits - 1, 0);
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  631
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  632          if (space >= nbits) {
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  633                  map[index] &= ~(GENMASK(nbits + offset - 1, offset));
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  634                  map[index] |= value << offset;
>
> if nbits > space...
>
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  635          } else {
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  636                  map[index] &= ~BITMAP_FIRST_WORD_MASK(start);
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  637                  map[index] |= value << offset;
>
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27 @638                  map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27 @639                  map[index + 1] |= (value >> space);
>
> space = 64...
>
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  640          }
> > e77c9b6f35c4bdf Syed Nayyar Waris      2020-06-27  641  }
>
> I don't see the test case for this. Can you provide one?
>
>
> --
> With Best Regards,
> Andy Shevchenko

Thanks Andy, but actually, I intentionally want and need 'space' to be
64, in the scenario (situation) mentioned above.

I think this same sparse warning was discussed earlier also, when this
same warning was reported by the buildbot in an earlier version of
patch. Later it was clarified by the sparse-check maintainer that this
warning is to be ignored and no code fix is required.

https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2202377.html

Regarding your question on the test case. Actually a 64 bit clump size
test is not added because the tests will always run; and when it runs
on machine where BITS_PER_LONG is 32 bits, it will fail, because in
'for_each_set_clump', it is needed that clump size is less than
BITS_PER_LONG.

I hope I have not misunderstood something.

Regards
Syed Nayyar Waris