Closed Bug 1138740 Opened 5 years ago Closed 5 years ago

Differential Testing: Different output message involving array buffers

Categories

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

x86_64
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox38.0.5 --- fixed
firefox39 --- fixed

People

(Reporter: gkw, Assigned: bhackett)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, testcase)

Attachments

(1 file)

x = Int8Array(1)
function f(y) {
    x[0] = y
}
f()
f(3)
f(7)
try {
    x.buffer(f())
} catch (e) {}
print(x[0])

$ ./js-dbg-64-dm-nsprBuild-darwin-0b3c520002ad --fuzzing-safe --no-threads --ion-eager testcase.js
7

$ ./js-dbg-64-dm-nsprBuild-darwin-0b3c520002ad --fuzzing-safe --no-threads --baseline-eager testcase.js
0

Tested this on m-c rev 0b3c520002ad.

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh ~/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build -R ~/trees/mozilla-central" -r 0b3c520002ad

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/d5b0e9e6a849
user:        Brian Hackett
date:        Mon Apr 07 13:04:37 2014 -0700
summary:     Bug 987508 - Create array buffers lazily for small typed arrays, r=sfink.

Setting s-s because typed arrays are involved.

Brian, is bug 987508 a likely regressor?
Flags: needinfo?(bhackett1024)
Attached patch patchSplinter Review
Ion needs to be notified when the base pointer of a possibly-singleton-typed buffer view changes.
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8573363 - Flags: review?(sphink)
The only problem here is that Ion might continue using the original inline data for the typed array's contents.  That memory will always be there, so this will only lead to correctness problems.
Group: core-security
Comment on attachment 8573363 [details] [diff] [review]
patch

Review of attachment 8573363 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good. It seems like there ought to be a simpler test case for this, but I was unable to come up with one. I couldn't get the compiler to emit the hardcoded data pointer. It created an MConstantElements MIR node with the pointer value, and lowered that to an LDefinition::SLOTS, but it seems like it never visited that node. I don't know anything about the compiler, so I don't know how to track it further. At any rate, it seemed like the generated code was loading it out of the private pointer.

The original test case does not reproduce for me.

Oh well. I'll remove my nose from the jit now. May as well just land as is.
Attachment #8573363 - Flags: review?(sphink) → review+
https://hg.mozilla.org/mozilla-central/rev/cb1c692e8963
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Can we please backport this to 38? It is the next ESR and this will help fuzzing on that ESR branch if need be.
Flags: needinfo?(bhackett1024)
Comment on attachment 8573363 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: Bug 987508
[User impact if declined]: Possible incorrect behavior.  See comment 6.
[Describe test coverage new/current, TreeHerder]: Fuzz test.
[Risks and why]: Minimal.
Flags: needinfo?(bhackett1024)
Attachment #8573363 - Flags: approval-mozilla-beta?
Attachment #8573363 - Flags: approval-mozilla-aurora?
Comment on attachment 8573363 [details] [diff] [review]
patch

[Triage Comment]
Sure, should be in 38 beta 8
39 is already fixed
Attachment #8573363 - Flags: approval-mozilla-release+
Attachment #8573363 - Flags: approval-mozilla-beta?
Attachment #8573363 - Flags: approval-mozilla-aurora?
Attachment #8573363 - Flags: approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.