Jacoco inserting instructions between catch handlers in monitor blocks

[mkj-gram]

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:

    val obj = Any()
    try {
        synchronized(obj) {
            Log.d("Test", "Succeeded")
        }
    } catch (ex: Exception) {
        Log.e("Test", "Failed", ex)
    }

That is, having an outer try catch around a synchronized block.

The dissassembled code before jacoco is:

  public void run();
    descriptor: ()V
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=7, locals=5, args_size=1
...
        71: monitorenter
        72: getfield      #73                 // Field com/fyber/inneractive/sdk/config/k.b:Lcom/fyber/inneractive/sdk/config/k$c;
        75: dup
        76: astore        4
        78: iload_1
        79: aload         4
        81: iload_2
        82: aload         4
        84: aload_0
        85: putfield      #78                 // Field com/fyber/inneractive/sdk/config/k$c.a:Ljava/lang/String;
        88: putfield      #81                 // Field com/fyber/inneractive/sdk/config/k$c.b:Z
        91: putfield      #84                 // Field com/fyber/inneractive/sdk/config/k$c.c:Z
        94: monitorexit
        95: goto          104
        98: astore_0
        99: aload_0
       100: aload_3
       101: monitorexit
       102: athrow
       103: pop
       104: return
       105: astore_0
...
      Exception table:
         from    to  target type
             0     6   105   Class java/lang/ClassNotFoundException
             7    17   105   Class java/lang/ClassNotFoundException
            35    38   103   Class android/provider/Settings$SettingNotFoundException
            40    45   103   Class android/provider/Settings$SettingNotFoundException
            55    62   103   Class android/provider/Settings$SettingNotFoundException
            65    68   103   Class android/provider/Settings$SettingNotFoundException
            71    72   103   Class android/provider/Settings$SettingNotFoundException
            72    75    98   any
            78    94    98   any
            94    98   103   Class android/provider/Settings$SettingNotFoundException
            99   103   103   Class android/provider/Settings$SettingNotFoundException

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:

10-31 06:01:51.084 14348 14348 E AndroidRuntime: java.lang.VerifyError: Verifier rejected class com.fyber.inneractive.sdk.config.s: void com.fyber.inneractive.sdk.config.s.run() failed to verify: void com.fyber.inneractive.sdk.config.s.run(): [0x70] expected to be within a catch-all for an instruction where a monitor is held (declaration of 'com.fyber.inneractive.sdk.config.s' appears in /data/app/com.example.monitorjacoco-UHj9aK0fXwUPdts0cNWNAQ==/base.apk)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at com.example.monitorjacoco.MainActivity$1.onClick(MainActivity.java:43)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.view.View.performClick(View.java:7125)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.view.View.performClickInternal(View.java:7102)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.view.View.access$3500(View.java:801)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.view.View$PerformClick.run(View.java:27336)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:883)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:100)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:214)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:7356)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
10-31 06:01:51.084 14348 14348 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

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 method run.

Steps to reproduce

• JaCoCo version: 0.8.8 and prior
• Complete executable reproducer: Android studio project
• Steps: run in debug mode with instrumentation

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

[marchof]

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...

[mkj-gram]

Can you please provide the class files without JaCoCo instrumentation

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

[Godin]

For provided MonitorJacoco.zip execution of

./gradlew build

doesn't fail, if

Complete executable reproducer: Android studio project
Steps: run in debug mode with instrumentation

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?

[mkj-gram]

I already provided the class files in raw form that you can use in classes.zip which should be sufficient for you to see the incorrect byte code pattern jacoco generate.

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 com.fyber.inneractive.sdk.config.s, in particular the run method.

[mkj-gram]

In any case I wonder what is the value of a JaCoCo code coverage report on obfuscated class files...

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:

//App build.gradle
buildTypes {
    debug{
        testCoverageEnabled true
    }
}

and then run:

> ./ gradlew installDebug

[michaelruppen]

Hey there
We are facing the same issue like described by @mkj-gram . Are there any updates/news on this problem? I tested it with jacoco 0.8.10 and agp 7.4.2
The crash is happening also during initialization of an external sdk, which is not under our control and we have set testCoverageEnabled true for our app.
Only way currently to prevent app from crashing is setting testCoverageEnabled false, but than obviously we don't get the reports anymore for sonar.

[Godin]

@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.

[mkj-gram]

@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.

[marchof]

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.

[michaelruppen]

