Title: [262830] trunk
- Revision
- 262830
- Author
- [email protected]
- Date
- 2020-06-09 19:04:14 -0700 (Tue, 09 Jun 2020)
Log Message
Stringifier::appendStringifiedValue() should not assume it is always safe to recurse.
https://bugs.webkit.org/show_bug.cgi?id=213006
<rdar://problem/64154840>
Reviewed by Keith Miller.
JSTests:
* stress/json-stringify-executing-in-reserved-zone.js: Added.
Source/_javascript_Core:
In r262727, I suggested that Alexey Shvayka add an assertion in
Stringifier::appendStringifiedValue() to assert that it is safe to recurse because
we don't expect it to recurse into itself. Turns out this is a bad idea because
a client may be doing the recursing before calling Stringifier::appendStringifiedValue().
As a result, Stringifier::appendStringifiedValue() ends up being executed with
the stack pointer already in the reserved zone. This is legal, and is what the
reserved zone is intended for as long as we don't recurse from here. However,
this also means that asserting vm.isSafeToRecurseSoft() here will surely fail
because we are already in the reserved zone area. The fix is simply to remove
this faulty assertion.
* runtime/JSONObject.cpp:
(JSC::Stringifier::appendStringifiedValue):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (262829 => 262830)
--- trunk/JSTests/ChangeLog 2020-06-10 01:57:47 UTC (rev 262829)
+++ trunk/JSTests/ChangeLog 2020-06-10 02:04:14 UTC (rev 262830)
@@ -1,5 +1,15 @@
2020-06-09 Mark Lam <[email protected]>
+ Stringifier::appendStringifiedValue() should not assume it is always safe to recurse.
+ https://bugs.webkit.org/show_bug.cgi?id=213006
+ <rdar://problem/64154840>
+
+ Reviewed by Keith Miller.
+
+ * stress/json-stringify-executing-in-reserved-zone.js: Added.
+
+2020-06-09 Mark Lam <[email protected]>
+
Disambiguate the OverridesGetPropertyNames structure flag
https://bugs.webkit.org/show_bug.cgi?id=212909
<rdar://problem/63823557>
Added: trunk/JSTests/stress/json-stringify-executing-in-reserved-zone.js (0 => 262830)
--- trunk/JSTests/stress/json-stringify-executing-in-reserved-zone.js (rev 0)
+++ trunk/JSTests/stress/json-stringify-executing-in-reserved-zone.js 2020-06-10 02:04:14 UTC (rev 262830)
@@ -0,0 +1,13 @@
+//@ requireOptions("--maxPerThreadStackUsage=153600", "--exceptionStackTraceLimit=0", "--defaultErrorStackTraceLimit=0")
+
+function foo() {
+ JSON.stringify();
+ foo();
+}
+
+try {
+ foo();
+} catch (e) {
+ if (e != "RangeError: Maximum call stack size exceeded.")
+ throw e;
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (262829 => 262830)
--- trunk/Source/_javascript_Core/ChangeLog 2020-06-10 01:57:47 UTC (rev 262829)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-06-10 02:04:14 UTC (rev 262830)
@@ -1,5 +1,27 @@
2020-06-09 Mark Lam <[email protected]>
+ Stringifier::appendStringifiedValue() should not assume it is always safe to recurse.
+ https://bugs.webkit.org/show_bug.cgi?id=213006
+ <rdar://problem/64154840>
+
+ Reviewed by Keith Miller.
+
+ In r262727, I suggested that Alexey Shvayka add an assertion in
+ Stringifier::appendStringifiedValue() to assert that it is safe to recurse because
+ we don't expect it to recurse into itself. Turns out this is a bad idea because
+ a client may be doing the recursing before calling Stringifier::appendStringifiedValue().
+ As a result, Stringifier::appendStringifiedValue() ends up being executed with
+ the stack pointer already in the reserved zone. This is legal, and is what the
+ reserved zone is intended for as long as we don't recurse from here. However,
+ this also means that asserting vm.isSafeToRecurseSoft() here will surely fail
+ because we are already in the reserved zone area. The fix is simply to remove
+ this faulty assertion.
+
+ * runtime/JSONObject.cpp:
+ (JSC::Stringifier::appendStringifiedValue):
+
+2020-06-09 Mark Lam <[email protected]>
+
Disambiguate the OverridesGetPropertyNames structure flag
https://bugs.webkit.org/show_bug.cgi?id=212909
<rdar://problem/63823557>
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (262829 => 262830)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2020-06-10 01:57:47 UTC (rev 262829)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2020-06-10 02:04:14 UTC (rev 262830)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -314,7 +314,8 @@
auto scope = DECLARE_THROW_SCOPE(vm);
// Recursion is avoided by !holderStackWasEmpty check and do/while loop at the end of this method.
- ASSERT(vm.isSafeToRecurseSoft());
+ // We're having this recursion check here as a fail safe in case the code
+ // below get modified such that recursion is no longer avoided.
if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
throwStackOverflowError(m_globalObject, scope);
return StringifyFailed;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
