ASAN Runtime: Fix ReAlloc to not deallocate original on alloc failure.

ASAN Runtime: Fix ReAlloc to not deallocate original on alloc failure.

R=chrisha@chromium.org, rogerm@chromium.org

Committed: https://code.google.com/p/sawbuck/source/detail?r=1578

[Siggi, 2013-06-18 14:31:22]

PTAL

[chrisha, 2013-06-18 14:33:05]

lgtm!

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap_un...
File syzygy/agent/asan/asan_heap_unittest.cc (right):

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap_un...
syzygy/agent/asan/asan_heap_unittest.cc:246:
proxy_.ReAlloc(HEAP_REALLOC_IN_PLACE_ONLY, NULL, kAllocSize));
Is this the right behaviour? Should an 'in place' NULL realloc succeed like a
NULL realloc?

[Roger McFarlane, 2013-06-18 14:48:57]

Nice!

LGTM with a nit.

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap.cc
File syzygy/agent/asan/asan_heap.cc (right):

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap.cc...
syzygy/agent/asan/asan_heap.cc:251: if (new_mem != NULL && mem != NULL)
This can be simplified now that you're checking for new_mem == NULL above.

if (mem != NULL) {
  ::memcpy(new_mem, mem, std::min(bytes, Size(0, mem)));
  Free(flags, mem)
}

return new_mem;

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap_un...
File syzygy/agent/asan/asan_heap_unittest.cc (right):

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap_un...
syzygy/agent/asan/asan_heap_unittest.cc:246:
proxy_.ReAlloc(HEAP_REALLOC_IN_PLACE_ONLY, NULL, kAllocSize));
On 2013/06/18 14:33:05, chrisha wrote:
> Is this the right behaviour? Should an 'in place' NULL realloc succeed like a
> NULL realloc?

"In place" with NULL seems non-sensical to me, so I think failure is the right
behaviour.

[Siggi, 2013-06-18 15:23:47]

Thanks, committing.

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap.cc
File syzygy/agent/asan/asan_heap.cc (right):

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap.cc...
syzygy/agent/asan/asan_heap.cc:251: if (new_mem != NULL && mem != NULL)
On 2013/06/18 14:48:57, Roger McFarlane wrote:
> This can be simplified now that you're checking for new_mem == NULL above.
> 
> if (mem != NULL) {
>   ::memcpy(new_mem, mem, std::min(bytes, Size(0, mem)));
>   Free(flags, mem)
> }
> 
> return new_mem;

Done.

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap_un...
File syzygy/agent/asan/asan_heap_unittest.cc (right):

https://codereview.appspot.com/10374044/diff/1/syzygy/agent/asan/asan_heap_un...
syzygy/agent/asan/asan_heap_unittest.cc:246:
proxy_.ReAlloc(HEAP_REALLOC_IN_PLACE_ONLY, NULL, kAllocSize));
On 2013/06/18 14:33:05, chrisha wrote:
> Is this the right behaviour? Should an 'in place' NULL realloc succeed like a
> NULL realloc?

I don't know, but I figure it won't matter for our use case.

[Siggi, 2013-06-18 15:25:24]

Committed patchset #2 manually as r1578 (presubmit successful).