Closed Bug 445818 Opened 12 years ago Closed 12 years ago

Crash in Javascript on N810 (arm).

Categories

(Core :: JavaScript Engine, defect)

x86
Maemo
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: romaxa, Assigned: igor)

References

()

Details

(Keywords: testcase)

Attachments

(2 files)

v1
3.14 KB, patch
brendan
: review+
Details | Diff | Splinter Review
1.03 KB, application/octet-stream
Details
Latest trunk, arm, N810.
Visit some google service pages mail/maps/documents

5/10 it will crash in:
#0  js_Dequeue (tl=0xb99dc) at js/src/jslock.cpp:993
#1  0x41e8d06c in js_AtomizeString (cx=0xb124d8, str=0xbed41c98, flags=8) at js/src/jsatom.cpp:704
#2  0x41e8d23c in js_AtomizeChars (cx=0x0, chars=0x0, length=760284, flags=0) at js/src/jsatom.cpp:756
#3  0x41eee93c in js_GetToken (cx=0xb124d8, ts=0xbed42e80) at js/src/jsscan.cpp:1258
#4  0x41ef0478 in js_MatchToken (cx=0x0, ts=0xbed42e80, tt=TOK_YIELD) at js/src/jsscan.cpp:1999
#5  0x41ed8304 in AssignExpr (cx=0xb124d8, ts=0xbed42e80, tc=0xbed422f8) at js/src/jsparse.cpp:3701
#6  0x41ede854 in Variables (cx=0xb124d8, ts=0xbed42e80, tc=0xbed422f8) at js/src/jsparse.cpp:3637
#7  0x41edac94 in Statement (cx=0xb124d8, ts=0xbed42e80, tc=0xbed422f8) at js/src/jsparse.cpp:3239
#8  0x41edc1a4 in Statements (cx=0xb124d8, ts=0xbed42e80, tc=0xbed422f8) at js/src/jsparse.cpp:1455
#9  0x41edc33c in FunctionBody (cx=0xb124d8, ts=0xbed42e80, tc=0xbed422f8) at js/src/jsparse.cpp:851
#10 0x41edc6a0 in FunctionDef (cx=0xb124d8, ts=0xbed42e80, tc=0xbed42710, lambda=8) at js/src/jsparse.cpp:1295
#11 0x41edd5a0 in PrimaryExpr (cx=0xb124d8, ts=0xbed42e80, tc=0xbed42710, tt=TOK_FUNCTION, afterDot=0) at js/src/jsparse.cpp:5352
....
#58 0x41ed8328 in AssignExpr (cx=0xb124d8, ts=0xbed42e80, tc=0xbed42cf8) at js/src/jsparse.cpp:3818
#59 0x41ed86f4 in Expr (cx=0xb124d8, ts=0xbed42e80, tc=0xbed42cf8) at js/src/jsparse.cpp:3662
#60 0x41ed9bb4 in Statement (cx=0xb124d8, ts=0xbed42e80, tc=0xbed42cf8) at js/src/jsparse.cpp:3454
#61 0x41edf540 in js_CompileScript (cx=0xb124d8, obj=0xbed42cf8, principals=0xb99dc, tcflags=2048, chars=0x49589010, length=1102854124, file=0x41edf540, 
    filename=0x41bc37ec "°\022\203@4¦\203@T=¹A°ÚºAT=¹AT=¹A\\\021\203@ð\005»A@ȹAT=¹AT=¹A̬ºAT=¹AT=¹AT=¹AT=¹AT=¹AT=¹A¸ÞºA\204v\177@T=¹A0$ºAT=¹A\200ÞºAøÕ¹Að\205\201@T=¹AÄ3ºA\214\017\203@T=¹A\f*ºA$¢¹At¨ºAT=¹AT=¹AT=¹AT=¹AT=¹AT=¹A¼#»Ax[¼A¬:¼A`¸y@\034]¼AÌ\023\002", lineno=11609348)
    at js/src/jsparse.cpp:611
(gdb) frame 0
#0  js_Dequeue (tl=0xb99dc) at /home/romaxa/microbcomponent/hg/mozilla/js/src/jslock.cpp:993
993	    PR_Lock(fl->slock);
(gdb) l 
988	    JSFatLock *fl = tl->fat;
989	    PRStatus stat;
990	
991	    JS_ASSERT(fl != NULL);
992	    JS_ASSERT(fl->susp > 0);
993	    PR_Lock(fl->slock);
994	    js_UnlockGlobal(tl);
995	    stat = PR_NotifyCondVar(fl->svar);
996	    JS_ASSERT(stat != PR_FAILURE);
997	    PR_Unlock(fl->slock);
(gdb) p *tl 
$3 = {owner = 0, fat = 0x0}
It seems this is a regression from the bug 444076. Can you check if USE_ARM_KUSER is defined during the build of js/src/jslock.cpp? 
Blocks: 444076
Attached patch v1Splinter Review
I was wrong about using generic __sync_bool_compare_and_swap available in GCC. Apparently one cannot trust the function as this bug demonstrates. When USE_ARM_KUSER is not defined GCC for ARM may either miscompile or generate the wrong code for __sync_bool_compare_and_swap.

