Skip to content

Commit 5b8434e

Browse files
targostargos
committed
deps: V8: cherry-pick 0188634
Original commit message: [ptr-compr][ubsan] Use [Read/Write]UnalignedValue for unaligned fields When pointer compression is enabled the [u]intptr_t and double fields are only kTaggedSize aligned so in order to avoid undefined behavior in C++ code we have to access these values in an unaligned pointer friendly way although both x64 and arm64 architectures (where pointer compression is supported) allow unaligned access. These changes will be removed once v8:8875 is fixed and all the kSystemPointerSize fields are properly aligned. Bug: v8:7703 Change-Id: I4df477cbdeab806303bb4f675d52b61c06342c8e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1528996 Commit-Queue: Igor Sheludko <ishell@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#60321} Refs: v8/v8@0188634 PR-URL: #27013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 8cc181c commit 5b8434e

14 files changed

+166
-96
lines changed

Diff for: ‎common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737

3838
# Reset this number to 0 on major V8 upgrades.
3939
# Increment by one for each non-official patch applied to deps/v8.
40-
'v8_embedder_string': '-node.15',
40+
'v8_embedder_string': '-node.16',
4141

4242
##### V8 defaults for Node.js #####
4343

Diff for: ‎deps/v8/include/v8-internal.h

+12
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <stddef.h>
99
#include <stdint.h>
10+
#include <string.h>
1011
#include <type_traits>
1112

1213
#include "v8-version.h" // NOLINT(build/include)
@@ -274,6 +275,17 @@ class Internals {
274275
V8_INLINE static T ReadRawField(internal::Address heap_object_ptr,
275276
int offset) {
276277
internal::Address addr = heap_object_ptr + offset - kHeapObjectTag;
278+
#ifdef V8_COMPRESS_POINTERS
279+
if (sizeof(T) > kApiTaggedSize) {
280+
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
281+
// fields (external pointers, doubles and BigInt data) are only
282+
// kTaggedSize aligned so we have to use unaligned pointer friendly way of
283+
// accessing them in order to avoid undefined behavior in C++ code.
284+
T r;
285+
memcpy(&r, reinterpret_cast<void*>(addr), sizeof(T));
286+
return r;
287+
}
288+
#endif
277289
return *reinterpret_cast<const T*>(addr);
278290
}
279291

Diff for: ‎deps/v8/include/v8.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ class Arguments;
118118
class DeferredHandles;
119119
class Heap;
120120
class HeapObject;
121+
class ExternalString;
121122
class Isolate;
122123
class LocalEmbedderHeapTracer;
123124
class MicrotaskQueue;
@@ -2797,7 +2798,7 @@ class V8_EXPORT String : public Name {
27972798
void operator=(const ExternalStringResourceBase&) = delete;
27982799

27992800
private:
2800-
friend class internal::Heap;
2801+
friend class internal::ExternalString;
28012802
friend class v8::String;
28022803
friend class internal::ScopedExternalStringLock;
28032804
};

Diff for: ‎deps/v8/src/heap/heap-inl.h

+1-9
Original file line numberDiff line numberDiff line change
@@ -300,15 +300,7 @@ void Heap::FinalizeExternalString(String string) {
300300
ExternalBackingStoreType::kExternalString,
301301
ext_string->ExternalPayloadSize());
302302

303-
v8::String::ExternalStringResourceBase** resource_addr =
304-
reinterpret_cast<v8::String::ExternalStringResourceBase**>(
305-
string->address() + ExternalString::kResourceOffset);
306-
307-
// Dispose of the C++ object if it has not already been disposed.
308-
if (*resource_addr != nullptr) {
309-
(*resource_addr)->Dispose();
310-
*resource_addr = nullptr;
311-
}
303+
ext_string->DisposeResource();
312304
}
313305

314306
Address Heap::NewSpaceTop() { return new_space_->top(); }

Diff for: ‎deps/v8/src/objects/bigint.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,7 @@ class MutableBigInt : public FreshlyAllocatedBigInt {
205205
}
206206
inline void set_digit(int n, digit_t value) {
207207
SLOW_DCHECK(0 <= n && n < length());
208-
Address address = FIELD_ADDR(*this, kDigitsOffset + n * kDigitSize);
209-
(*reinterpret_cast<digit_t*>(address)) = value;
208+
WRITE_UINTPTR_FIELD(*this, kDigitsOffset + n * kDigitSize, value);
210209
}
211210

212211
void set_64_bits(uint64_t bits);

