Bug 1265690 part 1 - Mark StringBuffer methods WARN_UNUSED_RESULT, fix OOM issues. r=jonco

js/src/builtin/TypedObject.cpp

@@ -691,12 +691,16 @@ ArrayMetaTypeDescr::construct(JSContext* cx, unsigned argc, Value* vp)
 
     // Construct a canonical string `new ArrayType(<elementType>, N)`:
     StringBuffer contents(cx);
-    contents.append("new ArrayType(");
-    contents.append(&elementType->stringRepr());
-    contents.append(", ");
+    if (!contents.append("new ArrayType("))
+        return false;
+    if (!contents.append(&elementType->stringRepr()))
+        return false;
+    if (!contents.append(", "))
+        return false;
     if (!NumberValueToStringBuffer(cx, NumberValue(length), contents))
         return false;
-    contents.append(")");
+    if (!contents.append(")"))
+        return false;
     RootedAtom stringRepr(cx, contents.finishAtom());
     if (!stringRepr)
         return false;

js/src/jit-test/tests/TypedObject/bug1265690.js

@@ -0,0 +1,14 @@
+if (!('oomTest' in this) || !this.hasOwnProperty("TypedObject"))
+    quit();
+lfCodeBuffer = `
+    ArrayType = TypedObject.ArrayType;
+    var StructType = TypedObject.StructType;
+    float32 = TypedObject.float32;
+    Point = new ArrayType(float32, 3);
+    var Line = new StructType({ Point });
+    new ArrayType(Line, 3);
+`;
+loadFile(lfCodeBuffer);
+function loadFile(lfVarx) {
+    oomTest(function() { eval(lfVarx) });
+}

js/src/jsexn.cpp

@@ -1028,49 +1028,49 @@ JS::CreateError(JSContext* cx, JSExnType type, HandleObject stack, HandleString
 const char*
 js::ValueToSourceForError(JSContext* cx, HandleValue val, JSAutoByteString& bytes)
 {
-    if (val.isUndefined()) {
+    if (val.isUndefined())
         return "undefined";
-    }
-    if (val.isNull()) {
+
+    if (val.isNull())
         return "null";
-    }
+
+    AutoClearPendingException acpe(cx);
 
     RootedString str(cx, JS_ValueToSource(cx, val));
-    if (!str) {
-        JS_ClearPendingException(cx);
+    if (!str)
         return "<<error converting value to string>>";
-    }
 
     StringBuffer sb(cx);
     if (val.isObject()) {
         RootedObject valObj(cx, val.toObjectOrNull());
         ESClassValue cls;
-        if (!GetBuiltinClass(cx, valObj, &cls)) {
-            JS_ClearPendingException(cx);
+        if (!GetBuiltinClass(cx, valObj, &cls))
             return "<<error determining class of value>>";
-        }
-        if (cls == ESClass_Array) {
-            sb.append("the array ");
-        } else if (cls == ESClass_ArrayBuffer) {
-            sb.append("the array buffer ");
-        } else if (JS_IsArrayBufferViewObject(valObj)) {
-            sb.append("the typed array ");
-        } else {
-            sb.append("the object ");
-        }
+        const char* s;
+        if (cls == ESClass_Array)
+            s = "the array ";
+        else if (cls == ESClass_ArrayBuffer)
+            s = "the array buffer ";
+        else if (JS_IsArrayBufferViewObject(valObj))
+            s = "the typed array ";
+        else
+            s = "the object ";
+        if (!sb.append(s, strlen(s)))
+            return "<<error converting value to string>>";
     } else if (val.isNumber()) {
-        sb.append("the number ");
+        if (!sb.append("the number "))
+            return "<<error converting value to string>>";
     } else if (val.isString()) {
-        sb.append("the string ");
+        if (!sb.append("the string "))
+            return "<<error converting value to string>>";
     } else {
         MOZ_ASSERT(val.isBoolean() || val.isSymbol());
         return bytes.encodeLatin1(cx, str);
     }
-    sb.append(str);
-    str = sb.finishString();
-    if (!str) {
-        JS_ClearPendingException(cx);
+    if (!sb.append(str))
+        return "<<error converting value to string>>";
+    str = sb.finishString();
+    if (!str)
         return "<<error converting value to string>>";
-    }
     return bytes.encodeLatin1(cx, str);
 }

js/src/jsopcode.cpp

@@ -1672,15 +1672,15 @@ js::GetPCCountScriptCount(JSContext* cx)
 
 enum MaybeComma {NO_COMMA, COMMA};
 
-static void
+static MOZ_WARN_UNUSED_RESULT bool
 AppendJSONProperty(StringBuffer& buf, const char* name, MaybeComma comma = COMMA)
 {
-    if (comma)
-        buf.append(',');
+    if (comma && !buf.append(','))
+        return false;
 
-    buf.append('\"');
-    buf.append(name, strlen(name));
-    buf.append("\":", 2);
+    return buf.append('\"') &&
+           buf.append(name, strlen(name)) &&
+           buf.append("\":", 2);
 }
 
 JS_FRIEND_API(JSString*)
@@ -1703,24 +1703,30 @@ js::GetPCCountScriptSummary(JSContext* cx, size_t index)
      */
     StringBuffer buf(cx);
 
-    buf.append('{');
+    if (!buf.append('{'))
+        return nullptr;
 
-    AppendJSONProperty(buf, "file", NO_COMMA);
+    if (!AppendJSONProperty(buf, "file", NO_COMMA))
+        return nullptr;
     JSString* str = JS_NewStringCopyZ(cx, script->filename());
     if (!str || !(str = StringToSource(cx, str)))
         return nullptr;
-    buf.append(str);
+    if (!buf.append(str))
+        return nullptr;
 
-    AppendJSONProperty(buf, "line");
+    if (!AppendJSONProperty(buf, "line"))
+        return nullptr;
     NumberValueToStringBuffer(cx, Int32Value(script->lineno()), buf);
 
     if (script->functionNonDelazifying()) {
         JSAtom* atom = script->functionNonDelazifying()->displayAtom();
         if (atom) {
-            AppendJSONProperty(buf, "name");
+            if (!AppendJSONProperty(buf, "name"))
+                return nullptr;
             if (!(str = StringToSource(cx, atom)))
                 return nullptr;
-            buf.append(str);
+            if (!buf.append(str))
+                return nullptr;
         }
     }
 
@@ -1734,11 +1740,15 @@ js::GetPCCountScriptSummary(JSContext* cx, size_t index)
         total += counts->numExec();
     }
 
