[llvm] r215841 - BumpPtrAllocator: remove 'no slabs allocated yet' check
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Sep 1 14:04:37 PDT 2014
> On 2014 Sep 1, at 00:29, Hans Wennborg <hans at hanshq.net> wrote:
> On 08/31/2014 08:36 PM, Chandler Carruth wrote:
>> On Sun, Aug 31, 2014 at 8:14 PM, Duncan P. N. Exon Smith
>> <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
>> > On 2014 Aug 17, at 14:31, Hans Wennborg <hans at hanshq.net
>> <mailto:hans at hanshq.net>> wrote:
>> > Author: hans
>> > Date: Sun Aug 17 13:31:18 2014
>> > New Revision: 215841
>> > URL: http://llvm.org/viewvc/llvm-project?rev=215841&view=rev
>> > Log:
>> > BumpPtrAllocator: remove 'no slabs allocated yet' check
>> > We already handle the no-slabs case when checking whether the
>> current slab
>> > is large enough: if no slabs have been allocated, CurPtr and End
>> are both 0.
>> > alignPtr(0), will still be 0, and so "if (Ptr + Size <= End)" fails.
>> I don't think this commit introduces the problem, but isn't the result
>> of `Ptr + Size` undefined when `Size` is too big?
>> Should this be changed to `if (Size <= End - Ptr)`?
> Ptr, which we got from calling alignPtr() can point beyond the current slab, so the End - Ptr operation could be undefined.
I assumed that slab sizes are always powers of 2 (so End will always be
well-aligned for reasonable values of `Alignment`), but I don't see any
asserts about that in the code now that I've looked. If we can add a
(static?) assertion to that effect then `alignPtr()` should be safe.
Or is there a use case for unaligned slab end pointers?
>> Should check the pre-commit review here, I think I brought this up and
>> there was some reason why it didn't make sense to fix it here....
> I think the reasoning was that it wouldn't cause problems in practice. I'd be happy to fix it, but I think that would mean doing the alignment and bounds check with intptr_t.
I think this would be good to fix. Never know when a compiler might
take advantage of UB.
More information about the llvm-commits