On Tue, 10 Mar 2026, Peter Maydell wrote:
On Tue, 10 Mar 2026 at 14:28, BALATON Zoltan <[email protected]> wrote:
I did not know I can get access to Coverity. Then I can take a look there.
I still think there's nothing to fix here because even if uint16_t
default_sc_bottom is promoted to int for the calculation we don't support
32 bit hosts any more so this can't get sign extension issue even in that
case. So I think Converity is just confused here and we should not make
code more complex for it.
None of the types here depend on whether the host is 32 bit or 64 bit:
* default_sc_bottom is "uint16_t", which is always 16 bits unsigned
* (default_sc_bottom << 16) is "int", which is 32-bit and signed
on any platform we have ever cared about
Yes you're right this is what causes the warning here.
* val is "uint64_t", which is always 64 bits unsigned
So if default_sc_bottom bit 15 is set (0x8000) then we will end up
with an "int" value 0x8000_0000 (shift by 16 into the sign bit),
which then gets sign-extended to give a uint64_t
0xffff_ffff_8000_0000 when it is assigned to val.
Coverity warns about this because "I took an unsigned type
and shifted it right and assigned it to a larger unsigned type,
and in the middle of that it got handled as signed" is one
of C's unintuitive corners.
The thing that makes this specific case not a bug is that the value
of default_sc_bottom is always masked so that bit 15 is zero
(which is not something Coverity's analysis is capable of
spotting, because it's too far removed from the site of the
sign-extension).
We are about 50:50 on whether we prefer to mark these as
false-positives or else to insert a cast or other thing
to enforce that the intermediate type in the shift is
unsigned.
If we want to fix it I think making val uint32_t would be better than a
cast just in this one case as that would avoid all similar problems in
other cases too, wouldn't it?
Regards,
BALATON Zoltan
