Skip to content

Commit 9936a22

Browse files
ConstellationConstellation
committed
TemplateObject passed to template literal tags are not always identical for the same source location.
https://bugs.webkit.org/show_bug.cgi?id=190756 Reviewed by Saam Barati. JSTests: * complex.yaml: * complex/tagged-template-regeneration-after.js: Added. (shouldBe): * complex/tagged-template-regeneration.js: Added. (call): (test): * modules/tagged-template-inside-module.js: Added. (from.string_appeared_here.call): * modules/tagged-template-inside-module/other-tagged-templates.js: Added. (call): (export.otherTaggedTemplates): * stress/call-and-construct-should-return-same-tagged-templates.js: Added. (shouldBe): (call): (poly): * stress/tagged-templates-in-direct-eval-should-not-produce-same-site-object.js: Added. (shouldBe): (call): * stress/tagged-templates-in-function-in-direct-eval.js: Added. (shouldBe): (call): (test): * stress/tagged-templates-in-global-function-should-not-produce-same-site-object.js: Added. (shouldBe): (call): * stress/tagged-templates-in-indirect-eval-should-not-produce-same-site-object.js: Added. (shouldBe): (call): * stress/tagged-templates-in-multiple-functions.js: Added. (shouldBe): (call): (a): (b): (c): * stress/tagged-templates-with-same-start-offset.js: Added. (shouldBe): Source/JavaScriptCore: Tagged template literal requires that the site object is allocated per source location. Previously, we create the site object when linking CodeBlock and cache it in CodeBlock. But this is wrong because, 1. CodeBlock can be jettisoned and regenerated. So every time CodeBlock is regenerated, we get the different site object. 2. Call and Construct can have different CodeBlock. Even if the function is called in call-form or construct-form, we should return the same site object. In this patch, we start caching these site objects in the top-level ScriptExecutable, this matches the spec's per source location since the only one top-level ScriptExecutable is created for the given script code. Each ScriptExecutable of JSFunction can be created multiple times because CodeBlock creates it. But the top-level one is not created by CodeBlock. This top-level ScriptExecutable is well-aligned to the Script itself. The top-level ScriptExecutable now has HashMap, which maps source locations to cached site objects. 1. This patch threads the top-level ScriptExecutable to each FunctionExecutable creation. Each FunctionExecutable has a reference to the top-level ScriptExecutable. 2. We put TemplateObjectMap in ScriptExecutable, which manages cached template objects. 3. We move FunctionExecutable::m_cachedPolyProtoStructure to the FunctionExecutable::RareDate to keep FunctionExecutable 128 bytes. 4. TemplateObjectMap is indexed with endOffset of TaggedTemplate. * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Combined.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Separate.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.prototype-Combined.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.prototype-Separate.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-Combined.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-Separate.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-InternalClashingNames-Combined.js-result: * Scripts/tests/builtins/expected/WebCore-AnotherGuardedInternalBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-GuardedInternalBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result: * Scripts/wkbuiltins/builtins_templates.py: * bytecode/CodeBlock.cpp: (JSC::CodeBlock::finishCreation): (JSC::CodeBlock::setConstantRegisters): * bytecode/CodeBlock.h: * bytecode/UnlinkedFunctionExecutable.cpp: (JSC::UnlinkedFunctionExecutable::link): * bytecode/UnlinkedFunctionExecutable.h: * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::addTemplateObjectConstant): (JSC::BytecodeGenerator::emitGetTemplateObject): * bytecompiler/BytecodeGenerator.h: * parser/ASTBuilder.h: (JSC::ASTBuilder::createTaggedTemplate): * runtime/CachedTypes.cpp: (JSC::CachedTemplateObjectDescriptor::encode): (JSC::CachedTemplateObjectDescriptor::decode const): (JSC::CachedJSValue::encode): (JSC::CachedJSValue::decode const): * runtime/EvalExecutable.cpp: (JSC::EvalExecutable::ensureTemplateObjectMap): (JSC::EvalExecutable::visitChildren): * runtime/EvalExecutable.h: * runtime/FunctionExecutable.cpp: (JSC::FunctionExecutable::finishCreation): (JSC::FunctionExecutable::visitChildren): (JSC::FunctionExecutable::fromGlobalCode): (JSC::FunctionExecutable::ensureRareDataSlow): (JSC::FunctionExecutable::ensureTemplateObjectMap): * runtime/FunctionExecutable.h: * runtime/JSModuleRecord.cpp: (JSC::JSModuleRecord::instantiateDeclarations): * runtime/JSTemplateObjectDescriptor.cpp: (JSC::JSTemplateObjectDescriptor::JSTemplateObjectDescriptor): (JSC::JSTemplateObjectDescriptor::create): * runtime/JSTemplateObjectDescriptor.h: * runtime/ModuleProgramExecutable.cpp: (JSC::ModuleProgramExecutable::ensureTemplateObjectMap): (JSC::ModuleProgramExecutable::visitChildren): * runtime/ModuleProgramExecutable.h: * runtime/ProgramExecutable.cpp: (JSC::ProgramExecutable::ensureTemplateObjectMap): (JSC::ProgramExecutable::visitChildren): * runtime/ProgramExecutable.h: * runtime/ScriptExecutable.cpp: (JSC::ScriptExecutable::topLevelExecutable): (JSC::ScriptExecutable::createTemplateObject): (JSC::ScriptExecutable::ensureTemplateObjectMapImpl): (JSC::ScriptExecutable::ensureTemplateObjectMap): * runtime/ScriptExecutable.h: * tools/JSDollarVM.cpp: (JSC::functionCreateBuiltin): (JSC::functionDeleteAllCodeWhenIdle): (JSC::JSDollarVM::finishCreation): Canonical link: https://commits.webkit.org/211819@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245040 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 980ae2e commit 9936a22

