Bug 1323183 - prevent memory leak from CheckTargetAndPopulate. r?bholley draft
authorAndi-Bogdan Postelnicu <bpostelnicu@mozilla.com>
Wed, 14 Dec 2016 20:56:46 +0200
changeset 449678 bcfbff29b329e4883d3c915aa18a6cb7af2fb3b4
parent 448717 f46f85dcfbc2b3098ea758825d18be6fab33cbc6
child 539538 185602bcffa16fdc96ee9644a19fc2976071d6c2
push id38620
push userbmo:bpostelnicu@mozilla.com
push dateWed, 14 Dec 2016 18:57:21 +0000
reviewersbholley
bugs1323183
milestone53.0a1
Bug 1323183 - prevent memory leak from CheckTargetAndPopulate. r?bholley MozReview-Commit-ID: 6iMg2eUHoU6
js/xpconnect/src/XPCConvert.cpp
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -1316,62 +1316,63 @@ XPCConvert::NativeArray2JS(MutableHandle
 }
 
 
 
 // Check that the tag part of the type matches the type
 // of the array. If the check succeeds, check that the size
 // of the output does not exceed UINT32_MAX bytes. Allocate
 // the memory and copy the elements by memcpy.
-static bool
+static void*
 CheckTargetAndPopulate(const nsXPTType& type,
                        uint8_t requiredType,
                        size_t typeSize,
                        uint32_t count,
                        JSObject* tArr,
-                       void** output,
                        nsresult* pErr)
 {
     // Check that the element type expected by the interface matches
     // the type of the elements in the typed array exactly, including
     // signedness.
     if (type.TagPart() != requiredType) {
         if (pErr)
             *pErr = NS_ERROR_XPC_BAD_CONVERT_JS;
 
-        return false;
+        return nullptr;
     }
 
-    // Calulate the maximum number of elements that can fit in
+    // Calculate the maximum number of elements that can fit in
     // UINT32_MAX bytes.
     size_t max = UINT32_MAX / typeSize;
 
     // This could overflow on 32-bit systems so check max first.
     size_t byteSize = count * typeSize;
-    if (count > max || !(*output = moz_xmalloc(byteSize))) {
+    if (count > max) {
         if (pErr)
             *pErr = NS_ERROR_OUT_OF_MEMORY;
 
-        return false;
+        return nullptr;
     }
 
     JS::AutoCheckCannotGC nogc;
     bool isShared;
     void* buf = JS_GetArrayBufferViewData(tArr, &isShared, nogc);
 
     // Require opting in to shared memory - a future project.
     if (isShared) {
         if (pErr)
             *pErr = NS_ERROR_XPC_BAD_CONVERT_JS;
 
-        return false;
+        return nullptr;
     }
 
-    memcpy(*output, buf, byteSize);
-    return true;
+    void* output = moz_xmalloc(byteSize);
+
+    memcpy(output, buf, byteSize);
+    return output;
 }
 
 // Fast conversion of typed arrays to native using memcpy.
 // No float or double canonicalization is done. Called by
 // JSarray2Native whenever a TypedArray is met. ArrayBuffers
 // are not accepted; create a properly typed array view on them
 // first. The element type of array must match the XPCOM
 // type in size, type and signedness exactly. As an exception,
@@ -1399,76 +1400,84 @@ XPCConvert::JSTypedArray2Native(void** d
 
         return false;
     }
 
     void* output = nullptr;
 
     switch (JS_GetArrayBufferViewType(jsArray)) {
     case js::Scalar::Int8:
-        if (!CheckTargetAndPopulate(nsXPTType::T_I8, type,
-                                    sizeof(int8_t), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_I8, type,
+                                        sizeof(int8_t), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Uint8:
     case js::Scalar::Uint8Clamped:
-        if (!CheckTargetAndPopulate(nsXPTType::T_U8, type,
-                                    sizeof(uint8_t), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_U8, type,
+                                        sizeof(uint8_t), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Int16:
-        if (!CheckTargetAndPopulate(nsXPTType::T_I16, type,
-                                    sizeof(int16_t), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_I16, type,
+                                        sizeof(int16_t), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Uint16:
-        if (!CheckTargetAndPopulate(nsXPTType::T_U16, type,
-                                    sizeof(uint16_t), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_U16, type,
+                                        sizeof(uint16_t), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Int32:
-        if (!CheckTargetAndPopulate(nsXPTType::T_I32, type,
-                                    sizeof(int32_t), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_I32, type,
+                                        sizeof(int32_t), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Uint32:
-        if (!CheckTargetAndPopulate(nsXPTType::T_U32, type,
-                                    sizeof(uint32_t), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_U32, type,
+                                        sizeof(uint32_t), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Float32:
-        if (!CheckTargetAndPopulate(nsXPTType::T_FLOAT, type,
-                                    sizeof(float), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_FLOAT, type,
+                                        sizeof(float), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     case js::Scalar::Float64:
-        if (!CheckTargetAndPopulate(nsXPTType::T_DOUBLE, type,
-                                    sizeof(double), count,
-                                    jsArray, &output, pErr)) {
+        output = CheckTargetAndPopulate(nsXPTType::T_DOUBLE, type,
+                                        sizeof(double), count,
+                                        jsArray, pErr);
+        if (!output) {
             return false;
         }
         break;
 
     // Yet another array type was defined? It is not supported yet...
     default:
         if (pErr)
             *pErr = NS_ERROR_XPC_BAD_CONVERT_JS;