#531 NPE in obfuscated code after switching to ProGuard 4.11 when optimizing field/propagation/value

[Frank-Michael Moser]

After switching from ProGuard version 4.10 to 4.11, I reproducably get a NullPointerException in my application which did not occur using ProGuard upto version 4.10 without having made any changes in the configuration of ProGuard. The interesting part of the stacktrace is:

java.lang.NullPointerException
    at com.decodon.awt.ar.a(SourceFile:387)
    at com.decodon._.HX.a(SourceFile:77)
    at com.decodon._.HX.revalidate(SourceFile:166)
    at javax.swing.JComponent.setFont(Unknown Source)
    at ...
    at javax.swing.JPanel.<init>(Unknown Source)
    at com.decodon.awt.ar.<init>(SourceFile:92)
    at com.decodon._.HX.<init>(SourceFile:29)
    at ...

With

com.decodon.awt.JMultiScrollPane -> com.decodon.awt.ar:
com.decodon.project.ui.roi.ROIMultiScrollPane -> com.decodon._.HX:

from the ProGuard mapping file you get the deobfuscated stacktrace:

java.lang.NullPointerException
    at com.decodon.awt.JMultiScrollPane.getViewComponents(SourceFile:387)
    at com.decodon.project.ui.roi.ROIMultiScrollPane.getROIs(SourceFile:77)
    at com.decodon.project.ui.roi.ROIMultiScrollPane.revalidate(SourceFile:166)
    at javax.swing.JComponent.setFont(Unknown Source)
    at ...
    at javax.swing.JPanel.<init>(Unknown Source)
    at com.decodon.awt.JMultiScrollPane.<init>(SourceFile:92)
    at com.decodon.project.ui.roi.ROIMultiScrollPane.<init>(SourceFile:29)
    at ...

In the source code of JMultiScrollPane at line 387 you find

385:    Map<JComponent, JScrollPane> scrollPaneMap = this.scrollPaneMap;
386:
387:    if (scrollPaneMap == null) {
388:        return new JComponent[0];
389:    }

You'll find, that comparing a local variable with "null" throws the NPE. Obviously this should not be possible at all. After a lot of investigation I found, that the problem

• starts to occur in ProGuard V 4.11, no problem upto V 4.10,
• persists in 5.0 Beta 1 and 2.
• only occurs if field/propagation/value is enabled.

When I add

-optimizations !field/propagation/value

to my ProGuard configuration (without other changes) there is no problem anymore.

For further investigation I have attached a Zip file with

• the complete obfuscated and deobfuscated stacktraces
• the source and (unobfuscated) class file for JMultiScrollPane
• the source and (unobfuscated) class file for ROIMultiScrollPane

Furthermore for the 4 possible cases when combining version 4.10 and 4.11 with "field/propagation/value" on and off, you'll find the obfuscated classes "ar" and "HX" as well as the interesting part from the ProGuard mapping file. The Java version used in the test cases is 1.6.0_45, but the problem can be reproduced with the latest Java 7 and even 8 if using version ProGuard 5 Beta.

Please let me know, if you need further information or help to investigate the problem.

[Eric Lafortune]

Thanks for the detailed report.

The error actually occurs in "synchronized (scrollPaneMap)", because ProGuard incorrectly removes "if (scrollPaneMap == null) ..." The field is final, and it is initialized early on in the initializer, but even so the code manages to access the field when it is still null, before its initialization. You can see the path in the stack trace.

It's not a pretty construct, but ProGuard should handle it correctly. I'll see what I can do; probably not for version 5.0 though.

If you ever need to analyze optimization issues, it's easier without obfuscation (-dontobfuscate).

[Frank-Michael Moser]

Thanks for the explanation, that sounds plausible for me. So far I can live with switching off "field/propagation/value", or is there an even more specific switch to disable removing checks for null on final variables?

[Eric Lafortune]

You can avoid the problem in ProGuard 5.0 beta3, by running it with -Doptimize.conservatively

[Éric Louvard]

The issue is always present in proGuard 5.2, I could solve it by removing the "final" keyword on the field, wich prevent the optimization.
A fix would be greatly appreciate, even if it is not a pretty construct.
Regards.

[Karsten]

I'm running into the same problem with ProGuard 5.2 when optimizing the Google AppCompat library, specifically android.support.v7.widget.AppCompatButton.setBackgroundDrawable()

A null-check on a final field mBackgroundTintHelper is optimized away because it gets set to a non-null value in the constructor, however the super constructor calls the setter before initialization has finished.

It seems that to make this optimization safe ProGuard would have to check which methods can possible be called directly or indirectly from a (super-class?) constructor, and not make assumptions about field initialization in those methods. This sounds like a very difficult static analysis because the offending call need not be in the class or any of it's super classes if a constructor lets a reference to the object under construction "escape" to any outside code.

[Karsten]

Actually the AppCompatButton case seems slightly different, in that the NPE is caused by invoking a method on a null reference where the null check has been optimized away. This is the method in question:

@Override
public void setBackgroundDrawable(Drawable background) {
    super.setBackgroundDrawable(background);
    if (mBackgroundTintHelper != null) {
        mBackgroundTintHelper.onSetBackgroundDrawable(background);
    }
}

mBackgroundTintHelper is non-null after initialization, but setBackgroundDrawable() gets called from the super constructor. This is the optimized code (-optimizations !class/merging/*,!class/unboxing/enum)

.method public setBackgroundDrawable(Landroid/graphics/drawable/Drawable;)V
    .locals 2
    .prologue
    .line 84
    invoke-super {p0, p1}, Landroid/widget/Button;->setBackgroundDrawable(Landroid/graphics/drawable/Drawable;)V
    .line 86
    iget-object v0, p0, Landroid/support/v7/widget/AppCompatButton;->b:Lsm;
    .line 1077
    const/4 v1, 0x0
    invoke-virtual {v0, v1}, Lsm;->b(Landroid/content/res/ColorStateList;)V
    .line 88
    return-void
.end method

[Eric Lafortune]

A more specific workaround is to avoid optimizations on the helper fields in this class:

-keepclassmembers,allowshrinking,allowobfuscation class  android.support.v7.widget.AppCompatButton {
    ** m*Helper;
}

The support library has a few more of these constructs, so you could generalize it to

-keepclassmembers,allowshrinking,allowobfuscation class  android.support.** {
    !static final <fields>;
}