Skip to content

Commit

Permalink
Add convenience option to enable/disable all Debug JITCode validations.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263811
rdar://117609282

Reviewed by Justin Michaud.

Sometimes, we just want to have the debuggability of a Debug build without all the JITCode
validations cluttering up the generated code.  Conversely, sometimes, we want all the
available validations enabled for a Release build test run.  We should have a jsc shell
convenience option to toggle all the JITCode validation options on or off.

What changed?
1. Added a Options::useJITAsserts() to allow us to disable all jitAssertXXX code emissions.
2. Changed Options::validateDFGExceptionHandling() to not be forced on for Debug builds.
3. Added a --useJITCodeValidations=<bool value> option to the jsc shell to enable/disable
   Options::useJITAsserts() and all JITCode validation codegen options.

* Source/JavaScriptCore/jit/AssemblyHelpers.cpp:
(JSC::AssemblyHelpers::jitAssertIsInt32):
(JSC::AssemblyHelpers::jitAssertIsJSInt32):
(JSC::AssemblyHelpers::jitAssertIsJSNumber):
(JSC::AssemblyHelpers::jitAssertIsJSDouble):
(JSC::AssemblyHelpers::jitAssertIsCell):
(JSC::AssemblyHelpers::jitAssertTagsInPlace):
(JSC::AssemblyHelpers::jitAssertHasValidCallFrame):
(JSC::AssemblyHelpers::jitAssertIsNull):
(JSC::AssemblyHelpers::jitAssertArgumentCountSane):
(JSC::AssemblyHelpers::jitAssertCodeBlockOnCallFrameWithType):
(JSC::AssemblyHelpers::jitAssertCodeBlockMatchesCurrentCalleeCodeBlockOnCallFrame):
(JSC::AssemblyHelpers::jitAssertCodeBlockOnCallFrameIsOptimizingJIT):
* Source/JavaScriptCore/jsc.cpp:
(CommandLine::parseArguments):
* Source/JavaScriptCore/runtime/Options.cpp:
(JSC::Options::setAllJITCodeValidations):
(JSC::Options::notifyOptionsChanged):
* Source/JavaScriptCore/runtime/Options.h:
* Source/JavaScriptCore/runtime/OptionsList.h:

Canonical link: https://commits.webkit.org/269879@main
  • Loading branch information
