Commit ec8c6b9
authored
deps: V8: cherry-pick 657d8de27427
Original commit message:
[maglev] Fix throwing node inside eager inlining
This commit refactors the exception handling logic to correctly identify
and associate nodes with their respective `catch` blocks, even
when multiple levels of inlining are involved.
Previously, the check `!IsInsideTryBlock() && !is_eager_inline()` was
insufficient to determine if catch block inside `CatchDetails` was
already created.
Specifically, consider the case where:
1. Function `bar` is non-eagerly inlined into `foo`.
2. `foo` contains a `catch` block.
3. `bar` calls `in_bar`, which is eagerly inlined.
4. A node within `in_bar` can `throw`.
In this scenario, `is_eager_inline` would be true when compiling
`in_bar`, leading to an incorrect assumption that the catch block didn't exist yet.
This change addresses the issue by propagating a boolean value via
`CatchDetails`. This boolean accurately indicates whether a `catch`
block is present in the call chain, allowing for correct exception
handling regardless of inlining depth or eagerness.
Fixed: 417768368
Change-Id: Ic52f72f302b4dc644bdcad939addf98111bc525b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6563500
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100380}
Refs: v8/v8@657d8de
PR-URL: #62784
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>1 parent b881658 commit ec8c6b9
3 files changed
Lines changed: 43 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -38,7 +38,7 @@ | |||
| 38 | 38 | ||
| 39 | 39 | # Reset this number to 0 on major V8 upgrades. | |
| 40 | 40 | # Increment by one for each non-official patch applied to deps/v8. | |
| 41 | - 'v8_embedder_string': '-node.48', | ||
| 41 | + 'v8_embedder_string': '-node.49', | ||
| 42 | 42 | ||
| 43 | 43 | ##### V8 defaults for Node.js ##### | |
| 44 | 44 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -178,6 +178,7 @@ NodeType StaticTypeForNode(compiler::JSHeapBroker* broker, | |||
| 178 | 178 | struct CatchBlockDetails { | |
| 179 | 179 | BasicBlockRef* ref = nullptr; | |
| 180 | 180 | bool exception_handler_was_used = false; | |
| 181 | + bool block_already_exists = false; | ||
| 181 | 182 | int deopt_frame_distance = 0; | |
| 182 | 183 | }; | |
| 183 | 184 | ||
@@ -1220,7 +1221,8 @@ class MaglevGraphBuilder { | |||
| 1220 | 1221 | } | |
| 1221 | 1222 | ||
| 1222 | 1223 | DCHECK_IMPLIES(!IsInsideTryBlock(), is_inline()); | |
| 1223 | - if (!IsInsideTryBlock() && !is_eager_inline()) { | ||
| 1224 | + if (catch_block.block_already_exists) { | ||
| 1225 | + DCHECK(!IsInsideTryBlock()); | ||
| 1224 | 1226 | // If we are inlining a function non-eagerly and we are not inside a | |
| 1225 | 1227 | // try block, then the catch block already exists. | |
| 1226 | 1228 | new (node->exception_handler_info()) ExceptionHandlerInfo( | |
@@ -1276,7 +1278,7 @@ class MaglevGraphBuilder { | |||
| 1276 | 1278 | // Inside a try-block. | |
| 1277 | 1279 | int offset = catch_block_stack_.top().handler; | |
| 1278 | 1280 | return {&jump_targets_[offset], | |
| 1279 | - merge_states_[offset]->exception_handler_was_used(), 0}; | ||
| 1281 | + merge_states_[offset]->exception_handler_was_used(), false, 0}; | ||
| 1280 | 1282 | } | |
| 1281 | 1283 | if (!is_inline()) { | |
| 1282 | 1284 | return CatchBlockDetails{}; | |
@@ -1286,7 +1288,8 @@ class MaglevGraphBuilder { | |||
| 1286 | 1288 | ||
| 1287 | 1289 | CatchBlockDetails GetTryCatchBlockFromInfo(ExceptionHandlerInfo* info) { | |
| 1288 | 1290 | if (IsInsideTryBlock()) { | |
| 1289 | - return {info->catch_block_ref_address(), !info->ShouldLazyDeopt(), 0}; | ||
| 1291 | + return {info->catch_block_ref_address(), !info->ShouldLazyDeopt(), true, | ||
| 1292 | + 0}; | ||
| 1290 | 1293 | } | |
| 1291 | 1294 | if (!is_inline()) { | |
| 1292 | 1295 | return CatchBlockDetails{}; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,36 @@ | |||
| 1 | + // Copyright 2025 the V8 project authors. All rights reserved. | ||
| 2 | + // Use of this source code is governed by a BSD-style license that can be | ||
| 3 | + // found in the LICENSE file. | ||
| 4 | + | ||
| 5 | + // Flags: --allow-natives-syntax | ||
| 6 | + | ||
| 7 | + function bar(a) { | ||
| 8 | + function in_bar(a, b) { | ||
| 9 | + a % b; // This emit a generic mod that can throw. | ||
| 10 | + return a; | ||
| 11 | + } | ||
| 12 | + %PrepareFunctionForOptimization(in_bar); | ||
| 13 | + in_bar({}); | ||
| 14 | + if (a) { | ||
| 15 | + throw_before_this_function_is_not_defined(); | ||
| 16 | + } | ||
| 17 | + } | ||
| 18 | + | ||
| 19 | + function foo() { | ||
| 20 | + try { | ||
| 21 | + function const_42() { | ||
| 22 | + return 42; | ||
| 23 | + } | ||
| 24 | + %PrepareFunctionForOptimization(const_42); | ||
| 25 | + const_42(); | ||
| 26 | + bar(true); | ||
| 27 | + } catch { | ||
| 28 | + } | ||
| 29 | + } | ||
| 30 | + | ||
| 31 | + %PrepareFunctionForOptimization(bar); | ||
| 32 | + %PrepareFunctionForOptimization(foo); | ||
| 33 | + foo(); | ||
| 34 | + | ||
| 35 | + %OptimizeMaglevOnNextCall(foo); | ||
| 36 | + foo(); | ||
0 commit comments