Merged: [snapshot] Fix copy-IET integration with Code Cache

Revision: d915b8d668615a7d6d75cf7a61d3ca5a3d139799 BUG=v8:9122 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=jgruber@chromium.org Change-Id: I7afc1a80036e95b5a15464c24ae875a4acc9ba96 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1597023 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/branch-heads/7.4@{#55} Cr-Branched-From: 3e8a733-refs/heads/7.4.288@{#1} Cr-Branched-From: d077f9b-refs/heads/master@{#60039}
nwjs · May 8, 2019 · ac6e72a · ac6e72a
1 parent 39ac3d0
commit ac6e72a
Show file tree

Hide file tree

Showing 4 changed files with 146 additions and 2 deletions.
diff --git a/src/snapshot/code-serializer.cc b/src/snapshot/code-serializer.cc
@@ -187,6 +187,18 @@ void CodeSerializer::SerializeObject(HeapObject obj) {
     return;
   }
 
+  // NOTE(mmarchini): If we try to serialize an InterpreterData our process
+  // will crash since it stores a code object. Instead, we serialize the
+  // bytecode array stored within the InterpreterData, which is the important
+  // information. On deserialization we'll create our code objects again, if
+  // --interpreted-frames-native-stack is on. See v8:9122 for more context
+#ifndef V8_TARGET_ARCH_ARM
+  if (V8_UNLIKELY(FLAG_interpreted_frames_native_stack) &&
+      obj->IsInterpreterData()) {
+    obj = InterpreterData::cast(obj)->bytecode_array();
+  }
+#endif  // V8_TARGET_ARCH_ARM
+
   if (obj->IsBytecodeArray()) {
     // Clear the stack frame cache if present
     BytecodeArray::cast(obj)->ClearFrameCacheFromSourcePositionTable();
@@ -210,6 +222,48 @@ void CodeSerializer::SerializeGeneric(HeapObject heap_object) {
   serializer.Serialize();
 }
 
+#ifndef V8_TARGET_ARCH_ARM
+// NOTE(mmarchini): when FLAG_interpreted_frames_native_stack is on, we want to
+// create duplicates of InterpreterEntryTrampoline for the deserialized
+// functions, otherwise we'll call the builtin IET for those functions (which
+// is not what a user of this flag wants).
+void CreateInterpreterDataForDeserializedCode(Isolate* isolate,
+                                              Handle<SharedFunctionInfo> sfi,
+                                              bool log_code_creation) {
+  Script script = Script::cast(sfi->script());
+  Handle<Script> script_handle(script, isolate);
+  String name = ReadOnlyRoots(isolate).empty_string();
+  if (script->name()->IsString()) name = String::cast(script->name());
+  Handle<String> name_handle(name, isolate);
+
+  SharedFunctionInfo::ScriptIterator iter(isolate, script);
+  for (SharedFunctionInfo info = iter.Next(); !info.is_null();
+       info = iter.Next()) {
+    if (!info->HasBytecodeArray()) continue;
+    Handle<Code> code = isolate->factory()->CopyCode(Handle<Code>::cast(
+        isolate->factory()->interpreter_entry_trampoline_for_profiling()));
+
+    Handle<InterpreterData> interpreter_data =
+        Handle<InterpreterData>::cast(isolate->factory()->NewStruct(
+            INTERPRETER_DATA_TYPE, TENURED));
+
+    interpreter_data->set_bytecode_array(info->GetBytecodeArray());
+    interpreter_data->set_interpreter_trampoline(*code);
+
+    info->set_interpreter_data(*interpreter_data);
+
+    if (!log_code_creation) continue;
+    Handle<AbstractCode> abstract_code = Handle<AbstractCode>::cast(code);
+    int line_num = script->GetLineNumber(info->StartPosition()) + 1;
+    int column_num = script->GetColumnNumber(info->StartPosition()) + 1;
+    PROFILE(isolate,
+            CodeCreateEvent(CodeEventListener::INTERPRETED_FUNCTION_TAG,
+                            *abstract_code, info, *name_handle, line_num,
+                            column_num));
+  }
+}
+#endif  // V8_TARGET_ARCH_ARM
+
 MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
     Isolate* isolate, ScriptData* cached_data, Handle<String> source,
     ScriptOriginOptions origin_options) {
@@ -253,6 +307,13 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
       isolate->logger()->is_listening_to_code_events() ||
       isolate->is_profiling() ||
       isolate->code_event_dispatcher()->IsListeningToCodeEvents();
+
+#ifndef V8_TARGET_ARCH_ARM
+  if (V8_UNLIKELY(FLAG_interpreted_frames_native_stack))
+    CreateInterpreterDataForDeserializedCode(isolate, result,
+                                             log_code_creation);
+#endif  // V8_TARGET_ARCH_ARM
+
   if (log_code_creation || FLAG_log_function_events) {
     String name = ReadOnlyRoots(isolate).empty_string();
     Script script = Script::cast(result->script());

diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status
@@ -580,6 +580,14 @@
   'test-cpu-profiler/TickLinesBaseline': [SKIP],
   'test-cpu-profiler/TickLinesOptimized': [SKIP],
   'test-cpu-profiler/Inlining2': [SKIP],
+
+  # TODO(mythria): Code logging tests that currently fail with lazy feedback
+  # allocation. Fix logging to work without feedback vectors and enable these
+  # tests in lite_mode.
+  'test-log/ExternalCodeEventListenerWithInterpretedFramesNativeStack': [SKIP],
+  'test-log/LogInterpretedFramesNativeStack': [SKIP],
+  'test-log/LogInterpretedFramesNativeStackWithSerialization': [SKIP],
+  'test-serialize/CodeSerializerOnePlusOneWithInterpretedFramesNativeStack': [SKIP]
 }], # lite_mode
 
 ##############################################################################
@@ -618,6 +626,8 @@
   # --interpreted-frames-native-stack tests
   'test-log/ExternalCodeEventListenerWithInterpretedFramesNativeStack': [SKIP],
   'test-log/LogInterpretedFramesNativeStack': [SKIP],
+  'test-log/LogInterpretedFramesNativeStackWithSerialization': [SKIP],
+  'test-serialize/CodeSerializerOnePlusOneWithInterpretedFramesNativeStack': [SKIP],
 
   # Crashes on native arm.
   'test-macro-assembler-arm/ExtractLane': [PASS, ['arch == arm and not simulator_run', SKIP]],

diff --git a/test/cctest/test-log.cc b/test/cctest/test-log.cc
@@ -31,6 +31,7 @@
 #include <vector>
 #include "src/api-inl.h"
 #include "src/builtins/builtins.h"
+#include "src/compilation-cache.h"
 #include "src/log-utils.h"
 #include "src/log.h"
 #include "src/objects-inl.h"
@@ -659,6 +660,68 @@ TEST(LogInterpretedFramesNativeStack) {
   }
   isolate->Dispose();
 }
