[llvm] r215841 - BumpPtrAllocator: remove 'no slabs allocated yet' check

Hans Wennborg hans at hanshq.net
Sun Aug 31 21:29:29 PDT 2014


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.

> 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.

  - Hans



More information about the llvm-commits mailing list