Skip to content

Commit

Permalink
deps: V8: cherry-pick 0188634
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
targos authored and BethGriggs committed Apr 4, 2019
1 parent 5d44bbd commit 07065a8
Show file tree
Hide file tree
Showing 14 changed files with 166 additions and 96 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

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

##### V8 defaults for Node.js #####

Expand Down
12 changes: 12 additions & 0 deletions deps/v8/include/v8-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <stddef.h>
#include <stdint.h>
#include <string.h>
#include <type_traits>

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

Expand Down
3 changes: 2 additions & 1 deletion deps/v8/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class Arguments;
class DeferredHandles;
class Heap;
class HeapObject;
class ExternalString;
class Isolate;
class LocalEmbedderHeapTracer;
class MicrotaskQueue;
Expand Down Expand Up @@ -2797,7 +2798,7 @@ class V8_EXPORT String : public Name {
void operator=(const ExternalStringResourceBase&) = delete;

private:
friend class internal::Heap;
friend class internal::ExternalString;
friend class v8::String;
friend class internal::ScopedExternalStringLock;
};
Expand Down
10 changes: 1 addition & 9 deletions deps/v8/src/heap/heap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,15 +300,7 @@ void Heap::FinalizeExternalString(String string) {
ExternalBackingStoreType::kExternalString,
ext_string->ExternalPayloadSize());

v8::String::ExternalStringResourceBase** resource_addr =
reinterpret_cast<v8::String::ExternalStringResourceBase**>(
string->address() + ExternalString::kResourceOffset);

// Dispose of the C++ object if it has not already been disposed.
if (*resource_addr != nullptr) {
(*resource_addr)->Dispose();
*resource_addr = nullptr;
}
ext_string->DisposeResource();
}

Address Heap::NewSpaceTop() { return new_space_->top(); }
Expand Down
3 changes: 1 addition & 2 deletions deps/v8/src/objects/bigint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@ class MutableBigInt : public FreshlyAllocatedBigInt {
}
inline void set_digit(int n, digit_t value) {
SLOW_DCHECK(0 <= n && n < length());
Address address = FIELD_ADDR(*this, kDigitsOffset + n * kDigitSize);
(*reinterpret_cast<digit_t*>(address)) = value;
WRITE_UINTPTR_FIELD(*this, kDigitsOffset + n * kDigitSize, value);
}

