[PATCH] Fix float to LONGEST conversion.
Ali Tamur via gdb-patches
gdb-patches@sourceware.org
Mon Sep 9 02:36:00 GMT 2019
Hi,
Can I submit if there aren't any other concerns?
Thanks,
Ali
On Wed, Sep 4, 2019 at 2:28 PM Ali Tamur <tamur@google.com> wrote:
>
> Updated the commit message:
> >
> Fix float to LONGEST conversion.
>
> The code used to have undefined behaviour when template parameter is float and
> host_float is NaN, because it attempted to convert NaN value to LONGEST at the
> last statement. This frequently caused crashes on tests that checked "info
> all-registers" (at least when the code is compiled with clang; I didn't test
> with gdb).
>
> On Wed, Aug 28, 2019 at 1:29 AM Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>>
>> * Ali Tamur via gdb-patches <gdb-patches@sourceware.org> [2019-08-27 20:28:08 -0700]:
>>
>> > The code used to have undefined behaviour.
>>
>> I would much prefer to see a more detailed explanation for _why_ the
>> previous behaviour is undefined. I ran my eye over the old code but
>> didn't see anything obvious.
>>
>> If at a later date I come back and want to figure out why this patch
>> went in it would be nice if the commit message could tell me
>> everything I need to know.
>>
>> Thanks,
>> Andrew
>>
>>
>>
>> >
>> >
>> > gdb/ChangeLog:
>> > *gdb/target-float.c (host_float_ops<T>::to_longest): Update
>> > implementation.
>> > ---
>> > gdb/target-float.c | 17 +++++++++++------
>> > 1 file changed, 11 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/gdb/target-float.c b/gdb/target-float.c
>> > index 39abb12696..0fd71c0dc3 100644
>> > --- a/gdb/target-float.c
>> > +++ b/gdb/target-float.c
>> > @@ -1007,13 +1007,18 @@ host_float_ops<T>::to_longest (const gdb_byte *addr,
>> > {
>> > T host_float;
>> > from_target (type, addr, &host_float);
>> > - /* Converting an out-of-range value is undefined behavior in C, but we
>> > - prefer to return a defined value here. */
>> > - if (host_float > std::numeric_limits<LONGEST>::max())
>> > - return std::numeric_limits<LONGEST>::max();
>> > - if (host_float < std::numeric_limits<LONGEST>::min())
>> > + T min_possible_range = static_cast<T>(std::numeric_limits<LONGEST>::min());
>> > + T max_possible_range = -min_possible_range;
>> > + /* host_float can be converted to an integer as long as it's in
>> > + the range [min_possible_range, max_possible_range). If not, it is either
>> > + too large, or too small, or is NaN; in this case return the maximum or
>> > + minimum possible value. */
>> > + if (host_float < max_possible_range && host_float >= min_possible_range)
>> > + return static_cast<LONGEST> (host_float);
>> > + if (host_float < min_possible_range)
>> > return std::numeric_limits<LONGEST>::min();
>> > - return (LONGEST) host_float;
>> > + /* This line will be executed if host_float is NaN. */
>> > + return std::numeric_limits<LONGEST>::max();
>> > }
>> >
>> > /* Convert signed integer VAL to a target floating-number of type TYPE
>> > --
>> > 2.23.0.187.g17f5b7556c-goog
>> >
More information about the Gdb-patches
mailing list