[Skiboot] [PATCH 6/7] llvm-scan-build: fix result of << is undefined

Stewart Smith stewart at linux.vnet.ibm.com
Wed Nov 11 10:54:13 AEDT 2015


Benjamin Herrenschmidt <benh at au1.ibm.com> writes:
>> --- a/hw/fsp/fsp.c
>> +++ b/hw/fsp/fsp.c
>> @@ -198,11 +198,15 @@ static struct fsp *fsp_get_active(void)
>>  
>>  static u64 fsp_get_class_bit(u8 class)
>>  {
>> +	u8 shift;
>> +
>>  	/* Alias classes CE and CF as the FSP has a single queue */
>>  	if (class == FSP_MCLASS_IPL)
>>  		class = FSP_MCLASS_SERVICE;
>>  
>> -	return 1ul << (class - FSP_MCLASS_FIRST);
>> +	shift = class - FSP_MCLASS_FIRST;
>> +	assert(shift < 64);
>> +	return 1ul << shift;
>>  }
>
> And that's gross. Don't do that.
>
> If you want to bound check class, do so with something like
>
> assert(class >= FSP_MCLASS_FIRST && class < ...)
>
> But that added shift variable is just hurting my eyes.

It's kind of incredibly awful, and this seems to be a hoop to jump
through to shut LLVM up.

This doesn't squash the warning:
        assert((class - FSP_MCLASS_FIRST) < 64);

Which would be semantically the best.. but what's worse is looking at it
again, class is u8, and FSP_MCLASS_FIRST is defined to 0xce - which
means that expression is only ever going to be <=49 which is, amazingly
enough, well within the allowed number of shifs for u64.

I scream LLVM bug instead and will just drop this patch.



More information about the Skiboot mailing list