[PATCH] [sanitizer] Fix overflow in SizeClassAllocator64::GetChunkIdx().

Sergey Matveev earthdok at google.com
Wed May 15 12:21:17 PDT 2013


We have about 600 lines of allocator tests, which is far too little.

Regarding this specific change, you can't test it without allocating 4gb of
memory (well, you can if you just feed the hand-crafted pointer to
GetBlockBegin - but that's a bit hacky), and the change itself is so
trivial that I don't think it's worth adding a regression test.


On Wed, May 15, 2013 at 11:17 PM, Alexander Potapenko <glider at google.com>wrote:

> Do we have tests for the allocator? Looks like it's time to add some.
>
> Sent from phone
> On May 15, 2013 11:11 PM, "Sergey Matveev" <earthdok at google.com> wrote:
>
>> Hi kcc, glider,
>>
>> The 32-bit offset overflowed when more than 4GB was allocated in a size
>> class. Also removed the misleading comment.
>>
>> http://llvm-reviews.chandlerc.com/D797
>>
>> Files:
>>   lib/sanitizer_common/sanitizer_allocator.h
>>
>> Index: lib/sanitizer_common/sanitizer_allocator.h
>> ===================================================================
>> --- lib/sanitizer_common/sanitizer_allocator.h
>> +++ lib/sanitizer_common/sanitizer_allocator.h
>> @@ -492,11 +492,7 @@
>>    }
>>
>>    static uptr GetChunkIdx(uptr chunk, uptr size) {
>> -    u32 offset = chunk % kRegionSize;
>> -    // Here we divide by a non-constant. This is costly.
>> -    // We require that kRegionSize is at least 2^32 so that offset is
>> 32-bit.
>> -    // We save 2x by using 32-bit div, but may need to use a 256-way
>> switch.
>> -    return offset / (u32)size;
>> +    return (chunk % kRegionSize) / size;
>>    }
>>
>>    NOINLINE Batch* PopulateFreeList(AllocatorStats *stat, AllocatorCache
>> *c,
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130515/532b8e66/attachment.html>


More information about the llvm-commits mailing list