Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] fix proguard rules for R8 compatibility
Browse files Browse the repository at this point in the history
Context: https://www.guardsquare.com/en/products/proguard/manual/usage#keepoptionmodifiers
Context: https://www.guardsquare.com/en/products/proguard/manual/examples
Fixes: dotnet#2684

Using R8 on a pre-Android 8.0 device was crashing with:

    E/mono    (16220): Unhandled Exception:
    E/mono    (16220): Java.Lang.LinkageError: no non-static method "Landroid/runtime/UncaughtExceptionHandler;.<init>()V"
    E/mono    (16220):   at Java.Interop.JniEnvironment+InstanceMethods.GetMethodID (Java.Interop.JniObjectReference type, System.String name, System.String signature) [0x0005b] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
    E/mono    (16220):   at Java.Interop.JniType.GetConstructor (System.String signature) [0x0000c] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
    E/mono    (16220):   at Java.Interop.JniPeerMembers+JniInstanceMethods.GetConstructor (System.String signature) [0x00035] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
    E/mono    (16220):   at Java.Interop.JniPeerMembers+JniInstanceMethods.FinishCreateInstance (System.String constructorSignature, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) [0x00036] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
    E/mono    (16220):   at Java.Lang.Object..ctor () [0x00054] in <d77389624c8c4948a12589c4bd4500eb>:0
    E/mono    (16220):   at Android.Runtime.UncaughtExceptionHandler..ctor (Java.Lang.Thread+IUncaughtExceptionHandler defaultHandler) [0x00000] in <d77389624c8c4948a12589c4bd4500eb>:0
    E/mono    (16220):   at Android.Runtime.JNIEnv.Initialize (Android.Runtime.JnienvInitializeArgs* args) [0x00202] in <d77389624c8c4948a12589c4bd4500eb>:0
    E/mono    (16220):   --- End of managed Java.Lang.LinkageError stack trace ---
    E/mono    (16220): java.lang.NoSuchMethodError: no non-static method "Landroid/runtime/UncaughtExceptionHandler;.<init>()V"
    E/mono    (16220): 	at mono.android.Runtime.init(Native Method)
    E/mono    (16220): 	at mono.MonoPackageManager.LoadApplication(:21)
    E/mono    (16220): 	at mono.MonoRuntimeProvider.attachInfo(:1)
    E/mono    (16220): 	at android.app.ActivityThread.installProvider(ActivityThread.java:5853)
    E/mono    (16220): 	at android.app.ActivityThread.installContentProviders(ActivityThread.java:5445)
    E/mono    (16220): 	at android.app.ActivityThread.handleBindApplication(ActivityThread.java:5384)
    E/mono    (16220): 	at android.app.ActivityThread.-wrap2(ActivityThread.java)
    E/mono    (16220): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1545)
    E/mono    (16220): 	at android.os.Handler.dispatchMessage(Handler.java:102)
    E/mono    (16220): 	at android.os.Looper.loop(Looper.java:154)
    E/mono    (16220): 	at android.app.ActivityThread.main(ActivityThread.java:6119)
    E/mono    (16220): 	at java.lang.reflect.Method.invoke(Native Method)
    E/mono    (16220): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
    E/mono    (16220): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

Looking into it, the following method was present when using ProGuard,
but not R8:

    #1              : (in Landroid/runtime/UncaughtExceptionHandler;)
      name          : '<init>'
      type          : '()V'
      access        : 0x10001 (PUBLIC CONSTRUCTOR)
      code          -
      registers     : 4
      ins           : 1
      outs          : 4
      insns size    : 22 16-bit code units
      catches       : (none)
      positions     :
        0x0000 line=22
        0x0003 line=23
        0x0010 line=24
      locals        :
        0x0000 - 0x0016 reg=3 this Landroid/runtime/UncaughtExceptionHandler;

Then I looked at `proguard_xamarin.cfg`, and noticed something strange:

    -keep class android.runtime.** { <init>(***); }

