[wasm][debug] Compile source only once for Debug-Evaluate.

We accidentally compiled the source for Debug-Evaluate on Wasm frames
twice, and used the first script result as outer function info for the
second, actual compile (which goes via the `eval` machinery).

Also unify DebugEvaluate::Local and DebugEvaluate::WebAssembly, since
they are essentially very similar, except for the context.

Bug: chromium:1127914
Change-Id: I8eb85c128c05ab1d4fcb73061de89b0942d1e5c8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2605198
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71890}

src/debug/debug-evaluate.cc

@@ -78,58 +78,37 @@ MaybeHandle<Object> DebugEvaluate::Local(Isolate* isolate,
 
   // Get the frame where the debugging is performed.
   StackTraceFrameIterator it(isolate, frame_id);
-  if (!it.is_javascript()) return isolate->factory()->undefined_value();
-  JavaScriptFrame* frame = it.javascript_frame();
+  if (it.is_javascript()) {
+    JavaScriptFrame* frame = it.javascript_frame();
+    // This is not a lot different than DebugEvaluate::Global, except that
+    // 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 {};
 
-  // This is not a lot different than DebugEvaluate::Global, except that
-  // 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 = context_builder.evaluation_context();
-  Handle<JSObject> receiver(context->global_proxy(), isolate);
-  MaybeHandle<Object> maybe_result =
-      Evaluate(isolate, context_builder.outer_info(), context, receiver, source,
-               throw_on_side_effect);
-  if (!maybe_result.is_null()) context_builder.UpdateValues();
-  return maybe_result;
-}
-
-V8_EXPORT MaybeHandle<Object> DebugEvaluate::WebAssembly(
-    Handle<WasmInstanceObject> instance, StackFrameId frame_id,
-    Handle<String> source, bool throw_on_side_effect) {
-  Isolate* isolate = instance->GetIsolate();
-
-  StackTraceFrameIterator it(isolate, frame_id);
-  if (!it.is_wasm()) return isolate->factory()->undefined_value();
-  WasmFrame* frame = WasmFrame::cast(it.frame());
-
-  Handle<JSProxy> context_extension = WasmJs::GetJSDebugProxy(frame);
-
-  DisableBreak disable_break_scope(isolate->debug(), /*disable=*/true);
-
-  Handle<SharedFunctionInfo> shared_info;
-  if (!GetFunctionInfo(isolate, source, REPLMode::kNo).ToHandle(&shared_info)) {
-    return {};
-  }
-
-  Handle<ScopeInfo> scope_info =
-      ScopeInfo::CreateForWithScope(isolate, Handle<ScopeInfo>::null());
-  Handle<Context> context = isolate->factory()->NewWithContext(
-      isolate->native_context(), scope_info, context_extension);
-
-  Handle<Object> result;
-  if (!DebugEvaluate::Evaluate(isolate, shared_info, context, context_extension,
-                               source, throw_on_side_effect)
-           .ToHandle(&result)) {
-    return {};
+    Handle<Context> context = context_builder.evaluation_context();
+    Handle<JSObject> receiver(context->global_proxy(), isolate);
+    MaybeHandle<Object> maybe_result =
+        Evaluate(isolate, context_builder.outer_info(), context, receiver,
+                 source, throw_on_side_effect);
+    if (!maybe_result.is_null()) context_builder.UpdateValues();
+    return maybe_result;
+  } else {
+    CHECK(it.is_wasm());
+    WasmFrame* frame = WasmFrame::cast(it.frame());
+    Handle<SharedFunctionInfo> outer_info(
+        isolate->native_context()->empty_function().shared(), isolate);
+    Handle<JSProxy> context_extension = WasmJs::GetJSDebugProxy(frame);
+    Handle<ScopeInfo> scope_info =
+        ScopeInfo::CreateForWithScope(isolate, Handle<ScopeInfo>::null());
+    Handle<Context> context = isolate->factory()->NewWithContext(
+        isolate->native_context(), scope_info, context_extension);
+    return Evaluate(isolate, outer_info, context, context_extension, source,
+                    throw_on_side_effect);
   }
-
-  return result;
 }
 
 MaybeHandle<Object> DebugEvaluate::WithTopmostArguments(Isolate* isolate,

src/debug/debug-evaluate.h

@@ -32,15 +32,14 @@ class DebugEvaluate : public AllStatic {
   // - Parameters and stack-allocated locals need to be materialized.  Altered
   //   values need to be written back to the stack afterwards.
   // - The arguments object needs to materialized.
+  // The stack frame can be either a JavaScript stack frame or a Wasm
+  // stack frame. In the latter case, a special Debug Proxy API is
+  // provided to peek into the Wasm state.
   static MaybeHandle<Object> Local(Isolate* isolate, StackFrameId frame_id,
                                    int inlined_jsframe_index,
                                    Handle<String> source,
                                    bool throw_on_side_effect);
 
-  static V8_EXPORT MaybeHandle<Object> WebAssembly(
-      Handle<WasmInstanceObject> instance, StackFrameId frame_id,
-      Handle<String> source, bool throw_on_side_effect);
-
   // This is used for break-at-entry for builtins and API functions.
   // Evaluate a piece of JavaScript in the native context, but with the
   // materialized arguments object and receiver of the current call.

src/debug/debug-stack-trace-iterator.cc

@@ -178,23 +178,10 @@ v8::MaybeLocal<v8::Value> DebugStackTraceIterator::Evaluate(
   Handle<Object> value;
 
   i::SafeForInterruptsScope safe_for_interrupt_scope(isolate_);
-  bool success = false;
-  if (iterator_.is_wasm()) {
-    FrameSummary summary = FrameSummary::Get(iterator_.frame(), 0);
-    const FrameSummary::WasmFrameSummary& wasmSummary = summary.AsWasm();
-    Handle<WasmInstanceObject> instance = wasmSummary.wasm_instance();
-
-    success = DebugEvaluate::WebAssembly(instance, iterator_.frame()->id(),
-                                         Utils::OpenHandle(*source),
-                                         throw_on_side_effect)
-                  .ToHandle(&value);
-  } else {
-    success = DebugEvaluate::Local(
-                  isolate_, iterator_.frame()->id(), inlined_frame_index_,
-                  Utils::OpenHandle(*source), throw_on_side_effect)
-                  .ToHandle(&value);
-  }
-  if (!success) {
+  if (!DebugEvaluate::Local(isolate_, iterator_.frame()->id(),
+                            inlined_frame_index_, Utils::OpenHandle(*source),
+                            throw_on_side_effect)
+           .ToHandle(&value)) {
     isolate_->OptionalRescheduleException(false);
     return v8::MaybeLocal<v8::Value>();
   }