Mark Lam committed Oct 28, 2023
1 parent 3839864 commit 77b974f
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 5 deletions.
38 changes: 37 additions & 1 deletion Source/JavaScriptCore/jit/AssemblyHelpers.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2011-2022 Apple Inc. All rights reserved.
* Copyright (C) 2011-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -107,6 +107,8 @@ void AssemblyHelpers::clearSamplingFlag(int32_t flag)
#if USE(JSVALUE64)
void AssemblyHelpers::jitAssertIsInt32(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
#if CPU(X86_64) || CPU(ARM64)
Jump checkInt32 = branch64(BelowOrEqual, gpr, TrustedImm64(static_cast<uintptr_t>(0xFFFFFFFFu)));
abortWithReason(AHIsNotInt32);
Expand All @@ -118,20 +120,26 @@ void AssemblyHelpers::jitAssertIsInt32(GPRReg gpr)

void AssemblyHelpers::jitAssertIsJSInt32(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkJSInt32 = branch64(AboveOrEqual, gpr, GPRInfo::numberTagRegister);
abortWithReason(AHIsNotJSInt32);
checkJSInt32.link(this);
}

void AssemblyHelpers::jitAssertIsJSNumber(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkJSNumber = branchTest64(MacroAssembler::NonZero, gpr, GPRInfo::numberTagRegister);
abortWithReason(AHIsNotJSNumber);
checkJSNumber.link(this);
}

void AssemblyHelpers::jitAssertIsJSDouble(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkJSInt32 = branch64(AboveOrEqual, gpr, GPRInfo::numberTagRegister);
Jump checkJSNumber = branchTest64(MacroAssembler::NonZero, gpr, GPRInfo::numberTagRegister);
checkJSInt32.link(this);
Expand All @@ -141,13 +149,17 @@ void AssemblyHelpers::jitAssertIsJSDouble(GPRReg gpr)

void AssemblyHelpers::jitAssertIsCell(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkCell = branchTest64(MacroAssembler::Zero, gpr, GPRInfo::notCellMaskRegister);
abortWithReason(AHIsNotCell);
checkCell.link(this);
}

void AssemblyHelpers::jitAssertTagsInPlace()
{
if (!Options::useJITAsserts())
return;
Jump ok = branch64(Equal, GPRInfo::numberTagRegister, TrustedImm64(JSValue::NumberTag));
abortWithReason(AHNumberTagNotInPlace);
breakpoint();
Expand All @@ -160,18 +172,24 @@ void AssemblyHelpers::jitAssertTagsInPlace()
#elif USE(JSVALUE32_64)
void AssemblyHelpers::jitAssertIsInt32(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
UNUSED_PARAM(gpr);
}

void AssemblyHelpers::jitAssertIsJSInt32(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkJSInt32 = branch32(Equal, gpr, TrustedImm32(JSValue::Int32Tag));
abortWithReason(AHIsNotJSInt32);
checkJSInt32.link(this);
}

void AssemblyHelpers::jitAssertIsJSNumber(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkJSInt32 = branch32(Equal, gpr, TrustedImm32(JSValue::Int32Tag));
Jump checkJSDouble = branch32(Below, gpr, TrustedImm32(JSValue::LowestTag));
abortWithReason(AHIsNotJSNumber);
Expand All @@ -181,46 +199,60 @@ void AssemblyHelpers::jitAssertIsJSNumber(GPRReg gpr)

void AssemblyHelpers::jitAssertIsJSDouble(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkJSDouble = branch32(Below, gpr, TrustedImm32(JSValue::LowestTag));
abortWithReason(AHIsNotJSDouble);
checkJSDouble.link(this);
}

void AssemblyHelpers::jitAssertIsCell(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkCell = branchIfCell(gpr);
abortWithReason(AHIsNotCell);
checkCell.link(this);
}

void AssemblyHelpers::jitAssertTagsInPlace()
{
if (!Options::useJITAsserts())
return;
}
#endif // USE(JSVALUE32_64)

void AssemblyHelpers::jitAssertHasValidCallFrame()
{
if (!Options::useJITAsserts())
return;
Jump checkCFR = branchTestPtr(Zero, GPRInfo::callFrameRegister, TrustedImm32(7));
abortWithReason(AHCallFrameMisaligned);
checkCFR.link(this);
}

void AssemblyHelpers::jitAssertIsNull(GPRReg gpr)
{
if (!Options::useJITAsserts())
return;
Jump checkNull = branchTestPtr(Zero, gpr);
abortWithReason(AHIsNotNull);
checkNull.link(this);
}

void AssemblyHelpers::jitAssertArgumentCountSane()
{
if (!Options::useJITAsserts())
return;
Jump ok = branch32(Below, payloadFor(CallFrameSlot::argumentCountIncludingThis), TrustedImm32(10000000));
abortWithReason(AHInsaneArgumentCount);
ok.link(this);
}

void AssemblyHelpers::jitAssertCodeBlockOnCallFrameWithType(GPRReg scratchGPR, JITType type)
{
if (!Options::useJITAsserts())
return;
JIT_COMMENT(*this, "jitAssertCodeBlockOnCallFrameWithType | ", scratchGPR, " = callFrame->codeBlock->jitCode->jitType == ", type);
emitGetFromCallFrameHeaderPtr(CallFrameSlot::codeBlock, scratchGPR);
loadPtr(Address(scratchGPR, CodeBlock::jitCodeOffset()), scratchGPR);
Expand All @@ -232,6 +264,8 @@ void AssemblyHelpers::jitAssertCodeBlockOnCallFrameWithType(GPRReg scratchGPR, J

void AssemblyHelpers::jitAssertCodeBlockMatchesCurrentCalleeCodeBlockOnCallFrame(GPRReg scratchGPR, GPRReg scratchGPR2, UnlinkedCodeBlock& block)
{
if (!Options::useJITAsserts())
return;
if (block.codeType() != FunctionCode)
return;
auto kind = block.isConstructor() ? CodeForConstruct : CodeForCall;
Expand All @@ -254,6 +288,8 @@ void AssemblyHelpers::jitAssertCodeBlockMatchesCurrentCalleeCodeBlockOnCallFrame

void AssemblyHelpers::jitAssertCodeBlockOnCallFrameIsOptimizingJIT(GPRReg scratchGPR)
{
if (!Options::useJITAsserts())
return;
emitGetFromCallFrameHeaderPtr(CallFrameSlot::codeBlock, scratchGPR);
loadPtr(Address(scratchGPR, CodeBlock::jitCodeOffset()), scratchGPR);
load8(Address(scratchGPR, JITCode::offsetOfJITType()), scratchGPR);
Expand Down
11 changes: 11 additions & 0 deletions Source/JavaScriptCore/jsc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3938,6 +3938,17 @@ void CommandLine::parseArguments(int argc, char** argv)
continue;
}

static const unsigned useJITCodeValidationsStrLength = strlen("--useJITCodeValidations=");
if (!strncmp(arg, "--useJITCodeValidations=", useJITCodeValidationsStrLength)) {
const char* valueStr = argv[i] + useJITCodeValidationsStrLength;
bool success = Options::setAllJITCodeValidations(valueStr);
if (!success) {
hasBadJSCOptions = true;
dataLogLn("ERROR: invalid value for --useJITCodeValidations: ", valueStr);
}
continue;
}

// See if the -- option is a JSC VM option.
if (strstr(arg, "--") == arg) {
if (!JSC::Options::setOption(&arg[2], /* verify = */ false)) {
Expand Down
20 changes: 17 additions & 3 deletions Source/JavaScriptCore/runtime/Options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,23 @@ static void overrideDefaults()
}
}

bool Options::setAllJITCodeValidations(const char* valueStr)
{
auto value = parse<OptionsStorage::Bool>(valueStr);
if (!value)
return false;
setAllJITCodeValidations(value.value());
return true;
}

void Options::setAllJITCodeValidations(bool value)
{
Options::validateDFGClobberize() = value;
Options::validateDFGExceptionHandling() = value;
Options::validateDoesGC() = value;
Options::useJITAsserts() = value;
}

static inline void disableAllJITOptions()
{
Options::useLLInt() = true;
Expand Down Expand Up @@ -653,9 +670,6 @@ void Options::notifyOptionsChanged()
if (thresholdForGlobalLexicalBindingEpoch == 0 || thresholdForGlobalLexicalBindingEpoch == 1)
Options::thresholdForGlobalLexicalBindingEpoch() = UINT_MAX;

#if !defined(NDEBUG)
Options::validateDFGExceptionHandling() = true;
#endif
#if !ENABLE(JIT)
Options::useJIT() = false;
#endif
Expand Down
4 changes: 4 additions & 0 deletions Source/JavaScriptCore/runtime/Options.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ class Options {
JS_EXPORT_PRIVATE static void initialize();
static void finalize();

JS_EXPORT_PRIVATE static bool setAllJITCodeValidations(const char* arg);

// Parses a string of options where each option is of the format "--<optionName>=<value>"
// and are separated by a space. The leading "--" is optional and will be ignored.
JS_EXPORT_PRIVATE static bool setOptions(const char* optionsList);
Expand Down Expand Up @@ -152,6 +154,8 @@ public: \
static bool overrideAliasedOptionWithHeuristic(const char* name);
#endif

static void setAllJITCodeValidations(bool);

static bool defaultTCSMValue();
static unsigned computeNumberOfGCMarkers(unsigned maxNumberOfGCMarkers);
static unsigned computeNumberOfWorkerThreads(int maxNumberOfWorkerThreads, int minimum = 1);
Expand Down
3 changes: 2 additions & 1 deletion Source/JavaScriptCore/runtime/OptionsList.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ bool canUseHandlerIC();
v(Bool, verboseFTLCompilation, false, Normal, nullptr) \
v(Bool, logCompilationChanges, false, Normal, nullptr) \
v(Bool, printEachOSRExit, false, Normal, nullptr) \
v(Bool, useJITAsserts, ASSERT_ENABLED, Normal, nullptr) \
v(Bool, validateDoesGC, ASSERT_ENABLED, Normal, nullptr) \
v(Bool, validateGraph, false, Normal, nullptr) \
v(Bool, validateGraphAtEachPhase, false, Normal, nullptr) \
Expand Down Expand Up @@ -411,7 +412,7 @@ bool canUseHandlerIC();
v(Bool, exitOnResourceExhaustion, false, Normal, nullptr) \
v(Bool, useExceptionFuzz, false, Normal, nullptr) \
v(Unsigned, fireExceptionFuzzAt, 0, Normal, nullptr) \
v(Bool, validateDFGExceptionHandling, false, Normal, "Causes the DFG to emit code validating exception handling for each node that can exit") /* This is true by default on Debug builds */\
v(Bool, validateDFGExceptionHandling, ASSERT_ENABLED, Normal, "Causes the DFG to emit code validating exception handling for each node that can exit") \
v(Bool, dumpSimulatedThrows, false, Normal, "Dumps the call stack of the last simulated throw if exception scope verification fails") \
v(Bool, validateExceptionChecks, false, Normal, "Verifies that needed exception checks are performed.") \
v(Unsigned, unexpectedExceptionStackTraceLimit, 100, Normal, "Stack trace limit for debugging unexpected exceptions observed in the VM") \
Expand Down

0 comments on commit 77b974f

Please sign in to comment.