#621 assumesideeffects leaves traces when inner classes are involved

[TWiStErRob]

First of all, sorry if it's a little convoluted, I worked on it for a day to reduce the problem to this size; this is how far I got. In the ticket description I only show the relevant parts for the explanation, the full example can be only reprodced if you download the attachment.

The original issue came up with an Android project, but I managed to reduce it to a Java project with a fake android.jar on the classpath. To reproduce all you need is a JDK and Gradle 2.12+, all the rest is done by the build script (even the Android deps).

My goal is to completely remove logging, that is any references to Logger and LoggerFactory; the LOG fields in all classes. For this the usual -assumenosideeffects ... { void info(...); } works most of the time, except in the below cases.

src/main/java/MyFragment.java

public class MyFragment extends android.support.v4.app.Fragment  {
    ? static final Logger LOG = LoggerFactory.getLogger(MyFragment.class);
    someMethod() {
        new InnerClass() {
            innerMethod() {
                LOG.info("log"); // accesssing the outer LOG variable
            }
        };
    }
}

LOG field visibility can be private or default, it triggers two different results, but both of them equally keep the LOG field and the <clinit> initialization.

non-private LOG (Fragment1.java)

I used default visibility, the key is that the inner class has access to the field. The result after proguard optimizations:

// declaration and initialization in <clinit>
static final Logger LOG = LoggerFactory.getLogger(MyFragment1.class);
// usage replaced with this:
8: getstatic #8 // Field MyFragment1.LOG:Lorg/slf4j/Logger;
11: pop

In my view this is equivalent to writing LOG; in Java, which is a no op (doesn't even compile).
And because this getstatic is referecing the field, it cannot be removed and the whole logging framework is kept. While trying to figure it out I've seen somewhere that just referencing a field can have side effects (I guess they meant that it can trigger loading a class and execute <clinit> . In desperation I tried: -assumenosideeffects class ** { final org.slf4j.Logger LOG; } which had no effect.

private LOG (Fragment2.java)

If the field is private, it triggers Java's by-design hacks and we get a synthetic accessor method in addition to the above:

// declaration and initialization in <clinit>
static final Logger LOG = LoggerFactory.getLogger(MyFragment2.class);
// synthetic in outer class
Logger access$000() { return LOG; }
// usage replaced with this:
8: invokestatic #9 // Method MyFragment2.access$000:()Lorg/slf4j/Logger;
11: pop

This is equivalent to access$000(); which could have side effects, because it's a method, but it only returns LOG. With this version I managed to get rid of leftovers via:

-assumenosideeffects class ** {
    static synthetic org.slf4j.Logger access$*();
}

Summary

I think the assumenosideeffects on getLogger should propagate through the field it is assigned to and/or the accessor method and the above cases should "just work" when trying to ignore logging calls, just like it works when there's no inner class involved (see MyFragment3).

I'm really sorry that I couldn't give you a simpler repro, for some reason if I change the super class the repro simply doesn't work. I tried to do something similar with JComponent and custom super class, but neither reproed the issue.

To run the attacment use (use gradlew if you don't have Gradle installed):

cls && gradle clean decompileJavap --info

Versions (Proguard 5.2.1, see build.gradle):

p:\projects\bugs\proguard-log>gradle --version
Gradle 2.14.1
Groovy:       2.4.4
Ant:          Apache Ant(TM) version 1.9.6 compiled on June 29 2015
JVM:          1.8.0_66 (Oracle Corporation 25.66-b17)
OS:           Windows 10 10.0 amd64

p:\projects\bugs\proguard-log>javap -version
1.8.0_66

p:\projects\bugs\proguard-log>java -version
java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)

[Eric Lafortune]

Thanks for your detailed report and sorry for the late reply. Your analysis is correct. The underlying cause is not entirely trivial. The getstatic/pop instructions are only preserved because they trigger the static initializer of the outer class. This static initializer is kept because it initializes the static LOG field. The static LOG field is kept because it is written and read by these classes. It's a bit of a catch-22 situation that would require a more complex analysis to resolve automatically.

You can work around it by explicitly stating that the static initializer doesn't have any side-effects (as far as other classes are concerned):

-assumenosideeffects class MyFragment* {
    void <clinit>();
}

This true for most classes; only very rarely code relies on the side-effects of class initialization triggered by accessing one of its static fields.

ProGuard's sibling DexGuard has a few more powerful options that can express this more accurately, in this case to the same effect:

-assumenoexternalsideeffects class MyFragment* {
    void <clinit>();
}

[TWiStErRob]

Thanks for taking a look. Does status "works-for-me" mean that you couldn't reproduce the issue based on my repro project? Can I help somehow?

[Eric Lafortune]

I can reproduce the behavior with the samples and similar code. Removing all traces of unused code is hard and sometimes impossible. The best solution for this case right now is to specify the extra assumption. I'll think about an automatic solution in the longer term, but it's not a real bug, hence the approximation "open-works-for-me". Thanks!