It seemed this rule was being ignored? All the other *working* rules
were using `<init>(...);` to refer to the instance constructor.

When I reviewed ProGuard's documented syntax/grammar:

    [@annotationtype] [[!]public|final|abstract|@ ...] [!]interface|class|enum classname
        [extends|implements [@annotationtype] classname]
    [{
        [@annotationtype] [[!]public|private|protected|static|volatile|transient ...] <fields> |
                                                                        (fieldtype fieldname);
        [@annotationtype] [[!]public|private|protected|static|synchronized|native|abstract|strictfp ...] <methods> |
                                                                                            <init>(argumenttype,...) |
                                                                                            classname(argumenttype,...) |
                                                                                            (returntype methodname(argumenttype,...));
        [@annotationtype] [[!]public|private|protected|static ... ] *;
        ...
    }]

It seems `(...)` is the proper way to specify a match against any set
of arguments. Looking through ProGuard's examples, I can't find any
usage of `(***)`. It must *happen* to work, but is undocumented.

R8 does not appear to accept the `(***)` syntax at all.

~~ Other changes ~~

I expanded upon the `DexUtils` class so it can look for methods within
classes. I also adjusted one test to verify this problem is solved.
  • Loading branch information
jonathanpeppers committed Feb 14, 2019
1 parent 23c3005 commit 99a737c
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
-keep class opentk_1_0.GameViewBase { *; <init>(...); }
-keep class com.xamarin.java_interop.ManagedPeer { *; <init>(...); }

-keep class android.runtime.** { <init>(***); }
-keep class assembly_mono_android.android.runtime.** { <init>(***); }
-keep class android.runtime.** { <init>(...); }
-keep class assembly_mono_android.android.runtime.** { <init>(...); }
# hash for android.runtime and assembly_mono_android.android.runtime.
-keep class md52ce486a14f4bcd95899665e9d932190b.** { *; <init>(...); }
-keepclassmembers class md52ce486a14f4bcd95899665e9d932190b.** { *; <init>(...); }

# Android's template misses fluent setters...
-keepclassmembers class * extends android.view.View {
*** set*(***);
*** set*(...);
}

# also misses those inflated custom layout stuff from xml...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,10 +774,15 @@ public void BuildProguardEnabledProject ([Values (true, false)] bool isRelease,
Assert.IsTrue (StringAssertEx.ContainsText (File.ReadAllLines (proguardProjectPrimary), "-keep class md52d9cf6333b8e95e8683a477bc589eda5.MainActivity"), "`md52d9cf6333b8e95e8683a477bc589eda5.MainActivity` should exist in `proguard_project_primary.cfg`!");
}

