Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Description • 11 years ago

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?

	Douglas Crosher [:dougc] 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.

	Douglas Crosher [:dougc] 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.

	Jan de Mooij [:jandem]
	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?

	Douglas Crosher [:dougc] 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.

	Andrew McCreight [:mccr8]
	Comment 5 • 11 years ago

Do you know how bad this problem is?  Is it just a null deref or something worse?

	Douglas Crosher [:dougc] 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.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) 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?

	Douglas Crosher [:dougc] Assignee
	Comment 8 • 11 years ago

I suggest re-evaluating this bug while checking and testing the changes in bug 760642, before closing it.

	Daniel Veditz [:dveditz]
	Comment 9 • 11 years ago

assigning to Gary for the suggested retesting

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 10 • 10 years ago

Bumping to :dougc, as he's working on rewriting the assembly buffer. Please feel free to change this as necessary.

	Christian Holler (:decoder)
	Comment 11 • 10 years ago

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 12 • 10 years ago

:dougc, is bug 1026919 a likely further fix for this?

	Douglas Crosher [:dougc] 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.