New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 724858 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue v8:6430



Sign in to add a comment

Page crashes when stepping through 'return' within a 'with' scope

Reported by rocca.jo...@gmail.com, May 21 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.96 Safari/537.36

Steps to reproduce the problem:
1. Open new browser tab and then open devtools console
2. Paste this in and execute it:

```
var prox = new Proxy({}, {
    has(target, property) { return true; },
});
function thing(input) {
	debugger;
	with(prox) {
		return input;
	}
}
console.log( thing(10) );
```

3. Execution will pause at debugger statement
4. Step through the code line-by-line
5. It will crash at the return statement

Note that this doesn't occur if you don't step through line-by-line. Without the debugger statement it returns `undefined` as expected.

If the developers would like some context for why on earth someone would use the above code, this may help: http://stackoverflow.com/questions/44093805/way-to-intercept-withs-property-lookup-check  - it doesn't explain why per se, but shows some motivation.

What is the expected behavior?
Return undefined, don't crash

What went wrong?
DevTools disconnected, page crashed

Did this work before? N/A 

Chrome version: 58.0.3029.96  Channel: stable
OS Version: 16.04
Flash Version: 

This is my first dev tools bug report and I have only tested this on my machine with my version of chrome.
 
devtools_crash.png
63.5 KB View Download
Labels: Needs-Triage-M58
Cc: brajkumar@chromium.org
Labels: -Type-Bug -Pri-2 hasbisect-per-revision M-59 OS-Mac OS-Windows Pri-1 Type-Bug-Regression
Owner: jgruber@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce on Windows-10, Ubuntu 14.04 and Mac OS 10.12 using chrome stable M58-58.0.3029.110.

Bisect Information:
=====================
Good build: 57.0.2929.0
Bad Build : 57.0.2931.0

Change Log URL: https://chromium.googlesource.com/chromium/src/+/0602448da6e71e82925a14517760dd0314ad94c2

From the above change log suspecting below change
Review URL: https://codereview.chromium.org/2519773003

jgruber@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks!
Note:
--------
You are probably looking for a change made after 434189 (known good), but no later than 434190 (first known bad).

Status: Started (was: Assigned)
Cc: kozyatinskiy@chromium.org yangguo@chromium.org
Labels: -Needs-Triage-M58
I can repro on 58.0.3029.110 and 60.0.3100.0.
Reduced repro:

```
function f(x) {
  debugger;          // B0 StepNext
  with ({}) {        // B1 StepNext
    return x;        // B2 StepNext
  }
}                    // B4 Continue

f(42);
```

The crash happens when stepping from the return statement within the with scope. Stacktrace:

#
# Fatal error in ../../src/debug/debug-scopes.cc, line 256
# Check failed: context_->IsFunctionContext() || !scope_info->HasContext().
#

#0  v8::base::OS::Abort () at ../../src/base/platform/platform-posix.cc:261
#1  0x00007ffff7fb8f47 in V8_Fatal (file=0x7ffff7643362 "../../src/debug/debug-scopes.cc", line=256, 
    format=0x7ffff75f523f "Check failed: %s.") at ../../src/base/logging.cc:74
#2  0x00007ffff6cc1807 in v8::internal::ScopeIterator::Type (this=0x7fffffffaf38) at ../../src/debug/debug-scopes.cc:256
#3  0x00007ffff6cc12c5 in v8::internal::ScopeIterator::MaterializeScopeDetails (this=0x7fffffffaf38)
    at ../../src/debug/debug-scopes.cc:178
#4  0x00007ffff71bfda7 in v8::internal::__RT_impl_Runtime_GetAllScopesDetails (args=..., isolate=0x555555600f10)
    at ../../src/runtime/runtime-debug.cc:861
#5  0x00007ffff71bf790 in v8::internal::Runtime_GetAllScopesDetails (args_length=4, args_object=0x7fffffffb530, isolate=0x555555600f10)
    at ../../src/runtime/runtime-debug.cc:826
[...]

==== JS stack trace =========================================

