1034280 - Differential Testing: inconsistent number of "Warning! Tried to access unreadable value allocation" with filterPar

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

x = []
try {
    var u
} catch (e) {}
x[12] = u
x.filterPar(function(e, i, s) {
    if (i % 93 == 11) {
        var y = 2 / i
        r = u
        var r = y * 1
    }
})

$ ./js-dbgDisabled-opt-64-prof-dm-ts-darwin-2b018836f449 --fuzzing-safe --ion-offthread-compile=off 1786.js

$ ./js-dbgDisabled-opt-64-prof-dm-ts-darwin-2b018836f449 --fuzzing-safe --ion-offthread-compile=off --ion-eager 1786.js
Warning! Tried to access unreadable value allocation (possible f.arguments).
Warning! Tried to access unreadable value allocation (possible f.arguments).
Warning! Tried to access unreadable value allocation (possible f.arguments).

(Tested this on 64-bit Mac js opt threadsafe deterministic shell off m-i rev 2b018836f449)

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-inbound/js/src/configure --target=x86_64-apple-darwin12.5.0 --disable-debug --enable-optimize --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>


autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/cd7125c33385
user:        Shu-yu Guo
date:        Fri Jun 20 18:39:14 2014 -0700
summary:     Bug 1019304 - Part 2: Overhaul PJS bailout mechanism to be like the normal bailout mechanism. (r=nmatsakis)

(not sure if this bisection is entirely true)

Shu-yu, since filterPar seems to be related (along with PJS), is bug 1019304 likely related?

Comment 1

[Shu-yu Guo [:shu]]

What is the differential output here? The warnings? Those don't matter for correctness.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

shu|europe: gkw: talk to nbp about those warnings
shu|europe: gkw: they're unconditional fprintf(stderr, ...) right now
shu|europe: gkw: probably should be under a flag, i agree
gkw: shu|europe: ok, I'll poke

Shifting needinfo? to :nbp. :)

Comment 3

[Shu-yu Guo [:shu]]

I guess the differential fuzzer cares about stderr because of thrown exceptions?

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Yep, we look out for differences in stderr and stdout.

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Shu-yu Guo [:shu] from comment #1)
> What is the differential output here? The warnings? Those don't matter for
> correctness.

Usually they do, as these warning are here saying that we are trying to read a value allocation, without the support for reading it.  This can happen if there is no MachineState / RecoverInstruction vector during the bailout or if there is a bad RValueAllocation (s-s critical if so).

In any case what this warning is saying is critical for Sequential executions, as we change the semantic and get "undefined" instead of the expected value.  But in PJS, if these values are not changing the behavior of JS, then PJS should rematerialized the frame silently when it is calling maybeRead, and with a warning / error when it is doing that from the debugger.

Comment 6

[Shu-yu Guo [:shu]]

(In reply to Nicolas B. Pierron [:nbp] from comment #5)
> (In reply to Shu-yu Guo [:shu] from comment #1)
> > What is the differential output here? The warnings? Those don't matter for
> > correctness.
> 
> Usually they do, as these warning are here saying that we are trying to read
> a value allocation, without the support for reading it.  This can happen if
> there is no MachineState / RecoverInstruction vector during the bailout or
> if there is a bad RValueAllocation (s-s critical if so).
> 
> In any case what this warning is saying is critical for Sequential
> executions, as we change the semantic and get "undefined" instead of the
> expected value.  But in PJS, if these values are not changing the behavior
> of JS, then PJS should rematerialized the frame silently when it is calling
> maybeRead, and with a warning / error when it is doing that from the
> debugger.

In that case, why not also assert when silentFailure = true instead of false?

Comment 7

[Shu-yu Guo [:shu]]

Attachment: Don't report warnings for recover instructions when snapshotting frames for PJS bailout warnings.

Comment 8

[Nicolas B. Pierron [:nbp]]

(In reply to Shu-yu Guo [:shu] from comment #6)
> (In reply to Nicolas B. Pierron [:nbp] from comment #5)
> > (In reply to Shu-yu Guo [:shu] from comment #1)
> > > What is the differential output here? The warnings? Those don't matter for
> > > correctness.
> > 
> > Usually they do, as these warning are here saying that we are trying to read
> > a value allocation, without the support for reading it.  This can happen if
> > there is no MachineState / RecoverInstruction vector during the bailout or
> > if there is a bad RValueAllocation (s-s critical if so).
> > 
> > In any case what this warning is saying is critical for Sequential
> > executions, as we change the semantic and get "undefined" instead of the
> > expected value.  But in PJS, if these values are not changing the behavior
> > of JS, then PJS should rematerialized the frame silently when it is calling
> > maybeRead, and with a warning / error when it is doing that from the
> > debugger.
> 
> In that case, why not also assert when silentFailure = true instead of false?

I guess we could do so now.

Comment 9

[Shu-yu Guo [:shu]]

Attachment: Don't report warnings for recover instructions when snapshotting frames for PJS bailout warnings.

Comment 10

[Shu-yu Guo [:shu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82b0cc883f41

Comment 11

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This landed:

https://hg.mozilla.org/mozilla-central/rev/82b0cc883f41