[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Possible undefined behavior in IOS
From: |
Jose E. Marchesi |
Subject: |
Re: Possible undefined behavior in IOS |
Date: |
Sun, 02 May 2021 23:54:01 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
>> GCC -fanalyzer reported the following warnings:
>>
>> 7326:../../libpoke/ios.c:700:35: warning: shift by count ('64') >= precision
>> of type ('64')
>> 7382:../../libpoke/ios.c:1478:19: warning: shift by count ('64') >=
>> precision of type ('64')
>>
>> These reference code in ios.c like this:
>>
>> /* We should shift to fill the least significant byte
>> which is the last 8 bits. */
>> *value = ((uint64_t) c[0] << (56 + lastbyte_bits))
>> | ((uint64_t) c[1] << (48 + lastbyte_bits))
>> | ((uint64_t) c[2] << (40 + lastbyte_bits))
>> | ((uint64_t) c[3] << (32 + lastbyte_bits))
>> | ((uint64_t) c[4] << (24 + lastbyte_bits))
>> | ((uint64_t) c[5] << (16 + lastbyte_bits))
>> | (c[6] << (8 + lastbyte_bits)) | (c[7] << lastbyte_bits)
>> | (c[8] >> (8 - lastbyte_bits));
>>
>> Note how the code above incurs in UB when lastbyte_bits >= 8. I suppose
>> that this is a false positive and that in these two particular locations
>> lastbyte_bits can't be 8, but it would be good to double-check it.
>>
>> Could you please take a look?
>> Thanks!
>
> Right.
>
> /lastbyte_bits/ keeps the number of bits (that we want to read) in the
> last byte. In general, it is a number between 1 and 8.
>
> The two cases reported by the compiler are cases where we want to read
> or write 9 bytes in total. *Assuming that the '**/bits/**' to be read
> or write can be at most 64 in these functions*, the warning is not
> legitimate. Because when 64 consecutive bits span 9 bytes in total,
> the last byte cannot be holding 8 of those bits. Therefore, the value
> of /lastbyte_bits/ cannot be 8 in these two specific cases.
>
> Based on the assumption above, these two warnings are false positives.
Excellent!
Thanks for checking this :)