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

RESOLVED FIXED in mozilla33

Status

()

Core
JavaScript Engine: JIT
--
major
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gkw, Assigned: shu)

Tracking

(Blocks: 2 bugs, {regression, testcase})

Trunk
mozilla33
x86_64
All
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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)
(Assignee)

Comment 1

4 years ago
What is the differential output here? The warnings? Those don't matter for correctness.
Flags: needinfo?(shu)
(Reporter)

Comment 2

4 years ago
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)
(Assignee)

Comment 3

4 years ago
I guess the differential fuzzer cares about stderr because of thrown exceptions?
(Reporter)

Comment 4

4 years ago
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)
(Assignee)

Comment 6

4 years ago
(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?
(Assignee)

Comment 7

4 years ago
Created attachment 8450968 [details] [diff] [review]
Don't report warnings for recover instructions when snapshotting frames for PJS bailout warnings.
(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.

Updated

4 years ago
Summary: Differential Testing: Different output message involving filterPar → Differential Testing: inconsistent number of "Warning! Tried to access unreadable value allocation" with filterPar
(Assignee)

Comment 9

4 years ago
Created attachment 8452831 [details] [diff] [review]
Don't report warnings for recover instructions when snapshotting frames for PJS bailout warnings.
Attachment #8450968 - Attachment is obsolete: true
Attachment #8452831 - Flags: review?(nicolas.b.pierron)
Attachment #8452831 - Flags: review?(nicolas.b.pierron) → review+
(Reporter)

Comment 11

4 years ago
This landed:

https://hg.mozilla.org/mozilla-central/rev/82b0cc883f41
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Reporter)

Updated

4 years ago
Assignee: nobody → shu
You need to log in before you can comment on or make changes to this bug.