Bug 27861 - VerifyError not being caught in jdt core tests
Summary: VerifyError not being caught in jdt core tests
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 2.1   Edit
Hardware: PC Windows 2000
: P2 blocker (vote)
Target Milestone: 2.1 M4   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2002-12-06 14:10 EST by Sonia Dimitrov CLA
Modified: 2002-12-18 09:08 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sonia Dimitrov CLA 2002-12-06 14:10:17 EST
When running the JDT Core automated JUnit tests using the mechanism used by 
releng, dialogs are opened with "Java Virtual Machine Launcher - Could not find 
the main class.  Program will exit!"

This problem occurs when the tests in the head stream are compiled using the 
JDT Core compiler from the M3 build.

Problem discussed with Olivier.
Comment 1 Olivier Thomann CLA 2002-12-06 14:25:39 EST
The problem is that our test suite should catch the problem when a VerifyError 
occurs. The only way I see is to use reflection to run the class.
We could have a generic runner that would take as an argument the class name we 
want to run and invoke its main method instead of directly invoking the main 
method of the generated .class files.
The M3 compiler compiles some tests and when the tests are executed, they throw 
a VerifyError. The related bugs have been fixed in the latest compiler, but not 
in the one used for the build. Therefore we should not release these tests in 
the public test suites as long as the compiler used for the build doesn't match 
the one that fixes these issues OR the build should compile our test suites 
using the newly generated compiler (the one we submit for a build). In fact I 
prefer this latter case, because this proves that the compiler provided with 
the new build passes the tests.
Comment 2 Olivier Thomann CLA 2002-12-06 16:42:11 EST
Ignore my previous note. The problem is completely different.
I didn't find the cause of the problem yet, but the problem itself is fairly
simple. When they use the build M3 to generate the new builds, they end up with
.class files that contain wrong local number information.
An example in the compiler test suite is the following.
In org.eclipse.jdt.core.tests.runtime.LocalVirtualMachine, the bytecodes for the
method cleanupTargetPath() looks like this:
	/*  Method descriptor  #12 ()V */
	Code attribute:
		/* Stack: 5, Locals: 4 */
		0	aload_0
		1	getfield #21 <Field org.eclipse.jdt.core.tests.runtime.LocalVirtualMachine#e
valTargetPath java.lang.String>
		4	ifnonnull 8
		7	return
		8	aload_0
		9	getfield #21 <Field org.eclipse.jdt.core.tests.runtime.LocalVirtualMachine#e
valTargetPath java.lang.String>
		12	pop
		13	aload_0
		14	new #44 java.io.File
		17	dup
		18	aload 255 <<============== THIS IS COMPLETELY BOGGUS. The max local is 4. 

The same source code compiled with 2.0.2 binaries led to these bytecodes:
	/*  Method descriptor  #12 ()V */
	Code attribute:
		/* Stack: 5, Locals: 5 */
		0	aload_0
		1	getfield #21 <Field org.eclipse.jdt.core.tests.runtime.LocalVirtualMachine#e
valTargetPath java.lang.String>
		4	ifnonnull 8
		7	return
		8	aload_0
		9	getfield #21 <Field org.eclipse.jdt.core.tests.runtime.LocalVirtualMachine#e
valTargetPath java.lang.String>
		12	astore_1
		13	aload_0
		14	new #44 java.io.File
		17	dup
		18	aload_1  <<==== THIS IS FINE.

I tried to reproduce by running a M3 build. I imported all required plugins for
the compiler test suite. I loaded the compiler test suite from source and the
bytecodes are ok.

So there is something wrong in the whole process used to generate the new builds
using a M3 build. I will try to understand what the differences are between the
two build processes. Maybe we should raise the priority of this bug to P1.
Comment 3 Olivier Thomann CLA 2002-12-07 12:47:05 EST
This is not a problem with the build scripts or the ant adapter.
This is a bug in the compiler with the FAKE_USED flag on the local binding. If
the source is compiled with -preserveAllLocals, the problem disappears.
Here is a simple test case that shows the problem:

[import java.io.File;

public class Test {
	public static final boolean DEBUG = true;
	
	private void foo(File f) {}
	
	String evalTargetPath;
	
	protected void cleanupTargetPath() throws Exception {
		String targetPath = this.evalTargetPath;
		if (DEBUG) {
			foo(new File(targetPath));
		} else {
			foo(new File(targetPath));
		}
	}
}]

If you compile this source withouth -preserveAllLocals on the command line, we
end up with these bytecodes:
	protected void cleanupTargetPath() throws java.lang.Exception ;
	/*  Method descriptor  #12 ()V */
	Code attribute:
		/* Stack: 4, Locals: 1 */
		0	aload_0
		1	getfield #29 <Field Test#evalTargetPath java.lang.String>
		4	pop
		5	aload_0
		6	new #31 java.io.File
		9	dup
		10	aload 255   <========== THIS IS BOGGUS
		12	invokespecial #34 <Constructor java.io.File(java.lang.String arg)>
		15	invokespecial #36 <Method Test#foo(java.io.File arg) void>
		18	return

The resolved position of this local is -1. Therefore we should never to a load
of this local binding, but we do.
With -preserveAllLocals it becomes:

	protected void cleanupTargetPath() throws java.lang.Exception ;
	/*  Method descriptor  #12 ()V */
	Code attribute:
		/* Stack: 4, Locals: 2 */
		0	aload_0
		1	getfield #29 <Field Test#evalTargetPath java.lang.String>
		4	astore_1
		5	aload_0
		6	new #31 java.io.File
		9	dup
		10	aload_1
		11	invokespecial #34 <Constructor java.io.File(java.lang.String arg)>
		14	invokespecial #36 <Method Test#foo(java.io.File arg) void>
		17	return

The max local is different and aload 255 becomes aload_1. This code can
successfully run whereas the previous version leads to a VerifyError.

The reason why it works with 2.0.2 is that 2.0.2 doesn't use this FAKE_USED
flag. That seemed to have change since 2.0.2.

I raise the priority to 1 and the severity to blocker, since it prevents the
releng team to move to the new build script.

I will provide a patch for the ant adapter that adds -preserveAllLocals on the
command line.
Comment 4 Philipe Mulet CLA 2002-12-08 14:35:48 EST
We will need to track down all failures, and provide them as new testcases 
without the -preserveAllLocal setting in the test harness.
Comment 5 Philipe Mulet CLA 2002-12-09 06:36:08 EST
Some local variables were incorrectly tagged as not being used, and thus ended 
up optimized out incorrectly (code generator was thus provided with variables 
with -1 positions).

Fixed in latest. Will issue a patch for M3.
Comment 6 Philipe Mulet CLA 2002-12-09 06:36:28 EST
Fixed
Comment 7 Philipe Mulet CLA 2002-12-09 11:00:32 EST
Patch is available at: http://dev.eclipse.org/viewcvs/index.cgi/%7Echeckout%
7E/jdt-core-home/patches/org.eclipse.jdt.core_2.1.0.zip
Comment 8 David Audel CLA 2002-12-18 09:08:06 EST
Verified.