VerifyError using Java 7 & Eclipse Juno #543

Closed
lombokissues opened this Issue Jul 14, 2015 · 13 comments

Projects

None yet

1 participant

@lombokissues
Collaborator

Migrated from Google Code (issue 470)

@lombokissues
Collaborator

👤 grant.forrester.mobiqa   🕗 Apr 10, 2013 at 16:34 UTC

What steps will reproduce the problem?

  1. Unzip the attachment.
  2. Import the Maven project to Eclipse Juno.
  3. Use Eclipse JUnit runner to run the Junit Test 'FooTest'.

What is the expected output? What do you see instead?
I'd expect the test to pass.
Instead the test fails with the following report.
java.lang.VerifyError: Bad type on operand stack in method Foo.applyTo(ZLFoo;)V at offset 25
at FooTest.verifies(FooTest.java:9)

What version of the product are you using? On what operating system?
Lombok 0.11.6
Windows 7 Professional Service Pack 1

I have also tried edge build 0.11.7 2013-03-26 01:46 UTC with same result.

Please provide any additional information below.
Eclipse Juno Service Release 1
Oracle JDK 1.7.0_17

The test passes when compiling and running from the Maven command line (both Oracle JDK6 & Oracle JDK7).
The test passes when using Eclipse Java 6 compiler and running the test in Eclipse using Oracle JDK
7.

The problem seems to relate be the inter-play between the Eclipse Java 7 compiler and Lombok.

@lombokissues
Collaborator

👤 grant.forrester.mobiqa   🕗 Apr 11, 2013 at 12:45 UTC

My initial attachment had the wrong java source and version numbers to demonstrate the problem. Here's the new attachment with source and target set to 1.7.

@lombokissues
Collaborator

👤 grant.forrester.mobiqa   🕗 Apr 11, 2013 at 12:45 UTC

🔗 LombokTest.zip

@lombokissues
Collaborator

👤 reinierz   🕗 Apr 23, 2013 at 01:41 UTC

Err, that's actually probably relatively time-consuming to solve. I'm going to push v0.11.8 out with this bug open. I am going to mark it as accepted though, because while I haven't personally verified it (that would involve downloading a bunch of tools), the bug reporter has obviously put in all due care.

It could be three different things:

  • Our rewrite-classfile-as-it-is-written-to-disk mechanism which actually removes the call to Lombok.sneakyThrow entirely is causing this issue. This should be easy to figure out, and easy to solve.
  • We have to read in your method, change it, and write it back out, at the class file level. ASM does this for us. Even though we only change a tiny bit, it is possible the operation of running the whole thing through ASM just breaks things in this specific case due to a bug in ASM.
  • This has nothing whatsoever to do with us rewriting your class file post compilation, instead, something in lombok is putting the ecj AST into an invalid state, and sometimes this leads to invalid class files instead of the more usual exceptions during compilation.

grant.forrester.mobiqa: The brand new release (0.11.8) has a new feature in it (not even the latest edge has this, I made it to help this bug report): If you add the 'lombok.disablePostCompiler=true' option to your eclipse.ini ( -Dlombok.disablePostCompiler=true immediately before -javaagent:lombok.jar in your eclipse.ini), then post compilation will be disabled. This means that @ SneakyThrows will actually compile down to Lombok.sneakyThrows and thus you'd need lombok.jar at runtime which is bad, but, if that solves this bug we now know it is either due to ASM or due to a bug in one of our rewriters. For extra credit, try -Dlombok.debugAsmOnly=true and see what happens now - this will cycle the code through ASM but actually not make any changes, so, if you get a buggy classfile with only that switch, it's an ASM issue which probably means we need to update that. If that is the issue, could you attach the class file produced when you use -Dlombok.disablePostCompiler=true, i.e., the one that DOES work? That gives us a specific class file to experiment with.

thanks a bunch!

NB: Download is here: https://projectlombok.org/download.html - and 0.11.8 should be available from maven central in a few hours.

***** WARNING: Before marking this bug as verified, remove the debugAsmOnly code throughout the codebase! *****

@lombokissues
Collaborator

👤 Yannis.BRES   🕗 May 27, 2013 at 14:33 UTC

I do confirm that adding -Dlombok.disablePostCompiler=true in eclipse.ini just before -javaagent:lombok.jar DOES solve the issue.
Setting only -Dlombok.debugAsmOnly=true in eclipse.ini just before -javaagent:lombok.jar DOES NOT solve the issue.
(Just in case, adding both parameters DOES solve the issue.)

