Skip to content

Commit 475c8cd

Browse files
isheludkoV8 LUCI CQ
isheludko
authored and
V8 LUCI CQ
committed
[ptr-compr] Fix multi-cage mode
This CL introduces PtrComprCageAccessScope which sets/restores current thread's pointer compression cage base values. It's supposed to be used by V8 jobs accessing V8 heap outside of v8::Isolate::Scope or i::LocalHeap or i::LocalIsolate scopes (they already ensure that the cage base values are properly initialized). For all other build modes PtrComprCageAccessScope is a no-op. For simplicity reasons the multi-cage mode is made incompatible with external code space. Bug: v8:13788, v8:14292 Change-Id: I06c2d19a1eb7254fa7af07a17617e22d98abea9f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846592 Reviewed-by: Jakob Linke <[email protected]> Reviewed-by: Jakob Kummerow <[email protected]> Commit-Queue: Igor Sheludko <[email protected]> Reviewed-by: Dominik Inführ <[email protected]> Cr-Commit-Position: refs/heads/main@{#90075}
1 parent a2b5bc0 commit 475c8cd

28 files changed

Lines changed: 325 additions & 104 deletions

‎BUILD.gn‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ if (v8_enable_short_builtin_calls == "") {
500500
if (v8_enable_external_code_space == "") {
501501
v8_enable_external_code_space =
502502
v8_enable_pointer_compression &&
503+
v8_enable_pointer_compression_shared_cage &&
503504
(v8_current_cpu == "x64" || v8_current_cpu == "arm64")
504505
}
505506
if (v8_enable_maglev == "") {
@@ -686,6 +687,12 @@ assert(
686687
!v8_enable_pointer_compression_shared_cage || v8_enable_pointer_compression,
687688
"Can't share a pointer compression cage if pointers aren't compressed")
688689

690+
assert(
691+
!v8_enable_pointer_compression ||
692+
v8_enable_pointer_compression_shared_cage ||
693+
!v8_enable_external_code_space,
694+
"Multi-cage pointer compression mode is not compatible with external code space")
695+
689696
assert(
690697
!v8_enable_pointer_compression_shared_cage || v8_current_cpu == "x64" ||
691698
v8_current_cpu == "arm64" || v8_current_cpu == "riscv64" ||

‎src/api/api.cc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9441,6 +9441,14 @@ void Isolate::TerminateExecution() {
94419441

94429442
bool Isolate::IsExecutionTerminating() {
94439443
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
9444+
#ifdef DEBUG
9445+
// This method might be called on a thread that's not bound to any Isolate
9446+
// and thus pointer compression schemes might have cage base value unset.
9447+
// Read-only roots accessors contain type DCHECKs which require access to
9448+
// V8 heap in order to check the object type. So, allow heap access here
9449+
// to let the checks work.
9450+
i::PtrComprCageAccessScope ptr_compr_cage_access_scope(i_isolate);
9451+
#endif // DEBUG
94449452
return i_isolate->is_execution_terminating();
94459453
}
94469454

@@ -10112,6 +10120,14 @@ void Isolate::LowMemoryNotification() {
1011210120
i::NestedTimedHistogramScope idle_notification_scope(
1011310121
i_isolate->counters()->gc_low_memory_notification());
1011410122
TRACE_EVENT0("v8", "V8.GCLowMemoryNotification");
10123+
#ifdef DEBUG
10124+
// This method might be called on a thread that's not bound to any Isolate
10125+
// and thus pointer compression schemes might have cage base value unset.
10126+
// Read-only roots accessors contain type DCHECKs which require access to
10127+
// V8 heap in order to check the object type. So, allow heap access here
10128+
// to let the checks work.
10129+
i::PtrComprCageAccessScope ptr_compr_cage_access_scope(i_isolate);
10130+
#endif // DEBUG
1011510131
i_isolate->heap()->CollectAllAvailableGarbage(
1011610132
i::GarbageCollectionReason::kLowMemoryNotification);
1011710133
}

‎src/common/ptr-compr-inl.h‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,16 @@ Address V8HeapCompressionScheme::DecompressTaggedSigned(Tagged_t raw_value) {
9191
template <typename TOnHeapAddress>
9292
Address V8HeapCompressionScheme::DecompressTagged(TOnHeapAddress on_heap_addr,
9393
Tagged_t raw_value) {
94-
#if defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE)
94+
#ifdef V8_COMPRESS_POINTERS
9595
Address cage_base = base();
96+
#ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
97+
DCHECK_WITH_MSG(cage_base != kNullAddress,
98+
"V8HeapCompressionScheme::base is not initialized for "
99+
"current thread");
100+
#endif // V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
96101
#else
97102
Address cage_base = GetPtrComprCageBaseAddress(on_heap_addr);
98-
#endif
103+
#endif // V8_COMPRESS_POINTERS
99104
Address result = cage_base + static_cast<Address>(raw_value);
100105
V8_ASSUME(static_cast<uint32_t>(result) == raw_value);
101106
return result;
@@ -191,11 +196,16 @@ Address ExternalCodeCompressionScheme::DecompressTaggedSigned(
191196
template <typename TOnHeapAddress>
192197
Address ExternalCodeCompressionScheme::DecompressTagged(
193198
TOnHeapAddress on_heap_addr, Tagged_t raw_value) {
194-
#if defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE)
199+
#ifdef V8_COMPRESS_POINTERS
195200
Address cage_base = base();
201+
#ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
202+
DCHECK_WITH_MSG(cage_base != kNullAddress,
203+
"ExternalCodeCompressionScheme::base is not initialized for "
204+
"current thread");
205+
#endif // V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
196206
#else
197207
Address cage_base = GetPtrComprCageBaseAddress(on_heap_addr);
198-
#endif
208+
#endif // V8_COMPRESS_POINTERS
199209
Address result = cage_base + static_cast<Address>(raw_value);
200210
V8_ASSUME(static_cast<uint32_t>(result) == raw_value);
201211
return result;
@@ -275,6 +285,19 @@ V8_INLINE PtrComprCageBase GetPtrComprCageBase(Tagged<HeapObject> object) {
275285
return GetPtrComprCageBaseFromOnHeapAddress(object.ptr());
276286
}
277287

288+
#ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
289+
290+
PtrComprCageAccessScope::PtrComprCageAccessScope(Isolate* isolate)
291+
: cage_base_(V8HeapCompressionScheme::base()) {
292+
V8HeapCompressionScheme::InitBase(isolate->cage_base());
293+
}
294+
295+
PtrComprCageAccessScope::~PtrComprCageAccessScope() {
296+
V8HeapCompressionScheme::InitBase(cage_base_);
297+
}
298+
299+
#endif // V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
300+
278301
} // namespace internal
279302
} // namespace v8
280303

‎src/common/ptr-compr.h‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ class V8HeapCompressionScheme {
5555
private:
5656
// These non-inlined accessors to base_ field are used in component builds
5757
// where cross-component access to thread local variables is not allowed.
58-
static Address base_non_inlined();
59-
static void set_base_non_inlined(Address base);
58+
static V8_EXPORT_PRIVATE Address base_non_inlined();
59+
static V8_EXPORT_PRIVATE void set_base_non_inlined(Address base);
6060

6161
#ifdef V8_COMPRESS_POINTERS_IN_SHARED_CAGE
6262
static V8_EXPORT_PRIVATE uintptr_t base_ V8_CONSTINIT;
@@ -156,6 +156,29 @@ static inline void WriteMaybeUnalignedValue(Address p, V value) {
156156
}
157157
}
158158

159+
// When multi-cage pointer compression mode is enabled this scope object
160+
// saves current cage's base values and sets them according to given Isolate.
161+
// For all other configurations this scope object is a no-op.
162+
class PtrComprCageAccessScope final {
163+
public:
164+
#ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
165+
V8_INLINE explicit PtrComprCageAccessScope(Isolate* isolate);
166+
V8_INLINE ~PtrComprCageAccessScope();
167+
#else
168+
V8_INLINE explicit PtrComprCageAccessScope(Isolate* isolate) {}
169+
V8_INLINE ~PtrComprCageAccessScope() {}
170+
#endif
171+
172+
private:
173+
#ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
174+
const Address cage_base_;
175+
#ifdef V8_EXTERNAL_CODE_SPACE
176+
// In case this configuration is necessary the code cage base must be saved too.
177+
#error Multi-cage pointer compression with external code space is not supported
178+
#endif // V8_EXTERNAL_CODE_SPACE
179+
#endif // V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
180+
};
181+
159182
} // namespace v8::internal
160183

161184
#endif // V8_COMMON_PTR_COMPR_H_

‎src/diagnostics/objects-printer.cc‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3504,9 +3504,15 @@ inline i::Tagged<i::Object> GetObjectFromRaw(void* object) {
35043504
#ifdef V8_COMPRESS_POINTERS
35053505
if (RoundDown<i::kPtrComprCageBaseAlignment>(object_ptr) == i::kNullAddress) {
35063506
// Try to decompress pointer.
3507-
i::Isolate* isolate = i::Isolate::Current();
3508-
object_ptr = i::V8HeapCompressionScheme::DecompressTagged(
3509-
isolate, static_cast<i::Tagged_t>(object_ptr));
3507+
i::Isolate* isolate = i::Isolate::TryGetCurrent();
3508+
if (isolate != nullptr) {
3509+
object_ptr = i::V8HeapCompressionScheme::DecompressTagged(
3510+
isolate, static_cast<i::Tagged_t>(object_ptr));
3511+
} else {
3512+
i::PtrComprCageBase cage_base = i::GetPtrComprCageBase();
3513+
object_ptr = i::V8HeapCompressionScheme::DecompressTagged(
3514+
cage_base, static_cast<i::Tagged_t>(object_ptr));
3515+
}
35103516
}
35113517
#endif
35123518
return i::Tagged<i::Object>(object_ptr);

‎src/execution/isolate.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3971,6 +3971,14 @@ Isolate::~Isolate() {
39713971

39723972
void Isolate::InitializeThreadLocal() {
39733973
thread_local_top()->Initialize(this);
3974+
#ifdef DEBUG
3975+
// This method might be called on a thread that's not bound to any Isolate
3976+
// and thus pointer compression schemes might have cage base value unset.
3977+
// Read-only roots accessors contain type DCHECKs which require access to
3978+
// V8 heap in order to check the object type. So, allow heap access here
3979+
// to let the checks work.
3980+
i::PtrComprCageAccessScope ptr_compr_cage_access_scope(this);
3981+
#endif // DEBUG
39743982
clear_pending_exception();
39753983
clear_pending_message();
39763984
clear_scheduled_exception();

‎src/execution/v8threads.cc‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ bool ThreadManager::RestoreThread() {
124124
InitThread(access);
125125
return false;
126126
}
127+
// In case multi-cage pointer compression mode is enabled ensure that
128+
// current thread's cage base values are properly initialized.
129+
PtrComprCageAccessScope ptr_compr_cage_access_scope(isolate_);
130+
127131
ThreadState* state = per_thread->thread_state();
128132
char* from = state->data();
129133
from = isolate_->handle_scope_implementer()->RestoreThread(from);
@@ -274,6 +278,14 @@ void ThreadManager::EagerlyArchiveThread() {
274278
}
275279

276280
void ThreadManager::FreeThreadResources() {
281+
#ifdef DEBUG
282+
// This method might be called on a thread that's not bound to any Isolate
283+
// and thus pointer compression schemes might have cage base value unset.
284+
// Read-only roots accessors contain type DCHECKs which require access to
285+
// V8 heap in order to check the object type. So, allow heap access here
286+
// to let the checks work.
287+
PtrComprCageAccessScope ptr_compr_cage_access_scope(isolate_);
288+
#endif // DEBUG
277289
DCHECK(!isolate_->has_pending_exception());
278290
DCHECK(!isolate_->external_caught_exception());
279291
DCHECK_NULL(isolate_->try_catch_handler());

‎src/heap/collection-barrier.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ class BackgroundCollectionInterruptTask : public CancelableTask {
5151

5252
private:
5353
// v8::internal::CancelableTask overrides.
54-
void RunInternal() override { heap_->CheckCollectionRequested(); }
54+
void RunInternal() override {
55+
// In case multi-cage pointer compression mode is enabled ensure that
56+
// current thread's cage base values are properly initialized.
57+
PtrComprCageAccessScope ptr_compr_cage_access_scope(heap_->isolate());
58+
heap_->CheckCollectionRequested();
59+
}
5560

5661
Heap* heap_;
5762
};

‎src/heap/concurrent-marking.cc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ class ConcurrentMarking::JobTaskMajor : public v8::JobTask {
154154

155155
// v8::JobTask overrides.
156156
void Run(JobDelegate* delegate) override {
157+
// In case multi-cage pointer compression mode is enabled ensure that
158+
// current thread's cage base values are properly initialized.
159+
PtrComprCageAccessScope ptr_compr_cage_access_scope(
160+
concurrent_marking_->heap_->isolate());
161+
157162
if (delegate->IsJoiningThread()) {
158163
// TRACE_GC is not needed here because the caller opens the right scope.
159164
concurrent_marking_->RunMajor(delegate, code_flush_mode_,
@@ -198,6 +203,11 @@ class ConcurrentMarking::JobTaskMinor : public v8::JobTask {
198203

199204
// v8::JobTask overrides.
200205
void Run(JobDelegate* delegate) override {
206+
// In case multi-cage pointer compression mode is enabled ensure that
207+
// current thread's cage base values are properly initialized.
208+
PtrComprCageAccessScope ptr_compr_cage_access_scope(
209+
concurrent_marking_->heap_->isolate());
210+
201211
if (delegate->IsJoiningThread()) {
202212
TRACE_GC_WITH_FLOW(concurrent_marking_->heap_->tracer(),
203213
GCTracer::Scope::MINOR_MS_MARK_PARALLEL, trace_id_,

‎src/heap/incremental-marking-job.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ void IncrementalMarkingJob::Task::RunInternal() {
9191
VMState<GC> state(isolate());
9292
TRACE_EVENT_CALL_STATS_SCOPED(isolate(), "v8",
9393
"V8.IncrementalMarkingJob.Task");
94+
// In case multi-cage pointer compression mode is enabled ensure that
95+
// current thread's cage base values are properly initialized.
96+
PtrComprCageAccessScope ptr_compr_cage_access_scope(isolate());
9497

9598
isolate()->stack_guard()->ClearStartIncrementalMarking();
9699

0 commit comments

Comments
 (0)