Skip to content

Commit a314247

Browse files
camillobruniCommit Bot
camillobruni
authored and
Commit Bot
committed
[runtime] Do not refer directly to the closure stored in the context
This is is a preparatory CL to detach the JSFunction from the Context. We mainly rewrite the DebugScopeInterator to no longer rely on the a JSFunction to be around. Additionally the empty_function needs to have a proper ScopeInfo now. Drive-by-fix: Improve ScopeInfo debug printing Bug: v8:7066 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: I2f2fa0e78914a12e076384e0e1234c2322ad1ee8 Reviewed-on: https://chromium-review.googlesource.com/918721 Commit-Queue: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#52791}
1 parent e570e67 commit a314247

19 files changed

Lines changed: 391 additions & 152 deletions

‎src/bootstrapper.cc‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ class Genesis BASE_EMBEDDED {
161161
// Creates some basic objects. Used for creating a context from scratch.
162162
void CreateRoots();
163163
// Creates the empty function. Used for creating a context from scratch.
164-
Handle<JSFunction> CreateEmptyFunction(Isolate* isolate);
164+
Handle<JSFunction> CreateEmptyFunction();
165165
// Returns the %ThrowTypeError% intrinsic function.
166166
// See ES#sec-%throwtypeerror% for details.
167167
Handle<JSFunction> GetThrowTypeErrorIntrinsic();
@@ -590,29 +590,33 @@ V8_NOINLINE void InstallSpeciesGetter(Handle<JSFunction> constructor) {
590590

591591
} // namespace
592592

593-
Handle<JSFunction> Genesis::CreateEmptyFunction(Isolate* isolate) {
594-
Factory* factory = isolate->factory();
595-
593+
Handle<JSFunction> Genesis::CreateEmptyFunction() {
596594
// Allocate the function map first and then patch the prototype later.
597-
Handle<Map> empty_function_map = factory->CreateSloppyFunctionMap(
595+
Handle<Map> empty_function_map = factory()->CreateSloppyFunctionMap(
598596
FUNCTION_WITHOUT_PROTOTYPE, MaybeHandle<JSFunction>());
599597
empty_function_map->set_is_prototype_map(true);
600598
DCHECK(!empty_function_map->is_dictionary_map());
601599

600+
// Allocate ScopeInfo for the empty function.
601+
Handle<ScopeInfo> scope_info = ScopeInfo::CreateForEmptyFunction(isolate());
602+
602603
// Allocate the empty function as the prototype for function according to
603604
// ES#sec-properties-of-the-function-prototype-object
604605
NewFunctionArgs args = NewFunctionArgs::ForBuiltin(
605-
factory->empty_string(), empty_function_map, Builtins::kEmptyFunction);
606-
Handle<JSFunction> empty_function = factory->NewFunction(args);
606+
factory()->empty_string(), empty_function_map, Builtins::kEmptyFunction);
607+
Handle<JSFunction> empty_function = factory()->NewFunction(args);
608+
native_context()->set_empty_function(*empty_function);
607609

608610
// --- E m p t y ---
609-
Handle<String> source = factory->NewStringFromStaticChars("() {}");
610-
Handle<Script> script = factory->NewScript(source);
611+
Handle<String> source = factory()->NewStringFromStaticChars("() {}");
612+
Handle<Script> script = factory()->NewScript(source);
611613
script->set_type(Script::TYPE_NATIVE);
612-
Handle<WeakFixedArray> infos = factory->NewWeakFixedArray(2);
614+
Handle<WeakFixedArray> infos = factory()->NewWeakFixedArray(2);
613615
script->set_shared_function_infos(*infos);
616+
// TODO(cbruni): fix position information here.
614617
empty_function->shared()->set_raw_start_position(0);
615618
empty_function->shared()->set_raw_end_position(source->length());
619+
empty_function->shared()->set_scope_info(*scope_info);
616620
empty_function->shared()->set_function_literal_id(1);
617621
empty_function->shared()->DontAdaptArguments();
618622
SharedFunctionInfo::SetScript(handle(empty_function->shared()), script);
@@ -5385,7 +5389,7 @@ Genesis::Genesis(
53855389
DCHECK_EQ(0u, context_snapshot_index);
53865390
// We get here if there was no context snapshot.
53875391
CreateRoots();
5388-
Handle<JSFunction> empty_function = CreateEmptyFunction(isolate);
5392+
Handle<JSFunction> empty_function = CreateEmptyFunction();
53895393
CreateSloppyModeFunctionMaps(empty_function);
53905394
CreateStrictModeFunctionMaps(empty_function);
53915395
CreateObjectFunction(empty_function);

‎src/builtins/builtins-typed-array-gen.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,8 @@ TF_BUILTIN(TypedArrayConstructor, TypedArrayBuiltinsAssembler) {
764764

765765
BIND(&throwtypeerror);
766766
{
767-
Node* name = CallRuntime(Runtime::kGetFunctionName, context, target);
767+
TNode<String> name =
768+
CAST(CallRuntime(Runtime::kGetFunctionName, context, target));
768769
ThrowTypeError(context, MessageTemplate::kConstructorNotFunction, name);
769770
}
770771
}

‎src/contexts.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ JSReceiver* Context::extension_receiver() {
114114
: extension_object();
115115
}
116116

117+
ScopeInfo* Context::raw_scope_info() {
118+
DCHECK(!IsNativeContext());
119+
return closure()->shared()->scope_info();
120+
}
121+
117122
ScopeInfo* Context::scope_info() {
118123
DCHECK(!IsNativeContext());
119124
if (IsFunctionContext() || IsModuleContext() || IsEvalContext()) {

‎src/contexts.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ enum ContextLookupFlags {
137137
V(DATA_VIEW_FUN_INDEX, JSFunction, data_view_fun) \
138138
V(DATE_FUNCTION_INDEX, JSFunction, date_function) \
139139
V(DEBUG_CONTEXT_ID_INDEX, Object, debug_context_id) \
140+
V(EMPTY_FUNCTION_INDEX, JSFunction, empty_function) \
140141
V(ERROR_MESSAGE_FOR_CODE_GEN_FROM_STRINGS_INDEX, Object, \
141142
error_message_for_code_gen_from_strings) \
142143
V(ERRORS_THROWN_INDEX, Smi, errors_thrown) \
@@ -487,6 +488,7 @@ class Context: public FixedArray {
487488
inline void set_extension(HeapObject* object);
488489
JSObject* extension_object();
489490
JSReceiver* extension_receiver();
491+
ScopeInfo* raw_scope_info();
490492
ScopeInfo* scope_info();
491493
String* catch_name();
492494

‎src/debug/debug-interface.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,9 @@ class ScopeIterator {
440440
virtual ScopeType GetType() = 0;
441441
virtual v8::Local<v8::Object> GetObject() = 0;
442442
virtual v8::Local<v8::Function> GetFunction() = 0;
443+
virtual v8::Local<v8::Value> GetFunctionDebugName() = 0;
444+
virtual int GetScriptId() = 0;
445+
virtual bool HasLocationInfo() = 0;
443446
virtual debug::Location GetStartLocation() = 0;
444447
virtual debug::Location GetEndLocation() = 0;
445448

@@ -463,7 +466,7 @@ class StackTraceIterator {
463466
virtual int GetContextId() const = 0;
464467
virtual v8::MaybeLocal<v8::Value> GetReceiver() const = 0;
465468
virtual v8::Local<v8::Value> GetReturnValue() const = 0;
466-
virtual v8::Local<v8::String> GetFunctionName() const = 0;
469+
virtual v8::Local<v8::String> GetFunctionDebugName() const = 0;
467470
virtual v8::Local<v8::debug::Script> GetScript() const = 0;
468471
virtual debug::Location GetSourceLocation() const = 0;
469472
virtual v8::Local<v8::Function> GetFunction() const = 0;

‎src/debug/debug-scope-iterator.cc‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -113,28 +113,34 @@ v8::Local<v8::Object> DebugScopeIterator::GetObject() {
113113

114114
v8::Local<v8::Function> DebugScopeIterator::GetFunction() {
115115
DCHECK(!Done());
116-
Handle<JSFunction> closure = iterator_.GetClosure();
116+
Handle<JSFunction> closure = iterator_.GetFunction();
117117
if (closure.is_null()) return v8::Local<v8::Function>();
118118
return Utils::ToLocal(closure);
119119
}
120+
int DebugScopeIterator::GetScriptId() {
121+
DCHECK(!Done());
122+
return iterator_.GetScript()->id();
123+
}
124+
125+
v8::Local<v8::Value> DebugScopeIterator::GetFunctionDebugName() {
126+
DCHECK(!Done());
127+
Handle<Object> name = iterator_.GetFunctionDebugName();
128+
return Utils::ToLocal(name);
129+
}
130+
131+
bool DebugScopeIterator::HasLocationInfo() {
132+
return iterator_.HasPositionInfo();
133+
}
120134

121135
debug::Location DebugScopeIterator::GetStartLocation() {
122136
DCHECK(!Done());
123-
Handle<JSFunction> closure = iterator_.GetClosure();
124-
if (closure.is_null()) return debug::Location();
125-
Object* obj = closure->shared()->script();
126-
if (!obj->IsScript()) return debug::Location();
127-
return ToApiHandle<v8::debug::Script>(handle(Script::cast(obj)))
137+
return ToApiHandle<v8::debug::Script>(iterator_.GetScript())
128138
->GetSourceLocation(iterator_.start_position());
129139
}
130140

131141
debug::Location DebugScopeIterator::GetEndLocation() {
132142
DCHECK(!Done());
133-
Handle<JSFunction> closure = iterator_.GetClosure();
134-
if (closure.is_null()) return debug::Location();
135-
Object* obj = closure->shared()->script();
136-
if (!obj->IsScript()) return debug::Location();
137-
return ToApiHandle<v8::debug::Script>(handle(Script::cast(obj)))
143+
return ToApiHandle<v8::debug::Script>(iterator_.GetScript())
138144
->GetSourceLocation(iterator_.end_position());
139145
}
140146

@@ -190,11 +196,23 @@ v8::Local<v8::Object> DebugWasmScopeIterator::GetObject() {
190196
return v8::Local<v8::Object>();
191197
}
192198

199+
int DebugWasmScopeIterator::GetScriptId() {
200+
DCHECK(!Done());
201+
return -1;
202+
}
203+
193204
v8::Local<v8::Function> DebugWasmScopeIterator::GetFunction() {
194205
DCHECK(!Done());
195206
return v8::Local<v8::Function>();
196207
}
197208

209+
v8::Local<v8::Value> DebugWasmScopeIterator::GetFunctionDebugName() {
210+
DCHECK(!Done());
211+
return Utils::ToLocal(isolate_->factory()->empty_string());
212+
}
213+
214+
bool DebugWasmScopeIterator::HasLocationInfo() { return false; }
215+
198216
debug::Location DebugWasmScopeIterator::GetStartLocation() {
199217
DCHECK(!Done());
200218
return debug::Location();

‎src/debug/debug-scope-iterator.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ class DebugScopeIterator final : public debug::ScopeIterator {
2424
ScopeType GetType() override;
2525
v8::Local<v8::Object> GetObject() override;
2626
v8::Local<v8::Function> GetFunction() override;
27+
v8::Local<v8::Value> GetFunctionDebugName() override;
28+
int GetScriptId() override;
29+
bool HasLocationInfo() override;
2730
debug::Location GetStartLocation() override;
2831
debug::Location GetEndLocation() override;
2932

@@ -46,12 +49,14 @@ class DebugWasmScopeIterator final : public debug::ScopeIterator {
4649
ScopeType GetType() override;
4750
v8::Local<v8::Object> GetObject() override;
4851
v8::Local<v8::Function> GetFunction() override;
52+
v8::Local<v8::Value> GetFunctionDebugName() override;
53+
int GetScriptId() override;
54+
bool HasLocationInfo() override;
4955
debug::Location GetStartLocation() override;
5056
debug::Location GetEndLocation() override;
5157

5258
bool SetVariableValue(v8::Local<v8::String> name,
5359
v8::Local<v8::Value> value) override;
54-
5560
private:
5661
Isolate* isolate_;
5762
StandardFrame* frame_;

0 commit comments

Comments
 (0)