void set_64_bits(uint64_t bits);
Expand Down
3 changes: 1 addition & 2 deletions deps/v8/src/objects/bigint.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ class BigIntBase : public HeapObject {

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

bool is_zero() const { return length() == 0; }
Expand Down
17 changes: 17 additions & 0 deletions deps/v8/src/objects/embedder-data-slot-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "src/objects-inl.h"
#include "src/objects/embedder-data-array.h"
#include "src/objects/js-objects-inl.h"
#include "src/v8memory.h"

// Has to be the last include (doesn't have include guards):
#include "src/objects/object-macros.h"
Expand Down Expand Up @@ -71,7 +72,15 @@ bool EmbedderDataSlot::ToAlignedPointer(void** out_pointer) const {
// are accessed this way only from the main thread via API during "mutator"
// phase which is propely synched with GC (concurrent marker may still look
// at the tagged part of the embedder slot but read-only access is ok).
#ifdef V8_COMPRESS_POINTERS
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
// aligned so we have to use unaligned pointer friendly way of accessing them
// in order to avoid undefined behavior in C++ code.
Address raw_value = ReadUnalignedValue<Address>(address());
#else
Address raw_value = *location();
#endif
*out_pointer = reinterpret_cast<void*>(raw_value);
return HAS_SMI_TAG(raw_value);
}
Expand All @@ -89,7 +98,15 @@ EmbedderDataSlot::RawData EmbedderDataSlot::load_raw(
// are accessed this way only by serializer from the main thread when
// GC is not active (concurrent marker may still look at the tagged part
// of the embedder slot but read-only access is ok).
#ifdef V8_COMPRESS_POINTERS
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
// aligned so we have to use unaligned pointer friendly way of accessing them
// in order to avoid undefined behavior in C++ code.
return ReadUnalignedValue<Address>(address());
#else
return *location();
#endif
}

void EmbedderDataSlot::store_raw(EmbedderDataSlot::RawData data,
Expand Down
23 changes: 21 additions & 2 deletions deps/v8/src/objects/fixed-array-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,17 @@ typename Traits::ElementType FixedTypedArray<Traits>::get_scalar_from_data_ptr(
// JavaScript memory model to have tear-free reads of overlapping accesses,
// and using relaxed atomics may introduce overhead.
TSAN_ANNOTATE_IGNORE_READS_BEGIN;
auto result = ptr[index];
ElementType result;
if (COMPRESS_POINTERS_BOOL && alignof(ElementType) > kTaggedSize) {
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
// aligned so we have to use unaligned pointer friendly way of accessing
// them in order to avoid undefined behavior in C++ code.
result = ReadUnalignedValue<ElementType>(reinterpret_cast<Address>(ptr) +
index * sizeof(ElementType));
} else {
result = ptr[index];
}
TSAN_ANNOTATE_IGNORE_READS_END;
return result;
}
Expand All @@ -664,7 +674,16 @@ void FixedTypedArray<Traits>::set(int index, ElementType value) {
// See the comment in FixedTypedArray<Traits>::get_scalar.
auto* ptr = reinterpret_cast<ElementType*>(DataPtr());
TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
ptr[index] = value;
if (COMPRESS_POINTERS_BOOL && alignof(ElementType) > kTaggedSize) {
// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
// fields (external pointers, doubles and BigInt data) are only kTaggedSize
// aligned so we have to use unaligned pointer friendly way of accessing
// them in order to avoid undefined behavior in C++ code.
WriteUnalignedValue<ElementType>(
reinterpret_cast<Address>(ptr) + index * sizeof(ElementType), value);
} else {
ptr[index] = value;
}
TSAN_ANNOTATE_IGNORE_WRITES_END;
}

Expand Down
23 changes: 13 additions & 10 deletions deps/v8/src/objects/object-macros-undef.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#undef DECL_INT_ACCESSORS
#undef DECL_INT32_ACCESSORS
#undef DECL_UINT16_ACCESSORS
#undef DECL_INT16_ACCESSORS
#undef DECL_UINT8_ACCESSORS
#undef DECL_ACCESSORS
#undef DECL_CAST
Expand Down Expand Up @@ -42,6 +43,7 @@
#undef BIT_FIELD_ACCESSORS
#undef INSTANCE_TYPE_CHECKER
#undef TYPE_CHECKER
#undef RELAXED_INT16_ACCESSORS
#undef FIELD_ADDR
#undef READ_FIELD
#undef READ_WEAK_FIELD
Expand All @@ -52,6 +54,7 @@
#undef WRITE_WEAK_FIELD
#undef RELEASE_WRITE_FIELD
#undef RELAXED_WRITE_FIELD
#undef RELAXED_WRITE_WEAK_FIELD
#undef WRITE_BARRIER
#undef WEAK_WRITE_BARRIER
#undef CONDITIONAL_WRITE_BARRIER
Expand All @@ -60,14 +63,7 @@
#undef WRITE_DOUBLE_FIELD
#undef READ_INT_FIELD
#undef WRITE_INT_FIELD
#undef ACQUIRE_READ_INTPTR_FIELD
#undef RELAXED_READ_INTPTR_FIELD
#undef READ_INTPTR_FIELD
#undef RELEASE_WRITE_INTPTR_FIELD
#undef RELAXED_WRITE_INTPTR_FIELD
#undef WRITE_INTPTR_FIELD
#undef READ_UINTPTR_FIELD
#undef WRITE_UINTPTR_FIELD
#undef ACQUIRE_READ_INT32_FIELD
#undef READ_UINT8_FIELD
#undef WRITE_UINT8_FIELD
#undef RELAXED_WRITE_INT8_FIELD
Expand All @@ -78,18 +74,25 @@
#undef WRITE_UINT16_FIELD
#undef READ_INT16_FIELD
#undef WRITE_INT16_FIELD
#undef RELAXED_READ_INT16_FIELD
#undef RELAXED_WRITE_INT16_FIELD
#undef READ_UINT32_FIELD
#undef RELAXED_READ_UINT32_FIELD
#undef WRITE_UINT32_FIELD
#undef RELAXED_WRITE_UINT32_FIELD
#undef READ_INT32_FIELD
#undef RELAXED_READ_INT32_FIELD
#undef WRITE_INT32_FIELD
#undef RELEASE_WRITE_INT32_FIELD
#undef RELAXED_WRITE_INT32_FIELD
#undef READ_FLOAT_FIELD
#undef WRITE_FLOAT_FIELD
#undef READ_INTPTR_FIELD
#undef WRITE_INTPTR_FIELD
#undef READ_UINTPTR_FIELD
#undef WRITE_UINTPTR_FIELD
#undef READ_UINT64_FIELD
#undef WRITE_UINT64_FIELD
#undef READ_INT64_FIELD
#undef WRITE_INT64_FIELD
#undef READ_BYTE_FIELD
#undef RELAXED_READ_BYTE_FIELD
#undef WRITE_BYTE_FIELD
Expand Down
74 changes: 39 additions & 35 deletions deps/v8/src/objects/object-macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,40 +323,10 @@
#define WRITE_INT_FIELD(p, offset, value) \
(*reinterpret_cast<int*>(FIELD_ADDR(p, offset)) = value)

#define ACQUIRE_READ_INTPTR_FIELD(p, offset) \
static_cast<intptr_t>(base::Acquire_Load( \
reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR(p, offset))))

#define ACQUIRE_READ_INT32_FIELD(p, offset) \
static_cast<int32_t>(base::Acquire_Load( \
reinterpret_cast<const base::Atomic32*>(FIELD_ADDR(p, offset))))

#define RELAXED_READ_INTPTR_FIELD(p, offset) \
static_cast<intptr_t>(base::Relaxed_Load( \
reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR(p, offset))))

