linux-kernel - RE: [PATCH v1 1/4] iio: afe: rescale: Don't use ^ for booleans

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2a7a87267feb4c35ad9ef493236b6035@AcuMS.aculab.com>
Date: Fri, 6 Dec 2024 20:13:46 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Andy Shevchenko' <andriy.shevchenko@...ux.intel.com>
CC: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Peter Rosin
	<peda@...ntia.se>, Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen
	<lars@...afoo.de>
Subject: RE: [PATCH v1 1/4] iio: afe: rescale: Don't use ^ for booleans

From: 'Andy Shevchenko'
> Sent: 06 December 2024 15:20
...
> ...
> 
> > >  		 * If only one of the rescaler elements or the schan scale is
> > >  		 * negative, the combined scale is negative.
> > >  		 */
> > > -		if (neg ^ ((rescale->numerator < 0) ^ (rescale->denominator < 0))) {
> > > +		if (neg != (rescale->numerator < 0 || rescale->denominator < 0)) {
> >
> > That is wrong, the || would also need to be !=.
> 
> Why do you think so? Maybe it's comment(s) that is(are) wrong?

The old code certainly negates for each of them - so you can get the double
and triple negative cases.
So believe the code not the comment.
> 
> > Which will all generate real pile of horrid code.
> > (I think the x86 version will stun you.)
> 
> I think your remark is based on something, can you show the output to elaborate
> what exactly becomes horrible in this case?

Ok it isn't quite as bad as I thought because all the compilers will use xor
for the equality test on sign bits. See https://www.godbolt.org/z/qxz5KPcTh
So changing ^ to != makes no difference at all.

But f3() shows the sort of thing that can happen.
Sometimes made worse because the x86 SETcc instruction only set 8bit registers.
(Odd since they got added for the 386)

	David

> 
> > I'm guessing that somewhere there is a:
> > 	neg = value < 0;
> 
> Nope.
> 
> > Provided all the values are the same size (eg int/s32), in which case:
> > 	neg = value;
> > ...
> > 	if ((neg ^ rescale->numerator ^ rescale->denominator) < 0)
> > will be the desired test.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

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