On 10/3/26 13:47, BALATON Zoltan wrote:
On Tue, 10 Mar 2026, Philippe Mathieu-Daudé wrote:
On 9/3/26 23:59, BALATON Zoltan wrote:
On Mon, 9 Mar 2026, Philippe Mathieu-Daudé wrote:
On 9/3/26 22:41, BALATON Zoltan wrote:
On Mon, 9 Mar 2026, Chad Jablonski wrote:
Cast default_sc_bottom to uint32_t before shifting left to prevent
sign extension when assigning to uint64_t.
Fixes: CID 1645615
Signed-off-by: Chad Jablonski <[email protected]>
---
hw/display/ati.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 6cf243bcf9..f9773e1154 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -514,7 +514,7 @@ static uint64_t ati_mm_read(void *opaque,
hwaddr addr, unsigned int size)
val |= s->regs.default_tile << 16;
break;
case DEFAULT_SC_BOTTOM_RIGHT:
- val = (s->regs.default_sc_bottom << 16) |
+ val = ((uint32_t)s->regs.default_sc_bottom << 16) |
s->regs.default_sc_right;
What about using deposit64() which is uncontroversial?
val = deposit64(s->regs.default_sc_right, 16,
13, s->regs.default_sc_bottom);
(alternatively deposit32)
I can't make sense of that without going and looking what the
arguments of this function are so I'd rather keep it simple. As I
said, the values here can't be large enough to sign extend and there
may be other similar cases that are already marked in Coverity. But
if we still want to fix all of them then maybe we could try to make
val uint32_t then do something about it once at the return for all
cases. That seems better than trying to handle in each case
inconsistently. The simplest would still be to tell Converity it's
wrong here.
Until the next register is implemented and Coverity complains again,
and another round-up on the mailing list until Peter manually marks it.
I was hoping for a way to avoid all that hassle.
Changing local val to uint32_t and cast it at the last return line
should also solve this if the issue is assigning to uint64_t without a
cast. To me that seems better than making changes to every case
separately and would allow me to avoid additional complexity. If you
think it would help I can make a patch but I can't test it.
No more time / energy for this topic, so do what you think is best for
the project, I don't mind much anymore. I shouldn't have intervened at
all first, but since I merged this patch, I felt a bit concerned.
Actually the simplest is you mark that issue yourself on Coverity;
anybody can request an account for free. I'm done with this thread.
Thanks,
Phil.
