Re: [crng-random:jd/get_random_u32_below 23/23] include/linux/random.h:64:69: sparse: sparse: cast truncates bits from constant value (1f4 becomes f4)
From: Jason A. Donenfeld
Date: Tue Oct 11 2022 - 19:31:17 EST
On Tue, Oct 11, 2022 at 11:24:50PM +0000, David Laight wrote:
> From: Jason A. Donenfeld
> > Sent: 10 October 2022 18:54
> >
> > On Mon, Oct 10, 2022 at 05:18:40PM +0000, David Laight wrote:
> > > From: kernel test robot <lkp@xxxxxxxxx>
> > > > Sent: 10 October 2022 00:32
> > > > To: Jason A. Donenfeld <zx2c4@xxxxxxxxxx>
> > > ...
> > >
> > > I'm missing the main mailing list email for this change.
> > > I'm guessing the non-inlined code for non-constant ceil
> > > is similar.
> >
> > It's part of a development tree I already linked you to. It's not done
> > yet. This alert is just about needing a __force.
>
> I keep thinking about this one.
> sparse is being stupid because it is looking at code that cade be reached.
> 'ceil' must be 500 so the test at line 62 is false.
> But what is the cast for?
> Line 64 is only executed when ceil is <= 256 and the condition
> only matters when ceil <= 255 - so the cast cannot be needed.
Yes, it's sparse being stupid.
> I think you might be trying to do 8-bit arithmetic - but you
> should know it gets promoted to 'int' (and then to unsigned int).
> What might do what you intended is (u8)~ceil % ceil.
> But that is the same as (0xff - ceil) % ceil.
Right. This already came up in a different thread and I made that change
already.
> Which is the same as 0xff % ceil.
Same as 0x100 % ceil, rather. It's compile-time evaluation though, keep
in mind, so it doesn't matter.
> A quick check with ceil == 5 gives the wrong answer
> as all values get accepted - and one need to be excluded.
> So you either need (mult & 0xff) > 0xff % ceil
> or (mult & 0xff) >= 0x100 % ceil.
Yes.
Jason