Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

java.lang.VerifyError: Bad type on operand stack in putfield #17

Open
boris-petrov opened this issue Dec 9, 2019 · 16 comments
Open

java.lang.VerifyError: Bad type on operand stack in putfield #17

boris-petrov opened this issue Dec 9, 2019 · 16 comments
Labels
bug Potential bug in ProGuard. needs-more-info

Comments

@boris-petrov
Copy link

ProGuard version 6.2.2 but this has been happening for a while now with older versions too.

This is when optimizations are on. With dontoptimize this error is gone. This is the full error:

java.lang.VerifyError: Bad type on operand stack in putfield
Exception Details:
  Location:
    com/company/fj.<init>(Ljava/lang/String;Z)V @6: putfield
  Reason:
    Type 'com/company/fj' (current frame, stack[0]) is not assignable to 'com/company/rest/b' (constant pool 29)
  Current Frame:
    bci: @6
    flags: { }
    locals: { 'com/company/fj', 'java/lang/String', integer }
    stack: { 'com/company/fj', 'java/lang/String' }
  Bytecode:
    0000000: 2ab7 002c 2a2b b500 1d2a 1cb5 001e b1  

Here is my configuration options:

	target '13'
	overloadaggressively
	allowaccessmodification
	mergeinterfacesaggressively
	keepparameternames
	optimizationpasses 5
	dontshrink
	dontusemixedcaseclassnames
	repackageclasses 'com.company'

Not sure how I can help with this. What information do you need from me?

@netomi
Copy link

netomi commented Dec 10, 2019

We would need more information, ideally a reproducible sample.

In your configuration I see that you use optimization in combination with -dontshrink.
This can lead to all kind of weird effects and is not suggested to be used in combination.

Can you check if the same problem appears when you remove -dontshrink?

@boris-petrov
Copy link
Author

@netomi - thanks for the answer.

Adding dontshrink is difficult as we have classes that we use dynamically and I would have to go through all of them and "keep" them manually... and I don't even know them all so it will be a tedious process. If that is very important for you and will help with debugging the issue, I could try doing it. But until a few months ago, all was working fine with optimizations enabled and dontshrink. Then after some changes in our code, that exception started happening and it still is today.

I was worried that you would need a reproduction. Let me see what I can do about that in the next days. Unfortunately we have a big app so that might be difficult. I'll let you know how it goes.

@netomi
Copy link

netomi commented Dec 10, 2019

We would need something, at least the class file of the class that fails verification.

If -dontshrink is too difficult, could you at least try disabling some optimizations that might result in structural changes:

-optimizations !class/merging/,!method/inlining/,**

@boris-petrov
Copy link
Author

boris-petrov commented Dec 10, 2019

With this configuration - optimizations '!class/merging/*', the project works fine.

If you want the class file, please give me an email to send it to and tell me whether you want the version before or after ProGuard and, if after, what configuration to use.

@netomi
Copy link

netomi commented Dec 10, 2019

Thanks for the test, that would be an indication that disabling shrinking is really the culprit. Fyi: when classes are being merged, the class that is merged into another one is not immediately being removed, this is done in the subsequent shrinking step. Now when you disable shrinking the class is not removed, but it might be out of sync. We will take a look into such cases and remove such merged classes in any case. Is disabling class merging for now an acceptable solution?

@boris-petrov
Copy link
Author

If you mean whether that configuration I pasted above is a solution - absolutely, even dontoptimize is. So am I to understand that you think you know what the issue is and you require nothing else from me for now? I'll let you know whether the problem is resolved for any new version that you release.

Thank you for the support!

@netomi
Copy link

netomi commented Dec 11, 2019

We have a clue, but without any more input its impossible to confirm. In any way, using optimization with -dontshrink is not advised. Disabling class merging will avoid most of the problems. That would also explain why it suddenly started to appear: which classes are being merged depends on many factors, even slight changes in the respective code can lead to different results.

