Bug 194493

Summary: Remove the RELEASE_ASSERT check for duplicate cases in the BinarySwitch constructor.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 194492    
Attachments:
Description Flags
proposed patch.
none
proposed patch.
none
proposed patch. ysuzuki: review+

Description Mark Lam 2019-02-10 23:34:17 PST
Having duplicate cases in the BinarySwitch if not a correctness issue.  It is however not good for performance and memory usage.  As such, a debug ASSERT will do.  We'll also do an audit of the clients of BinarySwitch to see if it's possible to be instantiated with duplicate cases in https://bugs.webkit.org/show_bug.cgi?id=194492 later.
Comment 1 Mark Lam 2019-02-10 23:34:38 PST
<rdar://problem/36380852>
Comment 2 Mark Lam 2019-02-10 23:49:22 PST
Created attachment 361662 [details]
proposed patch.
Comment 3 Mark Lam 2019-02-10 23:51:04 PST
Created attachment 361663 [details]
proposed patch.
Comment 4 Mark Lam 2019-02-11 00:04:18 PST
Created attachment 361664 [details]
proposed patch.
Comment 5 Yusuke Suzuki 2019-02-11 01:44:10 PST
Comment on attachment 361664 [details]
proposed patch.

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

r=me

> Source/JavaScriptCore/jit/BinarySwitch.cpp:65
> +        RELEASE_ASSERT(m_cases[i - 1] < m_cases[i], i, m_cases.size(), m_cases[i].value, m_cases[i].index);

Why not using `ASSERT` here?
Comment 6 Mark Lam 2019-02-11 10:11:16 PST
Thanks for the review.  I've changed the RELEASE_ASSERT to a debug ASSERT.  Landed in r241267: <http://trac.webkit.org/r241267>.