Closed
Bug 944972
Opened 11 years ago
Closed 10 years ago
Crash [@ js::jit::AssemblerBufferWithConstantPool]
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 1026919
People
(Reporter: gkw, Assigned: dougc)
References
Details
(4 keywords, Whiteboard: [jsbugmon:])
Crash Data
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
for (var f in []) {}
function testMathyFunction(f, inputs) {
var results = [];
for (var j = 0; j < inputs.length; ++j) {
for (var k = 0; k < inputs.length; ++k) {
try {
results.push(f(inputs[j], inputs[k]));
} catch (e) {
results.push(e);
}
}
}
uneval(results);
}
s = newGlobal();
s["testMathyFunction"] = this["testMathyFunction"].bind(this);
function f(code) {
try {
evalcx(code, s);
} catch (e) {}
};
f("\
(function(){})();\
testMathyFunction([\
Number.MAX_VALUE,\
Number.MIN_VALUE,\
Math.PI,\
]);\
");
f("\
m = (function(){});\
testMathyFunction(a, [\
Number.MIN_VALUE,,0xffffffff,,1/0,0x80000000,0xf,\
-0x100000000,-0,0x100000000,-1/0,0x100000001,\
Number.MAX_VALUE,0/0,,Math.PI,0x100000001,\
]);\
")
crashes js debug shell on m-c changeset 77b5c6edfe96 with --ion-eager at js::jit::AssemblerBufferWithConstantPool
My configure flags are:
CC="gcc -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" AR=ar CXX="g++ -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" sh ./configure --target=arm-linux-gnueabi --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --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: http://hg.mozilla.org/mozilla-central/rev/5959fac49afe
user: Douglas Crosher
date: Sun Nov 10 04:20:22 2013 +1100
summary: Bug 906964 - ARM: leave some head-room in the double pools to help avoid bailing out which causes compilation failure for asm.js code. r=mjrosenb
s-s because I'm not sure what this testcase is trying to do. Douglas, is bug 906964 a likely regressor?
Flags: needinfo?(dtc-moz)
Assignee | ||
Comment 1•11 years ago
|
||
This is a possible fix, but I am not familiar enough with the code to be sure. Let me try to reproduce it and test this. Marty might be working on the buffer pools and be familiar with this?
The bisected patch was just a hack, but was not expected to cause problems. It would be an easy decision to back it out, but let me explore it a little more first.
Flags: needinfo?(dtc-moz) → needinfo?(mrosenberg)
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to Douglas Crosher [:dougc] from comment #1)
> Created attachment 8340754 [details] [diff] [review]
> Guard against a null perforatedNode
>
> This is a possible fix, but I am not familiar enough with the code to be
> sure. Let me try to reproduce it and test this. Marty might be working on
> the buffer pools and be familiar with this?
>
> The bisected patch was just a hack, but was not expected to cause problems.
> It would be an easy decision to back it out, but let me explore it a little
> more first.
I can reproduce the crash and this patch alone is not adequate to fix the problem, so perhaps it would be best to backout the hack patch in bug 906964 and wait for a comprehensive solution. This patch might still be part of a solution, and it might be that the patch in bug 906964 just changed the packing of the pools and tickled the failure here.
Flags: needinfo?(mrosenberg)
Comment 3•11 years ago
|
||
(In reply to Douglas Crosher [:dougc] from comment #2)
> I can reproduce the crash and this patch alone is not adequate to fix the
> problem, so perhaps it would be best to backout the hack patch in bug 906964
> and wait for a comprehensive solution. This patch might still be part of a
> solution, and it might be that the patch in bug 906964 just changed the
> packing of the pools and tickled the failure here.
Can we find out if that's the case?
Flags: needinfo?(dtc-moz)
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #3)
> (In reply to Douglas Crosher [:dougc] from comment #2)
> > I can reproduce the crash and this patch alone is not adequate to fix the
> > problem, so perhaps it would be best to backout the hack patch in bug 906964
> > and wait for a comprehensive solution. This patch might still be part of a
> > solution, and it might be that the patch in bug 906964 just changed the
> > packing of the pools and tickled the failure here.
>
> Can we find out if that's the case?
I understand Marty is working of the buffer pools so I will defer to him.
Flags: needinfo?(dtc-moz)
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(mrosenberg)
Comment 5•11 years ago
|
||
Do you know how bad this problem is? Is it just a null deref or something worse?
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #3)
> (In reply to Douglas Crosher [:dougc] from comment #2)
> > I can reproduce the crash and this patch alone is not adequate to fix the
> > problem, so perhaps it would be best to backout the hack patch in bug 906964
> > and wait for a comprehensive solution. This patch might still be part of a
> > solution, and it might be that the patch in bug 906964 just changed the
> > packing of the pools and tickled the failure here.
>
> Can we find out if that's the case?
The problem should be analysed, but the patch in bug 906964 is being backed out and it might be informative to see if fuzzing can still tickle this crash.
Reporter | ||
Updated•11 years ago
|
Assignee: general → nobody
QA Contact: general
Reporter | ||
Comment 7•11 years ago
|
||
> to see if fuzzing can still tickle this crash.
This no longer crashes on m-c rev c8d5a871ae32 compiled with the patch for bug 952810. FIXED by the backout?
Flags: needinfo?(dtc-moz)
Assignee | ||
Comment 8•11 years ago
|
||
I suggest re-evaluating this bug while checking and testing the changes in bug 760642, before closing it.
Depends on: 760642
Flags: needinfo?(dtc-moz)
Updated•11 years ago
|
Group: javascript-core-security
Reporter | ||
Comment 10•11 years ago
|
||
Bumping to :dougc, as he's working on rewriting the assembly buffer. Please feel free to change this as necessary.
Assignee: gary → dtc-moz
Updated•10 years ago
|
Whiteboard: [jsbugmon:update]
Updated•10 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
Comment 11•10 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Updated•10 years ago
|
Group: javascript-core-security
Reporter | ||
Comment 12•10 years ago
|
||
:dougc, is bug 1026919 a likely further fix for this?
Flags: needinfo?(dtc-moz)
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] PTO Jul 19 to 27 from comment #12)
> :dougc, is bug 1026919 a likely further fix for this?
Yes, this issue looks resolved by bug 1026919. The failing line is still there, but the perforatedNode, now named perforatedSlice, is obtain just a few lines above and I can not see how it could be null or invalid.
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: needinfo?(dtc-moz)
Resolution: --- → DUPLICATE
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•