Crash in Javascript on N810 (arm).

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: romaxa, Assigned: igor)

Tracking

({testcase})

unspecified
x86
Maemo
testcase
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

v1
3.14 KB, patch
brendan
: review+
Details | Diff | Splinter Review
1.03 KB, application/octet-stream
Details
(Reporter)

Description

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

Comment 1

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

Comment 2

10 years ago
Created attachment 330091 [details] [diff] [review]
v1

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)
(Reporter)

Comment 3

10 years ago
Yep, with this patch it works without crashes.

Updated

10 years ago
Attachment #330091 - Flags: review?(brendan) → review+
(Assignee)

Comment 4

10 years ago
Created attachment 330138 [details]
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);


(Assignee)

Comment 6

10 years ago
(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. 

(Assignee)

Comment 8

10 years ago
(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? 
(Reporter)

Comment 9

10 years ago
Yes, we are using --with-arm-kuser option, because  it N8xx - arm + kernel
and USE_ARM_KUSER=1

(Assignee)

Comment 10

10 years ago
(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?
(Reporter)

Comment 11

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

Comment 12

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

Comment 15

10 years ago
(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.
(Reporter)

Comment 16

10 years ago
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.... :(


(Assignee)

Comment 17

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

Comment 18

10 years ago
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.
(Reporter)

Comment 19

10 years ago
var tmp = { "a"+i: 2 }; 
typein:1: SyntaxError: missing : after property id:
typein:1: var tmp = { "a"+i: 2 };

(Assignee)

Comment 20

10 years ago
(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;



(Reporter)

Comment 21

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

PS: testcase does not reproduce crash...

(Assignee)

Comment 22

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

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Keywords: testcase

Comment 24

10 years ago
/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.