Skip to content
Permalink
Browse files
CodeBlock should not add/remove LoopHintExecutionCounters.
https://bugs.webkit.org/show_bug.cgi?id=231209
rdar://83571235

Reviewed by Saam Barati.

JSTests:

* stress/codeBlock-should-not-add-remove-loop-hint-execution-counters-due-to-cached-unlinked-baseline-code.js: Added.

Source/JavaScriptCore:

This is because cached unlinked baseline JIT code would retain a pointer to those
counters.  Hence, the UnlinkedCodeBlock should do the add /remove of the counters
instead.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
(JSC::CodeBlock::~CodeBlock):
* bytecode/UnlinkedCodeBlock.cpp:
(JSC::UnlinkedCodeBlock::initializeLoopHintExecutionCounter):
(JSC::UnlinkedCodeBlock::~UnlinkedCodeBlock):
* bytecode/UnlinkedCodeBlock.h:
* bytecode/UnlinkedCodeBlockGenerator.cpp:
(JSC::UnlinkedCodeBlockGenerator::finalize):


Canonical link: https://commits.webkit.org/242532@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@283567 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Lam committed Oct 5, 2021
1 parent 830d0b6 commit bf9b4356a9ab6ae24cbd449c4ded3219cffe6eba
Showing 7 changed files with 64 additions and 13 deletions.
@@ -1,3 +1,13 @@
2021-10-05 Mark Lam <mark.lam@apple.com>

CodeBlock should not add/remove LoopHintExecutionCounters.
https://bugs.webkit.org/show_bug.cgi?id=231209
rdar://83571235

Reviewed by Saam Barati.

* stress/codeBlock-should-not-add-remove-loop-hint-execution-counters-due-to-cached-unlinked-baseline-code.js: Added.

2021-10-05 Yusuke Suzuki <ysuzuki@apple.com>

[JSC] JSPropertyNameEnumerator should not have cached prototype chain since empty JSPropertyNameEnumerator is shared
@@ -0,0 +1,10 @@
//@ runDefault("--returnEarlyFromInfiniteLoopsForFuzzing=true", "--forceCodeBlockToJettisonDueToOldAge=true", "--collectContinuously=true")

async function foo() {
for (let i = 0; i < 1000; i++);
}

for (let i = 0; i < 1000; i++) {
foo();
edenGC();
}
@@ -1,3 +1,25 @@
2021-10-05 Mark Lam <mark.lam@apple.com>

CodeBlock should not add/remove LoopHintExecutionCounters.
https://bugs.webkit.org/show_bug.cgi?id=231209
rdar://83571235

Reviewed by Saam Barati.

This is because cached unlinked baseline JIT code would retain a pointer to those
counters. Hence, the UnlinkedCodeBlock should do the add /remove of the counters
instead.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
(JSC::CodeBlock::~CodeBlock):
* bytecode/UnlinkedCodeBlock.cpp:
(JSC::UnlinkedCodeBlock::initializeLoopHintExecutionCounter):
(JSC::UnlinkedCodeBlock::~UnlinkedCodeBlock):
* bytecode/UnlinkedCodeBlock.h:
* bytecode/UnlinkedCodeBlockGenerator.cpp:
(JSC::UnlinkedCodeBlockGenerator::finalize):

2021-10-05 Kate Cheney <katherine_cheney@apple.com>

CSP: unsafe-eval tests timing out or failing
@@ -722,12 +722,6 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
break;
}

case op_loop_hint: {
if (UNLIKELY(Options::returnEarlyFromInfiniteLoopsForFuzzing()))
vm.addLoopHintExecutionCounter(instruction.ptr());
break;
}

default:
break;
}
@@ -850,13 +844,6 @@ CodeBlock::~CodeBlock()
// So, we can access member UnlinkedCodeBlock safely here. We bypass the assertion by using unvalidatedGet.
UnlinkedCodeBlock* unlinkedCodeBlock = m_unlinkedCode.unvalidatedGet();

if (UNLIKELY(Options::returnEarlyFromInfiniteLoopsForFuzzing() && JITCode::isBaselineCode(jitType()))) {
for (const auto& instruction : unlinkedCodeBlock->instructions()) {
if (instruction->is<OpLoopHint>())
vm.removeLoopHintExecutionCounter(instruction.ptr());
}
}

if (JITCode::isBaselineCode(jitType())) {
if (m_metadata) {
m_metadata->forEach<OpCatch>([&](auto& metadata) {
@@ -83,6 +83,16 @@ UnlinkedCodeBlock::UnlinkedCodeBlock(VM& vm, Structure* structure, CodeType code
m_llintExecuteCounter.setNewThreshold(thresholdForJIT(Options::thresholdForJITAfterWarmUp()));
}

void UnlinkedCodeBlock::initializeLoopHintExecutionCounter()
{
ASSERT(Options::returnEarlyFromInfiniteLoopsForFuzzing());
VM& vm = this->vm();
for (const auto& instruction : instructions()) {
if (instruction->is<OpLoopHint>())
vm.addLoopHintExecutionCounter(instruction.ptr());
}
}

template<typename Visitor>
void UnlinkedCodeBlock::visitChildrenImpl(JSCell* cell, Visitor& visitor)
{
@@ -273,6 +283,13 @@ bool UnlinkedCodeBlock::typeProfilerExpressionInfoForBytecodeOffset(unsigned byt

UnlinkedCodeBlock::~UnlinkedCodeBlock()
{
if (UNLIKELY(Options::returnEarlyFromInfiniteLoopsForFuzzing())) {
VM& vm = this->vm();
for (const auto& instruction : instructions()) {
if (instruction->is<OpLoopHint>())
vm.removeLoopHintExecutionCounter(instruction.ptr());
}
}
}

const InstructionStream& UnlinkedCodeBlock::instructions() const
@@ -141,6 +141,8 @@ class UnlinkedCodeBlock : public JSCell {

enum { CallFunction, ApplyFunction };

void initializeLoopHintExecutionCounter();

bool isConstructor() const { return m_isConstructor; }
bool usesCallEval() const { return m_usesCallEval; }
void setUsesCallEval() { m_usesCallEval = true; }
@@ -152,6 +152,9 @@ void UnlinkedCodeBlockGenerator::finalize(std::unique_ptr<InstructionStream> ins
m_codeBlock->m_rareData->m_bitVectors = WTFMove(m_bitVectors);
m_codeBlock->m_rareData->m_constantIdentifierSets = WTFMove(m_constantIdentifierSets);
}

if (UNLIKELY(Options::returnEarlyFromInfiniteLoopsForFuzzing()))
m_codeBlock->initializeLoopHintExecutionCounter();
}
m_vm.heap.writeBarrier(m_codeBlock.get());
m_vm.heap.reportExtraMemoryAllocated(m_codeBlock->m_instructions->sizeInBytes() + m_codeBlock->m_metadata->sizeInBytes());

0 comments on commit bf9b435

Please sign in to comment.