[Date Prev][Date Next][Thread Prev][Thread Next]
- Subject: Re: string.unpack segfaulting due to integer overflow and mixed sign comparisons
- From: William Ahern <william@...>
- Date: Wed, 27 Jun 2012 21:02:04 -0700
On Wed, Jun 27, 2012 at 05:37:19PM -0700, Sam Roberts wrote:
> On Wed, Jun 27, 2012 at 1:14 AM, Chris Emerson
> <firstname.lastname@example.org> wrote:
> >> - if (i+N>len) goto done;
> >> + if (i+N < i || i+N>len) goto done;
> > I don't think this is right - in C overflow of signed integers is undefined
> > behaviour. The C compiler can (and some now do) assume that "i+N < i"
> > (with N positive) can't happen, and that test can be optimised out.
> Interesting, is this true for unsigned, too?
> I hope that modular addition is well-defined over uint32_t, I use it a
> bunch, and haven't had any problems, yet, but maybe haven't hit the
> "right" kind of optomizing compiler.
Unsigned arithmetic is well-defined on overflow (or rather, unsigned types
cannot overflow at all because unsigned arithmetic is specifically defined
as modulo 2^N). I seem to remember someone on comp.lang.c or comp.std.c who
recently claimed that he used a modern C compiler (albeit for legacy
hardware) which had to emulate modulo arithemtic in software. You could
disable compliant mode for considerable performance gains.
C99 added several guarantees about object and value bit representations for
unsigned integers generally, and the extended fixed-width types especially.
But I don't think there was any existing implementation at the time that
would have been penalized, so they went ahead and made the guarantee.
> Anyhow, signed overflow is possible in other place in struct, such as
> it's atoi() variant, that does no bounds checking.
> I'd another patch that converted all the int values that are never
> allowed to be negative to size_t, but it was a lot more intrusive, and
> it wasn't clear, given how struct handles indexes that are out of
> bounds, how to maintain a backwards compatible API.
> Probably lua should have a luaL_checkunsigned().
Lua 5.2 does have luaL_checkunsigned.