[compiler] Add temporary merge validation code

It looks like we have crashes caused by compiling functions whose scripts have an empty string as the source. The most likely reason for this is a bad merge with an existing isolate-cached script after streaming script compilation (streamed scripts have their source set to empty string until finalization). If there is a merge, we would expect all merged SFIs that end up in the output to point to the old script (with source), not the new script (without source). This includes SFIs referenced via bytecode constant pools (for closure creation). Add some temporary verification code for debugging cases where this doesn't hold. Bug: 355575275 Change-Id: Id42198ad15b8126da1feae6af29d5beedbef2b66 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/ /5766375 Auto-Submit: Leszek Swirski <[email protected]> Reviewed-by: Patrick Thier <[email protected]> Commit-Queue: Leszek Swirski <[email protected]> Cr-Commit-Position: refs/heads/main@{#95517}
v8 · Aug 7, 2024 · fde959f · fde959f
1 parent ea7e53d
commit fde959f
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 6 deletions.
diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc
@@ -2446,6 2446,77 @@ Handle<SharedFunctionInfo> BackgroundMergeTask::CompleteMergeInForeground(
  SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, result);
  }
 
  {
  // TODO(355575275): Extra validation code to try to find a bug. Remove after
  // fixing.
  for (int i = 0; i < old_script->infos()->length(); i) {
  Tagged<MaybeObject> maybe_sfi = old_script->infos()->get(i);
  if (maybe_sfi.IsWeak() &&
  Is<SharedFunctionInfo>(maybe_sfi.GetHeapObjectAssumeWeak())) {
  Tagged<SharedFunctionInfo> sfi =
  Cast<SharedFunctionInfo>(maybe_sfi.GetHeapObjectAssumeWeak());
 
  // Check that the SFI has the right script.
  if (sfi->script() != *old_script) {
  isolate->PushStackTraceAndDie(
  reinterpret_cast<void*>(sfi.ptr()),
  reinterpret_cast<void*>(old_script->ptr()),
  reinterpret_cast<void*>(new_script->ptr()),
  reinterpret_cast<void*>(old_script->infos()->ptr() 
  WeakFixedArray::OffsetOfElementAt(i)),
  reinterpret_cast<void*>(new_script->infos()->ptr() 
  WeakFixedArray::OffsetOfElementAt(i)));
  }
 
  // Check that all SFIs in the bytecode array's constant pool are from
  // the same script.
  if (sfi->HasBytecodeArray()) {
  Tagged<BytecodeArray> bytecode = sfi->GetBytecodeArray(isolate);
  Tagged<TrustedFixedArray> constant_pool = bytecode->constant_pool();
  for (int i = 0; i < constant_pool->length(); i) {
  Tagged<Object> entry = constant_pool->get(i);
  if (Is<SharedFunctionInfo>(entry)) {
  Tagged<SharedFunctionInfo> inner_sfi =
  Cast<SharedFunctionInfo>(entry);
  if (MakeWeak(inner_sfi) !=
  old_script->infos()->get(inner_sfi->function_literal_id())) {
  isolate->PushStackTraceAndDie(
  reinterpret_cast<void*>(sfi.ptr()),
  reinterpret_cast<void*>(inner_sfi.ptr()),
  reinterpret_cast<void*>(constant_pool.ptr() 
  FixedArray::OffsetOfElementAt(i)),
  reinterpret_cast<void*>(
  old_script->infos()
  ->get(inner_sfi->function_literal_id())
  .ptr()),
  reinterpret_cast<void*>(
  new_script->infos()
  ->get(inner_sfi->function_literal_id())
  .ptr()));
  }
 
  if (inner_sfi->script() != *old_script) {
  isolate->PushStackTraceAndDie(
  reinterpret_cast<void*>(sfi.ptr()),
  reinterpret_cast<void*>(inner_sfi.ptr()),
  reinterpret_cast<void*>(old_script->ptr()),
  reinterpret_cast<void*>(new_script->ptr()),
  reinterpret_cast<void*>(
  old_script->infos()
  ->get(inner_sfi->function_literal_id())
  .ptr()),
  reinterpret_cast<void*>(
  new_script->infos()
  ->get(inner_sfi->function_literal_id())
  .ptr()));
  }
  }
  }
  }
  }
  }
  }
 
  if (v8_flags.verify_code_merge) {
  // Check that there aren't any duplicate scope infos. Every scope/context
  // should correspond to at most one scope info.
@@ -2457,6 2528,7 @@ Handle<SharedFunctionInfo> BackgroundMergeTask::CompleteMergeInForeground(
  old_script->infos()->get(i).GetHeapObjectAssumeWeak();
  if (Is<SharedFunctionInfo>(info)) {
  Tagged<SharedFunctionInfo> old_sfi = Cast<SharedFunctionInfo>(info);
  CHECK_EQ(old_sfi->script(), *old_script);
  if (!old_sfi->scope_info()->IsEmpty()) {
  scope_info = old_sfi->scope_info();
  } else if (old_sfi->HasOuterScopeInfo()) {

diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
@@ -668,10 668,10 @@ Handle<String> Isolate::StackTraceString() {
 }
 
 void Isolate::PushStackTraceAndDie(void* ptr1, void* ptr2, void* ptr3,
- void* ptr4) {
  void* ptr4, void* ptr5, void* ptr6) {
  StackTraceFailureMessage message(this,
  StackTraceFailureMessage::kIncludeStackTrace,
- ptr1, ptr2, ptr3, ptr4);
  ptr1, ptr2, ptr3, ptr4, ptr5, ptr6);
  message.Print();
  base::OS::Abort();
 }

diff --git a/src/execution/isolate.h b/src/execution/isolate.h
@@ -966,10 966,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  Handle<String> StackTraceString();
  // Stores a stack trace in a stack-allocated temporary buffer which will
  // end up in the minidump for debugging purposes.
- V8_NOINLINE void PushStackTraceAndDie(void* ptr1 = nullptr,
- void* ptr2 = nullptr,
- void* ptr3 = nullptr,
- void* ptr4 = nullptr);
  V8_NOINLINE void PushStackTraceAndDie(
  void* ptr1 = nullptr, void* ptr2 = nullptr, void* ptr3 = nullptr,
  void* ptr4 = nullptr, void* ptr5 = nullptr, void* ptr6 = nullptr);
  // Similar to the above but without collecting the stack trace.
  V8_NOINLINE void PushParamsAndDie(void* ptr1 = nullptr, void* ptr2 = nullptr,
  void* ptr3 = nullptr, void* ptr4 = nullptr,