We will try to ensure that merged classes are properly cleaned up even with dontshrink enabled, but the priority of this is not very high.

@boris-petrov
Copy link
Author

The low priority is totally fine - not a problem at all. Just a question - after you fix it some day, will it be safe to use dontshrink with optimizations enabled? You keep saying that's not a good idea but dontshrink is mandatory for us (and perhaps for lots of users) and I would like to enable optimizations and know that this is OK and not unadvised.

@netomi
Copy link

netomi commented Dec 11, 2019

It depends your use-case. From your configuration that you posted above, I assume that you have enabled obfuscation. Now when you access classes dynamically you will have to add a keep rule as otherwise they are renamed and you will not be able to access them via reflection.

Most of the time, if your app / library works with obfuscation enabled, you will get shrinking for free as all necessary keep rules are already in place.

I am not aware of many users that use -dontshrink, in fact thats the biggest benefit of ProGuard for most users, especially for Android applications.

@boris-petrov
Copy link
Author

You're right, mostly. For example, when I tried running our app without dontshrink, it blew up because the class that is used in resources/META-INF/services/ch.qos.logback.classic.spi.Configurator was removed. What I would have to do to run without dontshrink is to keep explicitly that class but allow obfuscation because that is what is done now by default - the class is obfuscated, protected from removal, and the resources/META-INF/services/ch.qos.logback.classic.spi.Configurator file is modified with the obfuscated name. There are probably more cases like this, but you are right that there aren't that many. I'll try to "fix" them and remove dontshrink if you're saying that this is better. Do you understand what I mean?

@netomi
Copy link

netomi commented Dec 11, 2019

Yes, I understand, what you describe makes sense. Such cases are an exception of course, but they should be very limited, if you can add explicit rule for such classes and can enable dontshrink you will gain a lot of benefit from using ProGuard.

@boris-petrov
Copy link
Author

A couple of more cases that come to my mind:

  1. JAX-RS endpoints - they are not used anywhere and have to be preserved manually when dontshrink is not used.
  2. When writing a library (or an application which, for example, exposes a plugin API and some utility methods) - practically nothing should be removed as that's the point - there are a bunch of methods that are entry points and nothing is used internally.

Especially in the second case dontshrink is very useful and without it it's going to be very painful (and error-prone) to protect everything from removal.

Now that I think about it, can't I just do keep allowoptimization: true, allowobfuscation: true, 'class com.company.* { *; }' to have the same effect as dontshrink and not hit the problems that you mention when using it?

@netomi
Copy link

netomi commented Dec 11, 2019

@1: I dont know how JAX-RS endpoints are declared. Via annotations or config files? Does Proguard replace them properly?

@2: you need to define the public API of your library that will also not be obfuscated, so dontshrink does not help you here imho. Basically you define the public API of your library and everything that is accessed dynamically, the remaining things will only be kept if they are actually accessed statically. Remaining things are removed.

Thats is how most of our customers apply ProGuard / DexGuard on their library.

@boris-petrov
Copy link
Author

OK, in order to not prolong this conversation too much, could you just tell me what you think of my last paragraph from the previous message (out of curiosity):

Now that I think about it, can't I just do keep allowoptimization: true, allowobfuscation: true, 'class com.company.* { *; }' to have the same effect as dontshrink and not hit the problems that you mention when using it?

Thank you again for your time!

@netomi
Copy link

netomi commented Dec 11, 2019

no that does not help. You basically allow optimization of these classes, but prevent shrinking of them, so ending up with the same problem as using -dontshrink globally.

@boris-petrov
Copy link
Author

I see. Thanks! Let's keep this issue open until you've fixed it if you're OK with that.

@netomi netomi self-assigned this Dec 18, 2019
@netomi netomi removed their assignment Dec 30, 2019
@mrjameshamilton mrjameshamilton added bug Potential bug in ProGuard. needs-more-info labels Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Potential bug in ProGuard. needs-more-info
Projects
None yet
Development

No branches or pull requests

3 participants