Skip to content

Commit

Permalink
src: remove unowned usages of GetBackingStore
Browse files Browse the repository at this point in the history
This removes all usages of GetBackingStore without any entries in the
`CODEOWNERS` file. For the most part this is a pretty straightforward
review; except `SPREAD_BUFFER_ARG` and the changes to `CopyArrayBuffer`.

See the linked issue for an explanation.

Refs: #32226
Refs: #43921
PR-URL: #44080
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
kvakil authored and danielleadams committed Aug 11, 2022
1 parent 8a79636 commit c7373b7
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 50 deletions.
7 changes: 3 additions & 4 deletions src/aliased_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class AliasedBufferBase {
// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, size_in_bytes);
buffer_ = static_cast<NativeT*>(ab->GetBackingStore()->Data());
buffer_ = static_cast<NativeT*>(ab->Data());

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
Expand Down Expand Up @@ -119,8 +119,7 @@ class AliasedBufferBase {
// be removed when we expand the snapshot support.
DCHECK_EQ(count_, arr->Length());
DCHECK_EQ(byte_offset_, arr->ByteOffset());
uint8_t* raw =
static_cast<uint8_t*>(arr->Buffer()->GetBackingStore()->Data());
uint8_t* raw = static_cast<uint8_t*>(arr->Buffer()->Data());
buffer_ = reinterpret_cast<NativeT*>(raw + byte_offset_);
js_array_.Reset(isolate_, arr);
index_ = nullptr;
Expand Down Expand Up @@ -278,7 +277,7 @@ class AliasedBufferBase {
isolate_, new_size_in_bytes);

// allocate new native buffer
NativeT* new_buffer = static_cast<NativeT*>(ab->GetBackingStore()->Data());
NativeT* new_buffer = static_cast<NativeT*>(ab->Data());
// copy old content
memcpy(new_buffer, buffer_, old_size_in_bytes);

Expand Down
59 changes: 35 additions & 24 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ bool HasInstance(Local<Object> obj) {
char* Data(Local<Value> val) {
CHECK(val->IsArrayBufferView());
Local<ArrayBufferView> ui = val.As<ArrayBufferView>();
return static_cast<char*>(ui->Buffer()->GetBackingStore()->Data()) +
ui->ByteOffset();
return static_cast<char*>(ui->Buffer()->Data()) + ui->ByteOffset();
}


Expand Down Expand Up @@ -1157,14 +1156,13 @@ static void EncodeInto(const FunctionCallbackInfo<Value>& args) {

Local<Uint8Array> dest = args[1].As<Uint8Array>();
Local<ArrayBuffer> buf = dest->Buffer();
char* write_result =
static_cast<char*>(buf->GetBackingStore()->Data()) + dest->ByteOffset();
char* write_result = static_cast<char*>(buf->Data()) + dest->ByteOffset();
size_t dest_length = dest->ByteLength();

// results = [ read, written ]
Local<Uint32Array> result_arr = args[2].As<Uint32Array>();
uint32_t* results = reinterpret_cast<uint32_t*>(
static_cast<char*>(result_arr->Buffer()->GetBackingStore()->Data()) +
static_cast<char*>(result_arr->Buffer()->Data()) +
result_arr->ByteOffset());

int nchars;
Expand Down Expand Up @@ -1228,6 +1226,27 @@ void DetachArrayBuffer(const FunctionCallbackInfo<Value>& args) {
}
}

namespace {

std::pair<void*, size_t> DecomposeBufferToParts(Local<Value> buffer) {
void* pointer;
size_t byte_length;
if (buffer->IsArrayBuffer()) {
Local<ArrayBuffer> ab = buffer.As<ArrayBuffer>();
pointer = ab->Data();
byte_length = ab->ByteLength();
} else if (buffer->IsSharedArrayBuffer()) {
Local<SharedArrayBuffer> ab = buffer.As<SharedArrayBuffer>();
pointer = ab->Data();
byte_length = ab->ByteLength();
} else {
UNREACHABLE(); // Caller must validate.
}
return {pointer, byte_length};
}

} // namespace

void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
// args[0] == Destination ArrayBuffer
// args[1] == Destination ArrayBuffer Offset
Expand All @@ -1241,32 +1260,24 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
CHECK(args[3]->IsUint32());
CHECK(args[4]->IsUint32());

std::shared_ptr<BackingStore> destination;
std::shared_ptr<BackingStore> source;
void* destination;
size_t destination_byte_length;
std::tie(destination, destination_byte_length) =
DecomposeBufferToParts(args[0]);

if (args[0]->IsArrayBuffer()) {
destination = args[0].As<ArrayBuffer>()->GetBackingStore();
} else if (args[0]->IsSharedArrayBuffer()) {
destination = args[0].As<SharedArrayBuffer>()->GetBackingStore();
}

if (args[2]->IsArrayBuffer()) {
source = args[2].As<ArrayBuffer>()->GetBackingStore();
} else if (args[0]->IsSharedArrayBuffer()) {
source = args[2].As<SharedArrayBuffer>()->GetBackingStore();
}
void* source;
size_t source_byte_length;
std::tie(source, source_byte_length) = DecomposeBufferToParts(args[2]);

uint32_t destination_offset = args[1].As<Uint32>()->Value();
uint32_t source_offset = args[3].As<Uint32>()->Value();
size_t bytes_to_copy = args[4].As<Uint32>()->Value();

CHECK_GE(destination->ByteLength() - destination_offset, bytes_to_copy);
CHECK_GE(source->ByteLength() - source_offset, bytes_to_copy);
CHECK_GE(destination_byte_length - destination_offset, bytes_to_copy);
CHECK_GE(source_byte_length - source_offset, bytes_to_copy);

uint8_t* dest =
static_cast<uint8_t*>(destination->Data()) + destination_offset;
uint8_t* src =
static_cast<uint8_t*>(source->Data()) + source_offset;
uint8_t* dest = static_cast<uint8_t*>(destination) + destination_offset;
uint8_t* src = static_cast<uint8_t*>(source) + source_offset;
memcpy(dest, src, bytes_to_copy);
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ static void GetLoadAvg(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> array = args[0].As<Float64Array>();
CHECK_EQ(array->Length(), 3);
Local<ArrayBuffer> ab = array->Buffer();
double* loadavg = static_cast<double*>(ab->GetBackingStore()->Data());
double* loadavg = static_cast<double*>(ab->Data());
uv_loadavg(loadavg);
}

Expand Down
6 changes: 3 additions & 3 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ static void CPUUsage(const FunctionCallbackInfo<Value>& args) {

// Get the double array pointer from the Float64Array argument.
Local<ArrayBuffer> ab = get_fields_array_buffer(args, 0, 2);
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
double* fields = static_cast<double*>(ab->Data());

// Set the Float64Array elements to be user / system values in microseconds.
fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
Expand Down Expand Up @@ -189,7 +189,7 @@ static void MemoryUsage(const FunctionCallbackInfo<Value>& args) {

// Get the double array pointer from the Float64Array argument.
Local<ArrayBuffer> ab = get_fields_array_buffer(args, 0, 5);
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
double* fields = static_cast<double*>(ab->Data());

size_t rss;
int err = uv_resident_set_memory(&rss);
Expand Down Expand Up @@ -311,7 +311,7 @@ static void ResourceUsage(const FunctionCallbackInfo<Value>& args) {
return env->ThrowUVException(err, "uv_getrusage");

Local<ArrayBuffer> ab = get_fields_array_buffer(args, 0, 16);
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
double* fields = static_cast<double*>(ab->Data());

fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
fields[1] = MICROS_PER_SEC * rusage.ru_stime.tv_sec + rusage.ru_stime.tv_usec;
Expand Down
4 changes: 1 addition & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -710,9 +710,7 @@ void Worker::GetResourceLimits(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, sizeof(resource_limits_));

memcpy(ab->GetBackingStore()->Data(),
resource_limits_,
sizeof(resource_limits_));
memcpy(ab->Data(), resource_limits_, sizeof(resource_limits_));
return Float64Array::New(ab, 0, kTotalResourceLimitCount);
}

Expand Down
3 changes: 1 addition & 2 deletions src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,7 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
CHECK(args[4]->IsUint32Array());
Local<Uint32Array> array = args[4].As<Uint32Array>();
Local<ArrayBuffer> ab = array->Buffer();
uint32_t* write_result = static_cast<uint32_t*>(
ab->GetBackingStore()->Data());
uint32_t* write_result = static_cast<uint32_t*>(ab->Data());

CHECK(args[5]->IsFunction());
Local<Function> write_js_callback = args[5].As<Function>();
Expand Down
3 changes: 1 addition & 2 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,7 @@ void ArrayBufferViewContents<T, S>::Read(v8::Local<v8::ArrayBufferView> abv) {
static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment");
length_ = abv->ByteLength();
if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) {
data_ = static_cast<T*>(abv->Buffer()->GetBackingStore()->Data()) +
abv->ByteOffset();
data_ = static_cast<T*>(abv->Buffer()->Data()) + abv->ByteOffset();
} else {
abv->CopyContents(stack_storage_, sizeof(stack_storage_));
data_ = stack_storage_;
Expand Down
19 changes: 8 additions & 11 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -535,17 +535,14 @@ class BufferValue : public MaybeStackBuffer<char> {
inline std::string ToString() const { return std::string(out(), length()); }
};

#define SPREAD_BUFFER_ARG(val, name) \
CHECK((val)->IsArrayBufferView()); \
v8::Local<v8::ArrayBufferView> name = (val).As<v8::ArrayBufferView>(); \
std::shared_ptr<v8::BackingStore> name##_bs = \
name->Buffer()->GetBackingStore(); \
const size_t name##_offset = name->ByteOffset(); \
const size_t name##_length = name->ByteLength(); \
char* const name##_data = \
static_cast<char*>(name##_bs->Data()) + name##_offset; \
if (name##_length > 0) \
CHECK_NE(name##_data, nullptr);
#define SPREAD_BUFFER_ARG(val, name) \
CHECK((val)->IsArrayBufferView()); \
v8::Local<v8::ArrayBufferView> name = (val).As<v8::ArrayBufferView>(); \
const size_t name##_offset = name->ByteOffset(); \
const size_t name##_length = name->ByteLength(); \
char* const name##_data = \
static_cast<char*>(name->Buffer()->Data()) + name##_offset; \
if (name##_length > 0) CHECK_NE(name##_data, nullptr);

// Use this when a variable or parameter is unused in order to explicitly
// silence a compiler warning about that.
Expand Down

0 comments on commit c7373b7

Please sign in to comment.