|
| 1 | +From 40f0c7577251cc5646d5216e493432e28c228cf3 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Victor Gomes < [email protected]> |
| 3 | +Date: Fri, 27 Jun 2025 12:40:10 +0200 |
| 4 | +Subject: [PATCH] Ensure InstructionAccurateScope is called with correct count |
| 5 | + |
| 6 | +Upstream Patch Link: https://chromium.googlesource.com/v8/v8/+/c58fda1f0ec46429dd66c2cacf6a98fac001e4fd%5E%21/ |
| 7 | +(Edited by < [email protected]> so it will apply correctly to our |
| 8 | +patched source) |
| 9 | + |
| 10 | +The scope prevents veneer pool generation. We need to pass the |
| 11 | +correct count of instructions to CheckVeneerPool inside the scope |
| 12 | +constructor, otherwise we might overflow the veneer distance |
| 13 | +margin in the next check (after the scope has ended). |
| 14 | + |
| 15 | +Fixed: 425583995 |
| 16 | +Change-Id: Iebb81898c4f7999137fc784ce6704773614c2bb5 |
| 17 | +Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6683635 |
| 18 | +Auto-Submit: Victor Gomes < [email protected]> |
| 19 | +Commit-Queue: Victor Gomes < [email protected]> |
| 20 | +Commit-Queue: Igor Sheludko < [email protected]> |
| 21 | +Reviewed-by: Igor Sheludko < [email protected]> |
| 22 | +Cr-Commit-Position: refs/heads/main@{#101089} |
| 23 | +--- |
| 24 | + .../codegen/arm64/macro-assembler-arm64.cc | 20 ++++++++--- |
| 25 | + .../src/codegen/arm64/macro-assembler-arm64.h | 11 +++---- |
| 26 | + .../test/mjsunit/regress/regress-425583995.js | 33 +++++++++++++++++++ |
| 27 | + 3 files changed, 53 insertions(+), 11 deletions(-) |
| 28 | + create mode 100644 deps/v8/test/mjsunit/regress/regress-425583995.js |
| 29 | + |
| 30 | +diff --git a/deps/v8/src/codegen/arm64/macro-assembler-arm64.cc b/deps/v8/src/codegen/arm64/macro-assembler-arm64.cc |
| 31 | +index 552425ed..e310fcd2 100644 |
| 32 | +--- a/deps/v8/src/codegen/arm64/macro-assembler-arm64.cc |
| 33 | ++++ b/deps/v8/src/codegen/arm64/macro-assembler-arm64.cc |
| 34 | +@@ -1178,7 +1178,7 @@ void TurboAssembler::PushHelper(int count, int size, const CPURegister& src0, |
| 35 | + const CPURegister& src2, |
| 36 | + const CPURegister& src3) { |
| 37 | + // Ensure that we don't unintentially modify scratch or debug registers. |
| 38 | +- InstructionAccurateScope scope(this); |
| 39 | ++ InstructionAccurateScope scope(this, count <= 2 ? 1 : 2); |
| 40 | + |
| 41 | + DCHECK(AreSameSizeAndType(src0, src1, src2, src3)); |
| 42 | + DCHECK(size == src0.SizeInBytes()); |
| 43 | +@@ -1215,7 +1215,7 @@ void TurboAssembler::PopHelper(int count, int size, const CPURegister& dst0, |
| 44 | + const CPURegister& dst1, const CPURegister& dst2, |
| 45 | + const CPURegister& dst3) { |
| 46 | + // Ensure that we don't unintentially modify scratch or debug registers. |
| 47 | +- InstructionAccurateScope scope(this); |
| 48 | ++ InstructionAccurateScope scope(this, count <= 2 ? 1 : 2); |
| 49 | + |
| 50 | + DCHECK(AreSameSizeAndType(dst0, dst1, dst2, dst3)); |
| 51 | + DCHECK(size == dst0.SizeInBytes()); |
| 52 | +@@ -1265,8 +1265,14 @@ void MacroAssembler::PeekPair(const CPURegister& dst1, const CPURegister& dst2, |
| 53 | + |
| 54 | + void MacroAssembler::PushCalleeSavedRegisters() { |
| 55 | + ASM_CODE_COMMENT(this); |
| 56 | ++#ifdef V8_ENABLE_CONTROL_FLOW_INTEGRITY |
| 57 | ++ constexpr int kInstrCount = 11; |
| 58 | ++#else |
| 59 | ++ constexpr int kInstrCount = 10; |
| 60 | ++#endif |
| 61 | ++ |
| 62 | + // Ensure that the macro-assembler doesn't use any scratch registers. |
| 63 | +- InstructionAccurateScope scope(this); |
| 64 | ++ InstructionAccurateScope scope(this, kInstrCount); |
| 65 | + |
| 66 | + MemOperand tos(sp, -2 * static_cast<int>(kXRegSize), PreIndex); |
| 67 | + |
| 68 | +@@ -1299,8 +1305,14 @@ void MacroAssembler::PushCalleeSavedRegisters() { |
| 69 | + |
| 70 | + void MacroAssembler::PopCalleeSavedRegisters() { |
| 71 | + ASM_CODE_COMMENT(this); |
| 72 | ++#ifdef V8_ENABLE_CONTROL_FLOW_INTEGRITY |
| 73 | ++ constexpr int kInstrCount = 11; |
| 74 | ++#else |
| 75 | ++ constexpr int kInstrCount = 10; |
| 76 | ++#endif |
| 77 | ++ |
| 78 | + // Ensure that the macro-assembler doesn't use any scratch registers. |
| 79 | +- InstructionAccurateScope scope(this); |
| 80 | ++ InstructionAccurateScope scope(this, kInstrCount); |
| 81 | + |
| 82 | + MemOperand tos(sp, 2 * kXRegSize, PostIndex); |
| 83 | + |
| 84 | +diff --git a/deps/v8/src/codegen/arm64/macro-assembler-arm64.h b/deps/v8/src/codegen/arm64/macro-assembler-arm64.h |
| 85 | +index ab56bba2..b232a4a9 100644 |
| 86 | +--- a/deps/v8/src/codegen/arm64/macro-assembler-arm64.h |
| 87 | ++++ b/deps/v8/src/codegen/arm64/macro-assembler-arm64.h |
| 88 | +@@ -2089,7 +2089,7 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler { |
| 89 | + // emitted is what you specified when creating the scope. |
| 90 | + class V8_NODISCARD InstructionAccurateScope { |
| 91 | + public: |
| 92 | +- explicit InstructionAccurateScope(TurboAssembler* tasm, size_t count = 0) |
| 93 | ++ explicit InstructionAccurateScope(TurboAssembler* tasm, size_t count) |
| 94 | + : tasm_(tasm), |
| 95 | + block_pool_(tasm, count * kInstrSize) |
| 96 | + #ifdef DEBUG |
| 97 | +@@ -2097,12 +2097,11 @@ class V8_NODISCARD InstructionAccurateScope { |
| 98 | + size_(count * kInstrSize) |
| 99 | + #endif |
| 100 | + { |
| 101 | ++ DCHECK_GT(count, 0); |
| 102 | + tasm_->CheckVeneerPool(false, true, count * kInstrSize); |
| 103 | + tasm_->StartBlockVeneerPool(); |
| 104 | + #ifdef DEBUG |
| 105 | +- if (count != 0) { |
| 106 | +- tasm_->bind(&start_); |
| 107 | +- } |
| 108 | ++ masm_->bind(&start_); |
| 109 | + previous_allow_macro_instructions_ = tasm_->allow_macro_instructions(); |
| 110 | + tasm_->set_allow_macro_instructions(false); |
| 111 | + #endif |
| 112 | +@@ -2111,9 +2110,7 @@ class V8_NODISCARD InstructionAccurateScope { |
| 113 | + ~InstructionAccurateScope() { |
| 114 | + tasm_->EndBlockVeneerPool(); |
| 115 | + #ifdef DEBUG |
| 116 | +- if (start_.is_bound()) { |
| 117 | +- DCHECK(tasm_->SizeOfCodeGeneratedSince(&start_) == size_); |
| 118 | +- } |
| 119 | ++ DCHECK(masm_->SizeOfCodeGeneratedSince(&start_) == size_); |
| 120 | + tasm_->set_allow_macro_instructions(previous_allow_macro_instructions_); |
| 121 | + #endif |
| 122 | + } |
| 123 | +diff --git a/deps/v8/test/mjsunit/regress/regress-425583995.js b/deps/v8/test/mjsunit/regress/regress-425583995.js |
| 124 | +new file mode 100644 |
| 125 | +index 00000000..eaed312c |
| 126 | +--- /dev/null |
| 127 | ++++ b/deps/v8/test/mjsunit/regress/regress-425583995.js |
| 128 | +@@ -0,0 +1,33 @@ |
| 129 | ++// Copyright 2025 the V8 project authors. All rights reserved. |
| 130 | ++// Use of this source code is governed by a BSD-style license that can be |
| 131 | ++// found in the LICENSE file. |
| 132 | ++ |
| 133 | ++// Flags: --allow-natives-syntax |
| 134 | ++ |
| 135 | ++const topLevel = %GetFunctionForCurrentFrame(); |
| 136 | ++%PrepareFunctionForOptimization(topLevel); |
| 137 | ++ |
| 138 | ++function g( |
| 139 | ++ // Too many arguments to fit here manually and overflow int32 calculation |
| 140 | ++) { |
| 141 | ++ return arguments.length + 1; |
| 142 | ++} |
| 143 | ++ |
| 144 | ++var num_args = 40000; // large number to cause overflow |
| 145 | ++ |
| 146 | ++// Construct argument list string |
| 147 | ++var argsList = ""; |
| 148 | ++for (var i = 0; i < num_args; i++) argsList += "a" + i + ","; |
| 149 | ++argsList = argsList.slice(0, -1); |
| 150 | ++ |
| 151 | ++// Construct function source to return sum of all args |
| 152 | ++var body = "return arguments.length + 1;"; |
| 153 | ++ |
| 154 | ++// Create large argument function dynamically |
| 155 | ++var bigArgFunc = new Function(argsList, body); |
| 156 | ++ |
| 157 | ++// Call many times to trigger OSR in v8 maglev |
| 158 | ++for (var i = 0; i < 2; i++) { |
| 159 | ++ bigArgFunc(0); |
| 160 | ++ %OptimizeOsr(); |
| 161 | ++} |
| 162 | +-- |
| 163 | +2.34.1 |
| 164 | + |
0 commit comments