* Re: [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63
2026-10-22 20:10 [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63 Peter Maydell
@ 2026-10-22 20:39 ` Daniel Henrique Barboza
2026-10-23 1:51 ` Chao Liu
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2026-10-22 20:39 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Liu Zhiwei, Chao Liu
On 10/22/2026 5:10 PM, Peter Maydell wrote:
> In prop_marchid_set() we set mxlen to either 32 or 64, and then
> calculate
> invalid_val = 1LL << (mxlen - 1);
>
> which shifts the signed value 1LL up into its sign bit when mxlen is
> 64, which Coverity complains about.
>
> This is not UB for QEMU because we use -fwrapv, but it makes more
> sense to use an explicitly unsigned 1ULL here when we clearly want to
> be treating it as an unsigned value and the 'invalid_val' variable is
> of type uint64_t.
>
> Resolves: Coverity CID 1547552
> Fixes: d6a427e2c0b24 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 08a4d14ea1f..d47a69bea7d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2298,7 +2298,7 @@ static void prop_marchid_set(Object *obj, Visitor *v, const char *name,
> g_assert_not_reached();
> }
>
> - invalid_val = 1LL << (mxlen - 1);
> + invalid_val = 1ULL << (mxlen - 1);
>
> if (value == invalid_val) {
> error_setg(errp, "Unable to set marchid with MSB (%u) bit set "
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63
2026-10-22 20:10 [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63 Peter Maydell
2026-10-22 20:39 ` Daniel Henrique Barboza
@ 2026-10-23 1:51 ` Chao Liu
2026-10-23 2:03 ` Alistair Francis
2026-10-23 2:06 ` Alistair Francis
3 siblings, 0 replies; 5+ messages in thread
From: Chao Liu @ 2026-10-23 1:51 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
On Thu, Oct 22, 2026 at 09:10:37PM +0800, Peter Maydell wrote:
> In prop_marchid_set() we set mxlen to either 32 or 64, and then
> calculate
> invalid_val = 1LL << (mxlen - 1);
>
> which shifts the signed value 1LL up into its sign bit when mxlen is
> 64, which Coverity complains about.
>
> This is not UB for QEMU because we use -fwrapv, but it makes more
> sense to use an explicitly unsigned 1ULL here when we clearly want to
> be treating it as an unsigned value and the 'invalid_val' variable is
> of type uint64_t.
>
> Resolves: Coverity CID 1547552
> Fixes: d6a427e2c0b24 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Chao Liu <chao.liu@processmission.com>
Thanks,
Chao
> ---
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 08a4d14ea1f..d47a69bea7d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2298,7 +2298,7 @@ static void prop_marchid_set(Object *obj, Visitor *v, const char *name,
> g_assert_not_reached();
> }
>
> - invalid_val = 1LL << (mxlen - 1);
> + invalid_val = 1ULL << (mxlen - 1);
>
> if (value == invalid_val) {
> error_setg(errp, "Unable to set marchid with MSB (%u) bit set "
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63
2026-10-22 20:10 [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63 Peter Maydell
2026-10-22 20:39 ` Daniel Henrique Barboza
2026-10-23 1:51 ` Chao Liu
@ 2026-10-23 2:03 ` Alistair Francis
2026-10-23 2:06 ` Alistair Francis
3 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2026-10-23 2:03 UTC (permalink / raw)
To: peter.maydell@linaro.org, qemu-devel@nongnu.org
Cc: qemu-riscv@nongnu.org, palmer@dabbelt.com,
daniel.barboza@oss.qualcomm.com, chao.liu@processmission.com,
liwei1518@gmail.com, zhiwei_liu@linux.alibaba.com
On Thu, 2026-10-22 at 21:10 +0100, Peter Maydell wrote:
> In prop_marchid_set() we set mxlen to either 32 or 64, and then
> calculate
> invalid_val = 1LL << (mxlen - 1);
>
> which shifts the signed value 1LL up into its sign bit when mxlen is
> 64, which Coverity complains about.
>
> This is not UB for QEMU because we use -fwrapv, but it makes more
> sense to use an explicitly unsigned 1ULL here when we clearly want to
> be treating it as an unsigned value and the 'invalid_val' variable is
> of type uint64_t.
>
> Resolves: Coverity CID 1547552
> Fixes: d6a427e2c0b24 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 08a4d14ea1f..d47a69bea7d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2298,7 +2298,7 @@ static void prop_marchid_set(Object *obj,
> Visitor *v, const char *name,
> g_assert_not_reached();
> }
>
> - invalid_val = 1LL << (mxlen - 1);
> + invalid_val = 1ULL << (mxlen - 1);
>
> if (value == invalid_val) {
> error_setg(errp, "Unable to set marchid with MSB (%u) bit
> set "
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63
2026-10-22 20:10 [PATCH] target/riscv: Use 1ULL for a value we're going to shift by 63 Peter Maydell
` (2 preceding siblings ...)
2026-10-23 2:03 ` Alistair Francis
@ 2026-10-23 2:06 ` Alistair Francis
3 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2026-10-23 2:06 UTC (permalink / raw)
To: peter.maydell@linaro.org, qemu-devel@nongnu.org
Cc: qemu-riscv@nongnu.org, palmer@dabbelt.com,
daniel.barboza@oss.qualcomm.com, chao.liu@processmission.com,
liwei1518@gmail.com, zhiwei_liu@linux.alibaba.com
On Thu, 2026-10-22 at 21:10 +0100, Peter Maydell wrote:
> In prop_marchid_set() we set mxlen to either 32 or 64, and then
> calculate
> invalid_val = 1LL << (mxlen - 1);
>
> which shifts the signed value 1LL up into its sign bit when mxlen is
> 64, which Coverity complains about.
>
> This is not UB for QEMU because we use -fwrapv, but it makes more
> sense to use an explicitly unsigned 1ULL here when we clearly want to
> be treating it as an unsigned value and the 'invalid_val' variable is
> of type uint64_t.
>
> Resolves: Coverity CID 1547552
> Fixes: d6a427e2c0b24 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 08a4d14ea1f..d47a69bea7d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2298,7 +2298,7 @@ static void prop_marchid_set(Object *obj,
> Visitor *v, const char *name,
> g_assert_not_reached();
> }
>
> - invalid_val = 1LL << (mxlen - 1);
> + invalid_val = 1ULL << (mxlen - 1);
>
> if (value == invalid_val) {
> error_setg(errp, "Unable to set marchid with MSB (%u) bit
> set "
^ permalink raw reply [flat|nested] 5+ messages in thread