Make ASan do bounds checks for WTF::Vector

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

We have some code to perform out of bounds checks in non-ASan builds, ASan checks would complement that.

Built-in checks are better, because they detect all overruns and underruns, while ASan only detects small ones, which make Vector access a memory address in a redzone. Also, they can be made either debug only or release too, depending on performance requirements.

ASan checks are better, because they check even those memory accesses that are performed via a raw pointer returned from data() or begin(). They have zero performance cost in non-ASan builds.

[Alexey Proskuryakov]

rdar://problem/19437718

[Alexey Proskuryakov]

Created attachment 244913 [details]
proposed patch

[WebKit Commit Bot]

Attachment 244913 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Vector.h:510:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 244913 [details]
proposed patch

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

Do you know why the Win EWS bot is red on this patch?

> Source/WTF/wtf/Vector.h:756
> +        if (this == &other) // ASan will crash if we try to restrict access to the same buffer twice.
> +            return;

It’s irritating to add this extra branch for all builds just to make ASAN work, especially since it only optimizes something that we should never be doing. Can we do better? Maybe just put this inside #if ASAN_ENABLED?

[Alexey Proskuryakov]

I don't yet understand why Windows failed to build; will need to figure it out before landing.

> Maybe just put this inside #if ASAN_ENABLED?

Will do.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r178722>.

Windows build was failing because of an incorrect macro value. Changed ASAN_ENABLED fallback value from false to 0 to fix this.