-    AppendJSONProperty(buf, "totals");
-    buf.append('{');
+    if (!AppendJSONProperty(buf, "totals"))
+        return nullptr;
+    if (!buf.append('{'))
+        return nullptr;
 
-    AppendJSONProperty(buf, PCCounts::numExecName, NO_COMMA);
-    NumberValueToStringBuffer(cx, DoubleValue(total), buf);
+    if (!AppendJSONProperty(buf, PCCounts::numExecName, NO_COMMA))
+        return nullptr;
+    if (!NumberValueToStringBuffer(cx, DoubleValue(total), buf))
+        return nullptr;
 
     uint64_t ionActivity = 0;
     jit::IonScriptCounts* ionCounts = sac.getIonCounts();
@@ -1748,15 +1758,18 @@ js::GetPCCountScriptSummary(JSContext* cx, size_t index)
         ionCounts = ionCounts->previous();
     }
     if (ionActivity) {
-        AppendJSONProperty(buf, "ion", COMMA);
-        NumberValueToStringBuffer(cx, DoubleValue(ionActivity), buf);
+        if (!AppendJSONProperty(buf, "ion", COMMA))
+            return nullptr;
+        if (!NumberValueToStringBuffer(cx, DoubleValue(ionActivity), buf))
+            return nullptr;
     }
 
-    buf.append('}');
-    buf.append('}');
-
-    if (cx->isExceptionPending())
+    if (!buf.append('}'))
         return nullptr;
+    if (!buf.append('}'))
+        return nullptr;
+
+    MOZ_ASSERT(!cx->isExceptionPending());
 
     return buf.finishString();
 }
