Reviewers: Yang,
Message:
PTAL
https://codereview.chromium.org/1157273002/diff/2/src/isolate.cc
File src/isolate.cc (right):
https://codereview.chromium.org/1157273002/diff/2/src/isolate.cc#newcode855
src/isolate.cc:855: return Throw(heap()->undefined_value(), nullptr);
Maybe it is better to throw "stack overflow" instead. WDYT?
Description:
Reland "Fixed a couple of failing DCHECK(has_pending_exception()).
(patchset #1
id:1 of https://codereview.chromium.org/1151373002/ )"
BUG=chromium:491062
LOG=N
Please review this at https://codereview.chromium.org/1157273002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+33, -16 lines):
M src/bootstrapper.cc
M src/debug.cc
M src/isolate.cc
M src/runtime/runtime-debug.cc
M test/mjsunit/mjsunit.status
A + test/mjsunit/regress/regress-crbug-491062.js
Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index
fcca1548443ca809bb8685efcebcf77039497e60..5521b18a60f8f3b3897567c6a287d35a96c9be1f
100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -1492,7 +1492,10 @@ bool Genesis::CompileNative(Isolate* isolate,
Vector<const char> name,
// environment has been at least partially initialized. Add a stack check
// before entering JS code to catch overflow early.
StackLimitCheck check(isolate);
- if (check.HasOverflowed()) return false;
+ if (check.HasOverflowed()) {
+ isolate->StackOverflow();
+ return false;
+ }
Handle<Context> context(isolate->context());
@@ -2969,7 +2972,10 @@ Genesis::Genesis(Isolate* isolate,
// environment has been at least partially initialized. Add a stack check
// before entering JS code to catch overflow early.
StackLimitCheck check(isolate);
- if (check.HasOverflowed()) return;
+ if (check.HasOverflowed()) {
+ isolate->StackOverflow();
+ return;
+ }
// The deserializer needs to hook up references to the global proxy.
// Create an uninitialized global proxy now if we don't have one
Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index
b5e81c55cb239042c63d7147b2b3c56ba4ba7f9e..85fdb14f36e6de4ca1939ff4eeecefa82c451786
100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -608,13 +608,7 @@ bool Debug::CompileDebuggerScript(Isolate* isolate,
int index) {
source_code, script_name, 0, 0, ScriptOriginOptions(),
Handle<Object>(),
context, NULL, NULL, ScriptCompiler::kNoCompileOptions, NATIVES_CODE,
false);
-
- // Silently ignore stack overflows during compilation.
- if (function_info.is_null()) {
- DCHECK(isolate->has_pending_exception());
- isolate->clear_pending_exception();
- return false;
- }
+ if (function_info.is_null()) return false;
// Execute the shared function in the debugger context.
Handle<JSFunction> function =
Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index
0b2478691bbf347aca4eb847998986ab854cedd4..38c8ed8fed921f3ee8bfdcc8f70e9e5c52e871a5
100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -849,9 +849,13 @@ Object* Isolate::StackOverflow() {
// constructor. Instead, we copy the pre-constructed boilerplate and
// attach the stack trace as a hidden property.
Handle<String> key = factory()->stack_overflow_string();
- Handle<JSObject> boilerplate = Handle<JSObject>::cast(
- Object::GetProperty(js_builtins_object(), key).ToHandleChecked());
- Handle<JSObject> exception = factory()->CopyJSObject(boilerplate);
+ Handle<Object> boilerplate =
+ Object::GetProperty(js_builtins_object(), key).ToHandleChecked();
+ if (boilerplate->IsUndefined()) {
+ return Throw(heap()->undefined_value(), nullptr);
+ }
+ Handle<JSObject> exception =
+ factory()->CopyJSObject(Handle<JSObject>::cast(boilerplate));
Throw(*exception, nullptr);
CaptureAndSetSimpleStackTrace(exception, factory()->undefined_value());
Index: src/runtime/runtime-debug.cc
diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index
d071cbf7f9ddfb721c417899d422ecf83c6e2520..91b6ee5db2d1e78742424458a5c9ab681f2b723a
100644
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -2748,6 +2748,10 @@ RUNTIME_FUNCTION(Runtime_DebugGetLoadedScripts) {
Handle<FixedArray> instances;
{
DebugScope debug_scope(isolate->debug());
+ if (debug_scope.failed()) {
+ DCHECK(isolate->has_pending_exception());
+ return isolate->heap()->exception();
+ }
// Fill the script objects.
instances = isolate->debug()->GetLoadedScripts();
}
Index: test/mjsunit/mjsunit.status
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index
3b6bfc5bf7660a1d9cce5c8485b7d2c8d047ee0f..7dc0aea2d4acdbac2939c943960a1b2817a14431
100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -178,6 +178,9 @@
# Issue 488: this test sometimes times out.
'array-constructor': [PASS, TIMEOUT],
+ # Run only on fast architectures, contains no architecture dependent
code.
+ 'regress/regress-crbug-491062': [PASS, ['arch != ia32 and arch != x64',
SKIP], NO_VARIANTS],
+
# Very slow on ARM and MIPS, contains no architecture dependent code.
'unicode-case-overoptimization': [PASS, NO_VARIANTS, ['arch == arm or
arch == android_arm or arch == android_arm64 or arch == mipsel or arch ==
mips64el or arch == mips', TIMEOUT]],
'regress/regress-3976': [PASS, NO_VARIANTS, ['arch == arm or arch ==
android_arm or arch == android_arm64 or arch == mipsel or arch == mips64el
or arch == mips', SKIP]],
Index: test/mjsunit/regress/regress-crbug-491062.js
diff --git a/test/mjsunit/harmony/regress/regress-crbug-465671.js
b/test/mjsunit/regress/regress-crbug-491062.js
similarity index 56%
copy from test/mjsunit/harmony/regress/regress-crbug-465671.js
copy to test/mjsunit/regress/regress-crbug-491062.js
index
24f4d054755bc241d87ad774787d595aad6c0657..e16f85b1eb87ac991fdcb65c7cc7527de807608a
100644
--- a/test/mjsunit/harmony/regress/regress-crbug-465671.js
+++ b/test/mjsunit/regress/regress-crbug-491062.js
@@ -2,15 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --harmony-arrow-functions
+// Flags: --allow-natives-syntax --stack-limit=32
-// This used to trigger crash because of an unhandled stack overflow.
+function g() {}
+
+var count = 0;
function f() {
- var a = [10];
try {
f();
} catch(e) {
- a.map(v => v + 1);
+ print(e.stack);
+ }
+ if (count < 50) {
+ count++;
+ %DebugGetLoadedScripts();
}
}
f();
+g();
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
