[webkit-reviews] review granted: [Bug 195982] [JSC] Do not create JIT related data under non-JIT mode : [Attachment 365476] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 195982: [JSC] Do not create JIT related data under non-JIT mode
https://bugs.webkit.org/show_bug.cgi?id=195982

Attachment 365476: Patch

https://bugs.webkit.org/attachment.cgi?id=365476&action=review




--- Comment #7 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 365476
  --> https://bugs.webkit.org/attachment.cgi?id=365476
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365476&action=review

r=me with fixes.  If you haven't already done so, please also do a cloop build
(build-jsc --cloop) to make sure it still builds.

> Source/JavaScriptCore/jit/ExecutableAllocator.h:66
> +    enum ProtectionSetting { Writable, Executable };

Looks like this is no longer used anywhere.  We can delete it.

> Source/JavaScriptCore/runtime/VM.cpp:195
> +    ExecutableAllocator::initializeUnderlyingAllocator();
> +    if (!ExecutableAllocator::singleton().isValid()) {

While you're in this function, I suggest you also fix it to check
getenv("JavaScriptCoreUseJIT") first (currently done below) before initializing
the ExecutableAllocator.  No point in doing so if
getenv("JavaScriptCoreUseJIT") disallows use of the JIT.

> Source/JavaScriptCore/tools/SigillCrashAnalyzer.cpp:-63
> -#if USE(ARM64_DISASSEMBLER)
> -    A64DOpcode m_arm64Opcode;
> -#endif

Undo this.  See the reason below.

> Source/JavaScriptCore/tools/SigillCrashAnalyzer.cpp:288
> +    auto arm64Opcode = std::make_unique<A64DOpcode>();

dumpCodeBlock() is called from SigillCrashAnalyzer::analyze(), which in turn is
called from a signal handler.  We don't want to malloc stuff while in the
signal handler.  So, let's keep the A64DOpcode where it's at right now, in the
SigillCrashAnalyzer.  Instead, let's change Options.cpp to not
enableSigillCrashAnalyzer() if the JIT is not in use.  The SigillCrashAnalyzer
is for analyzing JIT code that went haywire.  If the JIT is not in use, we
don't need to enable it, and hence, won't allocate the A64DOpcode.