Assigning ulan@ because ishell@ is on vacation.

Re-assigning to the current CF sheriff.

Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

Testcase 5050837557837824 is a top crash on ClusterFuzz for linux platform. Please prioritize fixing this crash.

Marking this crash as a Beta release blocker.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

rossberg: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Smaller repro:

-------------------------
class C {}
function constructor() {
 try { Reflect.construct(C, arguments, true); } catch(e) {}
}
function f() {
 new constructor; 
}

f(); 
%OptimizeFunctionOnNextCall(f); 
f(); 
-------------------------

Interestingly, the crash goes away when you remove the try/catch.

I've bisected the crash of the smaller repro to https://chromium-review.googlesource.com/615173. Leszek, please have a look.

Looking at --trace-turbo with --no-analyze-environment-liveness, it looks like the graphs differ after inlining, with the JSConstruct turning into JSConstructWithArrayLike if liveness is disabled.

Interestingly, if I break the Reflect.construct lowering, this stops failing:

-------------------------
class C {}
function constructor(x) {
 try { x(C, arguments, true); } catch(e) {}
}
function f(x) {
 new constructor(x); 
}

f(Reflect.construct);
f(undefined);
%OptimizeFunctionOnNextCall(f); 
f(Reflect.construct);
-------------------------

I think this is a bug in inlining/JSConstruct lowering, not in liveness, and that liveness is somehow exposing it. Reassigning to Jaro, feel free to assign back to me if it does turn out to be liveness (or similar) after all.

M63 is branching on this Thursday (10/12) and M63 beta promotion is coming very soon. Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP. Thank you.

The bug is in {JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread} where we lower directly to a {JSCreate} without checking whether the new target is constructible or not.

On a related note, the following two snippets also report a different error message. Not really a correctness issue but also exposes the "missing" explicit callability/constructability checks on the target (vs new target).

---

class C {}
function constructor() {
 try { Reflect.apply(1, 2, arguments); } catch(e) { print(e); }
}
function f() {
 new constructor; 
}
f();
%OptimizeFunctionOnNextCall(f); 
f();

---

class C {}
function constructor() {
 try { Reflect.construct(1, arguments); } catch(e) { print(e); }
}
function f() {
 new constructor; 
}

f();
%OptimizeFunctionOnNextCall(f); 
f();

---

I think it makes sense to block the 63 stable release for this but not the beta.

Clarification for #18: The CL in question simply flushed out the bug. The problem is in there a longer time already thus this is not a regression.

I am cooking up a fix ...

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

commit afd2f580c536343bf74d690a4bdd049cb6309113
Author: Michael Starzinger <mstarzinger@chromium.org>
Date: Fri Oct 13 13:13:12 2017

[turbofan] Fix new.target check in Reflect.construct.

This adds and explicit check for the constructability of the new.target
value in the lowering of {JSCall} nodes known to call Reflect.construct.
The {JSConstruct} operator does not perform this check and relies on the
implicit validity of new.target in all other use cases.

R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-768080
BUG= chromium:768080 

Change-Id: I7c1921e787bae64ba83de3eb08aa00fc5523e251
Reviewed-on: https://chromium-review.googlesource.com/718100
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48543}
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/opcodes.h
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/simplified-operator.cc
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/simplified-operator.h
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/typer.cc
[modify] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/src/compiler/verifier.cc
[add] https://crrev.com/afd2f580c536343bf74d690a4bdd049cb6309113/test/mjsunit/regress/regress-crbug-768080.js

ClusterFuzz has detected this issue as fixed in range 48542:48543.

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

Fuzzer: inferno_js_fuzzer
Job Type: linux_cfi_d8
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  args[1]->IsJSReceiver() in runtime-object.cc
  
Sanitizer: cfi (CFI)

Fixed: https://clusterfuzz.com/revisions?job=linux_cfi_d8&range=48542:48543

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5050837557837824

See https://github.com/google/clusterfuzz-tools for more information.

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

ClusterFuzz testcase 5050837557837824 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

awhalley@: This also affects Chrome 62. We would rather not merge this back to 62 because the fix will be quite involved. WDYT?

hablich@ - with M62 stable being released today, and this being an internally reported bug, I'm OK holding on until 53.  Thanks!

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

commit 2d80e84153db1774357b05544afefd68d03deb3b
Author: Michael Starzinger <mstarzinger@chromium.org>
Date: Wed Oct 18 12:02:44 2017

[turbofan] Properly restrict {JSCreate} to constructors.

This makes sure that the lowering of {JSCreate} operator during create
lowering is only applied to operations where both target and new.target
are known to be constructors.

R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-768080
BUG= chromium:774780 , chromium:768080 

Change-Id: I55a582a3453bba7e14655b594b7714a3940eeaae
Reviewed-on: https://chromium-review.googlesource.com/725332
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48680}
[modify] https://crrev.com/2d80e84153db1774357b05544afefd68d03deb3b/src/compiler/js-create-lowering.cc
[modify] https://crrev.com/2d80e84153db1774357b05544afefd68d03deb3b/test/mjsunit/regress/regress-crbug-768080.js

This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

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

+awhalley@ (Security TPM) for M63 merge review

govind@ - Good for 63

Approving merge to M63 branch 3239 based on comment #32. 

Note: There are  2 CLs at #22 and #28 after branch. Both need to be merged?

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

commit d12f799c23219ed318309a56efb584c933b7948b
Author: Michael Starzinger <mstarzinger@google.com>
Date: Fri Nov 03 09:38:29 2017

Merged: Squashed multiple commits.

Merged: [turbofan] Fix new.target check in Reflect.construct.
Revision: afd2f580c536343bf74d690a4bdd049cb6309113

Merged: [turbofan] Properly restrict {JSCreate} to constructors.
Revision: 2d80e84153db1774357b05544afefd68d03deb3b

R=bmeurer@chromium.org
BUG= chromium:768080 , chromium:774780 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Change-Id: I0d6aee91294aa8d2a187a908a4a9b517126ab1ea
Reviewed-on: https://chromium-review.googlesource.com/753088
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#50}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/js-create-lowering.cc
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/opcodes.h
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/simplified-operator.cc
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/simplified-operator.h
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/typer.cc
[modify] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/src/compiler/verifier.cc
[add] https://crrev.com/d12f799c23219ed318309a56efb584c933b7948b/test/mjsunit/regress/regress-crbug-768080.js

Re comment #33: Correct, both changes (#22 and #28) need to be merged. I have squashed them into one singe merge.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Per comment #34 and #35, this is already merged to M63.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

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