ASan support for the Zone

ASan support for the Zone

R=jarin@chromium.org, kcc@google.com

Committed: https://code.google.com/p/v8/source/detail?r=20266

[Jakob Kummerow, 2014-03-25 18:48:00]

Jaro: please take a look.

Kostya: FYI. Any comments?

[Jarin, 2014-03-25 20:50:48]

lgtm. Great stuff.

https://codereview.chromium.org/208743004/diff/20001/build/standalone.gypi
File build/standalone.gypi (right):

https://codereview.chromium.org/208743004/diff/20001/build/standalone.gypi#ne...
build/standalone.gypi:187: 'defines': [ 'ADDRESS_SANITIZER' ],
Perhaps we want to make this consistent with the formatting above [\n 
'ADDRESS_SANITIZER',\n],

[kcc1, 2014-03-26 06:43:40]

LGTM

https://codereview.chromium.org/208743004/diff/20001/src/zone-inl.h
File src/zone-inl.h (right):

https://codereview.chromium.org/208743004/diff/20001/src/zone-inl.h#newcode34
src/zone-inl.h:34: #include <sanitizer/asan_interface.h>
A style nit. I would do 
#ifdef ADDRESS_SANITIZER
# include <...>
#else
// define a few macros as empty
#endif

Then, instead of 
#ifdef ADDRESS_SANITIZER
ASAN_MACRO(...)
#endif

I would use 
  ASAN_MACRO(...)

This is just to make less ifdefs in the code. 
Otherwise looks good.
(I can't give a proper comment on the build file changes)

[Jakob Kummerow, 2014-03-26 09:59:12]

Thanks for the review, landing.

https://codereview.chromium.org/208743004/diff/20001/build/standalone.gypi
File build/standalone.gypi (right):

https://codereview.chromium.org/208743004/diff/20001/build/standalone.gypi#ne...
build/standalone.gypi:187: 'defines': [ 'ADDRESS_SANITIZER' ],
On 2014/03/25 20:50:48, jarin wrote:
> Perhaps we want to make this consistent with the formatting above [\n 
> 'ADDRESS_SANITIZER',\n],

As you wish. (I weakly prefer the more condensed form, but I agree that
consistency is good.)

https://codereview.chromium.org/208743004/diff/20001/src/zone-inl.h
File src/zone-inl.h (right):

https://codereview.chromium.org/208743004/diff/20001/src/zone-inl.h#newcode34
src/zone-inl.h:34: #include <sanitizer/asan_interface.h>
On 2014/03/26 06:43:41, kcc1 wrote:
> A style nit. I would do 
> #ifdef ADDRESS_SANITIZER
> # include <...>
> #else
> // define a few macros as empty
> #endif
> 
> Then, instead of 
> #ifdef ADDRESS_SANITIZER
> ASAN_MACRO(...)
> #endif
> 
> I would use 
>   ASAN_MACRO(...)
> 
> This is just to make less ifdefs in the code. 
> Otherwise looks good.
> (I can't give a proper comment on the build file changes)

Good idea, done for ASAN_UNPOISON_MEMORY_REGION, keeping zone.cc free of
additional #ifdefs. For the changes in this file below, however, I don't think
pulling stuff out into a #define would improve readability.

[Jakob Kummerow, 2014-03-26 10:02:06]

Committed patchset #3 manually as r20266 (presubmit successful).