diff mbox

block/qcow2.h: Avoid "1LL << 63" (shifts into sign bit)

Message ID 1377275745-8942-1-git-send-email-peter.maydell@linaro.org
State Accepted
Commit 127c84e1a52f11bf418cc2d3bf804da5091a190a
Headers show

Commit Message

Peter Maydell Aug. 23, 2013, 4:35 p.m. UTC
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(-)

Comments

Eric Blake Aug. 23, 2013, 4:41 p.m. UTC | #1
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>
Kevin Wolf Aug. 28, 2013, 9:15 a.m. UTC | #2
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
diff mbox

Patch

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 */