#define READ_INTPTR_FIELD(p, offset) \
(*reinterpret_cast<const intptr_t*>(FIELD_ADDR(p, offset)))

#define RELEASE_WRITE_INTPTR_FIELD(p, offset, value) \
base::Release_Store( \
reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
static_cast<base::AtomicWord>(value));

#define RELAXED_WRITE_INTPTR_FIELD(p, offset, value) \
base::Relaxed_Store( \
reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
static_cast<base::AtomicWord>(value));

#define WRITE_INTPTR_FIELD(p, offset, value) \
(*reinterpret_cast<intptr_t*>(FIELD_ADDR(p, offset)) = value)

#define READ_UINTPTR_FIELD(p, offset) \
(*reinterpret_cast<const uintptr_t*>(FIELD_ADDR(p, offset)))

#define WRITE_UINTPTR_FIELD(p, offset, value) \
(*reinterpret_cast<uintptr_t*>(FIELD_ADDR(p, offset)) = value)

#define READ_UINT8_FIELD(p, offset) \
(*reinterpret_cast<const uint8_t*>(FIELD_ADDR(p, offset)))

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

// TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size fields
// (external pointers, doubles and BigInt data) are only kTaggedSize aligned so
// we have to use unaligned pointer friendly way of accessing them in order to
// avoid undefined behavior in C++ code.
#ifdef V8_COMPRESS_POINTERS

#define READ_INTPTR_FIELD(p, offset) \
ReadUnalignedValue<intptr_t>(FIELD_ADDR(p, offset))

#define WRITE_INTPTR_FIELD(p, offset, value) \
WriteUnalignedValue<intptr_t>(FIELD_ADDR(p, offset), value)

#define READ_UINTPTR_FIELD(p, offset) \
ReadUnalignedValue<uintptr_t>(FIELD_ADDR(p, offset))

#define WRITE_UINTPTR_FIELD(p, offset, value) \
WriteUnalignedValue<uintptr_t>(FIELD_ADDR(p, offset), value)

#define READ_UINT64_FIELD(p, offset) \
ReadUnalignedValue<uint64_t>(FIELD_ADDR(p, offset))

#define WRITE_UINT64_FIELD(p, offset, value) \
WriteUnalignedValue<uint64_t>(FIELD_ADDR(p, offset), value)

#else // V8_COMPRESS_POINTERS

#define READ_INTPTR_FIELD(p, offset) \
(*reinterpret_cast<const intptr_t*>(FIELD_ADDR(p, offset)))

#define WRITE_INTPTR_FIELD(p, offset, value) \
(*reinterpret_cast<intptr_t*>(FIELD_ADDR(p, offset)) = value)

#define READ_UINTPTR_FIELD(p, offset) \
(*reinterpret_cast<const uintptr_t*>(FIELD_ADDR(p, offset)))

#define WRITE_UINTPTR_FIELD(p, offset, value) \
(*reinterpret_cast<uintptr_t*>(FIELD_ADDR(p, offset)) = value)

#define READ_UINT64_FIELD(p, offset) \
(*reinterpret_cast<const uint64_t*>(FIELD_ADDR(p, offset)))

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

#define READ_INT64_FIELD(p, offset) \
(*reinterpret_cast<const int64_t*>(FIELD_ADDR(p, offset)))

#define WRITE_INT64_FIELD(p, offset, value) \
(*reinterpret_cast<int64_t*>(FIELD_ADDR(p, offset)) = value)
#endif // V8_COMPRESS_POINTERS

#define READ_BYTE_FIELD(p, offset) \
(*reinterpret_cast<const byte*>(FIELD_ADDR(p, offset)))
Expand Down
Loading

0 comments on commit 07065a8

Please sign in to comment.