From 9dc0d640e89f40e93b27cf0a82217a3dfa4186a3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Mon, 2 May 2016 22:46:28 -0400 Subject: [PATCH] Fix: overflow of signed integer results in undefined behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The expression "min_value = -((int64_t)1 << (size - 1))" will result in a signed integer overflow when size is 64 ((1ULL << 63) > LONG_MAX). Note that larger sizes are unsupported and checked for in the setter. Signed overflows result in undefined behaviour and llvm takes advantage of this to optimize away the range check "if (value < min_value || value > max_value) {" Surprisingly, this was not catched by GCC, Coverity, scan-build or cppcheck. The fix consists in computing both bounds using an unsigned long long type and, in the case of the lower bound, negating it (resulting in a long long). Signed-off-by: Jérémie Galarneau --- formats/ctf/ir/fields.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/formats/ctf/ir/fields.c b/formats/ctf/ir/fields.c index 3ca57bb0..58f69fd4 100644 --- a/formats/ctf/ir/fields.c +++ b/formats/ctf/ir/fields.c @@ -928,8 +928,8 @@ int bt_ctf_field_signed_integer_set_value(struct bt_ctf_field *field, } size = integer_type->declaration.len; - min_value = -((int64_t)1 << (size - 1)); - max_value = ((int64_t)1 << (size - 1)) - 1; + min_value = -(1ULL << (size - 1)); + max_value = (1ULL << (size - 1)) - 1; if (value < min_value || value > max_value) { ret = -1; goto end; -- 2.34.1