Bug 211089

Summary: [JSC] Add $vm.assertEnabled() to suppress Debug crash expected tests in release+assert build
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch keith_miller: review+

Description Yusuke Suzuki 2020-04-27 12:11:33 PDT 
We have ASSERT which only fires when DOM object is wrongly implemented to catch wrong ones.
We are using ASSERT in JSC side since we don't know all the DOM objects are correctly implemented. If it is not correctly implemented, in Release build, JSC is returning sub-optimal results. But in Debug build, we can catch the wrong ones, which allows us to fix the wrong thing while avoiding catastrophic crashes in release build.

However, the test is assuming that ASSERT is not enabled if it is not Debug build. Release & Debug builds work, but Release+Assert does not meet this assumption.

So, skip this based on `$vm.assertEnabled()`.
Comment 1 Yusuke Suzuki 2020-04-27 12:22:59 PDT 
Created attachment 397713 [details]
Patch
Comment 2 Yusuke Suzuki 2020-04-27 12:25:48 PDT 
Created attachment 397715 [details]
Patch
Comment 3 Keith Miller 2020-04-27 12:33:43 PDT 
Comment on attachment 397715 [details]
Patch

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

r=me with suggestion

> JSTests/stress/incorrect-put-could-generate-invalid-ic-involving-dictionary-flatten.js:19
> +if (!$vm.assertEnabled()) {

Why not `if (!$vm.assertEnabled()) quit()`
Comment 4 Yusuke Suzuki 2020-04-27 13:39:15 PDT 
Comment on attachment 397715 [details]
Patch

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

>> JSTests/stress/incorrect-put-could-generate-invalid-ic-involving-dictionary-flatten.js:19
>> +if (!$vm.assertEnabled()) {
> 
> Why not `if (!$vm.assertEnabled()) quit()`

Sounds good!
Comment 5 Yusuke Suzuki 2020-04-27 14:14:23 PDT 
Committed r260784: <https://trac.webkit.org/changeset/260784>
Comment 6 Radar WebKit Bug Importer 2020-04-27 14:15:17 PDT 
<rdar://problem/62468324>