Changeset 262161 in webkit

Timestamp:

May 26, 2020 2:32:50 PM (4 years ago)

Author:

mark.lam@apple.com

Message:

SamplingProfiler::takeSample() should not assume that ENABLE(WEBASSEMBLY) means Wasm is enabled.
​https://bugs.webkit.org/show_bug.cgi?id=212382

Reviewed by Saam Barati.

Wasm can still be disabled at runtime with JSC options. Fixing this will allow
sampling profiler tests to run with JSC_useJIT=0 without crashing.

• runtime/SamplingProfiler.cpp:

(JSC::FrameWalker::FrameWalker):
(JSC::FrameWalker::recordJITFrame):
(JSC::CFrameWalker::CFrameWalker):
(JSC::SamplingProfiler::takeSample):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/SamplingProfiler.cpp (modified) (7 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-05-26  Mark Lam  <mark.lam@apple.com>
+
+        SamplingProfiler::takeSample() should not assume that ENABLE(WEBASSEMBLY) means Wasm is enabled.
+        https://bugs.webkit.org/show_bug.cgi?id=212382
+
+        Reviewed by Saam Barati.
+
+        Wasm can still be disabled at runtime with JSC options.  Fixing this will allow
+        sampling profiler tests to run with JSC_useJIT=0 without crashing.
+
+        * runtime/SamplingProfiler.cpp:
+        (JSC::FrameWalker::FrameWalker):
+        (JSC::FrameWalker::recordJITFrame):
+        (JSC::CFrameWalker::CFrameWalker):
+        (JSC::SamplingProfiler::takeSample):
+
 2020-05-26  Keith Rollin  <krollin@apple.com>
 

trunk/Source/JavaScriptCore/runtime/SamplingProfiler.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -43 +43 @@
 #include "WasmCallee.h"
 #include "WasmCalleeRegistry.h"
+#include "WasmCapabilities.h"
 #include <wtf/FilePrintStream.h>
 #include <wtf/HashSet.h>
@@ -74 +75 @@
 class FrameWalker {
 public:
-    FrameWalker(VM& vm, CallFrame* callFrame, const AbstractLocker& codeBlockSetLocker, const AbstractLocker& machineThreadsLocker, const AbstractLocker& wasmCalleeLocker)
+    FrameWalker(VM& vm, CallFrame* callFrame, const AbstractLocker& codeBlockSetLocker, const AbstractLocker& machineThreadsLocker, const Optional<LockHolder>& wasmCalleeLocker)
         : m_vm(vm)
         , m_callFrame(callFrame)
@@ -122 +123 @@
         stackTrace[m_depth] = UnprocessedStackFrame(codeBlock, unsafeCallee, callSiteIndex);
 #if ENABLE(WEBASSEMBLY)
-        if (unsafeCallee.isWasm()) {
+        if (Wasm::isSupported() && unsafeCallee.isWasm()) {
             auto* wasmCallee = unsafeCallee.asWasmCallee();
-            if (Wasm::CalleeRegistry::singleton().isValidCallee(m_wasmCalleeLocker, wasmCallee)) {
+            if (Wasm::CalleeRegistry::singleton().isValidCallee(*m_wasmCalleeLocker, wasmCallee)) {
                 // At this point, Wasm::Callee would be dying (ref count is 0), but its fields are still live.
                 // And we can safely copy Wasm::IndexOrName even when any lock is held by suspended threads.
@@ -200 +201 @@
     const AbstractLocker& m_codeBlockSetLocker;
     const AbstractLocker& m_machineThreadsLocker;
-    const AbstractLocker& m_wasmCalleeLocker;
+    const Optional<LockHolder>& m_wasmCalleeLocker;
     bool m_bailingOut { false };
     size_t m_depth { 0 };
@@ -209 +210 @@
     typedef FrameWalker Base;
 
-    CFrameWalker(VM& vm, void* machineFrame, CallFrame* callFrame, const AbstractLocker& codeBlockSetLocker, const AbstractLocker& machineThreadsLocker, const AbstractLocker& wasmCalleeLocker)
+    CFrameWalker(VM& vm, void* machineFrame, CallFrame* callFrame, const AbstractLocker& codeBlockSetLocker, const AbstractLocker& machineThreadsLocker, const Optional<LockHolder>& wasmCalleeLocker)
         : Base(vm, callFrame, codeBlockSetLocker, machineThreadsLocker, wasmCalleeLocker)
         , m_machineFrame(machineFrame)
@@ -352 +353 @@
         auto codeBlockSetLocker = holdLock(m_vm.heap.codeBlockSet().getLock());
         auto executableAllocatorLocker = holdLock(ExecutableAllocator::singleton().getLock());
+        Optional<LockHolder> wasmCalleesLocker;
 #if ENABLE(WEBASSEMBLY)
-        auto wasmCalleesLocker = holdLock(Wasm::CalleeRegistry::singleton().getLock());
-#else
-        LockHolder wasmCalleesLocker(NoLockingNecessary);
+        if (Wasm::isSupported())
+            wasmCalleesLocker = holdLock(Wasm::CalleeRegistry::singleton().getLock());
 #endif