New issue
Advanced search Search tips

Issue 592353 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

scope_contains_with_ bit not set consistently for arrow functions

Project Member Reported by ClusterFuzz, Mar 7 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6332930432958464

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_mipsel_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  old_target->kind() == new_target->kind() in src/objects-debug.cc
  
Regressed: V8: r32305:32306

Minimized Testcase (0.17 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv971TJULzHcRLNyFyW1Pfxc7SnNcRTWgoyRYU8G6HQAeDCN-Q1D8PegFGkxrH4GOVpFSFzfxJtSYEXF6ImsN1c5L4G-tKLAZ5T3nsvVWnEuWGC2yAArEXAxkuL-1gjkeN_Fhw-4aNiwU9VwA54R6d0szqUVS6A
with ({x: 'outer'}) {
  label: for (var __v_1 = 0; __v_1 < 10; ++__v_1) {
  }
}
__v_7 = ({x = {}}, {y = [], n = []}) => {
  };
%OptimizeFunctionOnNextCall(__v_7);
__v_7();


Filer: machenbach

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Owner: adamk@chromium.org
Status: Assigned (was: Available)
PTAL

Comment 2 by adamk@chromium.org, Mar 7 2016

Status: Started (was: Assigned)
Nothing ASAN or mipsel related about this, the trick is --no-lazy. Investigating.

Comment 3 by adamk@chromium.org, Mar 7 2016

Simplified test case:

with ({}) {}       
f = ({x}) => { };
%OptimizeFunctionOnNextCall(f);
f();

Comment 4 by adamk@chromium.org, Mar 7 2016

This is the same bug as  http://crbug.com/572589 , except for 'with' instead of 'eval'. The fix isn't going to be quite as easy, though, as we don't have the 'contains with' bit available on the ScopeInfo.

Comment 5 by adamk@chromium.org, Mar 14 2016

Summary: scope_contains_with_ bit not set consistently for arrow functions (was: old_target->kind() == new_target->kind() in src/objects-debug.cc)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83e1818175112b9a47d4403ade2357ecc1d43329

commit 83e1818175112b9a47d4403ade2357ecc1d43329
Author: Adam Klein <adamk@chromium.org>
Date: Tue Mar 15 20:54:18 2016

Mark inspector/sources/debugger-ui/function-details.html as NeedsManualRebaseline

This test's output will change after v8 CL https://codereview.chromium.org/1804783002/
rolls into Chromium. It slightly changes the way Scopes are exposed for
functions containing 'with' statements. Will rebaseline after the v8 roll.

BUG= 592353 
TBR=yangguo@chromium.org

Review URL: https://codereview.chromium.org/1797423003 .

Cr-Commit-Position: refs/heads/master@{#381307}

[modify] https://crrev.com/83e1818175112b9a47d4403ade2357ecc1d43329/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 15 2016

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

commit 108efd7f544e540cc741fdc3b74fcda650bf0728
Author: adamk <adamk@chromium.org>
Date: Tue Mar 15 22:41:13 2016

Remove Scope::scope_contains_with_ bit

This part of Scope has existed since V8's initial check in, but from what
I can tell it's not required to implement "with". The only tests that
depend upon it are tests of the debugger and the Scope mirrors, but the
resulting test behavior after removing the bit still seems perfectly
reasonable to me. In fact, with the included fix for scope name collection,
the scope mirror is actually improved with this change.

As a bi-product, this fixes the attached bug, about the contains_with
bit having inconsistent values in some arrow function compilation
scenarios.

BUG= chromium:592353 
LOG=n
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1804783002

Cr-Commit-Position: refs/heads/master@{#34802}

[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/src/ast/scopes.cc
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/src/ast/scopes.h
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/src/debug/debug-scopes.cc
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/src/parsing/parser.cc
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/test/cctest/interpreter/bytecode_expectations/WithStatement.golden
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/test/mjsunit/debug-evaluate-locals.js
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/test/mjsunit/debug-function-scopes.js
[modify] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/test/mjsunit/debug-scopes.js
[add] https://crrev.com/108efd7f544e540cc741fdc3b74fcda650bf0728/test/mjsunit/regress/regress-592353.js

Comment 8 by adamk@chromium.org, Mar 16 2016

Status: Fixed (was: Started)
Project Member

Comment 9 by ClusterFuzz, Mar 16 2016

ClusterFuzz has detected this issue as fixed in range 34801:34802.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6332930432958464

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_mipsel_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  old_target->kind() == new_target->kind() in src/objects-debug.cc
  
Regressed: V8: r32305:32306
Fixed: V8: r34801:34802

Minimized Testcase (0.17 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv971TJULzHcRLNyFyW1Pfxc7SnNcRTWgoyRYU8G6HQAeDCN-Q1D8PegFGkxrH4GOVpFSFzfxJtSYEXF6ImsN1c5L4G-tKLAZ5T3nsvVWnEuWGC2yAArEXAxkuL-1gjkeN_Fhw-4aNiwU9VwA54R6d0szqUVS6A
with ({x: 'outer'}) {
  label: for (var __v_1 = 0; __v_1 < 10; ++__v_1) {
  }
}
__v_7 = ({x = {}}, {y = [], n = []}) => {
  };
%OptimizeFunctionOnNextCall(__v_7);
__v_7();


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab13ce20b68732da249c769de9409cf82b4511f7

commit ab13ce20b68732da249c769de9409cf82b4511f7
Author: adamk <adamk@chromium.org>
Date: Fri Mar 18 23:26:28 2016

Rebaseline, fix, or remove various tests dealing with V8 behavior

Most tests were simply rebaselined. The Date test needed a slight
tweak to go from "failing" to "passing" (along with a rebaseline).
As for the 'const' tests, they were all testing "legacy" const,
which stopped shipping in M49 (in preference to ES2015 const),
so deletion seemed like the best remedy.

BUG= 535408 ,  570391 ,  574196 ,  575917 ,  576574 ,  592353 

Review URL: https://codereview.chromium.org/1815833002

Cr-Commit-Position: refs/heads/master@{#382133}

[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/const-expected.txt
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/const.html
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/fast/js/date-proto-generic-invocation-expected.txt
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/fast/js/function-bind-expected.txt
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/inc-const-valueOf-expected.txt
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/inc-const-valueOf.html
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/kde/const-expected.txt
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/kde/const.html
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/kde/resources/const.js
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/resources/const.js
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/fast/js/script-tests/date-proto-generic-invocation.js
[delete] https://crrev.com/937dcac73f5657d31fde4b79ac3ce8c91c68ea67/third_party/WebKit/LayoutTests/fast/js/script-tests/inc-const-valueOf.js
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/inspector/console/console-big-array-expected.txt
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/inspector/console/console-dir-expected.txt
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/function-details-expected.txt
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/inspector/sources/debugger/promise-events-expected.txt
[modify] https://crrev.com/ab13ce20b68732da249c769de9409cf82b4511f7/third_party/WebKit/LayoutTests/inspector/sources/debugger/properties-special-expected.txt

Project Member

Comment 11 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment