Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/44bed6a85e0775a77954ac17c0e551b55e43b32b (TF stubs out of ArrayIndexOf and ArrayInclude builtins).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

Verified the crash in the current ToT V8 (2028d1d8b1d2ce10aa27e7c73ea6dc398a0d7178).
Will try to understand and fix it.

I further squashed the repro case to:

for (var X = 0; X < 7452; ++X)
  [(Z) => 0, [, 2147483648 ][0]].includes('', -0);

The interesting (and necessary) part is the '-' in front of 0, and the exact values 7452 and 2147483648 -- using smaller ones (on my machine) avoided the crash.

Given that negative fromIndex is defined as the offset from the end towards the front, perhaps the '-0' results in some out-of-bounds access which manifests itself as soon as the surrounding memory has some strange values (resulting from the many cycles somehow)?

Still not very clear on this.

Hi Sigurd,
Do you think you could help me understand this problem?

The problem:

for (var X = 0; X < 7452; ++X)
  [(Z) => 0, [, 2147483648 ][0]].includes('', -0);

results in a NULL derefence (missing opcode?) during load
elimination: see the stack trace in
https://clusterfuzz.com/testcase-detail/6198506307715072

The particular numbers in the snippet seem to matter,
with lower numbers or with just "0" instead of "-0"
all works fine.

If JSCallReducer::ReduceArrayIndexOfIncludes becomes
just "return NoChange();" then all works fine as well.

Inspired by the "-" in "-0", I was trying to guard the
part of JSCallReducer::ReduceArrayIndexOfIncludes which
computes the new_from_index against potentially using
length + (-0) as the start indes. I injected
            graph()->NewNode(
                simplified()->NumberMin(),
                graph()->NewNode(simplified()->NumberAdd(), length, from_index),
                graph()->NewNode(simplified()->NumberAdd(), length,
                                 jsgraph()->MinusOneConstant())),
so that the whole expression became
    new_from_index = graph()->NewNode(
        common()->Select(MachineRepresentation::kTagged, BranchHint::kFalse),
        graph()->NewNode(simplified()->NumberLessThan(), from_index,
                         jsgraph()->ZeroConstant()),
        graph()->NewNode(
            simplified()->NumberMax(),
            graph()->NewNode(
                simplified()->NumberMin(),
                graph()->NewNode(simplified()->NumberAdd(), length, from_index),
                graph()->NewNode(simplified()->NumberAdd(), length,
                                 jsgraph()->MinusOneConstant())),
            jsgraph()->ZeroConstant()),
        from_index);
but to no avail. It seems that NumberLessThan() does return false
for "-0" and ZeroConstant().

And while this clearly breaks somewhere in the optimisation code,
just calling the includes() with %OptimizeFunctionOnNextCall alone
does not reproduce the crash (see the test and comments in
https://crrev.com/c/1340256).

Does this ring a bell with you? Do you understand what part of the
optimization process the stack trace corresponds to and what the
missing / null opcode can mean?

This is caused by an interaction between load elimination and dead code elimination. I'm looking into it.

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

commit b28637b4fe8037bbabcf8ff59eebb93734dcd8fc
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Wed Nov 21 15:23:01 2018

[turbofan] Apply duct-tape to load elimination

Load elimination is running together with to dead code elimination, the
latter of which might eliminate allocations (in particular FinishRegion
nodes). These are treated as alias nodes by load elimination, and load
elimination does not immediatelly learn that a node has been disconnected.
This causes load elimination to access the inputs of dead code eliminated
nodes while resolving renames, which causes nullptr dereferences.

This CL modifies load elimination to not resolve to a nullptr alias but
simply stop before that.

Change-Id: If4cef061c7c0e25f353727c9e27f790439b0beb5
Bug:  chromium:906406 
Reviewed-on: https://chromium-review.googlesource.com/c/1346491
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57688}
[modify] https://crrev.com/b28637b4fe8037bbabcf8ff59eebb93734dcd8fc/src/compiler/load-elimination.cc
[add] https://crrev.com/b28637b4fe8037bbabcf8ff59eebb93734dcd8fc/test/mjsunit/regress/regress-906406.js

Thanks a lot, Sigurd!
Given that the repro case is now added as a regression test, I'm marking this as fixed and assigning to you for credit.

Thanks!

ClusterFuzz has detected this issue as fixed in range 57687:57688.

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

Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  opcode
  IsRename
  ResolveRenames
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=51728:51729
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=57687:57688

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

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 6198506307715072 is verified as fixed, so closing issue as verified.

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