50 files changed

Lines changed: 547 additions & 57 deletions

File tree

Some content is hidden

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

‎JSTests/ChangeLog‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,47 @@
1+
2019-05-07 Yusuke Suzuki <[email protected]>
2+
3+
TemplateObject passed to template literal tags are not always identical for the same source location.
4+
https://bugs.webkit.org/show_bug.cgi?id=190756
5+
6+
Reviewed by Saam Barati.
7+
8+
* complex.yaml:
9+
* complex/tagged-template-regeneration-after.js: Added.
10+
(shouldBe):
11+
* complex/tagged-template-regeneration.js: Added.
12+
(call):
13+
(test):
14+
* modules/tagged-template-inside-module.js: Added.
15+
(from.string_appeared_here.call):
16+
* modules/tagged-template-inside-module/other-tagged-templates.js: Added.
17+
(call):
18+
(export.otherTaggedTemplates):
19+
* stress/call-and-construct-should-return-same-tagged-templates.js: Added.
20+
(shouldBe):
21+
(call):
22+
(poly):
23+
* stress/tagged-templates-in-direct-eval-should-not-produce-same-site-object.js: Added.
24+
(shouldBe):
25+
(call):
26+
* stress/tagged-templates-in-function-in-direct-eval.js: Added.
27+
(shouldBe):
28+
(call):
29+
(test):
30+
* stress/tagged-templates-in-global-function-should-not-produce-same-site-object.js: Added.
31+
(shouldBe):
32+
(call):
33+
* stress/tagged-templates-in-indirect-eval-should-not-produce-same-site-object.js: Added.
34+
(shouldBe):
35+
(call):
36+
* stress/tagged-templates-in-multiple-functions.js: Added.
37+
(shouldBe):
38+
(call):
39+
(a):
40+
(b):
41+
(c):
42+
* stress/tagged-templates-with-same-start-offset.js: Added.
43+
(shouldBe):
44+
145
2019-05-07 Robin Morisset <[email protected]>
246

347
All prototypes should call didBecomePrototype()

‎JSTests/complex.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,6 @@
2525

