aboutsummaryrefslogtreecommitdiff
path: root/tests/unit
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2023-05-22 14:04:40 -0500
committerEric Blake <eblake@redhat.com>2023-06-02 12:29:27 -0500
commitc25b1683443c6d658a82dc1c5587fdb0ae81663c (patch)
treee3d39baf7ced7022ba54e65943c970d63721443f /tests/unit
parentb87ac96651054fa89baab4e3a88a7feee7f92314 (diff)
downloadqemu-c25b1683443c6d658a82dc1c5587fdb0ae81663c.zip
qemu-c25b1683443c6d658a82dc1c5587fdb0ae81663c.tar.gz
qemu-c25b1683443c6d658a82dc1c5587fdb0ae81663c.tar.bz2
cutils: Improve qemu_strtod* error paths
Previous patches changed all integral qemu_strto*() error paths to guarantee that *value is never left uninitialized. Do likewise for qemu_strtod. Also, tighten qemu_strtod_finite() to never return a non-finite value (prior to this patch, we were rejecting "inf" with -EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE and HUGE_VAL - which is infinite on IEEE machines - despite our function claiming to recognize only finite values). Auditing callers, we have no external callers of qemu_strtod, and among the callers of qemu_strtod_finite: - qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and qapi/string-input-visitor.c:parse_type_number() which reject all errors (does not matter what we store) - utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points to '.' on all failures (that is, it is not distinguishing between EINVAL and ERANGE; and therefore still does the WRONG THING for "9.9e999". The change here does not entirely fix that (a later patch will tackle this more systematically), but at least it fixes the read-out-of-bounds first diagnosed in https://gitlab.com/qemu-project/qemu/-/issues/1629 - our testsuite, which we can update to match what we document Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> CC: qemu-stable@nongnu.org Message-Id: <20230522190441.64278-19-eblake@redhat.com>
Diffstat (limited to 'tests/unit')
-rw-r--r--tests/unit/test-cutils.c63
1 files changed, 36 insertions, 27 deletions
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 0e3215a..1ee410f 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2868,7 +2868,8 @@ static void test_qemu_strtod_einval(void)
res = 999;
err = qemu_strtod(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_null(endptr);
/* not recognizable */
@@ -3101,7 +3102,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* NULL */
@@ -3110,7 +3112,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_null(endptr);
/* not recognizable */
@@ -3119,7 +3122,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -3130,24 +3134,26 @@ static void test_qemu_strtod_finite_erange(void)
int err;
double res;
- /* overflow */
+ /* overflow turns into EINVAL */
str = "9e999";
endptr = "somewhere";
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
- g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpfloat(res, ==, HUGE_VAL);
- g_assert_true(endptr == str + 5);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str);
str = "-9e+999";
endptr = "somewhere";
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
- g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpfloat(res, ==, -HUGE_VAL);
- g_assert_true(endptr == str + 7);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str);
- /* underflow */
+ /* underflow is still possible */
str = "-9e-999";
endptr = "somewhere";
res = 999;
@@ -3172,7 +3178,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
str = "-infinity";
@@ -3180,7 +3187,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* not a number */
@@ -3189,7 +3197,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -3213,7 +3222,8 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_false(signbit(res));
/* trailing e is not an exponent */
str = ".5e";
@@ -3228,7 +3238,7 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.5);
/* trailing ( not part of long NaN */
str = "nan(";
@@ -3236,14 +3246,16 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
endptr = "somewhere";
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
}
static void test_qemu_strtod_finite_erange_junk(void)
@@ -3269,7 +3281,8 @@ static void test_qemu_strtod_finite_erange_junk(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
}
typedef int (*qemu_strtosz_fn)(const char *, const char **, uint64_t *);
@@ -3507,13 +3520,9 @@ static void test_qemu_strtosz_trailing(void)
0 /* FIXME EiB * 1.5 */, 0 /* FIXME 4 */,
-EINVAL, 0);
- /*
- * FIXME overflow in fraction is so buggy it can read beyond bounds
- * if we don't stuff extra \0 in our literal
- */
- do_strtosz_full("1.5E999\0\0" /* FIXME 1.5E999" */, qemu_strtosz,
- 0, 1 /* FIXME EiB * 1.5 */, 8 /* FIXME 4 */,
- 0 /* FIXME -EINVAL */, 1 /* FIXME 0 */);
+ /* FIXME overflow in fraction is still buggy */
+ do_strtosz_full("1.5E999", qemu_strtosz, 0, 1 /* FIXME EiB * 1.5 */,
+ 2 /* FIXME 4 */, -EINVAL, 0);
}
static void test_qemu_strtosz_erange(void)