On 07/06/2015 06:04 PM, Mikhail Maltsev wrote:
On 07.07.2015 1:55, Jeff Law wrote:
len = d_number (di);
- if (len <= 0)
+ if (len <= 0 || len > INT_MAX)
return NULL;
ret = d_identifier (di, len);
di->last_name = ret;
Isn't this only helpful if sizeof (long) > sizeof (int)? Otherwise the
compiler is going to eliminate that newly added test, right?
So with that in mind, what happens on i686-unknown-linux with this test?
Jeff
Probably it should be fine, because the problem occurred when len became
negative after implicit conversion to int (d_identifier does not check
for negative length, but it does check that length does not exceed total
string length). In this case (i.e. on ILP32 targets) len will not change
sign after conversion to int (because it's a no-op).
I'm not completely sure about compiler warnings, but AFAIR, in multilib
build libiberty is also built for 32-bit target, and I did not get any
additional warnings.
You may need -Wtype-limits to see the warning.
I'm not questioning whether or not the test will cause a problem, but
instead questioning if the test does what you expect it to do on a 32bit
host.
On a host where sizeof (int) == sizeof (long), that len > INT_MAX test
is always going to be false.
If you want to do overflow testing, you have to compute len in a wider
type. You might consider using "long long" or "int64_t" depending on
the outcome of a configure test. Falling back to a simple "long" if the
host compiler doesn't have "long long" or "int64_t".
Interesting exercise feeding those tests into demangle.com :-0 A
suitably interested party might be able to exploit that overflow.
jeff