So the patch does not trust GCC __sync_bool_compare_and_swap and just code it explicitly for x86-64.
Assignee: general → igor
Status: NEW → ASSIGNED
Attachment #330091 - Flags: review?(brendan)
Yep, with this patch it works without crashes.
Attachment #330091 - Flags: review?(brendan) → review+
Attached file v1 as hg bundle
Still crashes at the same place:
#0  js_Dequeue (tl=0xbf30c) at js/src/jslock.cpp:1001
#1  0x41cad06c in js_AtomizeString (cx=0x6c0b88, str=0xbee0e670, flags=8) at js/src/jsatom.cpp:704
#2  0x41cad23c in js_AtomizeChars (cx=0x0, chars=0x0, length=783116, flags=0)
(gdb) frame 0
#0  js_Dequeue (tl=0xbf30c) at /home/bifh4/diablo-arm-prereleased.gcc34qemu.distcc/work/microb-engine-20080616/build-tree/mozilla/js/src/jslock.cpp:1001
1001	    PR_Lock(fl->slock);
(gdb) p *tl
$7 = {owner = 0, fat = 0x0}
(gdb) l
996	    JSFatLock *fl = tl->fat;
997	    PRStatus stat;
998	
999	    JS_ASSERT(fl != NULL);
1000	    JS_ASSERT(fl->susp > 0);
1001	    PR_Lock(fl->slock);
1002	    js_UnlockGlobal(tl);
1003	    stat = PR_NotifyCondVar(fl->svar);
1004	    JS_ASSERT(stat != PR_FAILURE);
1005	    PR_Unlock(fl->slock);


(In reply to comment #5)
> Still crashes at the same place:
> #0  js_Dequeue (tl=0xbf30c) at js/src/jslock.cpp:1001

Do you mean it crashes with the patch?
(In reply to comment #6)
> (In reply to comment #5)
> > Still crashes at the same place:
> > #0  js_Dequeue (tl=0xbf30c) at js/src/jslock.cpp:1001
> 
> Do you mean it crashes with the patch?
Yep, it crashes with the patch. 

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Still crashes at the same place:
> > > #0  js_Dequeue (tl=0xbf30c) at js/src/jslock.cpp:1001
> > 
> > Do you mean it crashes with the patch?
> Yep, it crashes with the patch. 

Hm, but that mean that USE_ARM_KUSER is defined when js/src/jslock.cpp is compiled and neither this patch not the bug 444076 shpould not cause the regression. Can you check USE_ARM_KUSER? 
Yes, we are using --with-arm-kuser option, because  it N8xx - arm + kernel
and USE_ARM_KUSER=1

(In reply to comment #9)
> Yes, we are using --with-arm-kuser option, because  it N8xx - arm + kernel
> and USE_ARM_KUSER=1
> 

Then neither bug 444076 nor the patch can any effects on your crash since the code changes does not touch the code compiled on ARM.

Could you try to compile with -O0 to see if the crash would happen with no optimizations?
I have tested with -O0 already and it does not crash....
At least I was testing during 20 minutes and did not get any crash.
(In reply to comment #11)
> I have tested with -O0 already and it does not crash....
> At least I was testing during 20 minutes and did not get any crash.
> 

Then this looks like a compiler bug perhaps triggered by changes from the bug 440184 that added an extra level of inlining. Can you try with normal optimization level but with ThinkLock()/ThinkUnlock() changed  (they defined in two places, the changes has to go to functions defined right after js_Dequeue) from static JS_INLINE to just static.

If that would not help, can you try to remove both static and JS_INLINE?
tried to remove JS_INLINE only and still crashes in the same place.
I'll try to remove both.
removing both did not help.
(In reply to comment #14)
> removing both did not help.
> 

One more idea: could you try to remove static JS_INLINE from NativeCompareAndSwap defined under #elif defined(USE_ARM_KUSER)  ? And if that would not help, try to add __attributes__((noinline)) to NativeCompareAndSwap.
Hmm, ok I will try that... 

Igor, do you have any ideas which simple js testcase can reproduce this problem?
Usually it crashes on gmail ui=1/2, but
I tiered to load google services every time.... :(


(In reply to comment #16)
> Hmm, ok I will try that... 
> 
> Igor, do you have any ideas which simple js testcase can reproduce this
> problem?

Something like:

function test() {
    var obj = {};
    for (var i = 0; i != 42*42*42; ++i) {
        obj["a"+i] = 1;
        var tmp = { "a"+i: 2 }; 
    }
}

test();

But given the bug nature I suspect one needs at least 2 threads to trigger it. If you can compile and run thread-safe js shell, then you can try the above script in the shell replacing the "test()" invocation via:

scatter([test, test, test, test]);

That would run the function test on 4 threads. 
Actually if you can run the thread-safe shell, try something like:

var array = [{}, {}, {}, {}];

function test() {
    for (var i = 0; i != 42*42*42; ++i) {
        var obj = array[i % array.length];
        obj["a"+i] = 1;
        var tmp = { "a"+i: 2 }; 
    }
}

scatter([test, test, test, test]);


This will makes 4 threads to access the same objects over and over.
var tmp = { "a"+i: 2 }; 
typein:1: SyntaxError: missing : after property id:
typein:1: var tmp = { "a"+i: 2 };

(In reply to comment #19)
> var tmp = { "a"+i: 2 }; 
> typein:1: SyntaxError: missing : after property id:
> typein:1: var tmp = { "a"+i: 2 };

Sorry for that mental blunder. replace that var tmp = { "a"+i: 2 }; with

var tmp = {};
tmp["a"+i] = 2;



> would not help, try to add __attributes__((noinline)) to NativeCompareAndSwap.
> 
After ((noinline)) I did not found any crash.

PS: testcase does not reproduce crash...

I filed the bug 446169 to implement noinline workaround as the patch in this bug fix at least the linking problem when USE_ARM_KUSER is not defined.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: testcase
/cvsroot/mozilla/js/tests/js1_8/extensions/regress-445818.js,v  <--  regress-445818.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/f0e9fd501e63
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.