Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: remove unowned usages of GetBackingStore #44080

Merged
merged 2 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
CHECK(false); // Caller must validate.
kvakil marked this conversation as resolved.
Show resolved Hide resolved
}
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();
Comment on lines -1255 to -1256
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nodejs/whatwg-stream: this is a typo: the first args[0] should be args[2]. I don't think it ends up being an issue in practice. The only user of copyArrayBuffer is in lib/internal/webstreams/readablestream.js, and it doesn't seem possible to pass in a SharedArrayBuffer to either of its arguments.

Anyway, the code motion in this diff fixes the issue anyway.

}
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