Diff for: ‎deps/v8/src/objects/bigint.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ class BigIntBase : public HeapObject {
8787

8888
inline digit_t digit(int n) const {
8989
SLOW_DCHECK(0 <= n && n < length());
90-
Address address = FIELD_ADDR(*this, kDigitsOffset + n * kDigitSize);
91-
return *reinterpret_cast<digit_t*>(address);
90+
return READ_UINTPTR_FIELD(*this, kDigitsOffset + n * kDigitSize);
9291
}
9392

9493
bool is_zero() const { return length() == 0; }

Diff for: ‎deps/v8/src/objects/embedder-data-slot-inl.h

+17
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "src/objects-inl.h"
1212
#include "src/objects/embedder-data-array.h"
1313
#include "src/objects/js-objects-inl.h"
14+
#include "src/v8memory.h"
1415

1516
// Has to be the last include (doesn't have include guards):
1617
#include "src/objects/object-macros.h"
@@ -71,7 +72,15 @@ bool EmbedderDataSlot::ToAlignedPointer(void** out_pointer) const {
7172
// are accessed this way only from the main thread via API during "mutator"
7273
// phase which is propely synched with GC (concurrent marker may still look
7374
// at the tagged part of the embedder slot but read-only access is ok).
75+
#ifdef V8_COMPRESS_POINTERS
76+
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
77+
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
78+
// aligned so we have to use unaligned pointer friendly way of accessing them
79+
// in order to avoid undefined behavior in C++ code.
80+
Address raw_value = ReadUnalignedValue<Address>(address());
81+
#else
7482
Address raw_value = *location();
83+
#endif
7584
*out_pointer = reinterpret_cast<void*>(raw_value);
7685
return HAS_SMI_TAG(raw_value);
7786
}
@@ -89,7 +98,15 @@ EmbedderDataSlot::RawData EmbedderDataSlot::load_raw(
8998
// are accessed this way only by serializer from the main thread when
9099
// GC is not active (concurrent marker may still look at the tagged part
91100
// of the embedder slot but read-only access is ok).
101+
#ifdef V8_COMPRESS_POINTERS
102+
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
103+
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
104+
// aligned so we have to use unaligned pointer friendly way of accessing them
105+
// in order to avoid undefined behavior in C++ code.
106+
return ReadUnalignedValue<Address>(address());
107+
#else
92108
return *location();
109+
#endif
93110
}
94111

95112
void EmbedderDataSlot::store_raw(EmbedderDataSlot::RawData data,

Diff for: ‎deps/v8/src/objects/fixed-array-inl.h

+21-2
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,17 @@ typename Traits::ElementType FixedTypedArray<Traits>::get_scalar_from_data_ptr(
653653
// JavaScript memory model to have tear-free reads of overlapping accesses,
654654
// and using relaxed atomics may introduce overhead.
655655
TSAN_ANNOTATE_IGNORE_READS_BEGIN;
656-
auto result = ptr[index];
656+
ElementType result;
657+
if (COMPRESS_POINTERS_BOOL && alignof(ElementType) > kTaggedSize) {
658+
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
659+
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
660+
// aligned so we have to use unaligned pointer friendly way of accessing
661+
// them in order to avoid undefined behavior in C++ code.
662+
result = ReadUnalignedValue<ElementType>(reinterpret_cast<Address>(ptr) +
663+
index * sizeof(ElementType));
664+
} else {
665+
result = ptr[index];
666+
}
657667
TSAN_ANNOTATE_IGNORE_READS_END;
658668
return result;
659669
}
@@ -664,7 +674,16 @@ void FixedTypedArray<Traits>::set(int index, ElementType value) {
664674
// See the comment in FixedTypedArray<Traits>::get_scalar.
665675
auto* ptr = reinterpret_cast<ElementType*>(DataPtr());
666676
TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
667-
ptr[index] = value;
677+
if (COMPRESS_POINTERS_BOOL && alignof(ElementType) > kTaggedSize) {
678+
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
679+
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
680+
// aligned so we have to use unaligned pointer friendly way of accessing
681+
// them in order to avoid undefined behavior in C++ code.
682+
WriteUnalignedValue<ElementType>(
683+
reinterpret_cast<Address>(ptr) + index * sizeof(ElementType), value);
684+
} else {
685+
ptr[index] = value;
686+
}
668687
TSAN_ANNOTATE_IGNORE_WRITES_END;
669688
}
670689

Diff for: ‎deps/v8/src/objects/object-macros-undef.h

+13-10
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#undef DECL_INT_ACCESSORS
1616
#undef DECL_INT32_ACCESSORS
1717
#undef DECL_UINT16_ACCESSORS
18+
#undef DECL_INT16_ACCESSORS
1819
#undef DECL_UINT8_ACCESSORS
1920
#undef DECL_ACCESSORS
2021
#undef DECL_CAST
@@ -42,6 +43,7 @@
4243
#undef BIT_FIELD_ACCESSORS
4344
#undef INSTANCE_TYPE_CHECKER
4445
#undef TYPE_CHECKER
46+
#undef RELAXED_INT16_ACCESSORS
4547
#undef FIELD_ADDR
4648
#undef READ_FIELD
4749
#undef READ_WEAK_FIELD
@@ -52,6 +54,7 @@
5254
#undef WRITE_WEAK_FIELD
5355
#undef RELEASE_WRITE_FIELD
5456
#undef RELAXED_WRITE_FIELD
57+
#undef RELAXED_WRITE_WEAK_FIELD
5558
#undef WRITE_BARRIER
5659
#undef WEAK_WRITE_BARRIER
5760
#undef CONDITIONAL_WRITE_BARRIER
@@ -60,14 +63,7 @@
6063
#undef WRITE_DOUBLE_FIELD
6164
#undef READ_INT_FIELD
6265
#undef WRITE_INT_FIELD
63-
#undef ACQUIRE_READ_INTPTR_FIELD
64-
#undef RELAXED_READ_INTPTR_FIELD
65-
#undef READ_INTPTR_FIELD
66-
#undef RELEASE_WRITE_INTPTR_FIELD
67-
#undef RELAXED_WRITE_INTPTR_FIELD
68-
#undef WRITE_INTPTR_FIELD
69-
#undef READ_UINTPTR_FIELD
70-
#undef WRITE_UINTPTR_FIELD
66+
#undef ACQUIRE_READ_INT32_FIELD
7167
#undef READ_UINT8_FIELD
7268
#undef WRITE_UINT8_FIELD
7369
#undef RELAXED_WRITE_INT8_FIELD
@@ -78,18 +74,25 @@
7874
#undef WRITE_UINT16_FIELD
7975
#undef READ_INT16_FIELD
8076
#undef WRITE_INT16_FIELD
77+
#undef RELAXED_READ_INT16_FIELD
78+
#undef RELAXED_WRITE_INT16_FIELD
8179
#undef READ_UINT32_FIELD
80+
#undef RELAXED_READ_UINT32_FIELD
8281
#undef WRITE_UINT32_FIELD
82+
#undef RELAXED_WRITE_UINT32_FIELD
8383
#undef READ_INT32_FIELD
8484
#undef RELAXED_READ_INT32_FIELD
8585
#undef WRITE_INT32_FIELD
86+
#undef RELEASE_WRITE_INT32_FIELD
8687
#undef RELAXED_WRITE_INT32_FIELD
8788
#undef READ_FLOAT_FIELD
8889
#undef WRITE_FLOAT_FIELD
90+
#undef READ_INTPTR_FIELD
91+
#undef WRITE_INTPTR_FIELD
92+
#undef READ_UINTPTR_FIELD
93+
#undef WRITE_UINTPTR_FIELD
8994
#undef READ_UINT64_FIELD
9095
#undef WRITE_UINT64_FIELD
91-
#undef READ_INT64_FIELD
92-
#undef WRITE_INT64_FIELD
9396
#undef READ_BYTE_FIELD
9497
#undef RELAXED_READ_BYTE_FIELD
9598
#undef WRITE_BYTE_FIELD

Diff for: ‎deps/v8/src/objects/object-macros.h

+39-35
Original file line numberDiff line numberDiff line change
@@ -323,40 +323,10 @@
323323
#define WRITE_INT_FIELD(p, offset, value) \
324324
(*reinterpret_cast<int*>(FIELD_ADDR(p, offset)) = value)
325325

326-
#define ACQUIRE_READ_INTPTR_FIELD(p, offset) \
327-
static_cast<intptr_t>(base::Acquire_Load( \
328-
reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR(p, offset))))
329-
330326
#define ACQUIRE_READ_INT32_FIELD(p, offset) \
331327
static_cast<int32_t>(base::Acquire_Load( \
332328
reinterpret_cast<const base::Atomic32*>(FIELD_ADDR(p, offset))))
333329

334-
#define RELAXED_READ_INTPTR_FIELD(p, offset) \
335-
static_cast<intptr_t>(base::Relaxed_Load( \
336-
reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR(p, offset))))
337-
338-
#define READ_INTPTR_FIELD(p, offset) \
339-
(*reinterpret_cast<const intptr_t*>(FIELD_ADDR(p, offset)))
340-
341-
#define RELEASE_WRITE_INTPTR_FIELD(p, offset, value) \
342-
base::Release_Store( \
343-
reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
344-
static_cast<base::AtomicWord>(value));
345-
346-
#define RELAXED_WRITE_INTPTR_FIELD(p, offset, value) \
347-
base::Relaxed_Store( \
348-
reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
349-
static_cast<base::AtomicWord>(value));
350-
351-
#define WRITE_INTPTR_FIELD(p, offset, value) \
352-
(*reinterpret_cast<intptr_t*>(FIELD_ADDR(p, offset)) = value)
353-
354-
#define READ_UINTPTR_FIELD(p, offset) \
355-
(*reinterpret_cast<const uintptr_t*>(FIELD_ADDR(p, offset)))
356-
357-
#define WRITE_UINTPTR_FIELD(p, offset, value) \
358-
(*reinterpret_cast<uintptr_t*>(FIELD_ADDR(p, offset)) = value)
359-
360330
#define READ_UINT8_FIELD(p, offset) \
361331
(*reinterpret_cast<const uint8_t*>(FIELD_ADDR(p, offset)))
362332