@@ -1766,20 +1779,27 @@ GetPCCountJSON(JSContext* cx, const ScriptAndCounts& sac, StringBuffer& buf)
 {
     RootedScript script(cx, sac.script);
 
-    buf.append('{');
-    AppendJSONProperty(buf, "text", NO_COMMA);
+    if (!buf.append('{'))
+        return false;
+    if (!AppendJSONProperty(buf, "text", NO_COMMA))
+        return false;
 
     JSString* str = JS_DecompileScript(cx, script, nullptr, 0);
     if (!str || !(str = StringToSource(cx, str)))
         return false;
 
-    buf.append(str);
+    if (!buf.append(str))
+        return false;
 
-    AppendJSONProperty(buf, "line");
-    NumberValueToStringBuffer(cx, Int32Value(script->lineno()), buf);
+    if (!AppendJSONProperty(buf, "line"))
+        return false;
+    if (!NumberValueToStringBuffer(cx, Int32Value(script->lineno()), buf))
+        return false;
 
-    AppendJSONProperty(buf, "opcodes");
-    buf.append('[');
+    if (!AppendJSONProperty(buf, "opcodes"))
+        return false;
+    if (!buf.append('['))
+        return false;
     bool comma = false;
 
     SrcNoteLineScanner scanner(script->notes(), script->lineno());
@@ -1796,26 +1816,35 @@ GetPCCountJSON(JSContext* cx, const ScriptAndCounts& sac, StringBuffer& buf)
         if (counts)
             hits = counts->numExec();
 
-        if (comma)
-            buf.append(',');
+        if (comma && !buf.append(','))
+            return false;
         comma = true;
 
-        buf.append('{');
+        if (!buf.append('{'))
+            return false;
 
-        AppendJSONProperty(buf, "id", NO_COMMA);
-        NumberValueToStringBuffer(cx, Int32Value(offset), buf);
+        if (!AppendJSONProperty(buf, "id", NO_COMMA))
+            return false;
+        if (!NumberValueToStringBuffer(cx, Int32Value(offset), buf))
+            return false;
 
         scanner.advanceTo(offset);
 
-        AppendJSONProperty(buf, "line");
-        NumberValueToStringBuffer(cx, Int32Value(scanner.getLine()), buf);
+        if (!AppendJSONProperty(buf, "line"))
+            return false;
+        if (!NumberValueToStringBuffer(cx, Int32Value(scanner.getLine()), buf))
+            return false;
 
         {
             const char* name = CodeName[op];
-            AppendJSONProperty(buf, "name");
-            buf.append('\"');
-            buf.append(name, strlen(name));
-            buf.append('\"');
+            if (!AppendJSONProperty(buf, "name"))
+                return false;
+            if (!buf.append('\"'))
+                return false;
+            if (!buf.append(name, strlen(name)))
+                return false;
+            if (!buf.append('\"'))
+                return false;
         }
 
         {
@@ -1827,24 +1856,32 @@ GetPCCountJSON(JSContext* cx, const ScriptAndCounts& sac, StringBuffer& buf)
             char* text;
             if (!ed.getOutput(&text))
                 return false;
-            AppendJSONProperty(buf, "text");
             JSString* str = JS_NewStringCopyZ(cx, text);
             js_free(text);
+            if (!AppendJSONProperty(buf, "text"))
+                return false;
             if (!str || !(str = StringToSource(cx, str)))
                 return false;
-            buf.append(str);
+            if (!buf.append(str))
+                return false;
         }
 
-        AppendJSONProperty(buf, "counts");
-        buf.append('{');
+        if (!AppendJSONProperty(buf, "counts"))
+            return false;
+        if (!buf.append('{'))
+            return false;
 
         if (hits > 0) {
-            AppendJSONProperty(buf, PCCounts::numExecName, NO_COMMA);
-            NumberValueToStringBuffer(cx, DoubleValue(hits), buf);
+            if (!AppendJSONProperty(buf, PCCounts::numExecName, NO_COMMA))
+                return false;
+            if (!NumberValueToStringBuffer(cx, DoubleValue(hits), buf))
+                return false;
         }
 
-        buf.append('}');
-        buf.append('}');
+        if (!buf.append('}'))
+            return false;
+        if (!buf.append('}'))
+            return false;
 
         // If the current instruction has thrown,
         // then decrement the hit counts with the number of throws.
@@ -1853,57 +1890,79 @@ GetPCCountJSON(JSContext* cx, const ScriptAndCounts& sac, StringBuffer& buf)
             hits -= counts->numExec();
     }
 
