[webkit-reviews] review granted: [Bug 186957] Fix ASAN_ENABLED in GCC : [Attachment 343429] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at igalia.com> has granted Alicia Boya García
<aboya at igalia.com>'s request for review:
Bug 186957: Fix ASAN_ENABLED in GCC
https://bugs.webkit.org/show_bug.cgi?id=186957

Attachment 343429: Patch

https://bugs.webkit.org/attachment.cgi?id=343429&action=review




--- Comment #5 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 343429
  --> https://bugs.webkit.org/attachment.cgi?id=343429
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343429&action=review

> Source/WTF/wtf/Compiler.h:166
> +/* In GCC functions marked with no_sanitize_address cannot call functions
that are marked with always_inline and not marked with no_sanitize_address.
> + * Therefore we need to give up on the enforcement of ALWAYS_INLINE when
bulding with ASAN. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368 */
> +#if !defined(ALWAYS_INLINE) && COMPILER(GCC_OR_CLANG) && defined(NDEBUG) &&
!COMPILER(MINGW) && !(COMPILER(GCC) && ASAN_ENABLED)

I think it's easier to read without the comment... it should be clear from the
condition you've added that ALWAYS_INLINE + GCC + ASAN_ENABLED do not mix..

> Source/WTF/wtf/Vector.h:531
>	   ASSERT(buffer());
> +#if COMPILER(GCC)

I would leave a blank line between here.

> PerformanceTests/ChangeLog:8
> +	   Updated Compiler.h to reflect changes in WTF.

Hm, I don't think we should do this unless there's a compelling reason, and
surely performance tests are not meant to be run under asan. We can't be
updating StitchMarker every time