New issue
Advanced search Search tips

Issue 706761 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

V8 correctness failure in configs: x64,ignition_turbo_opt:x64,ignition_turbo_opt_eager

Project Member Reported by ClusterFuzz, Mar 30 2017

Issue description

Owner: machenb...@chromium.org
Status: Assigned (was: Untriaged)
Marja PTAL. Started with https://chromium.googlesource.com/v8/v8/+/bc39a5148a3824ea948fd7725674945ca0b1c56a
Owner: marja@chromium.org
Now also assigning :)

Comment 3 by marja@chromium.org, Mar 31 2017

Omg this is really bad. I might need to revert the fix.

As a side bug, I also noticed that the added test just crashes with --no-lazy. That's even worse!

Comment 4 by marja@chromium.org, Mar 31 2017

Found a related bug in Ignition which is independent of my fix (already fails before the fix).

https://bugs.chromium.org/p/v8/issues/detail?id=6182


Project Member

Comment 5 by ClusterFuzz, Apr 1 2017

ClusterFuzz has detected this issue as fixed in range 44307:44308.

Detailed report: https://clusterfuzz.com/testcase?key=5347427134734336

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition_turbo_opt:x64,ignition_turbo_opt_eager
  sources: eed
  
Sanitizer: address (ASAN)

Regressed: V8: 44176:44177
Fixed: V8: 44307:44308

Reproducer Testcase: https://clusterfuzz.com/download/AMIfv95pKLWnQYvTll7CxrwdnKXFhS0MWTiiK8MFz9GCvDw6v6IugLDUfsPPOovlWU32u1D9sgvjEVWnsejTwscDUAG1_qo4_Pz7HcyYwE6Gh5mC7BSnNUCWJguMaqRrISH2WeeJW9m3eWyH5hmp9mSh0wTjWZSvVPKXpY7niddapJC0_r3lq9SMxcXcNkSAJq5KWmQHwMPnM9wkq-VhDsloxGbJi-ATDLfgNYG3mCAkt2_WD4M3mClvS6agwylAGWc81gKCpZ__B7nUBmJwvD4LwoMKgNYRofqVRt5CM-TQJ_JHrrA08ZEL5uES7cI1e7Pg3QAkfWnfIdJBhkM61xNy-GWjy0h45KSYjNEi_FwUTHiBaPb_Y9ovWKoKHkVuoKcsXYcqaB27SZ6HOsovq-tsszBPB4KOjw?testcase_id=5347427134734336


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 6 by ClusterFuzz, Apr 1 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5347427134734336 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Cc: rmcilroy@chromium.org
Labels: ClusterFuzz-Wrong
Status: Assigned (was: Verified)
Since https://chromium.googlesource.com/v8/v8/+/a4c6126a836bae6af70220ce90386d322885e958 wasn't marked with this bug, please manually check if it really fixes also this issue. CC author.
Ah, and consider adding a regression test.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 4 2017

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

commit 5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f
Author: Caitlin Potter <caitp@igalia.com>
Date: Tue Apr 04 20:35:03 2017

[parser] don't rewrite destructuring assignments in params for lazy top level arrow functions

Remove destructuring assignments (parsed during arrow function formal
parameters) from queue for rewriting if parsing a lazy top-level arrow function.

Built ontop of https://chromium-review.googlesource.com/c/464769/

BUG= chromium:706234 ,  chromium:706761 ,  v8:6182 
R=marja@chromium.org, adamk@chromium.org, vogelheim@chromium.org

Change-Id: Ib35196b907350d1d78e4c3fcbf4cc971bf200948
Reviewed-on: https://chromium-review.googlesource.com/465415
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44393}
[modify] https://crrev.com/5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f/src/ast/scopes.cc
[modify] https://crrev.com/5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f/src/parsing/parser-base.h
[modify] https://crrev.com/5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f/src/parsing/parser.cc
[add] https://crrev.com/5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f/test/mjsunit/regress/regress-706234-2.js
[add] https://crrev.com/5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f/test/mjsunit/regress/regress-706234.js

Is this fixed now and do we have all regression tests necessary?
Status: Fixed (was: Assigned)
Afaik Caitlins fix above is enough.
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 11 2017

Labels: merge-merged-5.8
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/01d19f706ceb293d0449d80d0db7389cc509aea0

commit 01d19f706ceb293d0449d80d0db7389cc509aea0
Author: Marja Hölttä <marja@chromium.org>
Date: Tue Apr 11 08:19:48 2017

Merged: Squashed multiple commits.

Merged: [parser] Fix crash when lazy arrow func params contain destructuring assignments.
Revision: bc39a5148a3824ea948fd7725674945ca0b1c56a

Merged: [parser] don't rewrite destructuring assignments in params for lazy top level arrow functions
Revision: 5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f

BUG= chromium:704811 , chromium:706234 , chromium:706761 , v8:6182 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Change-Id: If5c04c3b9f6ac9c6879052b6a34446f895624200
Reviewed-on: https://chromium-review.googlesource.com/474746
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/branch-heads/5.8@{#58}
Cr-Branched-From: eda659cc5e307f20ac1ad542ba12ab32eaf4c7ef-refs/heads/5.8.283@{#1}
Cr-Branched-From: 4310cd02d2160b1457baed81a2f40063eb264a21-refs/heads/master@{#43429}
[modify] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/src/ast/scopes.cc
[modify] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/src/parsing/parser-base.h
[modify] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/src/parsing/parser.cc
[add] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/test/mjsunit/regress/regress-704811.js
[add] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/test/mjsunit/regress/regress-706234-2.js
[add] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/test/mjsunit/regress/regress-706234.js

Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment