checkNotNull() and getClass() NPE

ある時ランダムに押し付けれたクラッシュバグのスタックトレースを眺めていたら、Preconditions.checkNotNull() の中で NPE が起きている。NPE 自体はコードから期待される挙動だが、エラーメッセージは Object.getClass() を null に対して呼んでるよ" という。けれどコードを睨んでもそんなメソッドは見当たらない。

最近は R8/D8 が過剰なインライン化などおかしなことをしがちなので、これもその例かと Baksmali で DEX のバイトコードを disassemble し眺めると、たしかに getClass() が呼ばれている。しかしデバッグ情報を追加するとかではなく getClass() を呼んでいるだけである。

そして大量にある checkNotNull() のオーバーロードのうち getClass() を呼んでいるのは引数を一つとるバージョンだけだ。

その他の状況証拠からこれは R8 の仕業に違いないと当たりをつけコードを睨むと・・・ CheckNotNullConverter.java というのがありますねえ。

曰く:

  /**
   * Replace all calls to methods marked as a check-not-null method by a call to Object.getClass(),
   * using the first argument as the receiver for the new call.
   *
   * <p>If the invoke has an out-value, the out-value is replaced by the first argument to allow
   * removing the invoke.
   */

該当コミットはこちら

なぜ以下のような超シンプルなコードを:

  public static <T> T checkNotNull(@CheckForNull T reference) {
    if (reference == null) {
      throw new NullPointerException();
    }
    return reference;
  }

わざわざこんなふうに書き直すのだろうか:

  public static <T> T checkNotNull(@CheckForNull T reference) {
    reference.getClass();
    return reference;
  }

たしかに発生する例外は同じ NPE でバイトコードのサイズは短くなるかもしれませんがねえ・・・getClass() のコストは限りなく小さいんでしょうがねえ・・・エラーメッセージ引数なしの checkNotNull() なら情報は失われませんがねえ・・・

社内の static analysis は nullability にうるさく、 checkNotNull() は警告を消す目的で広く使われている。だからこの小さな省略も現実のアプリではそれなりにサイズ削減効果があった・・・のかもしれない。


なお同じバグで困っている人はいないのかと社内のバグトラッカーを眺めると、人々は特に文句を言うこともなく粛々と NPE を直していた。もしかして知らなかったのあたしだけ・・・?