+
+TEST(LogInterpretedFramesNativeStackWithSerialization) {
+  SETUP_FLAGS();
+  i::FLAG_interpreted_frames_native_stack = true;
+  i::FLAG_always_opt = false;
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+
+  v8::ScriptCompiler::CachedData* cache = nullptr;
+
+  bool has_cache = cache != nullptr;
+  // NOTE(mmarchini): Runs the test two times. The first time it will compile
+  // our script and will create a code cache for it. The second time we'll
+  // deserialize the cache and check if our function was logged correctly.
+  // We disallow compilation on the second run to ensure we're loading from
+  // cache.
+  do {
+    v8::Isolate* isolate = v8::Isolate::New(create_params);
+
+    {
+      ScopedLoggerInitializer logger(saved_log, saved_prof, isolate);
+
+      has_cache = cache != nullptr;
+      v8::ScriptCompiler::CompileOptions options =
+          has_cache ? v8::ScriptCompiler::kConsumeCodeCache
+                    : v8::ScriptCompiler::kEagerCompile;
+
+      v8::HandleScope scope(isolate);
+      v8::Isolate::Scope isolate_scope(isolate);
+      v8::Local<v8::Context> context = v8::Context::New(isolate);
+      v8::Local<v8::String> source = v8_str(
+          "function eyecatcher() { return a * a; } return eyecatcher();");
+      v8::Local<v8::String> arg_str = v8_str("a");
+      v8::ScriptOrigin origin(v8_str("filename"));
+
+      i::DisallowCompilation* no_compile_expected =
+          has_cache ? new i::DisallowCompilation(
+                          reinterpret_cast<i::Isolate*>(isolate))
+                    : nullptr;
+
+      v8::ScriptCompiler::Source script_source(source, origin, cache);
+      v8::Local<v8::Function> fun =
+          v8::ScriptCompiler::CompileFunctionInContext(
+              context, &script_source, 1, &arg_str, 0, nullptr, options)
+              .ToLocalChecked();
+      if (has_cache) {
+        logger.StopLogging();
+        CHECK(logger.ContainsLine({"InterpretedFunction", "eyecatcher"}));
+      }
+      v8::Local<v8::Value> arg = v8_num(3);
+      v8::Local<v8::Value> result =
+          fun->Call(context, v8::Undefined(isolate), 1, &arg).ToLocalChecked();
+      CHECK_EQ(9, result->Int32Value(context).FromJust());
+      cache = v8::ScriptCompiler::CreateCodeCacheForFunction(fun);
+
+      if (no_compile_expected != nullptr) delete no_compile_expected;
+    }
+
+    isolate->Dispose();
+  } while (!has_cache);
+  delete cache;
+}
 #endif  // V8_TARGET_ARCH_ARM
 
 TEST(ExternalCodeEventListener) {

diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
@@ -1578,7 +1578,7 @@ static Handle<SharedFunctionInfo> CompileScriptAndProduceCache(
   return sfi;
 }
 
-void TestCodeSerializerOnePlusOneImpl() {
+void TestCodeSerializerOnePlusOneImpl(bool verify_builtins_count = true) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
   isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
@@ -1622,13 +1622,23 @@ void TestCodeSerializerOnePlusOneImpl() {
       Execution::Call(isolate, copy_fun, global, 0, nullptr).ToHandleChecked();
   CHECK_EQ(2, Handle<Smi>::cast(copy_result)->value());
 
-  CHECK_EQ(builtins_count, CountBuiltins());
+  if (verify_builtins_count) CHECK_EQ(builtins_count, CountBuiltins());
 
   delete cache;
 }
 
 TEST(CodeSerializerOnePlusOne) { TestCodeSerializerOnePlusOneImpl(); }
 
+// See bug v8:9122
+#ifndef V8_TARGET_ARCH_ARM
+TEST(CodeSerializerOnePlusOneWithInterpretedFramesNativeStack) {
+  FLAG_interpreted_frames_native_stack = true;
+  // We pass false because this test will create IET copies (which are
+  // builtins).
+  TestCodeSerializerOnePlusOneImpl(false);
+}
+#endif
+
 TEST(CodeSerializerOnePlusOneWithDebugger) {
   v8::HandleScope scope(CcTest::isolate());
   static v8::debug::DebugDelegate dummy_delegate;