Closed Bug 1034280 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect, major)

x86_64
All
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: gkw, Assigned: shu)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, testcase)

Attachments

(1 file, 1 obsolete file)

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?
Flags: needinfo?(shu)
What is the differential output here? The warnings? Those don't matter for correctness.
Flags: needinfo?(shu)
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. :)
Flags: needinfo?(nicolas.b.pierron)
I guess the differential fuzzer cares about stderr because of thrown exceptions?
Yep, we look out for differences in stderr and stdout.
(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.
Flags: needinfo?(nicolas.b.pierron)
(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?
(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.
Summary: Differential Testing: Different output message involving filterPar → Differential Testing: inconsistent number of "Warning! Tried to access unreadable value allocation" with filterPar
Attachment #8452831 - Flags: review?(nicolas.b.pierron) → review+
This landed:

https://hg.mozilla.org/mozilla-central/rev/82b0cc883f41
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Assignee: nobody → shu
You need to log in before you can comment on or make changes to this bug.