RESOLVED FIXED204802
Enable security assertions on all ASAN builds 
https://bugs.webkit.org/show_bug.cgi?id=204802
Summary Enable security assertions on all ASAN builds
Sunny He
Reported 2019-12-03 11:20:05 PST
<rdar://57433774> To enable more effective testing, enable security assertions on all ASAN builds including release. This affects blocks guarded by ENABLE(SECURITY_ASSERTIONS) and ASSERT_WITH_SECURITY_IMPLICATION. Since ASAN builds are inherently for finding bugs at the cost of performance, the additional performance impact of enabling security assertions on ASAN builds should be acceptable.
Attachments
Patch (1.26 KB, patch)
2019-12-03 12:04 PST, Sunny He 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sunny He
Comment 1 2019-12-03 12:04:32 PST
Created attachment 384734 [details] Patch
Ryosuke Niwa
Comment 2 2019-12-03 14:02:27 PST
Windows bot failure is: C:\cygwin\home\buildbot\worker\Windows-EWS\build\Tools\DumpRenderTree\win\DumpRenderTree.cpp(800,61): error C2039: 'setWebAnimationsCompositeOperationsEnabled': is not a member of 'IWebPreferencesPrivate7' [C:\cygwin\home\buildbot\worker\Windows-EWS\build\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib.vcxproj] JSWrapper.cpp Do you think it might be related to your patch??
Ryosuke Niwa
Comment 3 2019-12-03 14:03:36 PST
iOS test failure is probably nothing to do with it.
Ryosuke Niwa
Comment 4 2019-12-03 15:08:55 PST
Oh looks, like Windows bot failure was an exciting issue.
WebKit Commit Bot
Comment 5 2019-12-03 17:27:08 PST
Comment on attachment 384734 [details] Patch Clearing flags on attachment: 384734 Committed r253072: <https://trac.webkit.org/changeset/253072>
WebKit Commit Bot
Comment 6 2019-12-03 17:27:09 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 7 2019-12-07 14:06:27 PST
> Since ASAN builds are inherently for finding bugs at the cost of performance, the additional performance impact of enabling security assertions on ASAN builds should be acceptable. This obviously depends on what the performance impact is. Looking at test runtime, it seems acceptable in this case (very roughly, 1-2%).
Note You need to log in before you can comment on or make changes to this bug.