Skip to content

Commit ccd648e

Browse files
o-nodejs-github-bot
authored andcommitted
deps: V8: backport b36cf9f6ccd2
Original commit message: [api] Prepare DisallowJavascriptExecution scope around api callbacks In `v8::Isolate::RequestInterrupt` the API contract says: API interrupt callbacks are forbidden from executing JavaScript on the interrupted Isolate. This CL prepares the infrastructure to enforce this contract. Bug: 540137721, 40067940 TAG=agy CONV=78573798-a951-4bd3-9c6f-3e3b6775da4b Change-Id: I43cc83c3cc3ebd34052a927dde55bbf4d49d2d29 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8173727 Auto-Submit: Olivier Flückiger <olivf@chromium.org> Reviewed-by: Simon Zünd <szuend@chromium.org> Commit-Queue: Olivier Flückiger <olivf@chromium.org> Cr-Commit-Position: refs/heads/main@{#109086} Refs: v8/v8@b36cf9f Signed-off-by: deepak1556 <hop2deep@gmail.com> PR-URL: #65397 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com>
1 parent 49e2608 commit ccd648e

11 files changed

Lines changed: 93 additions & 2 deletions

File tree

deps/v8/BUILD.bazel

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,11 @@ v8_flag(name = "v8_enable_trace_maps")
148148

149149
v8_flag(name = "v8_enable_v8_checks")
150150

151+
v8_flag(
152+
name = "v8_disallow_js_in_api_interrupts_is_checked",
153+
default = True,
154+
)
155+
151156
v8_flag(name = "v8_enable_verify_csa")
152157

153158
v8_flag(name = "v8_enable_verify_heap")
@@ -496,6 +501,7 @@ v8_config(
496501
"v8_android_log_stdout": "V8_ANDROID_LOG_STDOUT",
497502
"v8_code_comments": "V8_CODE_COMMENTS",
498503
"v8_deprecation_warnings": "V8_DEPRECATION_WARNINGS",
504+
"v8_disallow_js_in_api_interrupts_is_checked": "V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED",
499505
"v8_imminent_deprecation_warnings": "V8_IMMINENT_DEPRECATION_WARNINGS",
500506
"v8_enable_debug_code": "V8_ENABLE_DEBUG_CODE",
501507
"v8_enable_disassembler": "ENABLE_DISASSEMBLER",

deps/v8/BUILD.gn

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ declare_args() {
5959
# Enable compiler warnings when using V8_DEPRECATED apis.
6060
v8_deprecation_warnings = true
6161

62+
# Check that executing JavaScript inside API interrupt callbacks is
63+
# disallowed.
64+
v8_disallow_js_in_api_interrupts_is_checked = true
65+
6266
# Enable compiler warnings when using V8_DEPRECATE_SOON apis.
6367
v8_imminent_deprecation_warnings = true
6468

@@ -999,6 +1003,7 @@ external_v8_defines = [
9991003
"V8_COMPRESS_ZONES",
10001004
"V8_ENABLE_SANDBOX",
10011005
"V8_DEPRECATION_WARNINGS",
1006+
"V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED",
10021007
"V8_IMMINENT_DEPRECATION_WARNINGS",
10031008
"V8_USE_PERFETTO",
10041009
"V8_USE_PERFETTO_JSON_EXPORT",
@@ -1047,6 +1052,10 @@ if (v8_enable_sandbox) {
10471052
if (v8_deprecation_warnings) {
10481053
enabled_external_v8_defines += [ "V8_DEPRECATION_WARNINGS" ]
10491054
}
1055+
if (v8_disallow_js_in_api_interrupts_is_checked) {
1056+
enabled_external_v8_defines +=
1057+
[ "V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED" ]
1058+
}
10501059
if (v8_imminent_deprecation_warnings) {
10511060
enabled_external_v8_defines += [ "V8_IMMINENT_DEPRECATION_WARNINGS" ]
10521061
}
@@ -3093,6 +3102,8 @@ generated_file("v8_generate_features_json") {
30933102
output_conversion = "json"
30943103
contents = {
30953104
v8_deprecation_warnings = v8_deprecation_warnings
3105+
v8_disallow_js_in_api_interrupts_is_checked =
3106+
v8_disallow_js_in_api_interrupts_is_checked
30963107
v8_enable_31bit_smis_on_64bit_arch = v8_enable_31bit_smis_on_64bit_arch
30973108
v8_enable_direct_handle = v8_enable_direct_handle
30983109
v8_enable_extensible_ro_snapshot = v8_enable_extensible_ro_snapshot

deps/v8/src/common/assert-scope.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class V8_NODISCARD PerThreadAssertScope
110110
ScopeType& operator=(const ScopeType&) = delete; \
111111
V8_EXPORT_PRIVATE ~ScopeType(); \
112112
\
113-
static bool IsAllowed(Isolate* isolate); \
113+
V8_EXPORT_PRIVATE static bool IsAllowed(Isolate* isolate); \
114114
\
115115
V8_EXPORT_PRIVATE static void Open(Isolate* isolate, \
116116
bool* was_execution_allowed); \

deps/v8/src/execution/isolate.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2245,6 +2245,13 @@ void Isolate::InvokeApiInterruptCallbacks() {
22452245
}
22462246
VMState<EXTERNAL> state(this);
22472247
HandleScope handle_scope(this);
2248+
// API interrupt callbacks are forbidden from executing JavaScript on the
2249+
// interrupted Isolate (see v8::Isolate::RequestInterrupt contract in
2250+
// v8-isolate.h: "Registered |callback| must not reenter interrupted
2251+
// Isolate.").
2252+
#ifdef V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED
2253+
DisallowJavascriptExecution no_js(this);
2254+
#endif // V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED
22482255
entry.first(reinterpret_cast<v8::Isolate*>(this), entry.second);
22492256
}
22502257
}

deps/v8/src/inspector/v8-debugger.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,7 @@ void V8Debugger::handleProgramBreak(
532532
}
533533
});
534534
{
535+
v8::Isolate::AllowJavascriptExecutionScope allow_script(m_isolate);
535536
v8::Context::Scope scope(pausedContext);
536537

537538
m_inspector->forEachSession(

deps/v8/src/inspector/v8-inspector-impl.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ int V8InspectorImpl::resolveUniqueContextId(
9696

9797
v8::MaybeLocal<v8::Value> V8InspectorImpl::compileAndRunInternalScript(
9898
v8::Local<v8::Context> context, v8::Local<v8::String> source) {
99+
v8::Isolate::AllowJavascriptExecutionScope allow_script(m_isolate);
99100
v8::Local<v8::UnboundScript> unboundScript;
100101
if (!v8::debug::CompileInspectorScript(m_isolate, source)
101102
.ToLocal(&unboundScript))

deps/v8/src/inspector/v8-inspector-session-impl.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ void V8InspectorSessionImpl::reportAllContexts(V8RuntimeAgentImpl* agent) {
355355
}
356356

357357
void V8InspectorSessionImpl::dispatchProtocolMessage(StringView message) {
358+
v8::Isolate::AllowJavascriptExecutionScope allow_script(
359+
m_inspector->isolate());
358360
KeepSessionAliveScope keepAlive(*this);
359361

360362
using v8_crdtp::span;

deps/v8/src/inspector/v8-regex.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ V8Regex::V8Regex(V8InspectorImpl* inspector, const String16& pattern,
3838
v8::Local<v8::RegExp> regex;
3939
// Protect against reentrant debugger calls via interrupts.
4040
v8::debug::PostponeInterruptsScope no_interrupts(m_inspector->isolate());
41+
v8::Isolate::AllowJavascriptExecutionScope allow_js(m_inspector->isolate());
4142
if (v8::RegExp::New(context, toV8String(isolate, pattern),
4243
static_cast<v8::RegExp::Flags>(flags))
4344
.ToLocal(&regex))
@@ -69,6 +70,7 @@ int V8Regex::match(const String16& string, int startFrom,
6970
v8::MicrotasksScope::kDoNotRunMicrotasks);
7071
// Protect against reentrant debugger calls via interrupts.
7172
v8::debug::PostponeInterruptsScope no_interrupts(m_inspector->isolate());
73+
v8::Isolate::AllowJavascriptExecutionScope allow_js(m_inspector->isolate());
7274
v8::TryCatch tryCatch(isolate);
7375

7476
v8::Local<v8::RegExp> regex = m_regex.Get(isolate);

deps/v8/test/cctest/test-api.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22306,6 +22306,26 @@ TEST(RequestInterruptSmallScripts) {
2230622306
CHECK(interrupt_was_called);
2230722307
}
2230822308

22309+
#ifdef V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED
22310+
static bool interrupt_check_no_js = false;
22311+
void DisallowJsInterruptCallback(v8::Isolate* isolate, void* data) {
22312+
CHECK(!i::AllowJavascriptExecution::IsAllowed(
22313+
reinterpret_cast<i::Isolate*>(isolate)));
22314+
interrupt_check_no_js = true;
22315+
}
22316+
22317+
TEST(RequestInterruptDisallowsJavascript) {
22318+
LocalContext env;
22319+
v8::Isolate* isolate = CcTest::isolate();
22320+
v8::HandleScope scope(isolate);
22321+
22322+
interrupt_check_no_js = false;
22323+
isolate->RequestInterrupt(&DisallowJsInterruptCallback, nullptr);
22324+
CompileRun("(function(x){return x;})(1);");
22325+
CHECK(interrupt_check_no_js);
22326+
}
22327+
#endif // V8_DISALLOW_JS_IN_API_INTERRUPTS_IS_CHECKED
22328+
2230922329
static v8::Global<Value> function_new_expected_env_global;
2231022330
static void FunctionNewCallback(const v8::FunctionCallbackInfo<Value>& info) {
2231122331
v8::Isolate* isolate = info.GetIsolate();

deps/v8/test/cctest/test-regexp.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,13 @@ class InterruptTest {
124124
CHECK(string->ContainsOnlyOneByte());
125125
// Internalize the subject by using it as a computed property name in an
126126
// object.
127-
CompileRun("o = { [subject_string]: 'foo' }");
127+
{
128+
// This test is technically wrong for running JS in a C++ interrupt.
129+
// However we know that the interuptee here is the regexp engine, which
130+
// does not care.
131+
Isolate::AllowJavascriptExecutionScope allow_script(isolate);
132+
CompileRun("o = { [subject_string]: 'foo' }");
133+
}
128134
CHECK(string->IsOneByte());
129135
}
130136

0 commit comments

Comments
 (0)