Bugzilla – Full Text Bug Listing
| Summary: | [1.7][compiler]: VerifyError in try statement with finally and return statements | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | kurt <kurt2002> |
| Component: | Core | Assignee: | Olivier Thomann <Olivier_Thomann> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | deepakazad, kurt2002, Olivier_Thomann, srikanth_sankaran |
| Version: | 3.7 | Flags: | srikanth_sankaran:
review+
|
| Target Milestone: | 3.7.1 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Attachments: | |||
What is your build id ? On 3.7/HEAD I don't see this problem. I do see this issue however on the JAVA7 branch. Are you using the beta version ? Please confirm. (Also what JRE are you using ?) Interestingly, this problem shows up only when compiling with 1.7 level
and does not show up when compiling with source level 1.6.
More curiosly, the code generated is the same in both cases. A comparison
of the 1.6 class file and 1.7 class file shows that only the class file
major version number and check sum are different between the files. Everything
else is the same.
Slightly smaller test case:
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
public class E7Bug {
public boolean contentEquals(final Path src, final Path tar) throws IOException {
int lrSrc;
InputStream isSrc = null;
InputStream isTar = null;
try {
isSrc = Files.newInputStream(src);
if (isSrc == null)
return false;
isTar = Files.newInputStream(tar);
if (isTar == null)
return false;
do {
lrSrc = 1;
} while ((lrSrc >= 0));
} finally {
System.out.println("Finally");
}
return true;
}
public static void main(final String[] args) {
System.out.println("Done");
}
}
(In reply to comment #1) > What is your build id ? On 3.7/HEAD I don't see this problem. My mistake, I do see this problem on 3.7/HEAD, but only at 1.7 compliance. How did you set choose 1.7 compliance in 3.7 since it doesn't support choosing this level ? (there are other means to select 1.7, but checking your scenario). (In reply to comment #2) > Interestingly, this problem shows up only when compiling with 1.7 level > and does not show up when compiling with source level 1.6. > > More curiosly, the code generated is the same in both cases. A comparison > of the 1.6 class file and 1.7 class file shows that only the class file > major version number and check sum are different between the files. Everything > else is the same. If I binary edit the 1.6 class file and set the major version number to 1.7 (51), I get the verify error. Conversely, if I start with the 1.7 class file which fails to verify and change just the major version number and set it to 1.6 (50), it starts to verify fine. So this looks like a case of the VM tightening its behavior on code that used to verify alright earlier. Under investigation. At the outset this looks a problem in code generation for nio related
classes.
The local variable table looks odd:
Start Length Slot Name Signature
0 81 0 this LE7Bug;
0 81 1 src Ljava/nio/file/Path;
0 81 2 tar Ljava/nio/file/Path;
21 10 3 lrSrc I
51 7 3 lrSrc I
71 10 3 lrSrc I
3 78 4 isSrc Ljava/io/InputStream;
6 75 5 isTar Ljava/io/InputStream;
See the multiple entries for lrSrc - at first glance, this doesn't seem
to make sense.
(In reply to comment #4) > At the outset this looks a problem in code generation for nio related > classes. Not connected to nio classes at all. Here is a further simplified example: public class X { public static void main(String[] args) { int lrSrc; Object isSrc = null, isTar = null; try { if (isSrc == null) return; if (isTar == null) return; do { lrSrc = 1; } while ((lrSrc >= 0)); } finally { lrSrc = 0; } } } Even smaller test:
public class X {
public static void main(String[] p) {
int i;
try {
if (p == null || p == null)
return;
i = 0;
} finally {
i = 0;
}
}
}
(In reply to comment #6) Generated code for test case in comment#6 is: public static void main(java.lang.String[]); flags: ACC_PUBLIC, ACC_STATIC Code: stack=1, locals=3, args_size=1 0: aload_0 1: ifnull 8 4: aload_0 5: ifnonnull 11 8: iconst_0 9: istore_1 10: return 11: iconst_0 12: istore_1 13: goto 21 16: astore_2 17: iconst_0 18: istore_1 19: aload_2 20: athrow 21: iconst_0 22: istore_1 23: return Exception table: from to target type 0 8 16 any 11 16 16 any LineNumberTable: line 5: 0 line 9: 8 line 6: 10 line 7: 11 line 8: 16 line 9: 17 line 10: 19 line 9: 21 line 11: 23 LocalVariableTable: Start Length Slot Name Signature 0 24 0 p [Ljava/lang/String; 8 3 1 i I 13 3 1 i I 19 5 1 i I Messages from IBM JVM is: Exception in thread "main" java.lang.VerifyError: JVMVRFY012 stack shape inconsistent; class=X, method=main([Ljava/lang/String;)V, pc=1 at java.lang.J9VMInternals.verifyImpl(Native Method) at java.lang.J9VMInternals.verify(J9VMInternals.java:77) at java.lang.J9VMInternals.initialize(J9VMInternals.java:139) Message from Oracle JVM is more useful: it says: Exception in thread "main" java.lang.VerifyError: Inconsistent stackmap frames a t branch target 8 in method X.main([Ljava/lang/String;)V at offset 1 at java.lang.Class.getDeclaredMethods0(Native Method) at java.lang.Class.privateGetDeclaredMethods(Class.java:2442) at java.lang.Class.getMethod0(Class.java:2685) at java.lang.Class.getMethod(Class.java:1620) at sun.launcher.LauncherHelper.getMainMethod(LauncherHelper.java:484) at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:476) The operand stack is empty when you reach 8 irrespective of whether you branch to 8 or fall through to 8. If you initialize i = 0 along with its declaration, verify error goes away. What gives ? (In reply to comment #7) > (In reply to comment #6) > If you initialize i = 0 along with its declaration, verify error goes away. > > What gives ? When i is initialized to 0 the local variable looks like: LocalVariableTable: Start Length Slot Name Signature 0 26 0 p [Ljava/lang/String; 2 24 1 i I as opposed to the uninitialized case: LocalVariableTable: Start Length Slot Name Signature 0 24 0 p [Ljava/lang/String; 8 3 1 i I 13 3 1 i I 19 5 1 i I Similarly, the stack map table when i is initialized to 0 looks like: StackMapTable: number_of_entries = 4 frame_type = 252 /* append */ offset_delta = 10 locals = [ int ] frame_type = 2 /* same */ frame_type = 68 /* same_locals_1_stack_item */ stack = [ class java/lang/Throwable ] frame_type = 4 /* same */ as opposed to the uninitialized case: StackMapTable: number_of_entries = 4 frame_type = 252 /* append */ offset_delta = 8 locals = [ int ] frame_type = 250 /* chop */ offset_delta = 2 frame_type = 68 /* same_locals_1_stack_item */ stack = [ class java/lang/Throwable ] frame_type = 252 /* append */ offset_delta = 4 locals = [ int ] (In reply to comment #4) > At the outset this looks a problem in code generation for nio related > classes. > > The local variable table looks odd: [...] > See the multiple entries for lrSrc - at first glance, this doesn't seem > to make sense. These represent three distinct regions where the variable gets initialized and goes out of scope - so perhaps there is nothing wrong here. (In reply to comment #8) Olivier, I am not familiar with the stack map table. Does the below look reasonable ? > Similarly, the stack map table when i is initialized to 0 looks like: > > StackMapTable: number_of_entries = 4 > frame_type = 252 /* append */ > offset_delta = 10 > locals = [ int ] > frame_type = 2 /* same */ > frame_type = 68 /* same_locals_1_stack_item */ > stack = [ class java/lang/Throwable ] > frame_type = 4 /* same */ > > as opposed to the uninitialized case: > > StackMapTable: number_of_entries = 4 > frame_type = 252 /* append */ > offset_delta = 8 > locals = [ int ] > frame_type = 250 /* chop */ > offset_delta = 2 > frame_type = 68 /* same_locals_1_stack_item */ > stack = [ class java/lang/Throwable ] > frame_type = 252 /* append */ > offset_delta = 4 > locals = [ int ] Released (disabled) junit org.eclipse.jdt.core.tests.compiler.regression.TryStatementTest._test074(). (This fails on 1.6 as well, while outside of the IDE, I could see the problem only with 1.7 - not sure what is causing this difference) The bug comes from the first stack map entry.
Stack map table: number of frames 5
[pc: 8, append: {int, Object, Object}]
[pc: 11, full, stack: {}, locals: {String[], _, Object, Object}]
[pc: 18, same]
[pc: 27, same_locals_1_stack_item, stack: {Throwable}]
[pc: 34, full, stack: {}, locals: {String[], int, Object, Object}]
[pc: 8, append: {int, Object, Object}]
this means that the local lrSrc is initialized when getting to pc 8. We should have:
[pc: 8, append: {_, Object, Object}]
The sharing of the inlined subroutine might cause confusion in the local variable table shape. I'll take a look.
If we have:
[pc: 8, append: {_, Object, Object}]
then in 11, there is nothing to do and we would end up with:
[pc: 11, same]
...
(In reply to comment #11) > The bug comes from the first stack map entry. > Stack map table: number of frames 5 > [pc: 8, append: {int, Object, Object}] > [pc: 11, full, stack: {}, locals: {String[], _, Object, Object}] > [pc: 18, same] > [pc: 27, same_locals_1_stack_item, stack: {Throwable}] > [pc: 34, full, stack: {}, locals: {String[], int, Object, Object}] > > [pc: 8, append: {int, Object, Object}] > this means that the local lrSrc is initialized when getting to pc 8. We should > have: > [pc: 8, append: {_, Object, Object}] > > The sharing of the inlined subroutine might cause confusion in the local > variable table shape. I'll take a look. Thanks. Any idea why this would work alright with JDK6 ? is it just a case of the verification being tightened at JDK7 ? (In reply to comment #11) > The bug comes from the first stack map entry. > Stack map table: number of frames 5 > [pc: 8, append: {int, Object, Object}] > [pc: 11, full, stack: {}, locals: {String[], _, Object, Object}] > [pc: 18, same] > [pc: 27, same_locals_1_stack_item, stack: {Throwable}] > [pc: 34, full, stack: {}, locals: {String[], int, Object, Object}] Where is this output from BTW ? i.e which tool outputs this ? I would say this is a tightening of bytecode verification. I found one place where some variables from the natural exit state are injected and it looks bogus. Fixing it failed some tests, but looking at the failures (bytecode output mostly), it looks like the previous code was wrong. I am running all compiler tests with this patch and I am also looking at recompiling all eclipse SDK with this patch to see if Eclipse can start on JDK7. If everything is fine, I'll ask for a review. This is a really sensitive area where we must be extremely cautious. (In reply to comment #13) > Where is this output from BTW ? i.e which tool outputs this ? This is using our own disassembler in system mode. I almost never use javap to look at bytecode. I prefer our own output. Created attachment 199511 [details] Proposed fix + updated regression tests Srikanth, please review. Basically we cannot reinject locals from the natural exit for all subroutine inlining. This was a mistake made a long time ago and I am surprised nobody found it before. The state index that we set is used to reinject the desired locals at the right locations. I also moved the regression tests to the stack map test suite where I think it fits better. That patch passes all existing and new tests + recompilation of the SDK as explained in: http://jroller.com/andyl/entry/compiler_championship_continued (In reply to comment #16) > Created attachment 199511 [details] > Proposed fix + updated regression tests > > Srikanth, please review. AFAICT - the patch looks good. A couple of nits: (1) Java7 disclaimer is missing from copyright of StackMap*Test.java (2) The longer version of test is skipped for < 1.5 with a comment saying annotations are used - I didn't find annotations in the test. Varargs are used though. That said this test anyways runs only for 1.6 and above. (I had forgotten about 1.6 verifier fallback and that being disallowed for 1.7 - which explains both why things work at 1.6 as well as the junit vs IDE difference in behavior) (In reply to comment #17) > (1) Java7 disclaimer is missing from copyright of StackMap*Test.java Fixed. > (2) The longer version of test is skipped for < 1.5 with a comment saying > annotations are used - I didn't find annotations in the test. Varargs are > used though. That said this test anyways runs only for 1.6 and above. I removed the check as the StackMapAttributeTest runs only in 1.6 mode and above. Released in the BETA_JAVA7 branch. Sorry that I forgot to add the details about version - but you found the bug pretty soon anyway - congratulations! Here are my details - just in case: Eclipse: Version: 3.7.0 Build id: I20110613-1736 Java 7: java version "1.7.0" Java(TM) SE Runtime Environment (build 1.7.0-b147) Java HotSpot(TM) Server VM (build 21.0-b17, mixed mode) OS: Ubuntu 10.04 Just installed the latest JAVA_BETA_7 plugins and still get the error: Exception in thread "main" java.lang.VerifyError: Inconsistent stackmap frames at branch target 244 in method E7Bug2.copyDir(Ljava/util/List;ZZZLjava/io/File;Ljava/io/File;Ljava/io/FileFilter;)Ljava/util/List; at offset 236 at java.lang.Class.getDeclaredMethods0(Native Method) at java.lang.Class.privateGetDeclaredMethods(Class.java:2442) at java.lang.Class.getMethod0(Class.java:2685) at java.lang.Class.getMethod(Class.java:1620) at sun.launcher.LauncherHelper.getMainMethod(LauncherHelper.java:484) at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:476) I did not check for a workaround this time. Code in attachment (E7Bug2.java) Greetings Kurt Created attachment 199688 [details]
java.lang.VerifyError: Inconsistent stackmap frames at branch target 244 in method E7Bug2.copyDir(Ljava/util/List;ZZZLjava/io/File;Ljava/io/File;Ljava/io/FileFilter;)Ljava/util/List; at offset 236
see my last comment no. 19
I'll open a new bug report for this one as it is not related to the same bug. The previous one was a bug in the try statement specifically. Closing as FIXED. Please let me know the bug-id (and/or include me in the cc-list) - I am just too curious ;-) Thanks Kurt (In reply to comment #22) > Please let me know the bug-id (and/or include me in the cc-list) - I am just > too curious ;-) For sure. bug 352145. Thanks for your awesome test case. Should be fixed for the next refresh. Verified with v20110714-1300. |
Created attachment 199388 [details] java-source-example to reproduce the eclipse-compiler-bug Depending on where local variables are declared I get a VerifyError at runtime: Exception in thread "main" java.lang.VerifyError: Instruction type does not match stack map in method E7Bug.contentEquals(Ljava/nio/file/Path;Ljava/nio/file/Path;)Z at offset 73 at java.lang.Class.getDeclaredMethods0(Native Method) at java.lang.Class.privateGetDeclaredMethods(Class.java:2442) at java.lang.Class.getMethod0(Class.java:2685) at java.lang.Class.getMethod(Class.java:1620) at sun.launcher.LauncherHelper.getMainMethod(LauncherHelper.java:484) at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:476) In the attached example see two the "// --- defining variables here" comments on how to reproduce the bug. Greetings Kurt