From e76fb59e6e50871bae9c0fd3da9d955fc086b6a6 Mon Sep 17 00:00:00 2001 From: "D. Seifert" Date: Fri, 28 Jun 2024 00:26:16 +0800 Subject: [PATCH 1/2] Assert access checks --- src/execution/isolate.cc | 7 +++++++ src/objects/js-objects.cc | 2 ++ src/objects/objects.cc | 1 + 3 files changed, 10 insertions(+) diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index c4d0ae490059..c7c8d9150060 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -1540,6 +1540,11 @@ bool Isolate::MayAccess(Handle accessing_context, // Check for compatibility between the security tokens in the // current lexical context and the accessed object. + + REPLAY_ASSERT("[TT-366-1480] Isolate::MayAccess A %d %d %d", + bootstrapper()->IsActive(), + V8RecordReplayIsInReplayCode(), + receiver->IsJSGlobalProxy()); // During bootstrapping, callback functions are not enabled yet. if (bootstrapper()->IsActive()) return true; @@ -1563,6 +1568,8 @@ bool Isolate::MayAccess(Handle accessing_context, if (Context::cast(receiver_context).security_token() == native_context.security_token()) return true; + + REPLAY_ASSERT("[TT-366-1480] Isolate::MayAccess B"); } } diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc index 599438e45bc4..ae9fd77ef379 100644 --- a/src/objects/js-objects.cc +++ b/src/objects/js-objects.cc @@ -110,7 +110,9 @@ Maybe JSReceiver::HasProperty(LookupIterator* it) { break; } case LookupIterator::ACCESS_CHECK: { + REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] JSReceiver::HasProperty A"); if (it->HasAccess()) break; + REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] JSReceiver::HasProperty B"); Maybe result = JSObject::GetPropertyAttributesWithFailedAccessCheck(it); if (result.IsNothing()) return Nothing(); diff --git a/src/objects/objects.cc b/src/objects/objects.cc index 9ed9ec3e003d..070807e1ce1b 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -1183,6 +1183,7 @@ MaybeHandle Object::GetProperty(LookupIterator* it, break; } case LookupIterator::ACCESS_CHECK: + REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] Object::GetProperty"); if (it->HasAccess()) break; return JSObject::GetPropertyWithFailedAccessCheck(it); case LookupIterator::ACCESSOR: From 4902428d14d7b4244afbbdae1fbfdfa282eeb5b7 Mon Sep 17 00:00:00 2001 From: "D. Seifert" Date: Wed, 10 Jul 2024 02:26:15 +0800 Subject: [PATCH 2/2] Undo Asserts, but add `REPLAY_ASSERT_IF` --- include/replayio-macros.h | 6 ++++++ src/execution/isolate.cc | 7 ------- src/objects/js-objects.cc | 2 -- src/objects/objects.cc | 1 - 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/include/replayio-macros.h b/include/replayio-macros.h index 5a619253b2ab..056c6d590367 100644 --- a/include/replayio-macros.h +++ b/include/replayio-macros.h @@ -16,6 +16,12 @@ recordreplay::Assert(format, ##__VA_ARGS__); \ static_assert(true, "require semicolon") +// Same as |REPLAY_ASSERT| but won't Assert if the given condition is not true. +#define REPLAY_ASSERT_IF(cond, format, ...) \ + if (recordreplay::HasAsserts() && (cond)) \ + recordreplay::Assert(format, ##__VA_ARGS__); \ + static_assert(true, "require semicolon") + // Same as |REPLAY_ASSERT| but won't Assert when Events are disallowed. #define REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED(format, ...) \ if (recordreplay::HasAsserts() && !recordreplay::AreEventsDisallowed()) \ diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index c7c8d9150060..c4d0ae490059 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -1540,11 +1540,6 @@ bool Isolate::MayAccess(Handle accessing_context, // Check for compatibility between the security tokens in the // current lexical context and the accessed object. - - REPLAY_ASSERT("[TT-366-1480] Isolate::MayAccess A %d %d %d", - bootstrapper()->IsActive(), - V8RecordReplayIsInReplayCode(), - receiver->IsJSGlobalProxy()); // During bootstrapping, callback functions are not enabled yet. if (bootstrapper()->IsActive()) return true; @@ -1568,8 +1563,6 @@ bool Isolate::MayAccess(Handle accessing_context, if (Context::cast(receiver_context).security_token() == native_context.security_token()) return true; - - REPLAY_ASSERT("[TT-366-1480] Isolate::MayAccess B"); } } diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc index ae9fd77ef379..599438e45bc4 100644 --- a/src/objects/js-objects.cc +++ b/src/objects/js-objects.cc @@ -110,9 +110,7 @@ Maybe JSReceiver::HasProperty(LookupIterator* it) { break; } case LookupIterator::ACCESS_CHECK: { - REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] JSReceiver::HasProperty A"); if (it->HasAccess()) break; - REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] JSReceiver::HasProperty B"); Maybe result = JSObject::GetPropertyAttributesWithFailedAccessCheck(it); if (result.IsNothing()) return Nothing(); diff --git a/src/objects/objects.cc b/src/objects/objects.cc index 070807e1ce1b..9ed9ec3e003d 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -1183,7 +1183,6 @@ MaybeHandle Object::GetProperty(LookupIterator* it, break; } case LookupIterator::ACCESS_CHECK: - REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] Object::GetProperty"); if (it->HasAccess()) break; return JSObject::GetPropertyWithFailedAccessCheck(it); case LookupIterator::ACCESSOR: