diff options
author | Joel Brobecker <brobecker@adacore.com> | 2020-12-05 23:56:59 -0500 |
---|---|---|
committer | Joel Brobecker <brobecker@adacore.com> | 2020-12-05 23:56:59 -0500 |
commit | 63c457b911043aa6ebf3558e2d2050ee53d28a8e (patch) | |
tree | c398cb09dc5e8fa3c217f18bd907cfa08a9cff99 /gdb/unittests | |
parent | 6b1dce3a3d0c853dc11db2f942038af792cb7b3f (diff) | |
download | gdb-63c457b911043aa6ebf3558e2d2050ee53d28a8e.zip gdb-63c457b911043aa6ebf3558e2d2050ee53d28a8e.tar.gz gdb-63c457b911043aa6ebf3558e2d2050ee53d28a8e.tar.bz2 |
gmp-utils: protect gdb_mpz exports against out-of-range values
The gdb_mpz class currently provides a couple of methods which
essentially export an mpz_t value into either a buffer, or an integral
type. The export is based on using the mpz_export function which
we discovered can be a bit treacherous if used without caution.
In particular, the initial motivation for this patch was to catch
situations where the mpz_t value was so large that it would not fit
in the destination area. mpz_export does not know the size of
the buffer, and therefore can happily write past the end of our buffer.
While designing a solution to the above problem, I also discovered
that we also needed to be careful when exporting signed numbers.
In particular, numbers which are larger than the maximum value
for a given signed type size, but no so large as to fit in the
*unsigned* version with the same size, would end up being exported
incorrectly. This is related to the fact that mpz_export ignores
the sign of the value being exportd, and assumes an unsigned export.
Thus, for such large values, the appears as if mpz_export is able
to fit our value into our buffer, but in fact, it does not.
Also, I noticed that gdb_mpz::write wasn't taking its unsigned_p
parameter, which was a hole.
For all these reasons, a new low-level private method called
"safe_export" has been added to class gdb_mpz, whose goal is
to perform all necessary checks and manipulations for a safe
and correct export. As a bonus, this method allows us to factorize
the handling of negative value exports.
The gdb_mpz::as_integer and gdb_mpz::write methods are then simplified
to take advantage of this new safe_export method.
gdb/ChangeLog:
* gmp-utils.h (gdb_mpz::safe_export): New private method.
(gdb_mpz::as_integer): Reimplement using gdb_mpz::safe_export.
* gmp-utils.c (gdb_mpz::write): Rewrite using gdb_mpz::safe_export.
(gdb_mpz::safe_export): New method.
* unittests/gmp-utils-selftests .c (gdb_mpz_as_integer):
Update function description.
(check_as_integer_raises_out_of_range_error): New function.
(gdb_mpz_as_integer_out_of_range): New function.
(_initialize_gmp_utils_selftests): Register
gdb_mpz_as_integer_out_of_range as a selftest.
Diffstat (limited to 'gdb/unittests')
-rw-r--r-- | gdb/unittests/gmp-utils-selftests.c | 71 |
1 files changed, 68 insertions, 3 deletions
diff --git a/gdb/unittests/gmp-utils-selftests.c b/gdb/unittests/gmp-utils-selftests.c index 1365905..30c1902 100644 --- a/gdb/unittests/gmp-utils-selftests.c +++ b/gdb/unittests/gmp-utils-selftests.c @@ -26,9 +26,10 @@ namespace selftests { /* Perform a series of general tests of gdb_mpz's as_integer method. - This function tries to be reasonably exhaustive, by testing the edges, - as well as a resonable set of values including negative ones, zero, - and positive values. */ + This function limits itself to values which are in range (out-of-range + values will be tested separately). In doing so, it tries to be reasonably + exhaustive, by testing the edges, as well as a resonable set of values + including negative ones, zero, and positive values. */ static void gdb_mpz_as_integer () @@ -80,6 +81,68 @@ gdb_mpz_as_integer () SELF_CHECK (v.as_integer<ULONGEST> () == ul_expected); } +/* A helper function which calls the given gdb_mpz object's as_integer + method with the given type T, and verifies that this triggers + an error due to VAL's value being out of range for type T. */ + +template<typename T, typename = gdb::Requires<std::is_integral<T>>> +static void +check_as_integer_raises_out_of_range_error (const gdb_mpz &val) +{ + try + { + val.as_integer<T> (); + } + catch (const gdb_exception_error &ex) + { + SELF_CHECK (ex.reason == RETURN_ERROR); + SELF_CHECK (ex.error == GENERIC_ERROR); + SELF_CHECK (strstr (ex.what (), "Cannot export value") != nullptr); + return; + } + /* The expected exception did not get raised. */ + SELF_CHECK (false); +} + +/* Perform out-of-range tests of gdb_mpz's as_integer method. + + The goal of this function is to verify that gdb_mpz::as_integer + handles out-of-range values correctly. */ + +static void +gdb_mpz_as_integer_out_of_range () +{ + gdb_mpz v; + + /* Try LONGEST_MIN minus 1. */ + mpz_ui_pow_ui (v.val, 2, sizeof (LONGEST) * 8 - 1); + mpz_neg (v.val, v.val); + mpz_sub_ui (v.val, v.val, 1); + + check_as_integer_raises_out_of_range_error<ULONGEST> (v); + check_as_integer_raises_out_of_range_error<LONGEST> (v); + + /* Try negative one (-1). */ + v = -1; + + check_as_integer_raises_out_of_range_error<ULONGEST> (v); + SELF_CHECK (v.as_integer<LONGEST> () == (LONGEST) -1); + + /* Try LONGEST_MAX plus 1. */ + v = LONGEST_MAX; + mpz_add_ui (v.val, v.val, 1); + + SELF_CHECK (v.as_integer<ULONGEST> () == (ULONGEST) LONGEST_MAX + 1); + check_as_integer_raises_out_of_range_error<LONGEST> (v); + + /* Try ULONGEST_MAX plus 1. */ + v = ULONGEST_MAX; + mpz_add_ui (v.val, v.val, 1); + + check_as_integer_raises_out_of_range_error<ULONGEST> (v); + check_as_integer_raises_out_of_range_error<LONGEST> (v); +} + /* A helper function to store the given integer value into a buffer, before reading it back into a gdb_mpz. Sets ACTUAL to the value read back, while at the same time setting EXPECTED as the value @@ -445,6 +508,8 @@ _initialize_gmp_utils_selftests () { selftests::register_test ("gdb_mpz_as_integer", selftests::gdb_mpz_as_integer); + selftests::register_test ("gdb_mpz_as_integer_out_of_range", + selftests::gdb_mpz_as_integer_out_of_range); selftests::register_test ("gdb_mpz_read_all_from_small", selftests::gdb_mpz_read_all_from_small); selftests::register_test ("gdb_mpz_read_min_max", |