Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(762)

Unified Diff: src/debug/debug-evaluate.cc

Issue 1513183003: [debugger] debug-evaluate should not not modify local values. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@arrowthis
Patch Set: addressed comments. Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/debug/debug-evaluate.h ('k') | src/flag-definitions.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/debug/debug-evaluate.cc
diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc
index 9d0d38446e0a6cd27ec9bca7e614544c4026b30b..e19b93eebea0174e85ae44adb4e2c011589b6405 100644
--- a/src/debug/debug-evaluate.cc
+++ b/src/debug/debug-evaluate.cc
@@ -68,16 +68,19 @@ MaybeHandle<Object> DebugEvaluate::Local(Isolate* isolate,
// variables accessible by the function we are evaluating from are
// materialized and included on top of the native context. Changes to
// the materialized object are written back afterwards.
+ // Note that the native context is taken from the original context chain,
+ // which may not be the current native context of the isolate.
ContextBuilder context_builder(isolate, frame, inlined_jsframe_index);
if (isolate->has_pending_exception()) return MaybeHandle<Object>();
- Handle<Context> context = isolate->native_context();
+ Handle<Context> context = context_builder.native_context();
Handle<JSObject> receiver(context->global_proxy());
- Handle<SharedFunctionInfo> outer_info(context->closure()->shared(), isolate);
MaybeHandle<Object> maybe_result = Evaluate(
isolate, context_builder.outer_info(),
context_builder.innermost_context(), context_extension, receiver, source);
- if (!maybe_result.is_null()) context_builder.UpdateValues();
+ if (!maybe_result.is_null() && !FLAG_debug_eval_readonly_locals) {
+ context_builder.UpdateValues();
+ }
return maybe_result;
}
@@ -127,8 +130,8 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
Handle<JSFunction> local_function =
handle(JSFunction::cast(frame_inspector.GetFunction()));
Handle<Context> outer_context(local_function->context());
- Handle<Context> native_context = isolate->native_context();
- Handle<JSFunction> global_function(native_context->closure());
+ native_context_ = Handle<Context>(outer_context->native_context());
+ Handle<JSFunction> global_function(native_context_->closure());
outer_info_ = handle(global_function->shared());
Handle<Context> inner_context;
@@ -166,7 +169,7 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
// The "this" binding, if any, can't be bound via "with". If we need
// to, add another node onto the outer context to bind "this".
Handle<Context> receiver_context =
- MaterializeReceiver(native_context, local_context, local_function,
+ MaterializeReceiver(native_context_, local_context, local_function,
global_function, it.ThisIsNonLocal());
Handle<JSObject> materialized_function = NewJSObjectWithNullProto();
@@ -309,7 +312,7 @@ void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject(
MaybeHandle<Object> DebugEvaluate::ContextBuilder::LoadFromContext(
- Handle<Context> context, Handle<String> name) {
+ Handle<Context> context, Handle<String> name, bool* global) {
static const ContextLookupFlags flags = FOLLOW_CONTEXT_CHAIN;
int index;
PropertyAttributes attributes;
@@ -320,9 +323,13 @@ MaybeHandle<Object> DebugEvaluate::ContextBuilder::LoadFromContext(
Handle<Object> value;
if (index != Context::kNotFound) { // Found on context.
Handle<Context> context = Handle<Context>::cast(holder);
+ // Do not shadow variables on the script context.
+ *global = context->IsScriptContext();
return Handle<Object>(context->get(index), isolate_);
} else { // Found on object.
Handle<JSReceiver> object = Handle<JSReceiver>::cast(holder);
+ // Do not shadow properties on the global object.
+ *global = object->IsJSGlobalObject();
return JSReceiver::GetDataProperty(object, name);
}
}
@@ -333,7 +340,13 @@ void DebugEvaluate::ContextBuilder::MaterializeContextChain(
for (const Handle<String>& name : non_locals_) {
HandleScope scope(isolate_);
Handle<Object> value;
- if (!LoadFromContext(context, name).ToHandle(&value)) continue;
+ bool global;
+ if (!LoadFromContext(context, name, &global).ToHandle(&value) || global) {
+ // If resolving the variable fails, skip it. If it resolves to a global
+ // variable, skip it as well since it's not read-only and can be resolved
+ // within debug-evaluate.
+ continue;
+ }
JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
}
}
@@ -381,7 +394,8 @@ Handle<Context> DebugEvaluate::ContextBuilder::MaterializeReceiver(
Handle<Object> receiver = isolate_->factory()->undefined_value();
Handle<String> this_string = isolate_->factory()->this_string();
if (this_is_non_local) {
- LoadFromContext(lookup_context, this_string).ToHandle(&receiver);
+ bool global;
+ LoadFromContext(lookup_context, this_string, &global).ToHandle(&receiver);
} else if (local_function->shared()->scope_info()->HasReceiver()) {
receiver = handle(frame_->receiver(), isolate_);
}
« no previous file with comments | « src/debug/debug-evaluate.h ('k') | src/flag-definitions.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698