Issue1611

classification
Title: Jython bytecode violated JLS, causing NPE on Sun's JVM when using -Xcomp option
Type: crash Severity: normal
Components: Core Versions: 2.5.1
Milestone:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zyasoft Nosy List: alexey, draghuram, irogers, zyasoft
Priority: normal Keywords:

Created on 2010-05-15.15:58:19 by alexey, last changed 2010-08-22.06:45:04 by zyasoft.

Messages
msg5767 (view) Author: Alexey Aliev (alexey) Date: 2010-05-15.15:58:16
Consider the simplest example aaa.py with one line of code: 
print "Simple test"
Here is a fragment of bytecode, generated by Jython for this program (bytecode can be seen with javap, which is part of Sun’s JDK. It is might be helpful to create another single line file bbb.py, which contains “import aaa” and launch “jython.bat bbb.py” in order to see aaa$py.class in your local directory. Then run “javap -c aaa$py”):

public class aaa$py extends org.python.core.PyFunctionTable implements org.python.core.PyRunnable{
static final aaa$py self; //Field declared both static and final
...
public aaa$py(java.lang.String);  //Instance constructor
  Code:
   0:	aload_0
   1:	invokespecial	#35;
   4:	aload_0
   5:	putstatic	#39; //Write to field self:Laaa$py;
}

