From 57a2aa0d05f0963c7079117d1bee156d2f7399fa Mon Sep 17 00:00:00 2001 From: mustafa <971530+tosbaha@users.noreply.github.com> Date: Tue, 3 Dec 2019 13:44:28 +0300 Subject: [PATCH] GC concurrent issue potential fix --- patches/jsc_fix_concurrent_gc_issue.patch | 125 ++++++++++++++++++++++ scripts/patch.sh | 4 + 2 files changed, 129 insertions(+) create mode 100644 patches/jsc_fix_concurrent_gc_issue.patch diff --git a/patches/jsc_fix_concurrent_gc_issue.patch b/patches/jsc_fix_concurrent_gc_issue.patch new file mode 100644 index 000000000..7424cc387 --- /dev/null +++ b/patches/jsc_fix_concurrent_gc_issue.patch @@ -0,0 +1,125 @@ +--- target-org/webkit/Source/JavaScriptCore/ChangeLog 2019-09-23 18:14:45.000000000 +0800 ++++ target/webkit/Source/JavaScriptCore/ChangeLog 2019-12-18 08:36:29.000000000 +0800 +@@ -1,3 +1,40 @@ ++2019-10-18 Yusuke Suzuki ++ ++ [JSC] Make ConcurrentJSLock Lock even if ENABLE_CONCURRENT_JS=OFF ++ https://bugs.webkit.org/show_bug.cgi?id=202892 ++ ++ Reviewed by Mark Lam. ++ ++ We are using ConcurrentJSLock to guard data structure against concurrent compilers. ++ But these data structures should be guarded by GC concurrent collector, so we are using this ConcurrentJSLock ++ to guard them against concurrent collector too. ++ The problem is that ENABLE(CONCURRENT_JS) relies on ENABLE(DFG_JIT). If we configure JSC with the options like, ++ ++ ENABLE_DFG_JIT 0 ++ ENABLE_FTL_JIT 0 ++ ++ Then, the built JSC becomes ++ ++ ENABLE_CONCURRENT_JS 0 ++ But, Concurrent GC is enabled. ++ ++ This is wrong due to several reasons. ++ ++ 1. Baseline JIT can produce JIT related data structures that are traced by concurrent collector. In the above options, ++ these data structures are not guarded by lock. ++ 2. Baseline JIT also has concurrent JIT compiler. But ENABLE_CONCURRENT_JS does not reflect this. ++ ++ In this patch, we fix two things. ++ ++ 1. We should make ConcurrentJSLock always Lock. In 64bit environment we are supporting actively (including watchOS ARM64_32), ++ we are enabling ENABLE(JIT) regardless of we are actually using JIT. So, anyway, this is already a Lock. Flipping these ++ bits does not matter in 32bit architectures since they do not have concurrent compilers anyway. This makes things simpler: ++ it is always a Lock. And concurrent collector can use it. ++ 2. We should make `ENABLE(CONCURRENT_JS)` ON when `ENABLE(JIT)` is true, to reflect the fact that Baseline JIT has concurrent compiler. ++ ++ * runtime/ConcurrentJSLock.h: ++ (JSC::ConcurrentJSLocker::ConcurrentJSLocker): ++ + 2019-09-18 Saam Barati + + Phantom insertion phase may disagree with arguments forwarding about live ranges +--- target-org/webkit/Source/JavaScriptCore/runtime/ConcurrentJSLock.h 2018-12-19 20:41:11.000000000 -0800 ++++ target/webkit/Source/JavaScriptCore/runtime/ConcurrentJSLock.h 2019-10-21 11:43:39.000000000 -0700 +@@ -32,13 +32,8 @@ + + namespace JSC { + +-#if ENABLE(CONCURRENT_JS) +-typedef Lock ConcurrentJSLock; +-typedef LockHolder ConcurrentJSLockerImpl; +-#else +-typedef NoLock ConcurrentJSLock; +-typedef NoLockLocker ConcurrentJSLockerImpl; +-#endif ++using ConcurrentJSLock = Lock; ++using ConcurrentJSLockerImpl = LockHolder; + + static_assert(sizeof(ConcurrentJSLock) == 1, "Regardless of status of concurrent JS flag, size of ConurrentJSLock is always one byte."); + +@@ -103,7 +98,7 @@ + public: + ConcurrentJSLocker(ConcurrentJSLock& lockable) + : ConcurrentJSLockerBase(lockable) +-#if ENABLE(CONCURRENT_JS) && !defined(NDEBUG) ++#if !defined(NDEBUG) + , m_disallowGC(std::in_place) + #endif + { +@@ -111,7 +106,7 @@ + + ConcurrentJSLocker(ConcurrentJSLock* lockable) + : ConcurrentJSLockerBase(lockable) +-#if ENABLE(CONCURRENT_JS) && !defined(NDEBUG) ++#if !defined(NDEBUG) + , m_disallowGC(std::in_place) + #endif + { +@@ -119,7 +114,7 @@ + + ConcurrentJSLocker(NoLockingNecessaryTag) + : ConcurrentJSLockerBase(NoLockingNecessary) +-#if ENABLE(CONCURRENT_JS) && !defined(NDEBUG) ++#if !defined(NDEBUG) + , m_disallowGC(WTF::nullopt) + #endif + { +@@ -127,7 +122,7 @@ + + ConcurrentJSLocker(int) = delete; + +-#if ENABLE(CONCURRENT_JS) && !defined(NDEBUG) ++#if !defined(NDEBUG) + private: + Optional m_disallowGC; + #endif +--- target-org/webkit/Source/WTF/ChangeLog 2019-09-23 18:40:37.000000000 +0800 ++++ target/webkit/Source/WTF/ChangeLog 2019-12-18 08:35:53.000000000 +0800 +@@ -1,3 +1,15 @@ ++2019-10-18 Yusuke Suzuki ++ ++ [JSC] Make ConcurrentJSLock Lock even if ENABLE_CONCURRENT_JS=OFF ++ https://bugs.webkit.org/show_bug.cgi?id=202892 ++ ++ Reviewed by Mark Lam. ++ ++ BaselineJIT also has concurrent compiler. ENABLE(CONCURRENT_JS) should not rely on ENABLE(DFG_JIT). ++ It should rely on ENABLE(JIT) instead. ++ ++ * wtf/Platform.h: ++ + 2019-09-20 Libor Bukata + + UI process crash when using callOnMainThread() after the main thread dispatcher has been destroyed +--- target-org/webkit/Source/WTF/wtf/Platform.h 2019-08-23 14:21:51.000000000 -0700 ++++ target/webkit/Source/WTF/wtf/Platform.h 2019-10-21 11:44:30.000000000 -0700 +@@ -840,7 +840,7 @@ + values get stored to atomically. This is trivially true on 64-bit platforms, + but not true at all on 32-bit platforms where values are composed of two + separate sub-values. */ +-#if ENABLE(DFG_JIT) && USE(JSVALUE64) ++#if ENABLE(JIT) && USE(JSVALUE64) + #define ENABLE_CONCURRENT_JS 1 + #endif diff --git a/scripts/patch.sh b/scripts/patch.sh index 100769beb..3214a77fe 100755 --- a/scripts/patch.sh +++ b/scripts/patch.sh @@ -60,6 +60,10 @@ JSC_PATCHSET=( # Improve heap GC mechanism like iOS "jsc_heap_gc_like_ios.patch" + + # GC concurrent issue potential fix + # https://trac.webkit.org/changeset/251307/webkit + "jsc_fix_concurrent_gc_issue.patch" ) if [[ "$I18N" = false ]]