[JSC] Make ConcurrentJSLock Lock even if ENABLE_CONCURRENT_JS=OFF
https://bugs.webkit.org/show_bug.cgi?id=202892
Reviewed by Mark Lam.
Source/JavaScriptCore:
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):
Source/WTF:
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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251307 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 7781883..7a899b6 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,40 @@
+2019-10-18 Yusuke Suzuki <ysuzuki@apple.com>
+
+ [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-10-18 Devin Rousso <drousso@apple.com>
Web Inspector: Elements: allow WebKit engineers to edit UserAgent shadow trees
diff --git a/Source/JavaScriptCore/runtime/ConcurrentJSLock.h b/Source/JavaScriptCore/runtime/ConcurrentJSLock.h
index 4c7c10b..aa273ba 100644
--- a/Source/JavaScriptCore/runtime/ConcurrentJSLock.h
+++ b/Source/JavaScriptCore/runtime/ConcurrentJSLock.h
@@ -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<DisallowGC> m_disallowGC;
#endif
diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index 10ce81b..86faa0d 100644
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,15 @@
+2019-10-18 Yusuke Suzuki <ysuzuki@apple.com>
+
+ [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-10-17 Mark Lam <mark.lam@apple.com>
Use constexpr in more places and remove some unnecessary external linkage.
diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
index ce1785d..05a3c09 100644
--- a/Source/WTF/wtf/Platform.h
+++ b/Source/WTF/wtf/Platform.h
@@ -841,7 +841,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
