On Thu, Dec 02, 2010 at 02:47:30PM -0800, Gabriel Dos Reis wrote: > On Thu, Dec 2, 2010 at 2:20 PM, Joe Buck <joe.b...@synopsys.com> wrote: > > On Wed, Dec 01, 2010 at 10:26:58PM -0800, Florian Weimer wrote: > >> * Chris Lattner: > >> > >> > On overflow it just forces the size passed in to operator new to > >> > -1ULL, which throws bad_alloc. > >> > >> This is also what my patch tries to implement. > > > > Yes, but Chris's code just checks the overflow of the multiply. Your > > patch achieves the same result in a more complex way, by > > computing the largest non-overflowing value of n in > > > > new T[n]; > > > > and comparing n against that. Even though max_size_t/sizeof T is a > > compile-time constant, this is still more expensive. > > I would expect max_size_t/sizeof(T) to be actually an integer > constant that n is compared against. I would be surprised > if that one-time comparison is noticeable in real applications that > new an array of objects.
It's wasted code if the multiply instruction detects the overflow. It's true that the cost is small (maybe just one extra instruction and the same number of tests, maybe one more on architectures where you have to load a large constant), but it is slightly worse code than what Chris Lattner showed. Still, it's certainly an improvement on the current situation and the cost is negligible compared to the call to the allocator. Since it's a security issue, some form of the patch should go in.
