Attachment 361664 Details for Bug 194493 – proposed patch.

[patch] proposed patch.

bug-194493.patch (text/plain), 2.18 KB, created by Mark Lam on 2019-02-11 00:04:18 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2019-02-11 00:04:18 PST

Size: 2.18 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 241258)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2019-02-10  Mark Lam  <mark.lam@apple.com>
>+
>+        Remove the RELEASE_ASSERT check for duplicate cases in the BinarySwitch constructor.
>+        https://bugs.webkit.org/show_bug.cgi?id=194493
>+        <rdar://problem/36380852>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Having duplicate cases in the BinarySwitch is 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.
>+
>+        Also added some value dumps to the RELEASE_ASSERT to help debug the issue when we
>+        see duplicate cases.
>+
>+        * jit/BinarySwitch.cpp:
>+        (JSC::BinarySwitch::BinarySwitch):
>+
> 2019-02-10  Darin Adler  <darin@apple.com>
> 
>         Switch uses of StringBuilder with String::format for hex numbers to use HexNumber.h instead
>Index: Source/JavaScriptCore/jit/BinarySwitch.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/BinarySwitch.cpp	(revision 241258)
>+++ Source/JavaScriptCore/jit/BinarySwitch.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013, 2015 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -59,10 +59,12 @@ BinarySwitch::BinarySwitch(GPRReg value,
> 
>     if (BinarySwitchInternal::verbose)
>         dataLog("Sorted cases: ", listDump(m_cases), "\n");
>-    
>+
>+#if !ASSERT_DISABLED
>     for (unsigned i = 1; i < m_cases.size(); ++i)
>-        RELEASE_ASSERT(m_cases[i - 1] < m_cases[i]);
>-    
>+        RELEASE_ASSERT(m_cases[i - 1] < m_cases[i], i, m_cases.size(), m_cases[i].value, m_cases[i].index);
>+#endif
>+
>     build(0, false, m_cases.size());
> }
>

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194493: 361662 | 361663 | 361664