@@ -439,17 +409,51 @@
439409
#define WRITE_FLOAT_FIELD(p, offset, value) \
440410
(*reinterpret_cast<float*>(FIELD_ADDR(p, offset)) = value)
441411

412+
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size fields
413+
// (external pointers, doubles and BigInt data) are only kTaggedSize aligned so
414+
// we have to use unaligned pointer friendly way of accessing them in order to
415+
// avoid undefined behavior in C++ code.
416+
#ifdef V8_COMPRESS_POINTERS
417+
418+
#define READ_INTPTR_FIELD(p, offset) \
419+
ReadUnalignedValue<intptr_t>(FIELD_ADDR(p, offset))
420+
421+
#define WRITE_INTPTR_FIELD(p, offset, value) \
422+
WriteUnalignedValue<intptr_t>(FIELD_ADDR(p, offset), value)
423+
424+
#define READ_UINTPTR_FIELD(p, offset) \
425+
ReadUnalignedValue<uintptr_t>(FIELD_ADDR(p, offset))
426+
427+
#define WRITE_UINTPTR_FIELD(p, offset, value) \
428+
WriteUnalignedValue<uintptr_t>(FIELD_ADDR(p, offset), value)
429+
430+
#define READ_UINT64_FIELD(p, offset) \
431+
ReadUnalignedValue<uint64_t>(FIELD_ADDR(p, offset))
432+
433+
#define WRITE_UINT64_FIELD(p, offset, value) \
434+
WriteUnalignedValue<uint64_t>(FIELD_ADDR(p, offset), value)
435+
436+
#else // V8_COMPRESS_POINTERS
437+
438+
#define READ_INTPTR_FIELD(p, offset) \
439+
(*reinterpret_cast<const intptr_t*>(FIELD_ADDR(p, offset)))
440+
441+
#define WRITE_INTPTR_FIELD(p, offset, value) \
442+
(*reinterpret_cast<intptr_t*>(FIELD_ADDR(p, offset)) = value)
443+
444+
#define READ_UINTPTR_FIELD(p, offset) \
445+
(*reinterpret_cast<const uintptr_t*>(FIELD_ADDR(p, offset)))
446+
447+
#define WRITE_UINTPTR_FIELD(p, offset, value) \
448+
(*reinterpret_cast<uintptr_t*>(FIELD_ADDR(p, offset)) = value)
449+
442450
#define READ_UINT64_FIELD(p, offset) \
443451
(*reinterpret_cast<const uint64_t*>(FIELD_ADDR(p, offset)))
444452

445453
#define WRITE_UINT64_FIELD(p, offset, value) \
446454
(*reinterpret_cast<uint64_t*>(FIELD_ADDR(p, offset)) = value)
447455

448-
#define READ_INT64_FIELD(p, offset) \
449-
(*reinterpret_cast<const int64_t*>(FIELD_ADDR(p, offset)))
450-
451-
#define WRITE_INT64_FIELD(p, offset, value) \
452-
(*reinterpret_cast<int64_t*>(FIELD_ADDR(p, offset)) = value)
456+
#endif // V8_COMPRESS_POINTERS
453457

454458
#define READ_BYTE_FIELD(p, offset) \
455459
(*reinterpret_cast<const byte*>(FIELD_ADDR(p, offset)))

0 commit comments

Comments
 (0)