Attachment 362757 Details for Bug 194957 – Patch

[patch] Patch

bug-194957-20190222222702.patch (text/plain), 6.93 KB, created by Dominik Inführ on 2019-02-22 13:27:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Dominik Inführ

Created: 2019-02-22 13:27:03 PST

Size: 6.93 KB

patch

obsolete

>Subversion Revision: 241874
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index fcab6e3d3eb73d070e9a55c0a1b2e25be80c420d..cca6c513a9126be3733ecb809b201ec6a70256fb 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-02-22  Dominik Infuehr  <dinfuehr@igalia.com>
>+
>+        String overflow when using StringBuilder in JSC::createError
>+        https://bugs.webkit.org/show_bug.cgi?id=194957
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        StringBuilder in notAFunctionSourceAppender didn't check
>+        for overflows but just failed.
>+
>+        * runtime/ExceptionHelpers.cpp:
>+        (JSC::notAFunctionSourceAppender):
>+
> 2019-02-20  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Remove WatchpointSet creation for SymbolTable entries if VM::canUseJIT() returns false
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 4ecf7736ec6fd4a2ff0799e188231d5c38d5185c..dd4aead6cc4b246d5e278fc8a2c67a6d441efa0d 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-02-22  Dominik Infuehr  <dinfuehr@igalia.com>
>+
>+        String overflow when using StringBuilder in JSC::createError
>+        https://bugs.webkit.org/show_bug.cgi?id=194957
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When calculating the new capacity of a StringBuilder object,
>+        use a limit of MaxLength instead of MaxLength+1.  Allocating
>+        a string of size MaxLength+1 always fails, this meant that expanding
>+        a StringBuilder only works when doubling the capacity is smaller
>+        than that. A character cannot be appended to a String of size 1.4GB,
>+        since doubling the capacity doesn't fit into MaxLength anymore.
>+        Changing the maximum capacity to MaxLength allows this operation to
>+        succeed.
>+
>+        * wtf/text/StringBuilder.cpp:
>+
> 2019-02-21  Dean Jackson  <dino@apple.com>
> 
>         Rotation animations sometimes use the wrong origin (affects apple.com)
>diff --git a/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp b/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
>index a37446e4d2762e4c24cbeab66dce191b54726491..5ea8c4d762abdda2005d4251f4e728615ef50a9b 100644
>--- a/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
>+++ b/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
>@@ -193,7 +193,7 @@ static String notAFunctionSourceAppender(const String& originalMessage, const St
>     String base = functionCallBase(sourceText);
>     if (!base)
>         return defaultApproximateSourceError(originalMessage, sourceText);
>-    StringBuilder builder;
>+    StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
>     builder.append(base);
>     builder.appendLiteral(" is not a function. (In '");
>     builder.append(sourceText);
>@@ -209,6 +209,9 @@ static String notAFunctionSourceAppender(const String& originalMessage, const St
>     }
>     builder.append(')');
> 
>+    if (builder.hasOverflowed())
>+        return makeString("object is not a function."_s);
>+
>     return builder.toString();
> }
> 
>diff --git a/Source/WTF/wtf/text/StringBuilder.cpp b/Source/WTF/wtf/text/StringBuilder.cpp
>index 7d710971fbd5edb1af42d5fee41d2a2616bed391..4d73de9d280b23435082332887cf668ad1a9a04b 100644
>--- a/Source/WTF/wtf/text/StringBuilder.cpp
>+++ b/Source/WTF/wtf/text/StringBuilder.cpp
>@@ -34,7 +34,7 @@
> 
> namespace WTF {
> 
>-static constexpr unsigned maxCapacity = String::MaxLength + 1;
>+static constexpr unsigned maxCapacity = String::MaxLength;
> 
> static unsigned expandedCapacity(unsigned capacity, unsigned requiredLength)
> {
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 261d8178b3a76236d1fa30ce6ecabe0386111f4a..5a08d5bd993f064798804488d5b52eadb8b13c0e 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-02-22  Dominik Infuehr  <dinfuehr@igalia.com>
>+
>+        String overflow when using StringBuilder in JSC::createError
>+        https://bugs.webkit.org/show_bug.cgi?id=194957
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add test string-overflow-createError-bulder.js that overflows
>+        StringBuilder in notAFunctionSourceAppender. The second new test
>+        string-overflow-createError-fit.js has an error message that doesn't
>+        overflow, it still failed since the String's capacity can't be doubled.
>+        Run test string-overflow-createError.js only in the default
>+        configuration to reduce memory consumption when running the test
>+        in all configurations on multiple CPUs in parallel.
>+
>+        * stress/string-overflow-createError-builder.js: Copied from JSTests/stress/string-overflow-createError.js.
>+        (catch):
>+        * stress/string-overflow-createError-fit.js: Copied from JSTests/stress/string-overflow-createError.js.
>+        (catch):
>+        * stress/string-overflow-createError.js:
>+
> 2019-02-19  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
>diff --git a/JSTests/stress/string-overflow-createError-builder.js b/JSTests/stress/string-overflow-createError-builder.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..7af309ea00c93597b2f5c584794304efb9a63997
>--- /dev/null
>+++ b/JSTests/stress/string-overflow-createError-builder.js
>@@ -0,0 +1,15 @@
>+//@ skip if $memoryLimited
>+//@ runDefault
>+var exception;
>+try {
>+    bar = '2.3023e-320'
>+    foo = bar.padEnd(2147483620, 1);
>+    foo(true, 1).value;
>+} catch (e) {
>+    exception = e;
>+}
>+
>+// when the StringBuilder for the error message overflows,
>+// "object is not a function" is used as message for the TypeError.
>+if (exception.message != "object is not a function.")
>+    throw "FAILED";
>diff --git a/JSTests/stress/string-overflow-createError-fit.js b/JSTests/stress/string-overflow-createError-fit.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..dc0593751db9afa8837ac092fd33f948003c893f
>--- /dev/null
>+++ b/JSTests/stress/string-overflow-createError-fit.js
>@@ -0,0 +1,16 @@
>+//@ skip if $memoryLimited
>+//@ runDefault
>+var exception;
>+try {
>+    bar = '2.3023e-320'
>+    foo = bar.padEnd(2147480000, 1);
>+    foo(true, 1).value;
>+} catch (e) {
>+    exception = e;
>+}
>+
>+// Although the message of the TypeError is quite long,
>+// it still fits into String::MaxLength. Check the start
>+// of the error message.
>+if (!exception.message.startsWith("foo is not a function"))
>+    throw "FAILED";
>diff --git a/JSTests/stress/string-overflow-createError.js b/JSTests/stress/string-overflow-createError.js
>index 97ab1fa683ed4842faa591e6f22c3efef8237abf..80b93b538481def6cd9f3bd2dd1765ab74356257 100644
>--- a/JSTests/stress/string-overflow-createError.js
>+++ b/JSTests/stress/string-overflow-createError.js
>@@ -1,4 +1,5 @@
> //@ skip if $memoryLimited
>+//@ runDefault
> var exception;
> try {
>     bar = '2.3023e-320'
>@@ -8,5 +9,7 @@ try {
>     exception = e;
> }
> 
>+// Creating the error message for the TypeError overflows
>+// the string and therefore an out-of-memory error is thrown.
> if (exception != "Error: Out of memory")
>     throw "FAILED";

Actions: View | Formatted Diff | Diff

Attachments on bug 194957: 362746 | 362751 | 362755 | 362757 | 362759 | 363879