Skip to content

Commit 0ec3ce0

Browse files
geoffreygarengeoffreygaren
committed
Unreviewed, rolling back in r190450
https://bugs.webkit.org/show_bug.cgi?id=149727 The cause of the crash was a CodeBlock, after surviving a call to deleteAllCode by virtue of being in the remembered set, trying to mark its inlined CodeBlocks via pointers from its inlined executables. Since deleteAllCode clears those pointers, the CodeBlock would ASSERT. (Any other choice to retain a CodeBlock after deleteAllCode -- for example, conservative marking -- could trigger the same bug.) The fix is for InlineCallFrame to point directly to its inlined CodeBlock instead of pointing indirectly via an executable. This guarantees that CodeBlocks are GC safe regardless of whether we've called deleteAllCode. Restored changesets: "CodeBlock should be a GC object" https://bugs.webkit.org/show_bug.cgi?id=149727 http://trac.webkit.org/changeset/190450 Canonical link: https://commits.webkit.org/167878@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190522 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent fe97bc5 commit 0ec3ce0

56 files changed

Lines changed: 868 additions & 727 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
2015-10-01 Geoffrey Garen <ggaren@apple.com>
2+
3+
Unreviewed, rolling back in r190450
4+
https://bugs.webkit.org/show_bug.cgi?id=149727
5+
6+
The cause of the crash was a CodeBlock, after surviving a call to
7+
deleteAllCode by virtue of being in the remembered set, trying to mark
8+
its inlined CodeBlocks via pointers from its inlined executables.
9+
Since deleteAllCode clears those pointers, the CodeBlock would ASSERT.
10+
(Any other choice to retain a CodeBlock after deleteAllCode -- for
11+
example, conservative marking -- could trigger the same bug.)
12+
13+
The fix is for InlineCallFrame to point directly to its inlined CodeBlock
14+
instead of pointing indirectly via an executable. This guarantees that
15+
CodeBlocks are GC safe regardless of whether we've called deleteAllCode.
16+
17+
Restored changesets:
18+
19+
"CodeBlock should be a GC object"
20+
https://bugs.webkit.org/show_bug.cgi?id=149727
21+
http://trac.webkit.org/changeset/190450
22+
123
2015-10-02 Joseph Pecoraro <pecoraro@apple.com>
224

325
Web Inspector: Include Garbage Collection Event in Timeline

‎Source/JavaScriptCore/bytecode/CodeBlock.cpp‎

Lines changed: 276 additions & 173 deletions
Large diffs are not rendered by default.

‎Source/JavaScriptCore/bytecode/CodeBlock.h‎

Lines changed: 229 additions & 94 deletions
Large diffs are not rendered by default.

‎Source/JavaScriptCore/bytecode/CodeOrigin.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ bool CodeOrigin::isApproximatelyEqualTo(const CodeOrigin& other) const
7575
if (!a.inlineCallFrame)
7676
return true;
7777

78-
if (a.inlineCallFrame->executable.get() != b.inlineCallFrame->executable.get())
78+
if (a.inlineCallFrame->baselineCodeBlock.get() != b.inlineCallFrame->baselineCodeBlock.get())
7979
return false;
8080

8181
a = a.inlineCallFrame->directCaller;
@@ -98,7 +98,7 @@ unsigned CodeOrigin::approximateHash() const
9898
if (!codeOrigin.inlineCallFrame)
9999
return result;
100100

101-
result += WTF::PtrHash<JSCell*>::hash(codeOrigin.inlineCallFrame->executable.get());
101+
result += WTF::PtrHash<JSCell*>::hash(codeOrigin.inlineCallFrame->baselineCodeBlock.get());
102102

103103
codeOrigin = codeOrigin.inlineCallFrame->directCaller;
104104
}
@@ -115,11 +115,11 @@ Vector<CodeOrigin> CodeOrigin::inlineStack() const
115115
return result;
116116
}
117117

