From 14ab168aa8194eb82625f8c5ce35abadbfb0bd2a Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 14 Feb 2019 07:46:06 -0600 Subject: [PATCH] [Xamarin.Android.Build.Tasks] fix proguard rules for R8 compat (#2735) Fixes: https://github.com/xamarin/xamarin-android/issues/2684 Context: https://www.guardsquare.com/en/products/proguard/manual/usage#keepoptionmodifiers Context: https://www.guardsquare.com/en/products/proguard/manual/examples Using R8 on a pre-Android 8.0 device was crashing with: Unhandled Exception: Java.Lang.LinkageError: no non-static method "Landroid/runtime/UncaughtExceptionHandler;.()V" at Java.Interop.JniEnvironment+InstanceMethods.GetMethodID (Java.Interop.JniObjectReference type, System.String name, System.String signature) [0x0005b] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0 at Java.Interop.JniType.GetConstructor (System.String signature) [0x0000c] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0 at Java.Interop.JniPeerMembers+JniInstanceMethods.GetConstructor (System.String signature) [0x00035] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0 at Java.Interop.JniPeerMembers+JniInstanceMethods.FinishCreateInstance (System.String constructorSignature, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) [0x00036] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0 at Java.Lang.Object..ctor () [0x00054] in :0 at Android.Runtime.UncaughtExceptionHandler..ctor (Java.Lang.Thread+IUncaughtExceptionHandler defaultHandler) [0x00000] in :0 at Android.Runtime.JNIEnv.Initialize (Android.Runtime.JnienvInitializeArgs* args) [0x00202] in :0 --- End of managed Java.Lang.LinkageError stack trace --- java.lang.NoSuchMethodError: no non-static method "Landroid/runtime/UncaughtExceptionHandler;.()V" at mono.android.Runtime.init(Native Method) at mono.MonoPackageManager.LoadApplication(:21) at mono.MonoRuntimeProvider.attachInfo(:1) at android.app.ActivityThread.installProvider(ActivityThread.java:5853) at android.app.ActivityThread.installContentProviders(ActivityThread.java:5445) at android.app.ActivityThread.handleBindApplication(ActivityThread.java:5384) at android.app.ActivityThread.-wrap2(ActivityThread.java) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1545) at android.os.Handler.dispatchMessage(Handler.java:102) at android.os.Looper.loop(Looper.java:154) at android.app.ActivityThread.main(ActivityThread.java:6119) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886) 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 : '' 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; I looked at `proguard_xamarin.cfg`, and noticed something strange: -keep class android.runtime.** { (***); } It seemed this rule was being ignored? All the other *working* rules were using `(...);` to refer to the instance constructor. I reviewed [ProGuard's documented syntax/grammar][0]: [@annotationtype] [[!]public|final|abstract|@ ...] [!]interface|class|enum classname [extends|implements [@annotationtype] classname] [{ [@annotationtype] [[!]public|private|protected|static|volatile|transient ...] | (fieldtype fieldname); [@annotationtype] [[!]public|private|protected|static|synchronized|native|abstract|strictfp ...] | (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 *happened* to work, but is undocumented. R8 does not appear to accept the `(***)` syntax at all. Update `proguard_xamarin.cfg` so that it uses `(...)`, not `(***)`, allowing R8 to properly process `proguard_xamarin.cfg`. ~~ 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. [0]: https://www.guardsquare.com/en/products/proguard/manual/usage#classspecification --- .../Resources/proguard_xamarin.cfg | 6 +- .../Xamarin.Android.Build.Tests/BuildTest.cs | 9 +- .../Utilities/DexUtils.cs | 98 +++++++++++++++++-- 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Resources/proguard_xamarin.cfg b/src/Xamarin.Android.Build.Tasks/Resources/proguard_xamarin.cfg index 5d5c9affde5..e3feb7e92a1 100644 --- a/src/Xamarin.Android.Build.Tasks/Resources/proguard_xamarin.cfg +++ b/src/Xamarin.Android.Build.Tasks/Resources/proguard_xamarin.cfg @@ -14,15 +14,15 @@ -keep class opentk_1_0.GameViewBase { *; (...); } -keep class com.xamarin.java_interop.ManagedPeer { *; (...); } --keep class android.runtime.** { (***); } --keep class assembly_mono_android.android.runtime.** { (***); } +-keep class android.runtime.** { (...); } +-keep class assembly_mono_android.android.runtime.** { (...); } # hash for android.runtime and assembly_mono_android.android.runtime. -keep class md52ce486a14f4bcd95899665e9d932190b.** { *; (...); } -keepclassmembers class md52ce486a14f4bcd95899665e9d932190b.** { *; (...); } # Android's template misses fluent setters... -keepclassmembers class * extends android.view.View { - *** set*(***); + *** set*(...); } # also misses those inflated custom layout stuff from xml... diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs index f59607fe6cd..4aa8b2cdc5e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs @@ -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, "", "()V", dexFile, b.AndroidSdkDirectory), $"`{dexFile}` should include `{className}`!"); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/DexUtils.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/DexUtils.cs index f1dbe6ded7f..96b0fb2a57b 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/DexUtils.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/DexUtils.cs @@ -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 : '' + 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 : '' + 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; + */ + /// /// 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;' /// /// A Java class name of the form 'Landroid/app/ActivityTracker;' public static bool ContainsClass (string className, string dexFile, string androidSdkDirectory) @@ -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); + /// + /// Runs the dexdump command to see if a class exists in a dex file *and* has a public constructor + /// + /// A Java class name of the form 'Landroid/app/ActivityTracker;' + /// A Java method name of the form 'foo' + /// A Java method signature of the form '()V' + 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}`!"); @@ -53,8 +135,6 @@ public static bool ContainsClass (string className, string dexFile, string andro p.BeginOutputReadLine (); p.WaitForExit (); } - - return containsClass; } } }