Fix a DCHECK failure with broken asm.js functions

Fixed: chromium:1236286
Change-Id: I90106fce4d6e747f35c638ab00bf9a1696c8eb77
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3109668
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76462}

src/execution/runtime-profiler.cc

@@ -250,7 +250,7 @@ void RuntimeProfiler::MarkCandidatesForOptimization(JavaScriptFrame* frame) {
   MarkCandidatesForOptimizationScope scope(this);
 
   JSFunction function = frame->function();
-  CodeKind code_kind = function.GetActiveTier();
+  CodeKind code_kind = function.GetActiveTier().value();
 
   DCHECK(function.shared().is_compiled());
   DCHECK(function.shared().IsInterpreted());

src/objects/js-function-inl.h

@@ -55,7 +55,7 @@ void JSFunction::ClearOptimizationMarker() {
 }
 
 bool JSFunction::ChecksOptimizationMarker() {
-  return code(kAcquireLoad).checks_optimization_marker();
+  return code().checks_optimization_marker();
 }
 
 bool JSFunction::IsMarkedForOptimization() {

src/objects/js-function.cc

@@ -19,19 +19,10 @@ namespace v8 {
 namespace internal {
 
 CodeKinds JSFunction::GetAttachedCodeKinds() const {
-  // Note: There's a special case when bytecode has been aged away. After
-  // flushing the bytecode, the JSFunction will still have the interpreter
-  // entry trampoline attached, but the bytecode is no longer available.
-  Code code = this->code(kAcquireLoad);
-  if (code.is_interpreter_trampoline_builtin()) {
-    return CodeKindFlag::INTERPRETED_FUNCTION;
-  }
-
-  const CodeKind kind = code.kind();
+  const CodeKind kind = code().kind();
   if (!CodeKindIsJSFunction(kind)) return {};
-
-  if (CodeKindIsOptimizedJSFunction(kind) && code.marked_for_deoptimization()) {
-    // Nothing is attached.
+  if (CodeKindIsOptimizedJSFunction(kind) &&
+      code().marked_for_deoptimization()) {
     return {};
   }
   return CodeKindToCodeKindFlag(kind);
@@ -90,7 +81,8 @@ namespace {
 
 // Returns false if no highest tier exists (i.e. the function is not compiled),
 // otherwise returns true and sets highest_tier.
-bool HighestTierOf(CodeKinds kinds, CodeKind* highest_tier) {
+V8_WARN_UNUSED_RESULT bool HighestTierOf(CodeKinds kinds,
+                                         CodeKind* highest_tier) {
   DCHECK_EQ((kinds & ~kJSFunctionCodeKindsMask), 0);
   if ((kinds & CodeKindFlag::TURBOFAN) != 0) {
     *highest_tier = CodeKind::TURBOFAN;
@@ -111,33 +103,43 @@ bool HighestTierOf(CodeKinds kinds, CodeKind* highest_tier) {
 
 }  // namespace
 
-bool JSFunction::ActiveTierIsIgnition() const {
-  if (!shared().HasBytecodeArray()) return false;
-  bool result = (GetActiveTier() == CodeKind::INTERPRETED_FUNCTION);
+base::Optional<CodeKind> JSFunction::GetActiveTier() const {
+#if V8_ENABLE_WEBASSEMBLY
+  // Asm/Wasm functions are currently not supported. For simplicity, this
+  // includes invalid asm.js functions whose code hasn't yet been updated to
+  // CompileLazy but is still the InstantiateAsmJs builtin.
+  if (shared().HasAsmWasmData() ||
+      code().builtin_id() == Builtin::kInstantiateAsmJs) {
+    return {};
+  }
+#endif  // V8_ENABLE_WEBASSEMBLY
+
+  CodeKind highest_tier;
+  if (!HighestTierOf(GetAvailableCodeKinds(), &highest_tier)) return {};
+
 #ifdef DEBUG
-  Code code = this->code(kAcquireLoad);
-  DCHECK_IMPLIES(result, code.is_interpreter_trampoline_builtin() ||
-                             (CodeKindIsOptimizedJSFunction(code.kind()) &&
-                              code.marked_for_deoptimization()) ||
-                             (code.builtin_id() == Builtin::kCompileLazy &&
-                              shared().IsInterpreted()));
+  CHECK(highest_tier == CodeKind::TURBOFAN ||
+        highest_tier == CodeKind::BASELINE ||
+        highest_tier == CodeKind::TURBOPROP ||
+        highest_tier == CodeKind::INTERPRETED_FUNCTION);
+
+  if (highest_tier == CodeKind::INTERPRETED_FUNCTION) {
+    CHECK(code().is_interpreter_trampoline_builtin() ||
+          (CodeKindIsOptimizedJSFunction(code().kind()) &&
+           code().marked_for_deoptimization()) ||
+          (code().builtin_id() == Builtin::kCompileLazy &&
+           shared().IsInterpreted()));
+  }
 #endif  // DEBUG
-  return result;
-}
 
-CodeKind JSFunction::GetActiveTier() const {
-  CodeKind highest_tier;
-  DCHECK(shared().is_compiled());
-  HighestTierOf(GetAvailableCodeKinds(), &highest_tier);
-  DCHECK(highest_tier == CodeKind::TURBOFAN ||
-         highest_tier == CodeKind::BASELINE ||
-         highest_tier == CodeKind::TURBOPROP ||
-         highest_tier == CodeKind::INTERPRETED_FUNCTION);
   return highest_tier;
 }
 
+bool JSFunction::ActiveTierIsIgnition() const {
+  return GetActiveTier() == CodeKind::INTERPRETED_FUNCTION;
+}
+
 bool JSFunction::ActiveTierIsTurbofan() const {
-  if (!shared().HasBytecodeArray()) return false;
   return GetActiveTier() == CodeKind::TURBOFAN;
 }
 
@@ -145,27 +147,20 @@ bool JSFunction::ActiveTierIsBaseline() const {
   return GetActiveTier() == CodeKind::BASELINE;
 }
 
-bool JSFunction::ActiveTierIsIgnitionOrBaseline() const {
-  return ActiveTierIsIgnition() || ActiveTierIsBaseline();
-}
-
 bool JSFunction::ActiveTierIsToptierTurboprop() const {
-  if (!FLAG_turboprop_as_toptier) return false;
-  if (!shared().HasBytecodeArray()) return false;
-  return GetActiveTier() == CodeKind::TURBOPROP && FLAG_turboprop_as_toptier;
+  return FLAG_turboprop_as_toptier && GetActiveTier() == CodeKind::TURBOPROP;
 }
 
 bool JSFunction::ActiveTierIsMidtierTurboprop() const {
-  if (!FLAG_turboprop) return false;
-  if (!shared().HasBytecodeArray()) return false;
-  return GetActiveTier() == CodeKind::TURBOPROP && !FLAG_turboprop_as_toptier;
+  return FLAG_turboprop && !FLAG_turboprop_as_toptier &&
+         GetActiveTier() == CodeKind::TURBOPROP;
 }
 
 CodeKind JSFunction::NextTier() const {
   if (V8_UNLIKELY(FLAG_turboprop) && ActiveTierIsMidtierTurboprop()) {
     return CodeKind::TURBOFAN;
   } else if (V8_UNLIKELY(FLAG_turboprop)) {
-    DCHECK(ActiveTierIsIgnitionOrBaseline());
+    DCHECK(ActiveTierIsIgnition() || ActiveTierIsBaseline());
     return CodeKind::TURBOPROP;
   }
   return CodeKind::TURBOFAN;

src/objects/js-function.h

@@ -106,7 +106,8 @@ class JSFunction : public JSFunctionOrBoundFunction {
   //   indirect means such as the feedback vector's optimized code cache.
   // - Active: the single code kind that would be executed if this function
   //   were called in its current state. Note that there may not be an active
-  //   code kind if the function is not compiled.
+  //   code kind if the function is not compiled. Also, asm/wasm functions are
+  //   currently not supported.
   //
   // Note: code objects that are marked_for_deoptimization are not part of the
   // attached/available/active sets. This is because the JSFunction might have
@@ -120,11 +121,10 @@ class JSFunction : public JSFunctionOrBoundFunction {
   bool HasAttachedCodeKind(CodeKind kind) const;
   bool HasAvailableCodeKind(CodeKind kind) const;
 
-  CodeKind GetActiveTier() const;
+  base::Optional<CodeKind> GetActiveTier() const;
   V8_EXPORT_PRIVATE bool ActiveTierIsIgnition() const;
   bool ActiveTierIsTurbofan() const;
   bool ActiveTierIsBaseline() const;
-  bool ActiveTierIsIgnitionOrBaseline() const;
   bool ActiveTierIsMidtierTurboprop() const;
   bool ActiveTierIsToptierTurboprop() const;
 

src/runtime/runtime-test.cc

@@ -71,6 +71,18 @@ V8_WARN_UNUSED_RESULT Object ReturnFuzzSafe(Object value, Isolate* isolate) {
   if (!args[index].IsBoolean()) return CrashUnlessFuzzing(isolate); \
   bool name = args[index].IsTrue(isolate);
 
+bool IsAsmWasmFunction(Isolate* isolate, JSFunction function) {
+  DisallowGarbageCollection no_gc;
+#if V8_ENABLE_WEBASSEMBLY
+  // For simplicity we include invalid asm.js functions whose code hasn't yet
+  // been updated to CompileLazy but is still the InstantiateAsmJs builtin.
+  return function.shared().HasAsmWasmData() ||
+         function.code().builtin_id() == Builtin::kInstantiateAsmJs;
+#else
+  return false;
+#endif  // V8_ENABLE_WEBASSEMBLY
+}
+
 }  // namespace
 
 RUNTIME_FUNCTION(Runtime_ClearMegamorphicStubCache) {
@@ -243,11 +255,9 @@ bool CanOptimizeFunction(Handle<JSFunction> function, Isolate* isolate,
     return CrashUnlessFuzzingReturnFalse(isolate);
   }
 
-#if V8_ENABLE_WEBASSEMBLY
-  if (function->shared().HasAsmWasmData()) {
+  if (IsAsmWasmFunction(isolate, *function)) {
     return CrashUnlessFuzzingReturnFalse(isolate);
   }
-#endif  // V8_ENABLE_WEBASSEMBLY
 
   if (FLAG_testing_d8_test_runner) {
     PendingOptimizationTable::MarkedForOptimization(isolate, function);
@@ -425,9 +435,7 @@ RUNTIME_FUNCTION(Runtime_PrepareFunctionForOptimization) {
     return CrashUnlessFuzzing(isolate);
   }
 
-#if V8_ENABLE_WEBASSEMBLY
-  if (function->shared().HasAsmWasmData()) return CrashUnlessFuzzing(isolate);
-#endif  // V8_ENABLE_WEBASSEMBLY
+  if (IsAsmWasmFunction(isolate, *function)) return CrashUnlessFuzzing(isolate);
 
   // Hold onto the bytecode array between marking and optimization to ensure
   // it's not flushed.

test/mjsunit/regress/regress-crbug-1236286.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --fuzzing
+// Without --fuzzing, %PrepareFunctionForOptimization would fail.
+
+for (let i = 0; i < 2; i++) {
+  try { new this.invalid(); } catch {}
+
+  function invalid(x) {
+    "use asm";
+    var y = x.Math.fround;
+    function foo() {}
+    return {foo: foo};
+  }
+
+  %PrepareFunctionForOptimization(invalid);
+  %OptimizeFunctionOnNextCall(invalid);
+}