Line 5 (putstatic #39) in the instance constructor breaks JVM specifications, because fields declared both static and final must be initialized with compile-time values prior any instance of the class are created (see chapter 2.17.4 in the JVMS specification http://java.sun.com/docs/books/jvms/second_edition/html/VMSpecTOC.doc.html).
Seems that problem arises when Sun JVM performs some optimizations during bytecode compilation, which are correct in terms of JVMS, but don’t compatible with Jython. 
To observe the issue just force code compilation by passing –Xcomp option to JRE. 
I used:
set _JAVA_OPTS=-Xcomp
Because this problem exists in every file that Jython executes, it is not necessary to pass other arguments, just run jython.bat without parameters:
C:\jython2.5.1\test>..\jython.bat

Jython 2.5.1 (Release_2_5_1:6813, Sep 26 2009, 13:47:54)
[Java HotSpot(TM) Client VM (Sun Microsystems Inc.)] on java1.6.0_18
error importing site
Traceback (most recent call last):
  File "C:\jython2.5.1\Lib\site.py", line 0, in <module>
java.lang.NullPointerException
        at org.python.core.PyTableCode.call(PyTableCode.java:165)
        at org.python.core.PyCode.call(PyCode.java:18)
        at org.python.core.imp.createFromCode(imp.java:326)
        at org.python.core.imp.createFromPyClass(imp.java:145)
        at org.python.core.imp.loadFromSource(imp.java:505)
        at org.python.core.imp.find_module(imp.java:411)
        at org.python.core.imp.import_next(imp.java:635)
        at org.python.core.imp.import_first(imp.java:656)
        at org.python.core.imp.load(imp.java:564)
        at org.python.util.jython.run(jython.java:177)
        at org.python.util.jython.main(jython.java:129)
java.lang.NullPointerException: java.lang.NullPointerException

I believe that field “self” shouldn’t be declared as final in order to avoid this issue.

Java version is
C:\Program Files\Java\jdk1.6.0_18\bin>java.exe -version
java version "1.6.0_18"
Java(TM) SE Runtime Environment (build 1.6.0_18-b07)
Java HotSpot(TM) Client VM (build 16.0-b13, mixed mode, sharing)
msg5768 (view) Author: Alexey Aliev (alexey) Date: 2010-05-15.15:59:43
Correct link to JVMS is:
http://java.sun.com/docs/books/jvms/second_edition/html/VMSpecTOC.doc.html
msg5992 (view) Author: Ian Rogers (irogers) Date: 2010-08-20.18:06:46
So the interpretation of the virtual machine specification doesn't say that static final fields must be written before instances of classes can be created. If this were so then it would be impossible to make singleton instances of classes. Also putstatic may write to a final field in the same class from any method - from the virtual machine specification. So from this Jython's code looks valid and passes bytecode verification (-Xverify in HotSpot).

The Java Language Specification:
http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.5.1
details what the meaning of final is and here it can be seen why what jython is doing is buggy, although a discussion of class initializers isn't provided in this context - my discussion follows what happen in HotSpot (the reference Java implementation). A static final field is initialized by the class initializer. A freeze event occurs when the class initializer completes. Readers of the static final once a class is initialized can assume that it is constant, with some exceptions such as when the field is modified through a specialist mechanism such as reflection. The reader of the static final field (especially when -Xcomp is switched on) is the HotSpot compiler.

Consider jython generated code such as:

public sre_constants$py(java.lang.String);
  Code:
   Stack=13, Locals=3, Args_size=2
   0:   aload_0
   1:   invokespecial   #645; //Method org/python/core/PyFunctionTable."<init>":()V
   4:   aload_0
   5:   putstatic       #649; //Field self:Lsre_constants$py;
...
  1045:        iconst_1
   1046:        iconst_1
   1047:        anewarray       #543; //class java/lang/String
   1050:        astore_2
   1051:        aload_2
   1052:        iconst_0
   1053:        ldc_w   #858; //String a
   1056:        aastore
   1057:        aload_2
   1058:        aload_1
   1059:        ldc_w   #860; //String <lambda>
   1062:        sipush  222
   1065:        iconst_0
   1066:        iconst_0
   1067:        getstatic       #649; //Field self:Lsre_constants$py;
   1070:        iconst_4
   1071:        aconst_null
   1072:        aconst_null
   1073:        iconst_0
   1074:        sipush  4097
   1077:        invokestatic    #840; //Method org/python/core/Py.newCode:(I[Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IZZLorg/python/core/PyFunctionTable;I[Ljava/lang/String;[Ljava/lang/String;II)Lorg/python/core/PyCode;
   1080:        putstatic       #538; //Field f$4:Lorg/python/core/PyCode;
   1083:        return

The:
   5:   putstatic       #649; //Field self:Lsre_constants$py;
occurs after the class initialization freeze event. The compiler compiles:
   1067:        getstatic       #649; //Field self:Lsre_constants$py;
And because the field is considered frozen the value NULL is loaded and passed through to the subsequent call. Eventually this results in a NullPointerException.
msg5996 (view) Author: Jim Baker (zyasoft) Date: 2010-08-21.04:07:16
The "undocumented" option -Xcomp (AOT compilation of Java bytecode) is probably not a good idea, if it were to work, since profiled execution seems to be essential for Jython performance.

Bytecode layout is entirely done by the ASM library (http://asm.ow2.org/), so we really don't have much to here in Jython except upgrade if it has been fixed.

Marking this low for now.
msg5999 (view) Author: Ian Rogers (irogers) Date: 2010-08-22.05:32:39
Not all VMs perform interpretation, for example, Jikes RVM is a compilation only VM. It doesn't trip this bug as constant propagation isn't performed in the initial compiler, but it is easy to experiment with constant propagating in this case (it may even be a moderate win) and Jython would fail 100% of the time.

Claiming the bug is in ASM is lame. ASM is a *lightweight* way of creating bytecode and particularly injecting bytecode, it is not intended to be any kind of tool to imply generated bytecode that is correct to the intent of the Java language specification.

The simplest fix to this problem is that the self field shouldn't be marked as final as it is being modified outside of the class initializer breaking the frozen intent of the class initializer. A medium-level fix is to modify the field using reflection. As the class is a singleton, the best solution is to create the singleton instance in the class initializer then rather than constructing an instance the caller should just acquire the singleton instance.

As a VM engineer this bug is a PITA. We often want to make the compiler overly aggressive to expose new sets of compiler bugs to fix. Jython's behavior means it is unsuited to this kind of aggressive bug finding.
msg6000 (view) Author: Jim Baker (zyasoft) Date: 2010-08-22.05:52:59
Ian, thanks for your very helpful analysis. It's now clear to me what the violation is occurring, and possible solutions. Also, it sounds like we should use -Xcomp in our testing, to perform these more stringent optimizations.
msg6001 (view) Author: Jim Baker (zyasoft) Date: 2010-08-22.06:45:03
Fixed in r7101.

Following Ian's analysis, I chose the option of just modifying 'self' in the generated module so it's no longer final. We will be revisiting table code generation sometime in the near future, so a quick fix seems preferable.

Changing the bug title again (closer to Alexey's original submission).

Thanks everyone here for the help on identifying and resolving this bug.
History
Date User Action Args
2010-08-22 06:45:04zyasoftsetstatus: open -> closed
resolution: fixed
messages: + msg6001
title: Jython bytecode layout causes NPE on Sun's JVM using -Xcomp option -> Jython bytecode violated JLS, causing NPE on Sun's JVM when using -Xcomp option
2010-08-22 05:52:59zyasoftsetpriority: low -> normal
assignee: zyasoft
messages: + msg6000
2010-08-22 05:32:42irogerssetmessages: + msg5999
2010-08-21 04:08:27zyasoftsettitle: Jython generates wrong bytecode that can lead to NPE on Sun's JVM when run with -Xcomp -> Jython bytecode layout causes NPE on Sun's JVM using -Xcomp option
2010-08-21 04:07:17zyasoftsetpriority: low
nosy: + zyasoft
messages: + msg5996
severity: major -> normal
title: Jython generates wrong bytecode that can lead to NPE on Sun's JVM -> Jython generates wrong bytecode that can lead to NPE on Sun's JVM when run with -Xcomp
2010-08-20 18:06:49irogerssetnosy: + irogers
messages: + msg5992
2010-05-17 13:34:55draghuramsetnosy: + draghuram
2010-05-15 15:59:43alexeysetmessages: + msg5768
2010-05-15 15:58:19alexeycreate