- Revision
- 220432
- Author
- commit-qu...@webkit.org
- Date
- 2017-08-08 17:26:04 -0700 (Tue, 08 Aug 2017)
Log Message
Make JSC_validateExceptionChecks=1 succeed on JSTests/slowMicrobenchmarks/spread-small-array.js.
https://bugs.webkit.org/show_bug.cgi?id=175347
Patch by Robin Morisset <rmoris...@apple.com> on 2017-08-08
Reviewed by Saam Barati.
This is done by making finishCreation explicitely check for exceptions after setConstantRegister and setConstantIdentifiersSetRegisters.
I chose to have this check replace the boolean returned previously by these functions for readability. The performance impact should be
negligible considering how much more finishCreation does.
This fix then caused another issue to appear as it was now clear that finishCreation can throw. And since it is called by ProgramCodeBlock::create(),
FunctionCodeBlock::create() and friends, that are in turn called by ScriptExecutable::newCodeBlockFor, this last function also required a few tweaks.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
(JSC::CodeBlock::setConstantIdentifierSetRegisters):
(JSC::CodeBlock::setConstantRegisters):
* bytecode/CodeBlock.h:
* runtime/ScriptExecutable.cpp:
(JSC::ScriptExecutable::newCodeBlockFor):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (220431 => 220432)
--- trunk/Source/_javascript_Core/ChangeLog 2017-08-09 00:18:08 UTC (rev 220431)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-08-09 00:26:04 UTC (rev 220432)
@@ -1,3 +1,24 @@
+2017-08-08 Robin Morisset <rmoris...@apple.com>
+
+ Make JSC_validateExceptionChecks=1 succeed on JSTests/slowMicrobenchmarks/spread-small-array.js.
+ https://bugs.webkit.org/show_bug.cgi?id=175347
+
+ Reviewed by Saam Barati.
+
+ This is done by making finishCreation explicitely check for exceptions after setConstantRegister and setConstantIdentifiersSetRegisters.
+ I chose to have this check replace the boolean returned previously by these functions for readability. The performance impact should be
+ negligible considering how much more finishCreation does.
+ This fix then caused another issue to appear as it was now clear that finishCreation can throw. And since it is called by ProgramCodeBlock::create(),
+ FunctionCodeBlock::create() and friends, that are in turn called by ScriptExecutable::newCodeBlockFor, this last function also required a few tweaks.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::finishCreation):
+ (JSC::CodeBlock::setConstantIdentifierSetRegisters):
+ (JSC::CodeBlock::setConstantRegisters):
+ * bytecode/CodeBlock.h:
+ * runtime/ScriptExecutable.cpp:
+ (JSC::ScriptExecutable::newCodeBlockFor):
+
2017-08-08 Michael Catanzaro <mcatanz...@igalia.com>
Unreviewed, fix Ubuntu LTS build
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (220431 => 220432)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-08-09 00:18:08 UTC (rev 220431)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-08-09 00:26:04 UTC (rev 220432)
@@ -401,15 +401,16 @@
JSScope* scope)
{
Base::finishCreation(vm);
+ auto throwScope = DECLARE_THROW_SCOPE(vm);
if (vm.typeProfiler() || vm.controlFlowProfiler())
vm.functionHasExecutedCache()->removeUnexecutedRange(ownerExecutable->sourceID(), ownerExecutable->typeProfilingStartOffset(), ownerExecutable->typeProfilingEndOffset());
- if (!setConstantRegisters(unlinkedCodeBlock->constantRegisters(), unlinkedCodeBlock->constantsSourceCodeRepresentation()))
- return false;
+ setConstantRegisters(unlinkedCodeBlock->constantRegisters(), unlinkedCodeBlock->constantsSourceCodeRepresentation());
+ RETURN_IF_EXCEPTION(throwScope, false);
- if (!setConstantIdentifierSetRegisters(vm, unlinkedCodeBlock->constantIdentifierSets()))
- return false;
+ setConstantIdentifierSetRegisters(vm, unlinkedCodeBlock->constantIdentifierSets());
+ RETURN_IF_EXCEPTION(throwScope, false);
if (unlinkedCodeBlock->usesGlobalObject())
m_constantRegisters[unlinkedCodeBlock->globalObjectRegister().toConstantIndex()].set(*m_vm, this, m_globalObject.get());
@@ -869,7 +870,7 @@
#endif // ENABLE(JIT)
}
-bool CodeBlock::setConstantIdentifierSetRegisters(VM& vm, const Vector<ConstantIndentifierSetEntry>& constants)
+void CodeBlock::setConstantIdentifierSetRegisters(VM& vm, const Vector<ConstantIndentifierSetEntry>& constants)
{
auto scope = DECLARE_THROW_SCOPE(vm);
JSGlobalObject* globalObject = m_globalObject.get();
@@ -877,24 +878,21 @@
for (const auto& entry : constants) {
Structure* setStructure = globalObject->setStructure();
- RETURN_IF_EXCEPTION(scope, false);
+ RETURN_IF_EXCEPTION(scope, void());
JSSet* jsSet = JSSet::create(exec, vm, setStructure);
- RETURN_IF_EXCEPTION(scope, false);
+ RETURN_IF_EXCEPTION(scope, void());
const IdentifierSet& set = entry.first;
for (auto setEntry : set) {
JSString* jsString = jsOwnedString(&vm, setEntry.get());
jsSet->add(exec, jsString);
- RETURN_IF_EXCEPTION(scope, false);
+ RETURN_IF_EXCEPTION(scope, void());
}
m_constantRegisters[entry.second].set(vm, this, jsSet);
}
-
- scope.release();
- return true;
}
-bool CodeBlock::setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation)
+void CodeBlock::setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation)
{
auto scope = DECLARE_THROW_SCOPE(*m_vm);
JSGlobalObject* globalObject = m_globalObject.get();
@@ -921,7 +919,7 @@
constant = clone;
} else if (isTemplateRegistryKey(*m_vm, constant)) {
auto* templateObject = globalObject->templateRegistry().getTemplateObject(exec, jsCast<JSTemplateRegistryKey*>(constant));
- RETURN_IF_EXCEPTION(scope, false);
+ RETURN_IF_EXCEPTION(scope, void());
constant = templateObject;
}
}
@@ -930,8 +928,6 @@
}
m_constantsSourceCodeRepresentation = constantsSourceCodeRepresentation;
-
- return true;
}
void CodeBlock::setAlternative(VM& vm, CodeBlock* alternative)
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (220431 => 220432)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2017-08-09 00:18:08 UTC (rev 220431)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2017-08-09 00:26:04 UTC (rev 220432)
@@ -918,9 +918,9 @@
void updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
- bool setConstantIdentifierSetRegisters(VM&, const Vector<ConstantIndentifierSetEntry>& constants);
+ void setConstantIdentifierSetRegisters(VM&, const Vector<ConstantIndentifierSetEntry>& constants);
- bool setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation);
+ void setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation);
void replaceConstant(int index, JSValue value)
{
Modified: trunk/Source/_javascript_Core/runtime/ScriptExecutable.cpp (220431 => 220432)
--- trunk/Source/_javascript_Core/runtime/ScriptExecutable.cpp 2017-08-09 00:18:08 UTC (rev 220431)
+++ trunk/Source/_javascript_Core/runtime/ScriptExecutable.cpp 2017-08-09 00:26:04 UTC (rev 220432)
@@ -187,6 +187,7 @@
auto codeBlock = EvalCodeBlock::create(vm,
executable, executable->m_unlinkedEvalCodeBlock.get(), scope,
executable->source().provider());
+ ASSERT(throwScope.exception() || codeBlock);
if (!codeBlock) {
exception = throwException(
exec, throwScope,
@@ -204,6 +205,7 @@
auto codeBlock = ProgramCodeBlock::create(vm,
executable, executable->m_unlinkedProgramCodeBlock.get(), scope,
executable->source().provider(), startColumn());
+ ASSERT(throwScope.exception() || codeBlock);
if (!codeBlock) {
exception = throwException(
exec, throwScope,
@@ -221,6 +223,7 @@
auto codeBlock = ModuleProgramCodeBlock::create(vm,
executable, executable->m_unlinkedModuleProgramCodeBlock.get(), scope,
executable->source().provider(), startColumn());
+ ASSERT(throwScope.exception() || codeBlock);
if (!codeBlock) {
exception = throwException(
exec, throwScope,
@@ -251,6 +254,7 @@
return nullptr;
}
+ throwScope.release();
return FunctionCodeBlock::create(vm, executable, unlinkedCodeBlock, scope,
source().provider(), source().startOffset(), startColumn());
}
