Closed
Bug 504647
Opened 15 years ago
Closed 15 years ago
JITted regular expressions crash SPARC Nanojit
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.1 | --- | .2-fixed |
People
(Reporter: wes, Assigned: wes)
References
Details
(Keywords: crash, verified1.9.1)
Attachments
(1 file, 2 obsolete files)
771 bytes,
patch
|
dmandelin
:
review+
samuel.sidler+old
:
approval1.9.1.2+
|
Details | Diff | Splinter Review |
A reduced test case, "abc".match(/XYZ/) illustrates this problem, which was actually found via the String.match test in trace-test.js. The crash occurs because an unaligned memory read occurs; 16-bit jschars are loaded with the LIR 'ld' instruction, which reads 32-bit quantities. This is illegal on SPARC. The fix is simple; vlad already wrote it for ARM in bug 479525, just turn it on for SPARC too. Here is some debug info: - $pc is 0xfed95f58 at crash - why crash? %l1 is pointing 16-bit-aligned address: (gdb) print /x $l1 $3 = 0x2c630a (gdb) print (char)(((short *)$l1)[-1]) $7 = 97 'a' (gdb) print (char)(((short *)$l1)[0]) $8 = 98 'b' (gdb) print (char)(((short *)$l1)[1]) $9 = 99 'c' === Aggregated assembly output: BEGIN === 00fed95f20 [prologue] 00fed95f20 nop 00fed95f24 nop 00fed95f28 save %sp, -160, %sp 00fed95f2c patch entry: fed95f2c: ## patching branch at 00fed95fd4 to 00fed95f2c 00fed95f2c: ## compiling trunk 2f00d0 T1 state = iparam 0 %i0 gdata = iparam 1 %i1 start = ld gdata[16] cpend = ld gdata[24] le1 = le start, cpend jf le1 -> label1 00fed95f2c ld [%i1 + 16], %l1 %i0(state) %i1(gdata) 00fed95f30 ld [%i1 + 24], %l5 %l1(start) %i0(state) %i1(gdata) 00fed95f34 subcc %l1, %l5, %g0 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f38 bg fed95fb8 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f3c nop %l1(start) %l5(cpend) %i0(state) %i1(gdata) 2 sub1 = sub cpend, 2 lt1 = lt start, sub1 jf lt1 -> label2 00fed95f40 or %l5, 0, %l3 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f44 or %g0, 2, %l2 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f48 subcc %l3, %l2, %l3 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f4c subcc %l1, %l3, %g0 %l1(start) %l3(sub1) %l5(cpend) %i0(state) %i1(gdata) 00fed95f50 bge fed95fc4 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f54 nop %l1(start) %l5(cpend) %i0(state) %i1(gdata) ld1 = ld start[0] #580059 eq1 = eq ld1, #580059 jf eq1 -> label2 00fed95f58 ld [%l1 + 0], %l3 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f5c sethi 580059, %l2 %l1(start) %l3(ld1) %l5(cpend) %i0(state) %i1(gdata) 00fed95f60 or %l2, 89, %l2 %l1(start) %l3(ld1) %l5(cpend) %i0(state) %i1(gdata) 00fed95f64 subcc %l3, %l2, %g0 %l1(start) %l3(ld1) %l5(cpend) %i0(state) %i1(gdata) 00fed95f68 bne fed95fc4 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f6c nop %l1(start) %l5(cpend) %i0(state) %i1(gdata) 4 add1 = add start, 4 lt2 = lt add1, cpend jf lt2 -> label2 00fed95f70 or %l1, 0, %l3 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f74 or %g0, 4, %l2 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f78 addcc %l3, %l2, %l3 %l1(start) %l5(cpend) %i0(state) %i1(gdata) 00fed95f7c st %l3, [%fp + -4] %l1(start) %l3(add1) %l5(cpend) %i0(state) %i1(gdata) <= spill add1 00fed95f80 subcc %l3, %l5, %g0 %l1(start) %l3(add1) %l5(cpend) %i0(state) %i1(gdata) 00fed95f84 bge fed95fc4 %l1(start) %l3(add1) %i0(state) %i1(gdata) 00fed95f88 nop %l1(start) %l3(add1) %i0(state) %i1(gdata) ldcs1 = ldcs add1[0] 90 eq2 = eq ldcs1, 90 jf eq2 -> label2 00fed95f8c ld [%l3 + 0], %l3 %l1(start) %l3(add1) %i0(state) %i1(gdata) 00fed95f90 or %g0, 90, %l2 %l1(start) %l3(ldcs1) %i0(state) %i1(gdata) 00fed95f94 subcc %l3, %l2, %g0 %l1(start) %l3(ldcs1) %i0(state) %i1(gdata) 00fed95f98 bne fed95fc4 %l1(start) %i0(state) %i1(gdata) 00fed95f9c nop %l1(start) %i0(state) %i1(gdata) merging registers (union) with existing edge 00fed95fa0 or %i0, 0, %o0 %l1(start) %i0(state) restore add1 00fed95fa4 ld [%fp + -4], %l1 %o0(state) 2 add2 = add add1, 2 sti state[0] = add2 ret state 00fed95fa8 or %g0, 2, %l2 %o0(state) %l1(add1) 00fed95fac addcc %l1, %l2, %l1 %o0(state) %l1(add1) 00fed95fb0 st %l1, [%o0 + 0] %o0(state) %l1(add2) 00fed95fb4 ba fed95fe0 00fed95fb8 nop label1: 0 ret 0 00fed95fbc [label1] 00fed95fbc xor %o0, %o0, %o0 00fed95fc0 ba fed95fe0 %l1(start) %i0(state) %i1(gdata) 00fed95fc4 nop %l1(start) %i0(state) %i1(gdata) label2: 2 add3 = add start, 2 sti gdata[16] = add3 1 1 loop 00fed95fc8 [label2] %l1(start) %i0(state) %i1(gdata) 00fed95fc8 or %g0, 2, %l2 %l1(start) %i0(state) %i1(gdata) 00fed95fcc addcc %l1, %l2, %l1 %l1(start) %i0(state) %i1(gdata) 00fed95fd0 st %l1, [%i1 + 16] %l1(add3) %i0(state) %i1(gdata) jmp SOT 00fed95fd8 or %g2, 0, %g2 00fed95fdc jmpl [%g0 + %g2] 00fed95fe0 nop 00fed95fe4 [epilogue] 00fed95fe4 or %o0, 0, %i0 00fed95fe8 jmpl [%i7 + 8] 00fed95fec restore 00fed95ff0 sethi fedc2000, %g2 00fed95ff4 or %g2, 0, %g2 00fed95ff8 jmpl [%g0 + %g2] nop === === Aggregated assembly output: END === === END LIR::compile(2eac98, 2f00d0) ================================================================================
Attachment #388968 -
Flags: review?(dmandelin)
Assignee | ||
Comment 1•15 years ago
|
||
Same code, new comment
Assignee: general → wes
Attachment #388968 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #388973 -
Flags: review?(dmandelin)
Attachment #388968 -
Flags: review?(dmandelin)
Updated•15 years ago
|
Attachment #388973 -
Flags: review?(dmandelin) → review+
Assignee | ||
Comment 2•15 years ago
|
||
Same code, same comment, only /* is spelled correctly this time. Is it friday yet??
Attachment #388973 -
Attachment is obsolete: true
Attachment #388978 -
Flags: review?(dmandelin)
Updated•15 years ago
|
Attachment #388978 -
Flags: review?(dmandelin) → review+
Comment 3•15 years ago
|
||
Comment on attachment 388978 [details] [diff] [review] Patch to enable 16-bit loads for regexp jschars on SPARC Apparently I'm running on fumes too. :-)
Assignee | ||
Comment 5•15 years ago
|
||
Decreasing severity: not a blocker as browser is built with SunStudio, resulting in very poor regexp performance rather than a crash. Thanks Leon for the analysis in bug 504058.
Severity: blocker → normal
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/247524e19d0c http://hg.mozilla.org/tracemonkey/rev/8cf8bb50ec0b
Attachment #388978 -
Flags: approval1.9.1.2?
Comment on attachment 388978 [details] [diff] [review] Patch to enable 16-bit loads for regexp jschars on SPARC This is a NPOTB code change. Should be safe to land mozilla-1.9.1 branch.
Comment 9•15 years ago
|
||
Comment on attachment 388978 [details] [diff] [review] Patch to enable 16-bit loads for regexp jschars on SPARC Approved for 1.9.1.2. a=ss for release-drivers Please land on mozilla-1.9.1 and use the ".2-fixed" option of the "status1.9.1" flag.
Attachment #388978 -
Flags: approval1.9.1.2? → approval1.9.1.2+
Comment 10•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3738c64de46e
status1.9.1:
--- → .2-fixed
Keywords: crash
Keywords: verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•