@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).
Can you provide me an example how i can exclude a whole library? or can i only exclude some files? and do I have to exclude the obfuscated files or the not obfuscated?
F.e. the package wich causes the crash is com.fairtiq.sdk.internal and than some obfuscated Files in it. I would prefer if i can exclude the whole package or everything inside internal/* , is this possible?

[visttux]

Hello,

We faced the same issue and implemented the following steps to "workaround" it:

1. Replaced testCoverageEnabled with enableUnitTestCoverage in our gradle configuration. This change ensured that code coverage was only enabled for unit test builds, rather than all debug builds.

2. Took precautions to ensure that the crashing code (in this case from third-party SDK) was not invoked during our unit tests.

I hope it helps!

[marchof]

@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.

[michaelruppen]

Hey @visttux

Thanks for your input, with your settings the app compiles and runs without crashes!
But now i can't find the .xml report. Here is how our jacoco config looks like:

def fileFilter = [
        // data binding
        'android/databinding/**/*.class',
        '**/android/databinding/*Binding.class',
        '**/android/databinding/*',
        '**/androidx/databinding/*',
        '**/BR.*',
        // android
        '**/R.class',
        '**/R$*.class',
        '**/BuildConfig.*',
        '**/Manifest*.*',
        '**/*Test*.*',
        'android/**/*.*',
        // dagger
        '**/*_MembersInjector.class',
        '**/Dagger*Component.class',
        '**/Dagger*Component$Builder.class',
        '**/*Module_*Factory.class',
        '**/di/module/*',
        '**/*_Factory*.*',
        '**/*Module*.*',
        '**/*Dagger*.*',
        '**/*Hilt*.*',
        // kotlin
        '**/*MapperImpl*.*',
        '**/*$ViewInjector*.*',
        '**/*$ViewBinder*.*',
        '**/BuildConfig.*',
        '**/*Component*.*',
        '**/*BR*.*',
        '**/Manifest*.*',
        '**/*$Lambda$*.*',
        '**/*Companion*.*',
        '**/*Module*.*',
        '**/*MembersInjector*.*',
        '**/*_MembersInjector.class',
        '**/*_Factory*.*',
        '**/*_Provide*Factory*.*',
        '**/*Extensions*.*',
        // sealed and data classes
        '**/*$Result.*',
        '**/*$Result$*.*',
        // adapters generated by moshi
        '**/*JsonAdapter.*'
]

def javaTree = fileTree(dir: "${buildDir}/intermediates/javac/$sourceName/classes", excludes: fileFilter)
def kotlinTree = fileTree(dir: "${buildDir}/tmp/kotlin-classes/$sourceName", excludes: fileFilter)
classDirectories.setFrom(files([javaTree, kotlinTree]))

executionData.from = fileTree(dir: project.projectDir, includes: ['**/**/*.exec', '**/**/*.ec'])

def coverageSourceDirs = ["src/main/java"]

sourceDirectories.setFrom(files(coverageSourceDirs))
additionalSourceDirs.setFrom(files(coverageSourceDirs))

reports {
    csv {
        enabled false
    }
    xml {
        enabled true
        destination file("${buildDir}/reports/jacoco/report.xml")
    }
    html {
        enabled true
        destination file("${buildDir}/coverage-report")
    }
}

Do I have to adapt anything (this was the config in the project so far)? The HTML reports are correctly created in /build/coverage-report/... so I'm confused why the xml isn't (we need the xml report for Sonar). Can anyone help?

@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

def excludedClassLoaders =  ['com.fairtiq.sdk']
def javaTree = fileTree(dir: "${buildDir}/intermediates/javac/$sourceName/classes", excludes: fileFilter, exclclassloader : excludedClassLoaders)

I get the following gradle sync error:
Caused by: groovy.lang.MissingPropertyException: Could not set unknown property 'exclclassloader' for directory '/Users/myUser/AndroidStudioProjects/myProject/app/build/intermediates/javac/integDebug/classes' of type org.gradle.api.internal.file.collections.DefaultConfigurableFileTree.

[marchof]

@michaelruppen excludeclassloader is probably the wrong setting. Regarding Gradle you have to check with the Gradle project which maintains the JaCoCo integration. See https://www.jacoco.org/jacoco/trunk/doc/integrations.html

[bkronemeyer]

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.

[marchof]

@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.

[mkj-gram]

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 :)

[marchof]

@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.

[bkronemeyer]

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

[marchof]

@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:

-javaagent:jacocoagent.jar=excludes=com/thirdparty/*

As I have no experience with Android development I can only guess that the build is based on Gradle, which wraps our Ant tasks.

[bkronemeyer]

@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.

[bkronemeyer]

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

[daisy1754]

Hi @bkronemeyer - I am facing same issue with third party library. Have you find any workaround for this ?

[bkronemeyer]

@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

buildTypes {
    debug {
        minifyEnabled true
        proguardFile file("proguard-debug.cfg")
        // whatever else needed for your debug build type
    }
}

And then in a separate proguard file referenced above we defined the following:

-dontobfuscate
-dontshrink
-dontoptimize
-dontwarn **
-keep class * {*;}
-keepattributes *

[pidanxiangjiao]

@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

buildTypes {
    debug {
        minifyEnabled true
        proguardFile file("proguard-debug.cfg")
        // whatever else needed for your debug build type
    }
}

And then in a separate proguard file referenced above we defined the following:

-dontobfuscate
-dontshrink
-dontoptimize
-dontwarn **
-keep class * {*;}
-keepattributes *

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 🤔

[pidanxiangjiao]

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.

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?

[marchof]

I can only point you our documentation here at the JaCoCo project:

• Maven Goal
• Ant Task

The Gradle integration is developed by the Grade project, please check with them.