118-
ScriptExecutable* CodeOrigin::codeOriginOwner() const
118+
CodeBlock* CodeOrigin::codeOriginOwner() const
119119
{
120120
if (!inlineCallFrame)
121121
return 0;
122-
return inlineCallFrame->executable.get();
122+
return inlineCallFrame->baselineCodeBlock.get();
123123
}
124124

125125
int CodeOrigin::stackOffset() const
@@ -143,7 +143,7 @@ void CodeOrigin::dump(PrintStream& out) const
143143
out.print(" --> ");
144144

145145
if (InlineCallFrame* frame = stack[i].inlineCallFrame) {
146-
out.print(frame->briefFunctionInformation(), ":<", RawPointer(frame->executable.get()), "> ");
146+
out.print(frame->briefFunctionInformation(), ":<", RawPointer(frame->baselineCodeBlock.get()), "> ");
147147
if (frame->isClosureCall)
148148
out.print("(closure) ");
149149
}

‎Source/JavaScriptCore/bytecode/CodeOrigin.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct CodeOrigin {
8787

8888
// If the code origin corresponds to inlined code, gives you the heap object that
8989
// would have owned the code if it had not been inlined. Otherwise returns 0.
90-
ScriptExecutable* codeOriginOwner() const;
90+
CodeBlock* codeOriginOwner() const;
9191

9292
int stackOffset() const;
9393

‎Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace JSC {
3333
DeferredCompilationCallback::DeferredCompilationCallback() { }
3434
DeferredCompilationCallback::~DeferredCompilationCallback() { }
3535

36-
void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CompilationResult result)
36+
void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CodeBlock*, CompilationResult result)
3737
{
3838
dumpCompiledSourcesIfNeeded();
3939

‎Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ class DeferredCompilationCallback : public RefCounted<DeferredCompilationCallbac
4242
public:
4343
virtual ~DeferredCompilationCallback();
4444

45-
virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*) = 0;
46-
virtual void compilationDidComplete(CodeBlock*, CompilationResult);
45+
virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*, CodeBlock* profiledDFGCodeBlock) = 0;
46+
virtual void compilationDidComplete(CodeBlock*, CodeBlock* profiledDFGCodeBlock, CompilationResult);
4747

4848
Vector<DeferredSourceDump>& ensureDeferredSourceDump();
4949

‎Source/JavaScriptCore/bytecode/EvalCodeCache.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ namespace JSC {
5151
return 0;
5252
}
5353

54-
EvalExecutable* getSlow(ExecState* exec, ScriptExecutable* owner, bool inStrictContext, ThisTDZMode thisTDZMode, const String& evalSource, JSScope* scope)
54+
EvalExecutable* getSlow(ExecState* exec, JSCell* owner, bool inStrictContext, ThisTDZMode thisTDZMode, const String& evalSource, JSScope* scope)
5555
{
5656
VariableEnvironment variablesUnderTDZ;
5757
JSScope::collectVariablesUnderTDZ(scope, variablesUnderTDZ);

‎Source/JavaScriptCore/bytecode/InlineCallFrame.cpp‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,24 +47,17 @@ JSFunction* InlineCallFrame::calleeForCallFrame(ExecState* exec) const
4747

4848
CodeBlockHash InlineCallFrame::hash() const
4949
{
50-
return jsCast<FunctionExecutable*>(executable.get())->codeBlockFor(
51-
specializationKind())->hash();
50+
return baselineCodeBlock->hash();
5251
}
5352

5453
CString InlineCallFrame::hashAsStringIfPossible() const
5554
{
56-
return jsCast<FunctionExecutable*>(executable.get())->codeBlockFor(
57-
specializationKind())->hashAsStringIfPossible();
55+
return baselineCodeBlock->hashAsStringIfPossible();
5856
}
5957

6058
CString InlineCallFrame::inferredName() const
6159
{
62-
return jsCast<FunctionExecutable*>(executable.get())->inferredName().utf8();
63-
}
64-
65-
CodeBlock* InlineCallFrame::baselineCodeBlock() const
66-
{
67-
return jsCast<FunctionExecutable*>(executable.get())->baselineCodeBlockFor(specializationKind());
60+
return jsCast<FunctionExecutable*>(baselineCodeBlock->ownerExecutable())->inferredName().utf8();
6861
}
6962

7063
void InlineCallFrame::dumpBriefFunctionInformation(PrintStream& out) const
@@ -74,8 +67,8 @@ void InlineCallFrame::dumpBriefFunctionInformation(PrintStream& out) const
7467

7568
void InlineCallFrame::dumpInContext(PrintStream& out, DumpContext* context) const
7669
{
77-
out.print(briefFunctionInformation(), ":<", RawPointer(executable.get()));
78-
if (executable->isStrictMode())
70+
out.print(briefFunctionInformation(), ":<", RawPointer(baselineCodeBlock.get()));
71+
if (isStrictMode())
7972
out.print(" (StrictMode)");
8073
out.print(", bc#", directCaller.bytecodeIndex, ", ", static_cast<Kind>(kind));
8174
if (isClosureCall)

‎Source/JavaScriptCore/bytecode/InlineCallFrame.h‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "CodeBlock.h"
3030
#include "CodeBlockHash.h"
3131
#include "CodeOrigin.h"
32-
#include "Executable.h"
3332
#include "ValueRecovery.h"
3433
#include "WriteBarrier.h"
3534
#include <wtf/BitVector.h>
@@ -42,7 +41,6 @@ namespace JSC {
4241

4342
struct InlineCallFrame;
4443
class ExecState;
45-
class ScriptExecutable;
4644
class JSFunction;
4745

4846
struct InlineCallFrame {
@@ -174,7 +172,7 @@ struct InlineCallFrame {
174172
}
175173

176174
Vector<ValueRecovery> arguments; // Includes 'this'.
177-
WriteBarrier<ScriptExecutable> executable;
175+
WriteBarrier<CodeBlock> baselineCodeBlock;
178176
ValueRecovery calleeRecovery;
179177
CodeOrigin directCaller;
180178

@@ -209,8 +207,6 @@ struct InlineCallFrame {
209207
CodeBlockHash hash() const;
210208
CString hashAsStringIfPossible() const;
211209

212-
CodeBlock* baselineCodeBlock() const;
213-
214210
void setStackOffset(signed offset)
215211
{
216212
stackOffset = offset;
@@ -220,6 +216,8 @@ struct InlineCallFrame {
220216
ptrdiff_t callerFrameOffset() const { return stackOffset * sizeof(Register) + CallFrame::callerFrameOffset(); }
221217
ptrdiff_t returnPCOffset() const { return stackOffset * sizeof(Register) + CallFrame::returnPCOffset(); }
222218

219+
bool isStrictMode() const { return baselineCodeBlock->isStrictMode(); }
220+
223221
void dumpBriefFunctionInformation(PrintStream&) const;
224222
void dump(PrintStream&) const;
225223
void dumpInContext(PrintStream&, DumpContext*) const;
@@ -231,9 +229,7 @@ struct InlineCallFrame {
231229
inline CodeBlock* baselineCodeBlockForInlineCallFrame(InlineCallFrame* inlineCallFrame)
232230
{
233231
RELEASE_ASSERT(inlineCallFrame);
234-
ScriptExecutable* executable = inlineCallFrame->executable.get();
235-
RELEASE_ASSERT(executable->structure()->classInfo() == FunctionExecutable::info());
236-
return static_cast<FunctionExecutable*>(executable)->baselineCodeBlockFor(inlineCallFrame->specializationKind());
232+
return inlineCallFrame->baselineCodeBlock.get();
237233
}
238234

239235
inline CodeBlock* baselineCodeBlockForOriginAndBaselineCodeBlock(const CodeOrigin& codeOrigin, CodeBlock* baselineCodeBlock)

0 commit comments

Comments
 (0)