-    buf.append(']');
+    if (!buf.append(']'))
+        return false;
 
     jit::IonScriptCounts* ionCounts = sac.getIonCounts();
     if (ionCounts) {
-        AppendJSONProperty(buf, "ion");
-        buf.append('[');
+        if (!AppendJSONProperty(buf, "ion"))
+            return false;
+        if (!buf.append('['))
+            return false;
         bool comma = false;
         while (ionCounts) {
-            if (comma)
-                buf.append(',');
+            if (comma && !buf.append(','))
+                return false;
             comma = true;
 
-            buf.append('[');
+            if (!buf.append('['))
+                return false;
             for (size_t i = 0; i < ionCounts->numBlocks(); i++) {
-                if (i)
-                    buf.append(',');
+                if (i && !buf.append(','))
+                    return false;
                 const jit::IonBlockCounts& block = ionCounts->block(i);
 
-                buf.append('{');
-                AppendJSONProperty(buf, "id", NO_COMMA);
-                NumberValueToStringBuffer(cx, Int32Value(block.id()), buf);
-                AppendJSONProperty(buf, "offset");
-                NumberValueToStringBuffer(cx, Int32Value(block.offset()), buf);
-                AppendJSONProperty(buf, "successors");
-                buf.append('[');
+                if (!buf.append('{'))
+                    return false;
+                if (!AppendJSONProperty(buf, "id", NO_COMMA))
+                    return false;
+                if (!NumberValueToStringBuffer(cx, Int32Value(block.id()), buf))
+                    return false;
+                if (!AppendJSONProperty(buf, "offset"))
+                    return false;
+                if (!NumberValueToStringBuffer(cx, Int32Value(block.offset()), buf))
+                    return false;
+                if (!AppendJSONProperty(buf, "successors"))
+                    return false;
+                if (!buf.append('['))
+                    return false;
                 for (size_t j = 0; j < block.numSuccessors(); j++) {
-                    if (j)
-                        buf.append(',');
-                    NumberValueToStringBuffer(cx, Int32Value(block.successor(j)), buf);
+                    if (j && !buf.append(','))
+                        return false;
+                    if (!NumberValueToStringBuffer(cx, Int32Value(block.successor(j)), buf))
+                        return false;
                 }
-                buf.append(']');
-                AppendJSONProperty(buf, "hits");
-                NumberValueToStringBuffer(cx, DoubleValue(block.hitCount()), buf);
+                if (!buf.append(']'))
+                    return false;
+                if (!AppendJSONProperty(buf, "hits"))
+                    return false;
+                if (!NumberValueToStringBuffer(cx, DoubleValue(block.hitCount()), buf))
+                    return false;
 
-                AppendJSONProperty(buf, "code");
+                if (!AppendJSONProperty(buf, "code"))
+                    return false;
                 JSString* str = JS_NewStringCopyZ(cx, block.code());
                 if (!str || !(str = StringToSource(cx, str)))
                     return false;
-                buf.append(str);
-                buf.append('}');
+                if (!buf.append(str))
+                    return false;
+                if (!buf.append('}'))
+                    return false;
             }
-            buf.append(']');
+            if (!buf.append(']'))
+                return false;
 
             ionCounts = ionCounts->previous();
         }
-        buf.append(']');
+        if (!buf.append(']'))
+            return false;
     }
 
-    buf.append('}');
+    if (!buf.append('}'))
+        return false;
 
-    return !cx->isExceptionPending();
+    MOZ_ASSERT(!cx->isExceptionPending());
+    return true;
 }
 
 JS_FRIEND_API(JSString*)

js/src/vm/StringBuffer.h

@@ -72,7 +72,7 @@ class StringBuffer
         return cb.ref<TwoByteCharBuffer>();
     }
 
-    bool inflateChars();
+    MOZ_WARN_UNUSED_RESULT bool inflateChars();
 
   public:
     explicit StringBuffer(ExclusiveContext* cx)
@@ -85,25 +85,25 @@ class StringBuffer
         cb.construct<Latin1CharBuffer>(cx);
     }
 
