159334 - The javascript functions size is limited by a bug

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Yann Coquillard]

As far as I understand Rhino's source code, I think it's a bug. It is present 
on both versions 1.5R2 & 1.5R3. It occurs each time the number of token exceeds 
32767. 

V1.5R2 : org.mozilla.javascript.Interpreter:getString()
V1.5R3 : org.mozilla.javascript.Interpreter.getShort()

The way the index to enter the token's table (theStringTable) is computed can 
result in negative numbers. 

Here's the code in 1.5R2 ;

    private static String getString(String[] theStringTable, byte[] iCode,
                                    int pc){
        int index = (iCode[pc] << 8) + (iCode[pc + 1] & 0xFF);
        return theStringTable[index];
    }

As iCode is a table of bytes, iCode[pc] can result in a negative number. Here's 
an ugly-coded patch ;

    private static String getString(String[] theStringTable, byte[] iCode,
                                    int pc){
	int a1 = iCode[pc];
	if ( a1<0 ) a1 += 256;
	a1 = a1 << 8;
	int a2 = iCode[pc + 1];
	if ( a2<0 ) a2 += 256;
	a2 = a2  & 0xFF;
	int index = a1 + a2;
        return theStringTable[index];
    }

As I use some kind of code-generation, I have functions that exceed 2500 lines. 
By using this patch, everything works fine. Sorry for my bad english, I hope 
this will help :)

Comment 1

[Yann Coquillard]

(forgot the exception stack)

Exception stack for v1.5R2 :

java.lang.reflect.InvocationTargetException: 
java.lang.ArrayIndexOutOfBoundsException
        at org.mozilla.javascript.Interpreter.interpret(Interpreter.java:1782)
        at org.mozilla.javascript.InterpretedScript.call
(InterpretedScript.java:68)
        at org.mozilla.javascript.InterpretedScript.exec
(InterpretedScript.java:59)
        at org.mozilla.javascript.Context.evaluateReader(Context.java:778)
        at net.com_6.js.RhinoInterpretor.evaluateReader
(RhinoInterpretor.java:223)
        at net.com_6.js.RhinoInterpretor.processFile(RhinoInterpretor.java:203)
        at net.com_6.js.Global.include(Global.java:185)
        at inv1.invoke()
        at org.mozilla.javascript.FunctionObject.doInvoke
(FunctionObject.java:519)
        at org.mozilla.javascript.FunctionObject.callVarargs
(FunctionObject.java:534)
        at org.mozilla.javascript.FunctionObject.call(FunctionObject.java:399)
        at org.mozilla.javascript.ScriptRuntime.call(ScriptRuntime.java:1225)
        at org.mozilla.javascript.Interpreter.interpret(Interpreter.java:1935)
        at org.mozilla.javascript.InterpretedScript.call
(InterpretedScript.java:68)
        at org.mozilla.javascript.InterpretedScript.exec
(InterpretedScript.java:59)
        at org.mozilla.javascript.Context.evaluateReader(Context.java:778)
        at net.com_6.js.RhinoInterpretor.evaluateReader
(RhinoInterpretor.java:223)
        at net.com_6.js.RhinoInterpretor.processSource
(RhinoInterpretor.java:153)
        at net.com_6.js.RhinoInterpretor.run(RhinoInterpretor.java:327)
        at net.com_6.hiope.Main.<init>(Main.java:84)
        at net.com_6.hiope.Main.main(Main.java:42)

Comment 2

[Igor Bukanov]

Attachment: explicit test case for script with 64K diffrent tokens

Comment 3

[Igor Bukanov]

Attachment: Extending limit on number of different tokens to 64K

The Rhino tip already contains fixes to extend the limit from 32K name and
strings to 32K of different name and strings. This patch extends the limit
farther to 64K via storing indexes as uint16 and explicitly detects during byte
code generation the limit violations.

Comment 4

[Igor Bukanov]

Attachment: Test with 0xFFFF names and string literals

This version passes in SM as well

Comment 5

[Phil Schwartau]

Igor's testcase added to JS testsuite:

      mozilla/js/tests/js1_5/Regress/regress-159334.js

As Igor indicates, this test passes in the SpiderMonkey shell.
Without the above patch, it fails in the current Rhino shell:

*-* Testcase js1_5/Regress/regress-159334.js failed:
Expected exit code 0, got 1
Testcase terminated with signal 0
Complete testcase output was:

java.lang.ArrayIndexOutOfBoundsException: -32768
  at org.mozilla.javascript.Interpreter.interpret(Interpreter.java:2231)
  at org.mozilla.javascript.InterpretedScript.call(InterpretedScript.java:64)
  at org.mozilla.javascript.NativeGlobal.evalSpecial(NativeGlobal.java:530)
  at     
org.mozilla.javascript.ScriptRuntime.callOrNewSpecial(ScriptRuntime.java:1207)
  at org.mozilla.javascript.ScriptRuntime.callSpecial(ScriptRuntime.java:1247)
  at org.mozilla.javascript.gen.c17.call([path to]/regress-159334.js:67)
  at org.mozilla.javascript.gen.c17.exec([path to]/regress-159334.js)
  at org.mozilla.javascript.Context.evaluateReader(Context.java:820)
  at org.mozilla.javascript.tools.shell.Main.evaluateReader(Main.java:363)
  at org.mozilla.javascript.tools.shell.Main.processFileSecure(Main.java:354)
  at org.mozilla.javascript.tools.shell.Main.processFile(Main.java:291)
  at org.mozilla.javascript.tools.shell.Main.processSource(Main.java:283)
  at org.mozilla.javascript.tools.shell.Main.exec(Main.java:103)
  at org.mozilla.javascript.tools.shell.Main.main(Main.java:76)
Exception in thread "main"

Comment 6

[Igor Bukanov]

Attachment: New version js1_5/Regress/regress-159334.js

The current version of js1_5/Regress/regress-159334.js takes too long time and
too match memory to run under JDK 1.1. This patch changes it to bring time
down. It still necessary to specify 50M heap size to run it under JDK 1.1, but
at least timing is resonable.

Comment 7

[Igor Bukanov]

I commited the patch

Comment 8

[Phil Schwartau]

Igor's new version of the test has been checked in:

Checking in regress-159334.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-159334.js,v  <--  
regress-159334.js
new revision: 1.2; previous revision: 1.1
done


Is it OK to mark this bug fixed, or is there further work left?
The testcase (new version) used to fail in Rhino as in Comment #5.
Now, it passes in both the compiled and interpreted Rhino shells -

Comment 9

[Igor Bukanov]

Yes, then it should be marked as fixed

Comment 10

[Phil Schwartau]

Resolving as Fixed -

Comment 11

[Phil Schwartau]

Marking Verified -

Comment 12

[Igor Bukanov]

Targeting as resolved against 1.5R4