redex icon indicating copy to clipboard operation
redex copied to clipboard

Non-null static final field access replaced with null

Open benjaminRomano opened this issue 4 years ago • 3 comments

Background For the following RxKotlin class, D8 generates the following:

public final class Flowables {
    public static final Flowables INSTANCE = null;

    static {
        new Flowables();
    }

    private Flowables() {
        INSTANCE = this;
    }
    
    public final <T1, T2, R> Flowable<R> combineLatest(Flowable<T1> source1, Flowable<T2> source2, Function2<? super T1, ? super T2, ? extends R> combineFunction) {
        Intrinsics.checkParameterIsNotNull(source1, "source1");
        Intrinsics.checkParameterIsNotNull(source2, "source2");
        Intrinsics.checkParameterIsNotNull(combineFunction, "combineFunction");
        Flowable<R> combineLatest = Flowable.combineLatest(source1, source2, new Flowables$combineLatest$1(combineFunction));
        if (combineLatest == null) {
            Intrinsics.throwNpe();
        }
        return combineLatest;
    }
   // rest of methods...
}

With R8 this code reduces to the following:

public final class Flowables {
    public static final Flowables INSTANCE = null;

    static {
        new Flowables();
    }

    private Flowables() {
        INSTANCE = this;
    }
}

After R8 is ran, its possible for the following code to exist:

Objects.requireNonNull(Flowables.INSTANCE)

With Redex, this gets optimized to, Objects.requireNonNull(null) however, this is incorrect as INSTANCE is not guaranteed to be null. The bytecode from R8 looks suspect, but its probably good to have Redex gracefully handle this scenario. I suspect this is related to FinalInlinePassV2? Is there a recommended workaround for this beyond disabling optimizations on this class?

Ideally, this situation wouldn't occur as R8 should be able to strip out the Flowables class and consequently the Objects.requireNonNull(Flowables.INSTANCE) call.

benjaminRomano avatar Jul 30 '20 02:07 benjaminRomano

I've filed a bug report to R8 here with a minimal reproduction as well.

benjaminRomano avatar Jul 30 '20 07:07 benjaminRomano

With R8 this code reduces to the following

Can you share the dexdump output for the class after d8 and R8? I can't infer what the bytecode would like from the Java snippets, considering they will not compile.

Also, when I pull in the RxKotlin project and just built it w/ Android Studio I'm seeing different <clinit> and <init> methods than what is suggested here on the class, so I can't really give out any good info at this point.

wsanville avatar Jul 30 '20 17:07 wsanville

Before: https://gist.github.com/benjaminRomano/0c7b5d5e4e6b6b03b660cad6ec516203

After:

Class #780            -
  Class descriptor  : 'Lc/a/f/b;'
  Access flags      : 0x0011 (PUBLIC FINAL)
  Superclass        : 'Ljava/lang/Object;'
  Interfaces        -
  Static fields     -
    #0              : (in Lc/a/f/b;)
      name          : 'a'
      type          : 'Lc/a/f/b;'
      access        : 0x0019 (PUBLIC STATIC FINAL)
  Instance fields   -
  Direct methods    -
    #0              : (in Lc/a/f/b;)
      name          : '<clinit>'
      type          : '()V'
      access        : 0x10009 (PUBLIC STATIC CONSTRUCTOR)
      code          -
      registers     : 1
      ins           : 0
      outs          : 1
      insns size    : 6 16-bit code units
0fd90c:                                        |[0fd90c] c.a.f.b.<clinit>:()V
0fd91c: 2200 4006                              |0000: new-instance v0, Lc/a/f/b; // type@0640
0fd920: 7010 8827 0000                         |0002: invoke-direct {v0}, Lc/a/f/b;.<init>:()V // method@2788
0fd926: 0e00                                   |0005: return-void
      catches       : (none)
      positions     :
      locals        :

    #1              : (in Lc/a/f/b;)
      name          : '<init>'
      type          : '()V'
      access        : 0x10001 (PUBLIC CONSTRUCTOR)
      code          -
      registers     : 1
      ins           : 1
      outs          : 1
      insns size    : 6 16-bit code units
0fd928:                                        |[0fd928] c.a.f.b.<init>:()V
0fd938: 7010 df2c 0000                         |0000: invoke-direct {v0}, Ljava/lang/Object;.<init>:()V // method@2cdf
0fd93e: 6900 ce27                              |0003: sput-object v0, Lc/a/f/b;.a:Lc/a/f/b; // field@27ce
0fd942: 0e00                                   |0005: return-void
      catches       : (none)
      positions     :
      locals        :

  Virtual methods   -
  source_file_idx   : 0 ()

benjaminRomano avatar Aug 05 '20 08:08 benjaminRomano