WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
140631
Make ASan do bounds checks for WTF::Vector
https://bugs.webkit.org/show_bug.cgi?id=140631
Summary
Make ASan do bounds checks for WTF::Vector
Alexey Proskuryakov
Reported
2015-01-19 11:44:11 PST
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.
Attachments
proposed patch
(17.27 KB, patch)
2015-01-19 11:47 PST
,
Alexey Proskuryakov
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2015-01-19 11:46:16 PST
rdar://problem/19437718
Alexey Proskuryakov
Comment 2
2015-01-19 11:47:16 PST
Created
attachment 244913
[details]
proposed patch
WebKit Commit Bot
Comment 3
2015-01-19 11:50:10 PST
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 4
2015-01-20 09:21:09 PST
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
Comment 5
2015-01-20 10:00:41 PST
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
Comment 6
2015-01-20 10:28:50 PST
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug