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+

Mark Lam
Reported 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.
Attachments
proposed patch. (2.16 KB, patch)
2019-02-10 23:49 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
proposed patch. (2.16 KB, patch)
2019-02-10 23:51 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
proposed patch. (2.18 KB, patch)
2019-02-11 00:04 PST, Mark Lam 		ysuzuki: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2019-02-10 23:34:38 PST
<rdar://problem/36380852>
Mark Lam
Comment 2 2019-02-10 23:49:22 PST
Created attachment 361662 [details] proposed patch.
Mark Lam
Comment 3 2019-02-10 23:51:04 PST
Created attachment 361663 [details] proposed patch.
Mark Lam
Comment 4 2019-02-11 00:04:18 PST
Created attachment 361664 [details] proposed patch.
Yusuke Suzuki
Comment 5 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?
Mark Lam
Comment 6 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>.
Note You need to log in before you can comment on or make changes to this bug.