2

I have the following statement in my proguard-rules.pro to strip out logging in the production apk:

-assumenosideeffects class android.util.Log {
    public static boolean isLoggable(java.lang.String, int);
    public static int v(...);
    public static int i(...);
    public static int w(...);
    public static int d(...);
    public static int e(...);
}

This has the effect of removing statements like:

Log.d(TAG, "setup finished");

Now let's consider the following statement:

Log.d(TAG, "setup finished with status " + client.getStatus());

My assumption until now was that this Log.d() statement would be stripped out and therefore that client.getStatus() would never get called.

Specifically, if client happens to be null then my assumption had been that there would be no NullPointerException.

But now I'm not so sure. Perhaps the compiler looks at the above statement more like the following:

int status = client.getStatus();
Log.d(TAG, "setup finished with status " + status);

Now the client.getStatus() statement is executed and NullPointerException may result.

What is the actual situation?

Note: I'm actually now using R8 rather than ProGuard.

drmrbrewer
  • 11,491
  • 21
  • 85
  • 181
  • consider looking at a library like Timber https://github.com/JakeWharton/timber if you're really paranoid about production logs – a_local_nobody Jul 25 '19 at 07:43
  • @a_local_nobody I'm not paranoid about what appears in the logs... I'm pretty confident that in the above example the obfuscator will prevent the `status` from appearing in the logs, even if the `status` is actually fetched. I'm just trying to track down the elusive source of a `NullPointerException` in my production apk and now I'm starting to consider what I am executing in these `Log` statements, whereas previously I'd assumed that they would never result in a `NullPointerException` because they are stripped out completely. – drmrbrewer Jul 25 '19 at 07:51
  • if it is removed from code on preprocessing, how can it be executed ?? – Antoniossss Jul 25 '19 at 08:34
  • use a wrapper class around the normal logs....proguard did not work in my case and the logs were still on in production. I did a project-wide search and changed the name of the log statement to my custom class. I also have specific sequence of buttons that turn on logging in the production app. Pretty random clicks that no one would do. – Pemba Tamang Jul 25 '19 at 08:44
  • @Antoniossss look at the code snippet in the question in which the `client.getStatus()` statement is separated out from the `Log.d()` statement. Even if the `Log.d()` statement is removed from the code during preprocessing, this still leaves the `client.getStatus()` statement! And it is the `client.getStatus()` statement that may be the source of a `NullPointerException`, not the `Log.d()` statement per se. Or is it the case that R8/ProGuard will also (automatically) remove the `client.getStatus()` statement because its result is never used? – drmrbrewer Jul 25 '19 at 08:50
  • @Antoniossss but yes, that has always been my assumption, i.e. "if it is removed from code on preprocessing, how can it be executed??". The purpose of this question is to challenge whether this assumption is correct, and to ask whether proguard/R8 effectively perform their work only after the `Log.d()` statement has been refactored to move any inline statements like `client.getStatus()` outside of the `Log.d()` statement. – drmrbrewer Jul 25 '19 at 09:05
  • IMHO sounds like testable in 5 minutes. Just provide null client in test code... and you will see if you get NPE or not. – Antoniossss Jul 25 '19 at 09:26
  • @Antoniossss I just did... see my answer. – drmrbrewer Jul 25 '19 at 16:12
  • Possible duplicate of [Removing unused strings during ProGuard optimisation](https://stackoverflow.com/questions/6009078/removing-unused-strings-during-proguard-optimisation) – Antoniossss Jul 25 '19 at 16:38

1 Answers1

0

OK so this came as a surprise to me, and possibly to others (@Antoniossss), but it seems that statements executed inside a Log.d() are executed despite ProGuard/R8 being configured to remove all Log.d() statements.

So, even with ProGuard/R8 being configured to remove all Log.d() statements, the following will result in a NullPointerException:

String myString = null;
Log.d(TAG, "myString length: " + myString.length());

So, in future I will remember to check these Log statements more carefully when reviewing my code to find the source of a NullPointerException from my Developer Console, and not just pass over them assuming that they cannot have been the cause.

drmrbrewer
  • 11,491
  • 21
  • 85
  • 181
  • 1
    On which line it is thrown? – Antoniossss Jul 25 '19 at 16:34
  • nice link, thanks @Antoniossss. Which line? All the logcat says is `Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'int java.lang.String.length()' on a null object reference`, and below that it gives the method in which the call was made... how can I get more specific information about *where* in the method the offending call was made? – drmrbrewer Jul 25 '19 at 17:14
  • huh you wont. If the stacktrace does not contain line number, it simply means numbers were striped down during compilation. But anyway, we know what causes it. – Antoniossss Jul 25 '19 at 19:28