Security context: 0x2c55957e8f81 <JSObject>#0#
    2: allScopes [native mirrors.js:1] [bytecode=0x2c55957ec279 offset=50](this=0x736278aa3e1 <FrameMirror map = 0x2947f98a3fc1>#1#,at=0x22fc2002421 <false>)
    4: _frameMirrorToJSCallFrame [0x22fc2002311 <undefined>:33] [bytecode=0x2c55957a4b51 offset=184](this=0x1ac31a45b1f1 <Object map = 0x2947f98a8ed1>#2#,frameMirror=0x736278aa3e1 <FrameMirror map = 0x2947f98a3fc1>#1#)
    6: currentCallFrames [0x22fc2002311 <undefined>:20] [bytecode=0x2c55957bd911 offset=68](this=0x1ac31a45b1f1 <Object map = 0x2947f98a8ed1>#2#,execState=0x736278aa319 <ExecutionState map = 0x2947f98a45f1>#3#,limit=0)
Security context: 0x2c55957aa051 <JSObject>#4#
   11: f [test/debugger/regress/regress-724858.js:30] [bytecode=0x1ac31a435c21 offset=30](this=0x1ac31a422751 <JSGlobal Object>#5#,x=42)
   13: /* anonymous */ [test/debugger/regress/regress-724858.js:32] [bytecode=0x2c55957bf9c1 offset=86](this=0x1ac31a422751 <JSGlobal Object>#5#)
Blocking: v8:6430
Bisection points to

5f5300a [compiler] Ensure code unsupported by Crankshaft goes to Ignition. by rmcilroy ยท 6 months ago

Which just makes with statements go through ignition instead of crankshaft.
Cc: jgruber@chromium.org rmcilroy@chromium.org
Owner: mstarzinger@chromium.org
Status: Assigned (was: Started)
Summary: Page crashes when stepping through 'return' within a 'with' scope (was: Page crashes when stepping through code that has `with` statement and Proxy)
Assigning to mstarzinger@ as discussed offline.

The problem is similar to  http://crbug.com/v8/5610 : 
* the function context is popped before breaking at the return statement
* in debug-scopes.cc we reconstruct the scope chain, and finally
* fail since we expect the function context to be there but it's been popped.

The fix for v8:5610 does not apply here because of the ContextScope introduced by BytecodeGenerator::VisitInScope.

Attaching a d8 test - run with:

$ d8 --enable-inspector --allow-natives-syntax test/mjsunit/mjsunit.js test/debugger/test-api.js test/debugger/regress/regress-724858.js

regress-724858.js
932 bytes View Download
Status: Started (was: Assigned)
I have a fix in flight.
Project Member

Comment 11 by bugdroid1@chromium.org, May 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/02fee655b3637eadbf226cc6d4c922e3c2c41489

commit 02fee655b3637eadbf226cc6d4c922e3c2c41489
Author: Michael Starzinger <mstarzinger@chromium.org>
Date: Wed May 24 11:58:47 2017

[interpreter] Avoid redundant {PopContext} instructions.

This avoids emitting redundant {PopContext} bytecode instructions when
non-local control-flow leaves the method body. It also folds multiple
such {PopContext} instructions into one, in case several scoping levels
are crossed at one. Only the expected context of the target of a local
control-flow transfer matters.

R=rmcilroy@chromium.org
TEST=debugger/regress/regress-crbug-724858
BUG= chromium:724858 

Change-Id: Id4a47ae9fea25e75ae1af13619720b16a3975edf
Reviewed-on: https://chromium-review.googlesource.com/512545
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45507}
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/src/interpreter/bytecode-generator.cc
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/BasicLoops.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/ContextVariables.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/ForOfLoop.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/Generators.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/StandardForLoop.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/TryCatch.golden
[modify] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/cctest/interpreter/bytecode_expectations/WithStatement.golden
[add] https://crrev.com/02fee655b3637eadbf226cc6d4c922e3c2c41489/test/debugger/regress/regress-crbug-724858.js

Status: Fixed (was: Started)

Sign in to add a comment