var className = "Lmono/MonoRuntimeProvider;";
var dexFile = b.Output.GetIntermediaryPath (Path.Combine ("android", "bin", "classes.dex"));
FileAssert.Exists (dexFile);
Assert.IsTrue (DexUtils.ContainsClass (className, dexFile, b.AndroidSdkDirectory), $"`{dexFile}` should include `{className}`!");
var classes = new [] {
"Lmono/MonoRuntimeProvider;",
"Landroid/runtime/UncaughtExceptionHandler;",
};
foreach (var className in classes) {
Assert.IsTrue (DexUtils.ContainsClassWithMethod (className, "<init>", "()V", dexFile, b.AndroidSdkDirectory), $"`{dexFile}` should include `{className}`!");
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,60 @@ namespace Xamarin.ProjectTools
{
public static class DexUtils
{
/*
Example dexdump output:
Class #12 -
Class descriptor : 'Landroid/runtime/UncaughtExceptionHandler;'
Access flags : 0x0001 (PUBLIC)
Superclass : 'Ljava/lang/Object;'
Interfaces -
#0 : 'Ljava/lang/Thread$UncaughtExceptionHandler;'
#1 : 'Lmono/android/IGCUserPeer;'
Static fields -
#0 : (in Landroid/runtime/UncaughtExceptionHandler;)
name : '__md_methods'
type : 'Ljava/lang/String;'
access : 0x0019 (PUBLIC STATIC FINAL)
Instance fields -
#0 : (in Landroid/runtime/UncaughtExceptionHandler;)
name : 'refList'
type : 'Ljava/util/ArrayList;'
access : 0x0002 (PRIVATE)
Direct methods -
#0 : (in Landroid/runtime/UncaughtExceptionHandler;)
name : '<clinit>'
type : '()V'
access : 0x10008 (STATIC CONSTRUCTOR)
code -
registers : 3
ins : 0
outs : 3
insns size : 10 16-bit code units
catches : (none)
positions :
0x0002 line=16
locals :
#1 : (in Landroid/runtime/UncaughtExceptionHandler;)
name : '<init>'
type : '()V'
access : 0x10001 (PUBLIC CONSTRUCTOR)
code -
registers : 4
ins : 1
outs : 4
insns size : 22 16-bit code units
catches : (none)
positions :
0x0000 line=22
0x0003 line=23
0x0010 line=24
locals :
0x0000 - 0x0016 reg=3 this Landroid/runtime/UncaughtExceptionHandler;
*/

/// <summary>
/// Runs the dexdump command to see if a class exists in a dex file
/// dexdump returns data of the form:
/// Class descriptor : 'Landroid/app/ActivityTracker;'
/// </summary>
/// <param name="className">A Java class name of the form 'Landroid/app/ActivityTracker;'</param>
public static bool ContainsClass (string className, string dexFile, string androidSdkDirectory)
Expand All @@ -22,13 +72,45 @@ public static bool ContainsClass (string className, string dexFile, string andro
if (e.Data != null && e.Data.Contains ("Class descriptor") && e.Data.Contains (className))
containsClass = true;
};
DexDump (handler, dexFile, androidSdkDirectory);
return containsClass;
}

var androidSdk = new AndroidSdkInfo ((l, m) => {
Console.WriteLine ($"{l}: {m}");
if (l == TraceLevel.Error) {
throw new Exception (m);
/// <summary>
/// Runs the dexdump command to see if a class exists in a dex file *and* has a public constructor
/// </summary>
/// <param name="className">A Java class name of the form 'Landroid/app/ActivityTracker;'</param>
/// <param name="method">A Java method name of the form 'foo'</param>
/// <param name="type">A Java method signature of the form '()V'</param>
public static bool ContainsClassWithMethod (string className, string method, string type, string dexFile, string androidSdkDirectory)
{
bool inClass = false;
bool hasName = false;
bool hasType = false;
DataReceivedEventHandler handler = (s, e) => {
if (e.Data != null) {
if (e.Data.Contains ("Class descriptor")) {
inClass = e.Data.Contains (className);
hasName = false;
} else if (inClass && e.Data.Contains ("name") && e.Data.Contains (method)) {
hasName = true;
} else if (hasName && e.Data.Contains ("type") && e.Data.Contains (type)) {
hasType = true;
}
}, androidSdkDirectory);
}
};
DexDump (handler, dexFile, androidSdkDirectory);
return hasType;
}

static void DexDump (DataReceivedEventHandler handler, string dexFile, string androidSdkDirectory)
{
var androidSdk = new AndroidSdkInfo ((l, m) => {
Console.WriteLine ($"{l}: {m}");
if (l == TraceLevel.Error) {
throw new Exception (m);
}
}, androidSdkDirectory);
var buildToolsPath = androidSdk.GetBuildToolsPaths ().FirstOrDefault ();
if (string.IsNullOrEmpty (buildToolsPath)) {
throw new Exception ($"Unable to find build-tools in `{androidSdkDirectory}`!");
Expand All @@ -53,8 +135,6 @@ public static bool ContainsClass (string className, string dexFile, string andro
p.BeginOutputReadLine ();
p.WaitForExit ();
}

return containsClass;
}
}
}

0 comments on commit 99a737c

Please sign in to comment.