504647 - JITted regular expressions crash SPARC Nanojit

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Wesley W. Garland]

Attachment: Patch to enable 16-bit loads for regexp jschars on SPARC

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)
================================================================================

Comment 1

[Wesley W. Garland]

Attachment: Patch to enable 16-bit loads for regexp jschars on SPARC

Same code, new comment

Comment 2

[Wesley W. Garland]

Attachment: Patch to enable 16-bit loads for regexp jschars on SPARC

Same code, same comment, only /* is spelled correctly this time.

Is it friday yet??

Comment 3

[David Mandelin [:dmandelin]]

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. :-)

Comment 5

[Wesley W. Garland]

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.

Comment 6

[Leon Sha]

http://hg.mozilla.org/mozilla-central/rev/247524e19d0c
http://hg.mozilla.org/tracemonkey/rev/8cf8bb50ec0b

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/8cf8bb50ec0b

Comment 8

[Leon Sha]

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

[Samuel Sidler (old account; do not CC)]

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.

Comment 10

[Leon Sha]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3738c64de46e