2626
- path: complex/generator-regeneration.js
2727
cmd: runComplexTest [], ["generator-regeneration-after.js"], "--useDollarVM=1"
28+
29+
- path: complex/tagged-template-regeneration.js
30+
cmd: runComplexTest [], ["tagged-template-regeneration-after.js"], "--useDollarVM=1"

‎JSTests/complex/tagged-template-regeneration-after.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
function shouldBe(actual, expected) {
2+
if (actual !== expected)
3+
throw new Error('bad value: ' + actual);
4+
}
5+
6+
var second = test();
7+
shouldBe(first, second);

‎JSTests/complex/tagged-template-regeneration.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
function call(site)
2+
{
3+
return site;
4+
}
5+
6+
function test()
7+
{
8+
return call`Cocoa`;
9+
}
10+
11+
var first = test();
12+
$vm.deleteAllCodeWhenIdle();
13+
fullGC();

‎JSTests/modules/tagged-template-inside-module.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { shouldThrow, shouldBe } from "./resources/assert.js";
2+
import { otherTaggedTemplates } from "./tagged-template-inside-module/other-tagged-templates.js"
3+
4+
function call(site) {
5+
return site;
6+
}
7+
8+
var template = otherTaggedTemplates();
9+
shouldBe(call`Cocoa` !== template, true);
10+
shouldBe(template, otherTaggedTemplates());
11+
shouldBe(template, new otherTaggedTemplates());

‎JSTests/modules/tagged-template-inside-module/other-tagged-templates.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
function call(site)
2+
{
3+
return site;
4+
}
5+
6+
export function otherTaggedTemplates()
7+
{
8+
return call`Cocoa`;
9+
}

‎JSTests/stress/call-and-construct-should-return-same-tagged-templates.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
function shouldBe(actual, expected)
2+
{
3+
if (actual !== expected)
4+
throw new Error('bad value: ' + actual);
5+
}
6+
7+
function call(site)
8+
{
9+
return site;
10+
}
11+
12+
function poly()
13+
{
14+
return call`Cocoa`;
15+
}
16+
17+
var first = poly();
18+
var second = new poly();
19+
20+
shouldBe(first, second);

‎JSTests/stress/tagged-templates-in-direct-eval-should-not-produce-same-site-object.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
function shouldBe(actual, expected) {
2+
if (actual !== expected)
3+
throw new Error('bad value: ' + actual);
4+
}
5+
6+
function call(site)
7+
{
8+
return site;
9+
}
10+
11+
var expr = "call`Cocoa`";
12+
var first = eval(expr);
13+
var second = eval(expr);
14+
shouldBe(first !== second, true);

‎JSTests/stress/tagged-templates-in-function-in-direct-eval.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
function shouldBe(actual, expected)
2+
{
3+
if (actual !== expected)
4+
throw new Error('bad value: ' + actual);
5+
}
6+
7+
function call(site)
8+
{
9+
return site;
10+
}
11+
12+
function test()
13+
{
14+
return eval("(function ok() { return call`Cocoa`; })()");
15+
}
16+
17+
var first = test();
18+
var second = test();
19+
shouldBe(first !== second, true);

‎JSTests/stress/tagged-templates-in-global-function-should-not-produce-same-site-object.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
function shouldBe(actual, expected) {
2+
if (actual !== expected)
3+
throw new Error('bad value: ' + actual);
4+
}
5+
6+
function call(site)
7+
{
8+
return site;
9+
}
10+
11+
var expr = "return call`Cocoa`";
12+
var firstFunction = Function(expr);
13+
var secondFunction = Function(expr);
14+
var first = firstFunction();
15+
var second = secondFunction();
16+
shouldBe(first !== second, true);
17+
18+
shouldBe(first, firstFunction());
19+
shouldBe(first, new firstFunction());
20+
shouldBe(second, secondFunction());
21+
shouldBe(second, new secondFunction());

0 commit comments

Comments
 (0)