[Qemu-devel] [PATCH] block/qcow2.h: Avoid "1LL << 63" (shifts into sign bit)

[Qemu-devel] [PATCH] block/qcow2.h: Avoid "1LL << 63" (shifts into sign bit)

[Peter Maydell]

The expression "1LL << 63" tries to shift the 1 into the sign bit of a
'long long', which provokes a clang sanitizer warning:

runtime error: left shift of 1 by 63 places cannot be represented in type 'long long'

Use "1ULL << 63" as the definition of QCOW_OFLAG_COPIED instead
to avoid this. For consistency, we also update the other QCOW_OFLAG
definitions to use the ULL suffix rather than LL, though only the
shift by 63 is undefined behaviour.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/qcow2.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index dba9771..365a17e 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -40,11 +40,11 @@
 #define QCOW_MAX_CRYPT_CLUSTERS 32
 
 /* indicate that the refcount of the referenced cluster is exactly one. */
-#define QCOW_OFLAG_COPIED     (1LL << 63)
+#define QCOW_OFLAG_COPIED     (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
-#define QCOW_OFLAG_COMPRESSED (1LL << 62)
+#define QCOW_OFLAG_COMPRESSED (1ULL << 62)
 /* The cluster reads as all zeros */
-#define QCOW_OFLAG_ZERO (1LL << 0)
+#define QCOW_OFLAG_ZERO (1ULL << 0)
 
 #define REFCOUNT_SHIFT 1 /* refcount size is 2 bytes */
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] block/qcow2.h: Avoid "1LL << 63" (shifts into sign bit)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

On 08/23/2013 10:35 AM, Peter Maydell wrote:
> The expression "1LL << 63" tries to shift the 1 into the sign bit of a
> 'long long', which provokes a clang sanitizer warning:
> 
> runtime error: left shift of 1 by 63 places cannot be represented in type 'long long'

Yep, C99 6.5.7p3 states it is undefined to shift a signed number left
across the sign bit:

"The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
bits are filled with zeros. If E1 has an unsigned type, the value of the
result is E1 × 2E2 , reduced modulo one more than the maximum value
representable in the result type. If E1 has a signed type and
nonnegative value, and E1 × 2E2 is representable in the result type,
then that is the resulting value; otherwise, the behavior is undefined."

Qemu assumes twos-complement arithmetic with sane signed left shifts,
but without a way to tell the compiler our assumptions, it's easier to
just stick with well-defined unsigned shifts.

> 
> Use "1ULL << 63" as the definition of QCOW_OFLAG_COPIED instead
> to avoid this. For consistency, we also update the other QCOW_OFLAG
> definitions to use the ULL suffix rather than LL, though only the
> shift by 63 is undefined behaviour.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  block/qcow2.h |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] block/qcow2.h: Avoid "1LL << 63" (shifts into sign bit)

[Kevin Wolf]

Am 23.08.2013 um 18:35 hat Peter Maydell geschrieben:
> The expression "1LL << 63" tries to shift the 1 into the sign bit of a
> 'long long', which provokes a clang sanitizer warning:
> 
> runtime error: left shift of 1 by 63 places cannot be represented in type 'long long'
> 
> Use "1ULL << 63" as the definition of QCOW_OFLAG_COPIED instead
> to avoid this. For consistency, we also update the other QCOW_OFLAG
> definitions to use the ULL suffix rather than LL, though only the
> shift by 63 is undefined behaviour.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Thanks, applied to the block branch.

Kevin