Skip to content

Commit 774d5a6

Browse files
verwaestV8 LUCI CQ
verwaest
authored and
V8 LUCI CQ
committed
[parser] Fix eval handling in scope info reuse
Only reuse scope infos up to the outer scope info of the shared function info we're compiling. That guarantees we'll only reuse scope infos belonging to the current script (the outer scope info of the sfi is the first scope info that could belong to an outer script in case of eval); so we can also drop EvalState again \o/ Bug: 352739458 Change-Id: I8e9dd66b5a2c5367e2def716a4cf990abadd0264 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5703778 Commit-Queue: Toon Verwaest <[email protected]> Auto-Submit: Toon Verwaest <[email protected]> Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Leszek Swirski <[email protected]> Cr-Commit-Position: refs/heads/main@{#95026}
1 parent 9840eed commit 774d5a6

8 files changed

Lines changed: 21 additions & 41 deletions

File tree

‎src/ast/scopes.cc‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,6 @@ void DeclarationScope::SetDefaults() {
329329
is_skipped_function_ = false;
330330
preparse_data_builder_ = nullptr;
331331
class_scope_has_private_brand_ = false;
332-
eval_state_ = false;
333332
#ifdef DEBUG
334333
DeclarationScope* outer_declaration_scope =
335334
outer_scope_ ? outer_scope_->GetDeclarationScope() : nullptr;
@@ -354,7 +353,6 @@ void Scope::SetDefaults() {
354353

355354
calls_eval_ = false;
356355
sloppy_eval_can_extend_vars_ = false;
357-
eval_state_ = false;
358356
scope_nonlinear_ = false;
359357
is_hidden_ = false;
360358
is_debug_evaluate_scope_ = false;
@@ -558,16 +556,6 @@ const DeclarationScope* Scope::AsDeclarationScope() const {
558556
return static_cast<const DeclarationScope*>(this);
559557
}
560558

561-
void DeclarationScope::set_eval_state() {
562-
DCHECK(is_eval_scope());
563-
if (outer_scope_->scope_info_.is_null()) {
564-
CHECK(outer_scope_->is_script_scope());
565-
eval_state_ = true;
566-
} else {
567-
eval_state_ = !outer_scope_->scope_info()->EvalState();
568-
}
569-
}
570-
571559
ModuleScope* Scope::AsModuleScope() {
572560
SBXCHECK(is_module_scope());
573561
return static_cast<ModuleScope*>(this);
@@ -2813,6 +2801,10 @@ void DeclarationScope::AllocateScopeInfos(ParseInfo* info,
28132801
Tagged<WeakFixedArray> infos = script->shared_function_infos();
28142802
std::unordered_map<int, Handle<ScopeInfo>> scope_infos_to_reuse;
28152803
if (v8_flags.reuse_scope_infos && infos->length() != 0) {
2804+
Tagged<SharedFunctionInfo> sfi = *info->literal()->shared_function_info();
2805+
Tagged<ScopeInfo> outer = sfi->HasOuterScopeInfo()
2806+
? sfi->GetOuterScopeInfo()
2807+
: Tagged<ScopeInfo>();
28162808
// Look at all the existing inner functions (they are numbered id+1 until
28172809
// max_id+1) to reattach their outer scope infos to corresponding scopes.
28182810
for (int i = info->literal()->function_literal_id() + 1;
@@ -2826,8 +2818,7 @@ void DeclarationScope::AllocateScopeInfos(ParseInfo* info,
28262818
if (!sfi->HasOuterScopeInfo()) continue;
28272819
Tagged<ScopeInfo> scope_info = sfi->GetOuterScopeInfo();
28282820
while (true) {
2829-
if (scope_info->EvalState() != scope->eval_state()) break;
2830-
if (scope_info->StartPosition() < scope->start_position()) break;
2821+
if (scope_info == outer) break;
28312822
int id = scope_info->UniqueIdInScript();
28322823
auto it = scope_infos_to_reuse.find(id);
28332824
if (it != scope_infos_to_reuse.end()) {

‎src/ast/scopes.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
386386
bool has_await_using_declaration() const {
387387
return has_await_using_declaration_;
388388
}
389-
bool eval_state() const { return eval_state_; }
390389

391390
#if V8_ENABLE_WEBASSEMBLY
392391
bool IsAsmModule() const;
@@ -741,7 +740,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
741740
inner_scope->sibling_ = inner_scope_;
742741
inner_scope_ = inner_scope;
743742
inner_scope->outer_scope_ = this;
744-
inner_scope->eval_state_ = eval_state_;
745743
}
746744

747745
void SetDefaults();
@@ -828,8 +826,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
828826

829827
bool must_use_preparsed_scope_data_ : 1;
830828

831-
bool eval_state_ : 1;
832-
833829
// True if this is a deserialized scope which caches its lookups on another
834830
// Scope's variable map. This will be true for every scope above the first
835831
// non-eval declaration scope above the compilation entry point, e.g. for
@@ -889,8 +885,6 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
889885
return is_function_scope() && IsArrowFunction(function_kind_);
890886
}
891887

892-
void set_eval_state();
893-
894888
// Inform the scope and outer scopes that the corresponding code contains an
895889
// eval call.
896890
void RecordDeclarationScopeEvalCall() {

‎src/codegen/compiler.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2024,10 +2024,8 @@ class ConstantPoolPointerForwarder {
20242024
Tagged<SharedFunctionInfo> old_sfi =
20252025
Cast<SharedFunctionInfo>(maybe_old_sfi.GetHeapObjectAssumeWeak());
20262026
if (!old_sfi->HasOuterScopeInfo()) return;
2027-
bool eval_state = old_sfi->scope_info()->EvalState();
20282027
Tagged<ScopeInfo> scope_info = old_sfi->GetOuterScopeInfo();
20292028
while (true) {
2030-
CHECK_EQ(scope_info->EvalState(), eval_state);
20312029
if (scope_infos_to_update_.find(scope_info->StartPosition()) !=
20322030
scope_infos_to_update_.end()) {
20332031
return;

‎src/objects/scope-info.cc‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,6 @@ Handle<ScopeInfo> ScopeInfo::Create(IsolateT* isolate, Zone* zone, Scope* scope,
248248
PrivateNameLookupSkipsOuterClassBit::encode(
249249
scope->private_name_lookup_skips_outer_class()) |
250250
HasContextExtensionSlotBit::encode(scope->HasContextExtensionSlot()) |
251-
EvalStateBit::encode(scope->eval_state()) |
252251
HasLocalsBlockListBit::encode(false);
253252
scope_info->set_flags(flags);
254253

@@ -450,7 +449,7 @@ Handle<ScopeInfo> ScopeInfo::CreateForWithScope(
450449
IsDebugEvaluateScopeBit::encode(false) |
451450
ForceContextAllocationBit::encode(false) |
452451
PrivateNameLookupSkipsOuterClassBit::encode(false) |
453-
HasContextExtensionSlotBit::encode(true) | EvalStateBit::encode(false) |
452+
HasContextExtensionSlotBit::encode(true) |
454453
HasLocalsBlockListBit::encode(false);
455454
scope_info->set_flags(flags);
456455

@@ -467,8 +466,6 @@ Handle<ScopeInfo> ScopeInfo::CreateForWithScope(
467466
if (has_outer_scope_info) {
468467
Tagged<ScopeInfo> outer = *outer_scope.ToHandleChecked();
469468
scope_info->set(index++, outer);
470-
scope_info->set_flags(
471-
EvalStateBit::update(scope_info->flags(), !outer->EvalState()));
472469
}
473470
DCHECK_EQ(index, scope_info->length());
474471
DCHECK_EQ(0, scope_info->ParameterCount());
@@ -546,7 +543,7 @@ Handle<ScopeInfo> ScopeInfo::CreateForBootstrapping(Isolate* isolate,
546543
PrivateNameLookupSkipsOuterClassBit::encode(false) |
547544
HasContextExtensionSlotBit::encode(is_native_context ||
548545
has_const_tracking_let_side_data) |
549-
EvalStateBit::encode(false) | HasLocalsBlockListBit::encode(false);
546+
HasLocalsBlockListBit::encode(false);
550547
Tagged<ScopeInfo> raw_scope_info = *scope_info;
551548
raw_scope_info->set_flags(flags);
552549
raw_scope_info->set_parameter_count(parameter_count);
@@ -829,8 +826,6 @@ bool ScopeInfo::HasLocalsBlockList() const {
829826
return HasLocalsBlockListBit::decode(Flags());
830827
}
831828

832-
bool ScopeInfo::EvalState() const { return EvalStateBit::decode(Flags()); }
833-
834829
Tagged<StringSet> ScopeInfo::LocalsBlockList() const {
835830
DCHECK(HasLocalsBlockList());
836831
return Cast<StringSet>(locals_block_list());

‎src/objects/scope-info.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,6 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
249249
// come from debug evaluate but are different to IsDebugEvaluateScope().
250250
bool IsReplModeScope() const;
251251

252-
// 1 bit of information that, paired with a bit from the script, allows us to
253-
// identify whether the scope info still belongs to the script, or already to
254-
// a parent. Once a scope info belongs to a parent, the bit may match up again
255-
// with the bit of this script once we reach the parent of a parent (in case
256-
// of nested eval).
257-
bool EvalState() const;
258-
259252
#ifdef DEBUG
260253
// For LiveEdit we ignore:
261254
// - position info: "unchanged" functions are allowed to move in a script

‎src/objects/scope-info.tq‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ bitfield struct ScopeFlags extends uint32 {
8080
private_name_lookup_skips_outer_class: bool: 1 bit;
8181
has_context_extension_slot: bool: 1 bit;
8282
has_locals_block_list: bool: 1 bit;
83-
eval_state: bool: 1 bit;
8483
is_empty: bool: 1 bit;
8584
}
8685

‎src/parsing/parser-base.h‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -801,10 +801,7 @@ class ParserBase {
801801
}
802802

803803
DeclarationScope* NewEvalScope(Scope* parent) const {
804-
DeclarationScope* result =
805-
zone()->template New<DeclarationScope>(zone(), parent, EVAL_SCOPE);
806-
result->set_eval_state();
807-
return result;
804+
return zone()->template New<DeclarationScope>(zone(), parent, EVAL_SCOPE);
808805
}
809806

810807
ClassScope* NewClassScope(Scope* parent, bool is_anonymous) const {

‎test/mjsunit/regress-352739458.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2024 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --reuse-scope-infos --allow-natives-syntax
6+
7+
Realm.eval(Realm.current(), `function f(s) {
8+
return eval(s);
9+
}
10+
let g = f("0,function(y) { return ()=>{ eval() } }");
11+
let i = g(1);
12+
%ForceFlush(g);
13+
print(g(2));`);

0 commit comments

Comments
 (0)