From f6ea0c203aa08c4817c7739461f8e7015d390279 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Fri, 14 Mar 2014 23:22:27 +0100
Subject: [PATCH] src: fix segfaults, fix 32 bits integer negation

Make calls to v8::Isolate::AdjustAmountOfExternalAllocatedMemory() take
special care when negating 32 bits unsigned types like size_t.

Before this commit, values were negated before they got promoted to
64 bits, meaning that on 32 bits architectures, a value like 42 got
cast to 4294967254 instead of -42.

That in turn made the garbage collector start scavenging like crazy
because it thought the system was out of memory.

That's bad enough but calls to AdjustAmountOfExternalAllocatedMemory()
were made from weak callbacks, i.e. at a time when the garbage collector
was already busy.  It triggered asserts in debug builds and caused
random crashes and memory corruption in release builds.

The behavior in release builds is arguably a V8 bug and should perhaps
be reported upstream.

Partially fixes #7309 but requires further bug fixes to src/smalloc.cc
that I'll address in a follow-up commit.
---
 src/node_zlib.cc    | 8 ++++----
 src/smalloc.cc      | 9 ++++++---
 src/string_bytes.cc | 3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/node_zlib.cc b/src/node_zlib.cc
index 81d98d17c9..08a20ca553 100644
--- a/src/node_zlib.cc
+++ b/src/node_zlib.cc
@@ -110,13 +110,13 @@ class ZCtx : public AsyncWrap {
 
     if (mode_ == DEFLATE || mode_ == GZIP || mode_ == DEFLATERAW) {
       (void)deflateEnd(&strm_);
-      env()->isolate()
-          ->AdjustAmountOfExternalAllocatedMemory(-kDeflateContextSize);
+      int64_t change_in_bytes = -static_cast<int64_t>(kDeflateContextSize);
+      env()->isolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
     } else if (mode_ == INFLATE || mode_ == GUNZIP || mode_ == INFLATERAW ||
                mode_ == UNZIP) {
       (void)inflateEnd(&strm_);
-      env()->isolate()
-          ->AdjustAmountOfExternalAllocatedMemory(-kInflateContextSize);
+      int64_t change_in_bytes = -static_cast<int64_t>(kInflateContextSize);
+      env()->isolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
     }
     mode_ = NONE;
 
diff --git a/src/smalloc.cc b/src/smalloc.cc
index 6a812be51e..a5c5015a98 100644
--- a/src/smalloc.cc
+++ b/src/smalloc.cc
@@ -291,7 +291,8 @@ void TargetCallback(const WeakCallbackData<Object, char>& data) {
   assert(array_size * len >= len);
   len *= array_size;
   if (info != NULL && len > 0) {
-    data.GetIsolate()->AdjustAmountOfExternalAllocatedMemory(-len);
+    int64_t change_in_bytes = -static_cast<int64_t>(len);
+    data.GetIsolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
     free(info);
   }
 
@@ -338,7 +339,8 @@ void AllocDispose(Environment* env, Handle<Object> obj) {
     free(data);
   }
   if (length != 0) {
-    env->isolate()->AdjustAmountOfExternalAllocatedMemory(-length);
+    int64_t change_in_bytes = -static_cast<int64_t>(length);
+    env->isolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
   }
 }
 
@@ -407,7 +409,8 @@ void TargetFreeCallback(Isolate* isolate,
     assert(len * array_size > len);
     len *= array_size;
   }
-  isolate->AdjustAmountOfExternalAllocatedMemory(-(len + sizeof(*cb_info)));
+  int64_t change_in_bytes = -static_cast<int64_t>(len + sizeof(*cb_info));
+  isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
   cb_info->p_obj.Reset();
   cb_info->cb(data, cb_info->hint);
   delete cb_info;
diff --git a/src/string_bytes.cc b/src/string_bytes.cc
index d1ad4a1f4a..1546ca252e 100644
--- a/src/string_bytes.cc
+++ b/src/string_bytes.cc
@@ -50,7 +50,8 @@ class ExternString: public ResourceType {
   public:
     ~ExternString() {
       delete[] data_;
-      isolate()->AdjustAmountOfExternalAllocatedMemory(-length_);
+      int64_t change_in_bytes = -static_cast<int64_t>(length_);
+      isolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
     }
 
     const TypeName* data() const {