Attachment 341405 Details for Bug 186014 – the patch

[patch] the patch

blah.patch (text/plain), 5.00 KB, created by Filip Pizlo on 2018-05-26 12:35:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-05-26 12:35:58 PDT

Size: 5.00 KB

patch

obsolete

>Index: Source/JavaScriptCore/runtime/JSString.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSString.cpp	(revision 232209)
>+++ Source/JavaScriptCore/runtime/JSString.cpp	(working copy)
>@@ -53,7 +53,10 @@ void JSRopeString::RopeBuilder<RecordOve
> 
> void JSString::destroy(JSCell* cell)
> {
>-    static_cast<JSString*>(cell)->JSString::~JSString();
>+    JSString* string = static_cast<JSString*>(cell);
>+    if (!string->m_value.isNull())
>+        dataLog("Really destroying string.\n");
>+    string->JSString::~JSString();
> }
> 
> void JSString::dumpToStream(const JSCell* cell, PrintStream& out)
>Index: Source/JavaScriptCore/runtime/JSString.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSString.h	(revision 232209)
>+++ Source/JavaScriptCore/runtime/JSString.h	(working copy)
>@@ -192,11 +192,12 @@ public:
>         Is8Bit = 1u
>     };
> 
>+    bool isRope() const { return m_value.isNull(); }
>+
> protected:
>     friend class JSValue;
> 
>     JS_EXPORT_PRIVATE bool equalSlowCase(ExecState*, JSString* other) const;
>-    bool isRope() const { return m_value.isNull(); }
>     bool isSubstring() const;
>     bool is8Bit() const { return m_flags & Is8Bit; }
>     void setIs8Bit(bool flag) const
>Index: Source/JavaScriptCore/runtime/JSStringHeapCellType.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSStringHeapCellType.cpp	(revision 232209)
>+++ Source/JavaScriptCore/runtime/JSStringHeapCellType.cpp	(working copy)
>@@ -34,7 +34,8 @@ namespace JSC {
> struct JSStringDestroyFunc {
>     ALWAYS_INLINE void operator()(VM&, JSCell* cell) const
>     {
>-        static_cast<JSString*>(cell)->JSString::~JSString();
>+        JSString* string = static_cast<JSString*>(cell);
>+        string->JSString::~JSString();
>     }
> };
> 
>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 232226)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2018-05-26  Filip Pizlo  <fpizlo@apple.com>
>+
>+        testair sometimes crashes due to races in initialization of ARC4RandomNumberGenerator
>+        https://bugs.webkit.org/show_bug.cgi?id=186014
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        testair launches a bunch of threads and the threads do B3 things that use random numbers.
>+        Sometimes two threads will initialize the random number generator at the same time, because
>+        that's what happens when you use static NeverDestroyed<>.
>+        
>+        This changes that code to use std::call_once to initialize the shared
>+        ARC4RandomNumberGenerator.
>+        
>+        Also, this adds a diagnostic message to the lock's assertion. This assertion was the symptom
>+        of the race, and knowing the state of the lock when the assertion fired gave a darn good clue
>+        about what was going on: the lock's value was 0 at time of unlock, implying that another
>+        thread reinitialized the lock to zero by rerunning the constructor.
>+
>+        * wtf/CryptographicallyRandomNumber.cpp:
>+        * wtf/LockAlgorithmInlines.h:
>+        (WTF::Hooks>::unlockSlow):
>+
> 2018-05-25  Michael Saboff  <msaboff@apple.com>
> 
>         JavaScriptCore: Disable 32-bit JIT on Windows
>Index: Source/WTF/wtf/CryptographicallyRandomNumber.cpp
>===================================================================
>--- Source/WTF/wtf/CryptographicallyRandomNumber.cpp	(revision 232209)
>+++ Source/WTF/wtf/CryptographicallyRandomNumber.cpp	(working copy)
>@@ -159,9 +159,15 @@ void ARC4RandomNumberGenerator::randomVa
> 
> ARC4RandomNumberGenerator& sharedRandomNumberGenerator()
> {
>-    static NeverDestroyed<ARC4RandomNumberGenerator> randomNumberGenerator;
>+    static ARC4RandomNumberGenerator* randomNumberGenerator;
>+    static std::once_flag onceFlag;
>+    std::call_once(
>+        onceFlag,
>+        [] {
>+            randomNumberGenerator = new ARC4RandomNumberGenerator();
>+        });
> 
>-    return randomNumberGenerator;
>+    return *randomNumberGenerator;
> }
> 
> }
>Index: Source/WTF/wtf/LockAlgorithmInlines.h
>===================================================================
>--- Source/WTF/wtf/LockAlgorithmInlines.h	(revision 232209)
>+++ Source/WTF/wtf/LockAlgorithmInlines.h	(working copy)
>@@ -110,9 +110,11 @@ void LockAlgorithm<LockType, isHeldBit, 
>     // be held and parked if someone attempts to lock just as we are unlocking.
>     for (;;) {
>         uint8_t oldByteValue = lock.load();
>-        RELEASE_ASSERT(
>-            (oldByteValue & mask) == isHeldBit
>-            || (oldByteValue & mask) == (isHeldBit | hasParkedBit));
>+        if ((oldByteValue & mask) != isHeldBit
>+            && (oldByteValue & mask) != (isHeldBit | hasParkedBit)) {
>+            dataLog("Invalid value for lock: ", oldByteValue, "\n");
>+            RELEASE_ASSERT_NOT_REACHED();
>+        }
>         
>         if ((oldByteValue & mask) == isHeldBit) {
>             if (lock.compareExchangeWeak(oldByteValue, Hooks::unlockHook(oldByteValue & ~isHeldBit)))

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186014: 341405