Skip to content

Commit

Permalink
src: normalize malloc, realloc
Browse files Browse the repository at this point in the history
malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
that the standard allows them to either return a unique pointer or a
nullptr for zero-sized allocation requests.  Normalize by always using
a nullptr.

- Introduce node::malloc, node::realloc and node::calloc that should
  be used throught our source.
- Update all existing node source files to use the new functions
  instead of the native allocation functions.

Fixes: #7549
PR-URL: #7564
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

 Conflicts:
	src/node.cc
  • Loading branch information
mhdawson authored and Fishrock123 committed Sep 9, 2016
1 parent d0a92be commit d1d1433
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 32 deletions.
3 changes: 2 additions & 1 deletion src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ static void ares_poll_close_cb(uv_handle_t* watcher) {

/* Allocates and returns a new node_ares_task */
static node_ares_task* ares_task_create(Environment* env, ares_socket_t sock) {
node_ares_task* task = static_cast<node_ares_task*>(malloc(sizeof(*task)));
node_ares_task* task =
static_cast<node_ares_task*>(node::Malloc(sizeof(*task)));

if (task == nullptr) {
/* Out of memory. */
Expand Down
4 changes: 2 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1023,9 +1023,9 @@ void* ArrayBufferAllocator::Allocate(size_t size) {
if (env_ == nullptr ||
!env_->array_buffer_allocator_info()->no_zero_fill() ||
zero_fill_all_buffers)
return calloc(size, 1);
return node::Calloc(size, 1);
env_->array_buffer_allocator_info()->reset_fill_flag();
return malloc(size);
return node::Malloc(size);
}

static bool DomainHasErrorHandler(const Environment* env,
Expand Down
12 changes: 4 additions & 8 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
size_t length = end - start;

#define BUFFER_MALLOC(length) \
zero_fill_all_buffers ? calloc(length, 1) : malloc(length)
zero_fill_all_buffers ? node::Calloc(length, 1) : node::Malloc(length)

#if defined(__GNUC__) || defined(__clang__)
#define BSWAP_INTRINSIC_2(x) __builtin_bswap16(x)
Expand Down Expand Up @@ -265,10 +265,6 @@ MaybeLocal<Object> New(Isolate* isolate,
size_t actual = 0;
char* data = nullptr;

// malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
// that the standard allows them to either return a unique pointer or a
// nullptr for zero-sized allocation requests. Normalize by always using
// a nullptr.
if (length > 0) {
data = static_cast<char*>(BUFFER_MALLOC(length));

Expand All @@ -282,7 +278,7 @@ MaybeLocal<Object> New(Isolate* isolate,
free(data);
data = nullptr;
} else if (actual < length) {
data = static_cast<char*>(realloc(data, actual));
data = static_cast<char*>(node::Realloc(data, actual));
CHECK_NE(data, nullptr);
}
}
Expand Down Expand Up @@ -361,7 +357,7 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
void* new_data;
if (length > 0) {
CHECK_NE(data, nullptr);
new_data = malloc(length);
new_data = node::Malloc(length);
if (new_data == nullptr)
return Local<Object>();
memcpy(new_data, data, length);
Expand Down Expand Up @@ -1083,7 +1079,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
offset,
is_forward);
} else if (enc == LATIN1) {
uint8_t* needle_data = static_cast<uint8_t*>(malloc(needle_length));
uint8_t* needle_data = static_cast<uint8_t*>(node::Malloc(needle_length));
if (needle_data == nullptr) {
return args.GetReturnValue().Set(-1);
}
Expand Down
20 changes: 10 additions & 10 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2279,7 +2279,7 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
size_t len = Buffer::Length(obj);

// OpenSSL takes control of the pointer after accepting it
char* data = reinterpret_cast<char*>(malloc(len));
char* data = reinterpret_cast<char*>(node::Malloc(len));
CHECK_NE(data, nullptr);
memcpy(data, resp, len);

Expand Down Expand Up @@ -3330,7 +3330,7 @@ bool CipherBase::GetAuthTag(char** out, unsigned int* out_len) const {
if (initialised_ || kind_ != kCipher || !auth_tag_)
return false;
*out_len = auth_tag_len_;
*out = static_cast<char*>(malloc(auth_tag_len_));
*out = static_cast<char*>(node::Malloc(auth_tag_len_));
CHECK_NE(*out, nullptr);
memcpy(*out, auth_tag_, auth_tag_len_);
return true;
Expand Down Expand Up @@ -4906,7 +4906,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
// NOTE: field_size is in bits
int field_size = EC_GROUP_get_degree(ecdh->group_);
size_t out_len = (field_size + 7) / 8;
char* out = static_cast<char*>(malloc(out_len));
char* out = static_cast<char*>(node::Malloc(out_len));
CHECK_NE(out, nullptr);

int r = ECDH_compute_key(out, out_len, pub, ecdh->key_, nullptr);
Expand Down Expand Up @@ -4942,7 +4942,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
if (size == 0)
return env->ThrowError("Failed to get public key length");

unsigned char* out = static_cast<unsigned char*>(malloc(size));
unsigned char* out = static_cast<unsigned char*>(node::Malloc(size));
CHECK_NE(out, nullptr);

int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
Expand All @@ -4968,7 +4968,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
return env->ThrowError("Failed to get ECDH private key");

int size = BN_num_bytes(b);
unsigned char* out = static_cast<unsigned char*>(malloc(size));
unsigned char* out = static_cast<unsigned char*>(node::Malloc(size));
CHECK_NE(out, nullptr);

if (size != BN_bn2bin(b, out)) {
Expand Down Expand Up @@ -5099,7 +5099,7 @@ class PBKDF2Request : public AsyncWrap {
saltlen_(saltlen),
salt_(salt),
keylen_(keylen),
key_(static_cast<char*>(malloc(keylen))),
key_(static_cast<char*>(node::Malloc(keylen))),
iter_(iter) {
if (key() == nullptr)
FatalError("node::PBKDF2Request()", "Out of Memory");
Expand Down Expand Up @@ -5262,7 +5262,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Salt");

pass = static_cast<char*>(malloc(passlen));
pass = static_cast<char*>(node::Malloc(passlen));
if (pass == nullptr) {
FatalError("node::PBKDF2()", "Out of Memory");
}
Expand All @@ -5274,7 +5274,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
goto err;
}

salt = static_cast<char*>(malloc(saltlen));
salt = static_cast<char*>(node::Malloc(saltlen));
if (salt == nullptr) {
FatalError("node::PBKDF2()", "Out of Memory");
}
Expand Down Expand Up @@ -5367,7 +5367,7 @@ class RandomBytesRequest : public AsyncWrap {
: AsyncWrap(env, object, AsyncWrap::PROVIDER_CRYPTO),
error_(0),
size_(size),
data_(static_cast<char*>(malloc(size))) {
data_(static_cast<char*>(node::Malloc(size))) {
if (data() == nullptr)
FatalError("node::RandomBytesRequest()", "Out of Memory");
Wrap(object, this);
Expand Down Expand Up @@ -5596,7 +5596,7 @@ void GetCurves(const FunctionCallbackInfo<Value>& args) {

if (num_curves) {
alloc_size = sizeof(*curves) * num_curves;
curves = static_cast<EC_builtin_curve*>(malloc(alloc_size));
curves = static_cast<EC_builtin_curve*>(node::Malloc(alloc_size));

CHECK_NE(curves, nullptr);

Expand Down
3 changes: 2 additions & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
inline void set_env(Environment* env) { env_ = env; }

virtual void* Allocate(size_t size); // Defined in src/node.cc
virtual void* AllocateUninitialized(size_t size) { return malloc(size); }
virtual void* AllocateUninitialized(size_t size)
{ return node::Malloc(size); }
virtual void Free(void* data, size_t) { free(data); }

private:
Expand Down
4 changes: 2 additions & 2 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ void StreamWrap::OnAlloc(uv_handle_t* handle,


void StreamWrap::OnAllocImpl(size_t size, uv_buf_t* buf, void* ctx) {
buf->base = static_cast<char*>(malloc(size));
buf->base = static_cast<char*>(node::Malloc(size));
buf->len = size;

if (buf->base == nullptr && size > 0) {
Expand Down Expand Up @@ -204,7 +204,7 @@ void StreamWrap::OnReadImpl(ssize_t nread,
return;
}

char* base = static_cast<char*>(realloc(buf->base, nread));
char* base = static_cast<char*>(node::Realloc(buf->base, nread));
CHECK_LE(static_cast<size_t>(nread), buf->len);

if (pending == UV_TCP) {
Expand Down
8 changes: 4 additions & 4 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class ExternString: public ResourceType {
return scope.Escape(String::Empty(isolate));

TypeName* new_data =
static_cast<TypeName*>(malloc(length * sizeof(*new_data)));
static_cast<TypeName*>(node::Malloc(length * sizeof(*new_data)));
if (new_data == nullptr) {
return Local<String>();
}
Expand Down Expand Up @@ -624,7 +624,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,

case ASCII:
if (contains_non_ascii(buf, buflen)) {
char* out = static_cast<char*>(malloc(buflen));
char* out = static_cast<char*>(node::Malloc(buflen));
if (out == nullptr) {
return Local<String>();
}
Expand Down Expand Up @@ -659,7 +659,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,

case BASE64: {
size_t dlen = base64_encoded_size(buflen);
char* dst = static_cast<char*>(malloc(dlen));
char* dst = static_cast<char*>(node::Malloc(dlen));
if (dst == nullptr) {
return Local<String>();
}
Expand All @@ -678,7 +678,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,

case HEX: {
size_t dlen = buflen * 2;
char* dst = static_cast<char*>(malloc(dlen));
char* dst = static_cast<char*>(node::Malloc(dlen));
if (dst == nullptr) {
return Local<String>();
}
Expand Down
2 changes: 1 addition & 1 deletion src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ void TLSWrap::OnReadImpl(ssize_t nread,


void TLSWrap::OnAllocSelf(size_t suggested_size, uv_buf_t* buf, void* ctx) {
buf->base = static_cast<char*>(malloc(suggested_size));
buf->base = static_cast<char*>(node::Malloc(suggested_size));
CHECK_NE(buf->base, nullptr);
buf->len = suggested_size;
}
Expand Down
4 changes: 2 additions & 2 deletions src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ void UDPWrap::OnSend(uv_udp_send_t* req, int status) {
void UDPWrap::OnAlloc(uv_handle_t* handle,
size_t suggested_size,
uv_buf_t* buf) {
buf->base = static_cast<char*>(malloc(suggested_size));
buf->base = static_cast<char*>(node::Malloc(suggested_size));
buf->len = suggested_size;

if (buf->base == nullptr && suggested_size > 0) {
Expand Down Expand Up @@ -426,7 +426,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,
return;
}

char* base = static_cast<char*>(realloc(buf->base, nread));
char* base = static_cast<char*>(node::Realloc(buf->base, nread));
argv[2] = Buffer::New(env, base, nread).ToLocalChecked();
argv[3] = AddressToJS(env, addr);
wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv);
Expand Down
26 changes: 26 additions & 0 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,32 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
return true;
}

// These should be used in our code as opposed to the native
// versions as they abstract out some platform and or
// compiler version specific functionality.
// malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
// that the standard allows them to either return a unique pointer or a
// nullptr for zero-sized allocation requests. Normalize by always using
// a nullptr.
void* Realloc(void* pointer, size_t size) {
if (size == 0) {
free(pointer);
return nullptr;
}
return realloc(pointer, size);
}

// As per spec realloc behaves like malloc if passed nullptr.
void* Malloc(size_t size) {
return Realloc(nullptr, size);
}

void* Calloc(size_t n, size_t size) {
if ((n == 0) || (size == 0)) return nullptr;
CHECK_GE(n * size, n); // Overflow guard.
return calloc(n, size);
}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
13 changes: 12 additions & 1 deletion src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@

namespace node {

// These should be used in our code as opposed to the native
// versions as they abstract out some platform and or
// compiler version specific functionality
// malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
// that the standard allows them to either return a unique pointer or a
// nullptr for zero-sized allocation requests. Normalize by always using
// a nullptr.
inline void* Realloc(void* pointer, size_t size);
inline void* Malloc(size_t size);
inline void* Calloc(size_t n, size_t size);

#ifdef __GNUC__
#define NO_RETURN __attribute__((noreturn))
#else
Expand Down Expand Up @@ -300,7 +311,7 @@ class MaybeStackBuffer {
// Guard against overflow.
CHECK_LE(storage, sizeof(T) * storage);

buf_ = static_cast<T*>(malloc(sizeof(T) * storage));
buf_ = static_cast<T*>(Malloc(sizeof(T) * storage));
CHECK_NE(buf_, nullptr);
}

Expand Down

0 comments on commit d1d1433

Please sign in to comment.