Fix a bug in Shadow::IsBeginningOfBlockBody

Fix a bug in Shadow::IsBeginningOfBlockBody

The beginning of a block might be in its trailer in the case of a 0 bytes block.


BUG=
R=chrisha@chromium.org

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

[Sébastien Marchand, 2014-08-27 20:58:34]

PTAL.

[chrisha, 2014-08-27 21:06:18]

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

https://codereview.appspot.com/133110044/diff/1/syzygy/agent/asan/shadow.cc#n...
syzygy/agent/asan/shadow.cc:237: if (IsLeftRedzone(addr))
Redzones aren't always split into 'left' and 'right', so it's possible that the
address is a kAsanReserved redzone, etc. I think you want this:

// If the block has a non-zero body size then the beginning 
// of the body will be accessible.
// If the block has an empty body then the beginning of
// the body will be a right redzone.
if (!IsAccessible(addr) || !IsRightRedzone(addr))

Rename this to IsBeginningOfAccessibleBlockBody? Or adjust the logic to handle
quarantined blocks as well? In which case it would be:

if (!IsAccessible(addr) || !IsRightRedzone(addr) ||
    GetShadowMarker(addr) == kHeapFreedMarker)

[Sébastien Marchand, 2014-08-28 02:16:50]

Thanks, PTAnL.

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

https://codereview.appspot.com/133110044/diff/1/syzygy/agent/asan/shadow.cc#n...
syzygy/agent/asan/shadow.cc:237: if (IsLeftRedzone(addr))
On 2014/08/27 21:06:18, chrisha wrote:
> Redzones aren't always split into 'left' and 'right', so it's possible that
the
> address is a kAsanReserved redzone, etc. I think you want this:
> 
> // If the block has a non-zero body size then the beginning 
> // of the body will be accessible.
> // If the block has an empty body then the beginning of
> // the body will be a right redzone.
> if (!IsAccessible(addr) || !IsRightRedzone(addr))
> 
> Rename this to IsBeginningOfAccessibleBlockBody? Or adjust the logic to handle
> quarantined blocks as well? In which case it would be:
> 
> if (!IsAccessible(addr) || !IsRightRedzone(addr) ||
>     GetShadowMarker(addr) == kHeapFreedMarker)

Done. The principal reason of this function is to avoid touching the block while
it's in the quarantine of the zebra heap (and so it's protected against the read
accesses) so I haven't changed the name.

[chrisha, 2014-08-28 13:50:20]

lgtm!

[Sébastien Marchand, 2014-08-28 16:09:36]

Thanks, committing.

[Sébastien Marchand, 2014-08-28 16:28:32]

Committed patchset #2 (id:20001) manually as 2276 (presubmit successful).