Opened 11 years ago

Closed 11 years ago

#6193 closed Bugs (fixed)

lexical_cast overflow processing does not always work correctly

Reported by: David W. Birdsall <dave.birdsall@…> Owned by: nasonov
Milestone: To Be Determined Component: lexical_cast
Version: Boost 1.47.0 Severity: Regression
Keywords: lexical_cast overflow Cc: dave.birdsall@…

Description

In the attached program, supplying a really long non-zero number with lots of trailing zeros is not detected as an invalid number; instead no exception is thrown and a value of zero is returned for the parameter.

Here are some sample executions of the attached program. The first three executions behave correctly (note the third throws the expected exception); the fourth execution manifests the bug:

/home/birdsall>./temp.exe --number 16 The value this code thinks was supplied for --number was: 16 /home/birdsall>./temp.exe --number 1600000 The value this code thinks was supplied for --number was: 1600000 /home/birdsall>./temp.exe --number 160000000000 ERROR: in option 'number': invalid option value Allowed options:

--help produce this help message --number arg (=0) a number parameter

/home/birdsall>./temp.exe --number 1600000000000000000000000000000000000000 The value this code thinks was supplied for --number was: 0 /home/birdsall>

I found this bug in Boost 1.47.0, and the code snippets below assume that version. However, I checked the code in 1.48.0, and in the latest repository, and the bug exists there as well.

The bug does not exist in Boost 1.43.0; the code there functions correctly.

The bug seems to be in lexical_cast.hpp. In the 1.47.0 version, starting at line 702, is the following code:

while ( begin <= end )

{ T const new_sub_value = multiplier * 10 * (*end - czero);

if (*end < czero
*end >= czero + 10

/* detecting overflow */

new_sub_value/10 != multiplier * (*end - czero)
static_cast<T>((std::numeric_limits<T>::max)()-new_sub_value) < value

)

return false;

value += new_sub_value; multiplier *= 10; --end; }

The problem is, there is no check for overflow after the statement, "multiplier *= 10". If this statement is executed enough times, and overflows enough times, multiplier gets a value of zero. This defeats the overflow checking code earlier in the loop; once multiplier becomes zero, new_sub_value will be zero no matter what digit *end is. Therefore, value remains zero, new_sub_value/10 remains zero and multiplier * anything remains zero.

I fixed the bug in my own copy in the following way. I don't claim that this is the most elegant or efficient fix.

bool multiplier_overflow = false;

while ( begin <= end ) {

T const new_sub_value = multiplier * 10 * (*end - czero);

if (*end < czero
*end >= czero + 10

/* detecting overflow */

new_sub_value/10 != multiplier * (*end - czero)
static_cast<T>((std::numeric_limits<T>::max)()-new_sub_value) < value
(multiplier_overflow && (value != 0))

)

return false;

value += new_sub_value; T previous_multiplier = multiplier; multiplier *= 10; If we let multiplier =* 10 happen enough times (e.g., because of a long string of trailing zeroes), it will overflow multiple times and eventually happen to overflow to zero, defeating the overflow checks above. The check below prevents multiplier from becoming zero. if (multiplier/10 != previous_multiplier) {

multiplier_overflow = true; remember that multiplier overflowed multiplier = 10;

} --end;

}

A similar copy of the bug occurs earlier in the function, at lines 659 through 674.

Attachments (1)

temp.cpp (1.6 KB ) - added by David W. Birdsall <dave.birdsall@…> 11 years ago.
Sample program that demonstrates the defect

Download all attachments as: .zip

Change History (2)

by David W. Birdsall <dave.birdsall@…>, 11 years ago

Attachment: temp.cpp added

Sample program that demonstrates the defect

comment:1 by Antony Polukhin, 11 years ago

Resolution: fixed
Status: newclosed

(In [76318]) Fixes #6193

Note: See TracTickets for help on using tickets.