-    inline bool reserve(size_t len) {
+    MOZ_WARN_UNUSED_RESULT bool reserve(size_t len) {
         if (len > reserved_)
             reserved_ = len;
         return isLatin1() ? latin1Chars().reserve(len) : twoByteChars().reserve(len);
     }
-    inline bool resize(size_t len) {
+    MOZ_WARN_UNUSED_RESULT bool resize(size_t len) {
         return isLatin1() ? latin1Chars().resize(len) : twoByteChars().resize(len);
     }
-    inline bool empty() const {
+    bool empty() const {
         return isLatin1() ? latin1Chars().empty() : twoByteChars().empty();
     }
-    inline size_t length() const {
+    size_t length() const {
         return isLatin1() ? latin1Chars().length() : twoByteChars().length();
     }
-    inline char16_t getChar(size_t idx) const {
+    char16_t getChar(size_t idx) const {
         return isLatin1() ? latin1Chars()[idx] : twoByteChars()[idx];
     }
 
-    inline bool ensureTwoByteChars() {
+    MOZ_WARN_UNUSED_RESULT bool ensureTwoByteChars() {
         if (isLatin1() && !inflateChars())
             return false;
 
@@ -113,7 +113,7 @@ class StringBuffer
         return true;
     }
 
-    inline bool append(const char16_t c) {
+    MOZ_WARN_UNUSED_RESULT bool append(const char16_t c) {
         if (isLatin1()) {
             if (c <= JSString::MAX_LATIN1_CHAR)
                 return latin1Chars().append(Latin1Char(c));
@@ -122,43 +122,44 @@ class StringBuffer
         }
         return twoByteChars().append(c);
     }
-    inline bool append(Latin1Char c) {
+    MOZ_WARN_UNUSED_RESULT bool append(Latin1Char c) {
         return isLatin1() ? latin1Chars().append(c) : twoByteChars().append(c);
     }
-    inline bool append(char c) {
+    MOZ_WARN_UNUSED_RESULT bool append(char c) {
         return append(Latin1Char(c));
     }
 
-    inline bool append(const char16_t* begin, const char16_t* end);
-    inline bool append(const char16_t* chars, size_t len) {
+    inline MOZ_WARN_UNUSED_RESULT bool append(const char16_t* begin, const char16_t* end);
+
+    MOZ_WARN_UNUSED_RESULT bool append(const char16_t* chars, size_t len) {
         return append(chars, chars + len);
     }
 
-    inline bool append(const Latin1Char* begin, const Latin1Char* end) {
+    MOZ_WARN_UNUSED_RESULT bool append(const Latin1Char* begin, const Latin1Char* end) {
         return isLatin1() ? latin1Chars().append(begin, end) : twoByteChars().append(begin, end);
     }
-    inline bool append(const Latin1Char* chars, size_t len) {
+    MOZ_WARN_UNUSED_RESULT bool append(const Latin1Char* chars, size_t len) {
         return append(chars, chars + len);
     }
 
-    inline bool append(const JS::ConstCharPtr chars, size_t len) {
+    MOZ_WARN_UNUSED_RESULT bool append(const JS::ConstCharPtr chars, size_t len) {
         return append(chars.get(), chars.get() + len);
     }
-    inline bool appendN(Latin1Char c, size_t n) {
+    MOZ_WARN_UNUSED_RESULT bool appendN(Latin1Char c, size_t n) {
         return isLatin1() ? latin1Chars().appendN(c, n) : twoByteChars().appendN(c, n);
     }
 
-    inline bool append(JSString* str);
-    inline bool append(JSLinearString* str);
-    inline bool appendSubstring(JSString* base, size_t off, size_t len);
-    inline bool appendSubstring(JSLinearString* base, size_t off, size_t len);
+    inline MOZ_WARN_UNUSED_RESULT bool append(JSString* str);
+    inline MOZ_WARN_UNUSED_RESULT bool append(JSLinearString* str);
+    inline MOZ_WARN_UNUSED_RESULT bool appendSubstring(JSString* base, size_t off, size_t len);
+    inline MOZ_WARN_UNUSED_RESULT bool appendSubstring(JSLinearString* base, size_t off, size_t len);
 
-    inline bool append(const char* chars, size_t len) {
+    MOZ_WARN_UNUSED_RESULT bool append(const char* chars, size_t len) {
         return append(reinterpret_cast<const Latin1Char*>(chars), len);
     }
 
     template <size_t ArrayLength>
-    bool append(const char (&array)[ArrayLength]) {
+    MOZ_WARN_UNUSED_RESULT bool append(const char (&array)[ArrayLength]) {
         return append(array, ArrayLength - 1); /* No trailing '\0'. */
     }