This issue is actually not related to JRebel and attached is a very simple class that reproduces the issue.
compile with : javac -cp lombok.jar SneakyThrowsTest.java
run with : java SneakyThrowsTest
Exception in thread "main" java.lang.VerifyError: Bad type on operand stack in method SneakyThrowsTest.MethodWithInvalidByteCode()V at offset 29
at java.lang.Class.getDeclaredMethods0(Native Method)
at java.lang.Class.privateGetDeclaredMethods(Class.java:2451)
at java.lang.Class.getMethod0(Class.java:2694)
at java.lang.Class.getMethod(Class.java:1622)
at sun.launcher.LauncherHelper.getMainMethod(LauncherHelper.java:494)
at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:486)
MethodWithInvalidByteCode calls a method with an argument that may have multiple possible dynamic types and, apparently,

I hope this will help and thanks for your work !

In case you have ideas for a blind fix, we can try a snapshot build !

@lombokissues
Collaborator

👤 Yannis.BRES   🕗 May 27, 2013 at 14:33 UTC

🔗 SneakyThrowsTest.java View file

@lombokissues
Collaborator

👤 reinierz   🕗 May 27, 2013 at 18:06 UTC

Wow, awesome. Test case reliably triggers this problem. Will you be at devoxx or near Delft at any time soon for your complementary beverage of choice for awesome bug reports?

We're working on it now.

@lombokissues
Collaborator

👤 Yannis.BRES   🕗 May 27, 2013 at 22:13 UTC

Oh come on ! I will buy you a drink at Devoxx ! (If we ever manage to organize a meeting without reliable WiFi... ;-) )
Thanks again for this great project !

@lombokissues
Collaborator

👤 reinierz   🕗 May 28, 2013 at 10:52 UTC

Eeeeeesh, okay, this is basically about how re-analysing class files, at least with objectweb's ASM, is basically impossible now. To do anything beyong the trivial, you need to muck about with inserting frames into the class file, but ASM's handling of this seems kinda weird. If you let ASM sort it all out, you MUST provide a method that determines, given 2 classnames in string form, the classname of the common supertype. We thought you could just return 'Object' safely here but that is what is triggering this bug.

To avoid this, we have to do our own frame calculation but the code we would prefer to generate is incompatible with that idea, because the compiler cannot handle opcodes following an ATHROW.

Fortunately, we do have a fix which is reasonable:

proper usage of Lombok.sneakyThrow, which is:

  • via @ SneakyThrows
  • by writing 'throw Lombok.sneakyThrow(someEx);'

will work fine, but improper usage of Lombok.sneakyThrow, which is:

  • writing 'Lombok.sneakyThrow(someEx);' without throwing the return value of this method invocation.

will no longer get the call expunged from the class file. Meaning, you would need lombok.jar on the classpath.

We emit a warning in javac, but we don't do anything in eclipse because we can't report errors reliably at the point where we realize this is happening (or we have to add some relatively significant overhead to ALL eclipse processing by scanning for calls to Lombok.sneakyThrow which doesn't seem to be worth it).

We've got this in a local branch now and will probably commit it as the fix for this issue if we can't think of something better.

More generally, I hope people aren't making big plans for the post processor API of lombok itself, because, basically, you're going to have to do your own frame calculation which is pretty much undoable. Presumably there's a reason ASM can't actually pull it off without a 'common type' oracle.

NB: One alternative is to actually implement the 'what is the common supertype' class properly for each compiler environment. After all, the compiler would know what the common supertype is of any 2 given types present in a file it just compiled. We don't need to go that far just for sneakyThrows fortunately.

@lombokissues
Collaborator

👤 Yaya.at.Work   🕗 Jul 05, 2013 at 08:34 UTC

Hi !

Any progress on this issue ?

Thanks in advance for your answer !

@lombokissues
Collaborator

👤 Yaya.at.Work   🕗 Aug 01, 2013 at 17:52 UTC

The changelog for release 0.12 mentions
BUGFIX: Usage of Lombok.sneakyThrow() or @ SneakyThrows would sometimes result in invalid classes (classes which fail with VerifyError). Issue #543
although this issue is not marked as fixed.

However, one of our developers has tried the new version and noticed that it now works without adding -Dlombok.disablePostCompiler=true in eclipse.ini.

Thanks a lot and let's not forget this beer @ Devoxx 2013 !

@lombokissues
Collaborator

👤 reinierz   🕗 Oct 26, 2013 at 21:20 UTC

Awesome. I'm googling for a different issue, randomly stumble on this, read through this because it's on 'accepted', and finally realize that we should have marked this bug as 'verified' earlier. My mistake. verified now :0

@lombokissues lombokissues removed the accepted label Jul 14, 2015
@lombokissues
Collaborator

End of migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment