Cleanup in the interface and fix of a small issue.

Cleanup in the interface and fix of a small issue.

We can't report an error in the HeapProxy::OnBadAccess function, in the case that there's several heap in our application it would report an unknown error for each of them until the heap containing the accessed address is found...

BUG=

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

[Sébastien Marchand, 2012-10-26 16:52:57]

PTAL.

[Roger McFarlane, 2012-10-26 20:02:38]

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

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:154: DCHECK(bad_access_kind !=
UNKNOWN_BAD_ACCESS);
nit: Generally, prefer DCHECK_<condition>(a, b).

In this case: DCHECK_NE(UNKNOWN_BAD_ACCESS, bad_access_kind);

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:278: uint8* mem =
reinterpret_cast<uint8*>(const_cast<void*>(alloc));
const uint8* mem = static_cast<const uint8*>(alloc);

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:279: BlockHeader* header =
reinterpret_cast<BlockHeader*>(mem - kRedZoneSize);
const BlockHeader* header = reinterpret_cast<const BlockHeader*>(mem -
kRedZoneSize);

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:288: return header;
return const_cast<BlockHeader*>(header);

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:336:
reinterpret_cast<void*>(const_cast<uint8*>(addr)));
neither cast should be necessary

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:360: HeapProxy::BlockHeader*
HeapProxy::FindAddressBlock(const uint8* addr) {
can be const void*

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:389: const uint8* casted_addr = static_cast<const
uint8*>(addr);
This cast is only necessary because OnBadAccess wants to call other funcs that
take a uint8* when their interface doesn't actually require one. OnBadAccess
doesn't perform any pointer arithmetic of its own.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:407: ReportAsanErrorBase("unknown-crash",
static_cast<const uint8*>(addr),
ditto.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:407: ReportAsanErrorBase("unknown-crash",
static_cast<const uint8*>(addr),
this could take const void*

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:422: const uint8* addr,
this can be const void*

[Sébastien Marchand, 2012-10-26 20:29:55]

PTAnL.

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

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:154: DCHECK(bad_access_kind !=
UNKNOWN_BAD_ACCESS);
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> nit: Generally, prefer DCHECK_<condition>(a, b).
> 
> In this case: DCHECK_NE(UNKNOWN_BAD_ACCESS, bad_access_kind);

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:278: uint8* mem =
reinterpret_cast<uint8*>(const_cast<void*>(alloc));
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> const uint8* mem = static_cast<const uint8*>(alloc);

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:279: BlockHeader* header =
reinterpret_cast<BlockHeader*>(mem - kRedZoneSize);
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> const BlockHeader* header = reinterpret_cast<const BlockHeader*>(mem -
> kRedZoneSize);

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:288: return header;
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> return const_cast<BlockHeader*>(header);

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:336:
reinterpret_cast<void*>(const_cast<uint8*>(addr)));
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> neither cast should be necessary

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:360: HeapProxy::BlockHeader*
HeapProxy::FindAddressBlock(const uint8* addr) {
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> can be const void*

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:389: const uint8* casted_addr = static_cast<const
uint8*>(addr);
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> This cast is only necessary because OnBadAccess wants to call other funcs that
> take a uint8* when their interface doesn't actually require one. OnBadAccess
> doesn't perform any pointer arithmetic of its own.

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:407: ReportAsanErrorBase("unknown-crash",
static_cast<const uint8*>(addr),
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> this could take const void*

Done.

https://codereview.appspot.com/6785055/diff/1/syzygy/agent/asan/asan_heap.cc#...
syzygy/agent/asan/asan_heap.cc:422: const uint8* addr,
On 2012/10/26 20:02:38, Roger McFarlane wrote:
> this can be const void*

Done.

[Sébastien Marchand, 2012-10-29 15:06:39]

Any update on this CL ?

[Roger McFarlane, 2012-10-29 15:17:30]

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

https://codereview.appspot.com/6785055/diff/5001/syzygy/agent/asan/asan_heap....
syzygy/agent/asan/asan_heap.cc:339: HeapProxy::BadAccessKind
HeapProxy::GetBadAccessKind(const uint8* addr,
const void*

https://codereview.appspot.com/6785055/diff/5001/syzygy/agent/asan/asan_heap....
syzygy/agent/asan/asan_heap.cc:394: bad_access_kind =
GetBadAccessKind(static_cast<const uint8*>(addr), header);
no cast needed if GetBadAccessKind takes const void*

[Sébastien Marchand, 2012-10-29 15:26:05]

Thanks, PTAnL.

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

https://codereview.appspot.com/6785055/diff/5001/syzygy/agent/asan/asan_heap....
syzygy/agent/asan/asan_heap.cc:339: HeapProxy::BadAccessKind
HeapProxy::GetBadAccessKind(const uint8* addr,
On 2012/10/29 15:17:30, Roger McFarlane wrote:
> const void*

Done.

https://codereview.appspot.com/6785055/diff/5001/syzygy/agent/asan/asan_heap....
syzygy/agent/asan/asan_heap.cc:394: bad_access_kind =
GetBadAccessKind(static_cast<const uint8*>(addr), header);
On 2012/10/29 15:17:30, Roger McFarlane wrote:
> no cast needed if GetBadAccessKind takes const void*

Done.

[Roger McFarlane, 2012-10-30 18:53:17]

lgtm with a nit.

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

https://codereview.appspot.com/6785055/diff/10002/syzygy/agent/asan/asan_heap...
syzygy/agent/asan/asan_heap.cc:301: void
HeapProxy::PrintAddressInformation(const uint8* addr,
Just one left...

[Sébastien Marchand, 2012-10-30 19:01:12]

Merci, committing.

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

https://codereview.appspot.com/6785055/diff/10002/syzygy/agent/asan/asan_heap...
syzygy/agent/asan/asan_heap.cc:301: void
HeapProxy::PrintAddressInformation(const uint8* addr,
On 2012/10/30 18:53:17, Roger McFarlane wrote:
> Just one left...

Done.