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
Jacoco inserting instructions between catch handlers in monitor blocks #1381
Comments
Hi, we'll investigate when we find time for this. Can you please provide the class files without JaCoCo instrumentation? Thx? In any case I wonder what is the value of a JaCoCo code coverage report on obfuscated class files... |
classes.zip and the MonitorJacoco.zip contains only class files without the instrumentation. You will have to run either the android project to get jacoco to insert the instructions or do it manually on the classes.zip |
For provided MonitorJacoco.zip execution of
doesn't fail, if
means that one has to run it from Android Studio, then could you please provide an example that can be executed from the command line? |
I already provided the class files in raw form that you can use in You do not need the android studio project for anything, and I do not know how to trigger an instrumented test run through gradlew anymore than you do apparently. If unwilling to download Android Studio to test, then use the provided class files and see the output of class |
I assume there may be none but the developer is not doing it on their obfuscated files, they are doing it on an app that uses a library that has been obfuscated Potentially a way to create the debug apk with instrumentation is to modify app/build.gradle:
and then run:
|
Hey there |
@michaelruppen could you please provide source code of the project that triggers this issue and that we can easily build ourselves? This will help us investigate the root cause of the issue and so will help to find a proper way to fix it. I already asked it here, but unfortunately got only already compiled class files, which contain unusual and nonsensical exception handlers, and unclear whether these exception handlers produced by compiler or obfuscator. |
@Godin , your jacoco transformer is a class-file to class-file transformer. The input classes that jacoco fail to rewrite correctly is in classes.zip. Ergo, it is a repro of the problem. True, there is no "runner", but the input class files needed for figuring out the issue has been provided. I also provided an android studio project if you want a larger example. If you wonder why they are already compiled and not direct java sources, I cannot help with that. It is an external library that someone uploaded to maven. I do not have the sources. But again, jacoco is a class-file to class-file transformer, if you get valid input you should produce valid output. The bottom line here is that Jacoco produces invalid bytecode on input in classes.zip and I pinpointed the incorrect produced bytecode. |
Hi, JaCoCo maintainer here trying to manage expectations. This is a tough problem to solve (especially in absence of reproducer) and a solution may result in a breaking change in exec file format. Therefore it has currently no priority for us. As a workaround please consider excluding this class from instrumentation - it is an obfuscated library anyways. |
@marchof I'm not quiet familiar with this excluding in jacoco (I have never used jacoco before and haven't done the setup in the project). |
Hello, We faced the same issue and implemented the following steps to "workaround" it:
I hope it helps! |
@michaelruppen Yes, the JaCoCo agent allows exclusion by class name. See: https://www.jacoco.org/jacoco/trunk/doc/agent.html I don't know how the build/test tool you're using allows to set these parameters. |
Hey @visttux Thanks for your input, with your settings the app compiles and runs without crashes!
Do I have to adapt anything (this was the config in the project so far)? The HTML reports are correctly created in @marchof according to my config above where do i have to add "exclclassloader" (i assume i have to use that to exclude a whole sdk)? When i try adding it like this
I get the following gradle sync error: |
@michaelruppen |
Hi, is there any update on the status of this ticket? I am experiencing the same VerifyError exception with a third party dependency which may or may not be able to update the synchronized try/catch blocks in their source code. |
@bkronemeyer As we don't have an reproducer we cannot analyze this (see tag "reproducer required"). If you get the error please provide an executable reproducer, preferably a GitHub repository. |
As the original poster of this problem I feel it is my duty to report that all the information has been provided and everything needed to reproduce the issue to see the incorrectly generated byte code from Jacoco is here. I also believe that anyone reading this thread is under the same impression. Seems like @marchof has not even tried to run Jacoco on classes.zip and looked at the output and they will not consider an Android project as a reproduction. Pretty hard to be more helpful :) |
@mkj-gram @bkronemeyer I appologies, I now see that you already provided the classes. Please understand that this project is maintained by two people in their free time. Everything we do is fully transparent here on GitHub. As you can see the status is that we didn't find the time yet to work on this. And no, we don't commit on future activities (see above: "free time"). As proposed above you can exclude the affected classes in the meantime as a workaround. |
Understandable @marchof , I am curious how excluding the third party dependency would work in the jacoco task? I have attempted using the jacoco exclude mentioned above but to no degree of success so far. I want to completely exclude the third party dependency from any code injection/alteration by jacoco at all |
@bkronemeyer If I understand the issue correctly this happens at runtime. Here the only option is to instruct the agent to not instrument certain packages. See agent documentation. For example:
As I have no experience with Android development I can only guess that the build is based on Gradle, which wraps our Ant tasks. |
@marchof Thank you very much for the help, I will look into that documentation and see if I can get things to work on my end. Will post results here with details if I am successful, as it may help someone in the future as well. |
As an update: Unfortunately, I could not seem to get the third party classes to not be instrumented. I tried the jvm options you posted above and tried an offline instrumentation implementation with the jacoco ant dependency as well but for some reason it would not exclude the dependency. I have managed to reach out to the third party dependency though and they said they will look into how much effort changing all of the synchronized try/catch blocks would be |
Hi @bkronemeyer - I am facing same issue with third party library. Have you find any workaround for this ? |
@daisy1754 Actually yes, we did find a workaround for our product. I am unsure if it will work for you as well but we enabled minify in our debug build and added a proguard file to keep everything the way it was. We are unsure why this works. Probably something to do with the order of how jacoco is injected but it did prevent the build from crashing in this way so in our build.gradle we added
And then in a separate proguard file referenced above we defined the following:
|
when config like this, is the third party library code still be jacoco injected? i think the key is to prevent jacoco from instrumenting the code of specified third party library 🤔 |
Hello , after i look up for some doc, didn't find any solution to exclude classes from instrumentation, is there any configuration for jacoco offline to achieve that? |
I can only point you our documentation here at the JaCoCo project: The Gradle integration is developed by the Grade project, please check with them. |
An issue was reported to the D8 issue tracker regarding jacoco and monitor instructions:
https://issuetracker.google.com/issues/247932119
The problem seems to be come from kotlin code like the following:
That is, having an outer try catch around a synchronized block.
The dissassembled code before jacoco is:
Note that the any handler on the monitor body is split in two, from 72-75 target 98 and 78-94 target 98. Jacoco will insert an aput-boolean in between here, but assign it the outer-most try-catch handler thereby escaping a catch handler where the monitor exit instruction should be called. As a result all android vms will report a verify-error:
I've attached a debug test example where the class and method that cause the problem is in class
com/fyber/inneractive/sdk/config/s
methodrun
.Steps to reproduce
Expected behaviour
Not inserting instructions between catch handlers in monitors.
MonitorJacoco.zip
If you just want the class files without going through a reproduction, I've also attached the classes.jar here, from https://mvnrepository.com/artifact/com.fyber/marketplace-sdk.
classes.zip
The text was updated successfully, but these errors were encountered: