Fix crash when hooked static method invokes class initialization

appveyor.yml

@@ -1,4 +1,4 @@
-version: '0.4.4.7_alpha({build})'
+version: '0.4.5.0_alpha({build})'
 
 environment:
   ANDROID_HOME: C:\android-sdk-windows

edxp-common/src/main/java/com/elderdrivers/riru/edxp/art/ClassLinker.java

@@ -1,8 +1,10 @@
 package com.elderdrivers.riru.edxp.art;
 
 import com.elderdrivers.riru.common.KeepAll;
+import com.elderdrivers.riru.edxp.core.Yahfa;
 
 import java.lang.reflect.Member;
+import java.util.function.Consumer;
 
 import de.robv.android.xposed.PendingHooks;
 
@@ -10,7 +12,13 @@ public class ClassLinker implements KeepAll {
 
     public static native void setEntryPointsToInterpreter(Member method);
 
+    public static void onPreFixupStaticTrampolines(Class clazz) {
+        // remove modified native flags to let FixupStaticTrampolines fill in right entrypoints
+        PendingHooks.removeNativeFlags(clazz);
+    }
+
     public static void onPostFixupStaticTrampolines(Class clazz) {
+        // native flags will be re-set in hooking logic
         PendingHooks.hookPendingMethod(clazz);
     }
 }

edxp-common/src/main/java/com/elderdrivers/riru/edxp/config/BaseHookProvider.java

@@ -1,5 +1,6 @@
 package com.elderdrivers.riru.edxp.config;
 
+import com.elderdrivers.riru.edxp.core.Yahfa;
 import com.elderdrivers.riru.edxp.deopt.PrebuiltMethodsDeopter;
 import com.elderdrivers.riru.edxp.hook.HookProvider;
 
@@ -37,4 +38,9 @@ public abstract class BaseHookProvider implements HookProvider {
     public boolean initXResourcesNative() {
         return false;
     }
+
+    @Override
+    public void setNativeFlag(Member hookMethod, boolean isNative) {
+        Yahfa.setNativeFlag(hookMethod, isNative);
+    }
 }

edxp-common/src/main/java/com/elderdrivers/riru/edxp/core/Yahfa.java

@@ -15,4 +15,6 @@ public class Yahfa {
     public static native void init(int SDK_version);
 
     public static native void setMethodNonCompilable(Member member);
+
+    public static native boolean setNativeFlag(Member member, boolean isNative);
 }

edxp-common/src/main/java/com/elderdrivers/riru/edxp/proxy/NormalProxy.java

@@ -29,14 +29,8 @@ public class NormalProxy extends BaseProxy {
     }
 
     public void forkAndSpecializePost(int pid, String appDataDir, String niceName) {
-        // TODO consider processes without forkAndSpecializePost called
+        // TODO consider processes without forkAndSpecializePost being called
-        ConfigManager.appDataDir = appDataDir;
+        forkPostCommon(pid, false, appDataDir, niceName);
-        ConfigManager.niceName = niceName;
-        mRouter.prepare(false);
-        mRouter.onEnterChildProcess();
-        // load modules for each app process on its forked if dynamic modules mode is on
-        mRouter.loadModulesSafely(false, true);
-        mRouter.onForkFinish();
     }
 
     public void forkSystemServerPre(int uid, int gid, int[] gids, int debugFlags, int[][] rlimits,
@@ -58,12 +52,20 @@ public class NormalProxy extends BaseProxy {
 
     public void forkSystemServerPost(int pid) {
         // in system_server process
-        ConfigManager.appDataDir = getDataPathPrefix() + "android";
+        forkPostCommon(pid, true,
-        ConfigManager.niceName = "system_server";
+                getDataPathPrefix() + "android", "system_server");
-        mRouter.prepare(true);
+    }
+
+
+    private void forkPostCommon(int pid, boolean isSystem, String appDataDir, String niceName) {
+        ConfigManager.appDataDir = appDataDir;
+        ConfigManager.niceName = niceName;
+        mRouter.prepare(isSystem);
         mRouter.onEnterChildProcess();
         // reload module list if dynamic mode is on
         if (ConfigManager.isDynamicModulesEnabled()) {
+            // FIXME this could be error-prone because hooks installed inside old versions
+            //  of initZygote instances of same module are not unhooked
             mRouter.loadModulesSafely(false, true);
         }
         mRouter.onForkFinish();

edxp-common/src/main/java/com/elderdrivers/riru/edxp/util/ClassUtils.java

@@ -2,6 +2,7 @@ package com.elderdrivers.riru.edxp.util;
 
 import android.os.Build;
 
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Member;
 import java.lang.reflect.Modifier;
 
@@ -37,7 +38,7 @@ public class ClassUtils {
     }
 
     public static boolean shouldDelayHook(Member hookMethod) {
-        if (hookMethod == null) {
+        if (hookMethod == null || hookMethod instanceof Constructor) {
             return false;
         }
         Class declaringClass = hookMethod.getDeclaringClass();

edxp-core/build.gradle

@@ -4,7 +4,7 @@ import org.gradle.internal.os.OperatingSystem
 apply plugin: 'com.android.library'
 
 // Values set here will be overriden by AppVeyor, feel free to modify during development.
-def buildVersionName = 'v0.4.4.7_alpha'
+def buildVersionName = 'v0.4.5.0_alpha'
 def buildVersionCode = 10000
 
 if (System.env.APPVEYOR_BUILD_VERSION != null) {
@@ -165,7 +165,7 @@ afterEvaluate {
                         into "${zipPathMagiskRelease}/common"
                         filter { line -> line
                                 .replaceAll('%VERSION%', "$version")
-                                .replaceAll('%VERSION_CODE%', "$versionCode")
+                                    .replaceAll('%VERSION_CODE%', "$versionCode")
                                 .replaceAll('%BACKEND%', "$backendCapped") }
                     }
                     copy {

edxp-core/src/main/cpp/external/yahfa/include/HookMain.h

@@ -23,6 +23,8 @@ void Java_lab_galaxy_yahfa_HookMain_ensureMethodCached(JNIEnv *env, jclass clazz
 
 void setNonCompilable(void *method);
 
+bool setNativeFlag(void *method, bool isNative);
+
 static void *getResolvedMethodsAddr(JNIEnv *, jobject);
 
 #ifdef __cplusplus

edxp-core/src/main/cpp/external/yahfa/src/HookMain.c

@@ -2,6 +2,7 @@
 #include <string.h>
 #include <sys/mman.h>
 #include <stdlib.h>
+#include <stdbool.h>
 
 #include "common.h"
 #include "env.h"
@@ -128,6 +129,26 @@ void setNonCompilable(void *method) {
     );
 }
 
+bool setNativeFlag(void *method, bool isNative) {
+    int access_flags = read32((char *) method + OFFSET_access_flags_in_ArtMethod);
+    LOGI("setNativeFlag: access flags is 0x%x", access_flags);
+    int old_access_flags = access_flags;
+    if (isNative) {
+        access_flags |= kAccNative;
+    } else {
+        access_flags &= ~kAccNative;
+    }
+    if (access_flags != old_access_flags) {
+        memcpy(
+                (char *) method + OFFSET_access_flags_in_ArtMethod,
+                &access_flags,
+                4
+        );
+        return true;
+    }
+    return false;
+}
+
 static int doBackupAndHook(JNIEnv *env, void *targetMethod, void *hookMethod, void *backupMethod) {
     if (hookCount >= hookCap) {
         LOGI("not enough capacity. Allocating...");
@@ -182,13 +203,7 @@ static int doBackupAndHook(JNIEnv *env, void *targetMethod, void *hookMethod, vo
 
     // set the target method to native so that Android O wouldn't invoke it with interpreter
     if (SDKVersion >= ANDROID_O) {
-        int access_flags = read32((char *) targetMethod + OFFSET_access_flags_in_ArtMethod);
+        setNativeFlag(targetMethod, true);
-        access_flags |= kAccNative;
-        memcpy(
-                (char *) targetMethod + OFFSET_access_flags_in_ArtMethod,
-                &access_flags,
-                4
-        );
         LOGI("access flags is 0x%x", access_flags);
     }
 

edxp-core/src/main/cpp/main/include/art/runtime/class_linker.h

@@ -3,9 +3,12 @@
 
 #include <JNIHelper.h>
 #include <base/object.h>
+#include <art/runtime/mirror/class.h>
+#include <android-base/strings.h>
 #include "runtime.h"
 #include "jni_env_ext.h"
 #include "edxp_context.h"
+#include "jni/edxp_pending_hooks.h"
 
 namespace art {
 
@@ -28,8 +31,17 @@ namespace art {
         }
 
         CREATE_HOOK_STUB_ENTRIES(void, FixupStaticTrampolines, void *thiz, void *clazz_ptr) {
+            art::mirror::Class clazz(clazz_ptr);
+            std::string storage;
+            const char *desc = clazz.GetDescriptor(&storage);
+            bool should_intercept = edxp::IsClassPending(desc);
+            if (UNLIKELY(should_intercept)) {
+                edxp::Context::GetInstance()->CallOnPreFixupStaticTrampolines(clazz_ptr);
+            }
             FixupStaticTrampolinesBackup(thiz, clazz_ptr);
-            edxp::Context::GetInstance()->CallOnPostFixupStaticTrampolines(clazz_ptr);
+            if (UNLIKELY(should_intercept)) {
+                edxp::Context::GetInstance()->CallOnPostFixupStaticTrampolines(clazz_ptr);
+            }
         }
 
     public:

edxp-core/src/main/cpp/main/src/edxp_context.cpp

@@ -14,6 +14,7 @@
 #include <art/runtime/mirror/class.h>
 #include <android-base/strings.h>
 #include <nativehelper/scoped_local_ref.h>
+#include <jni/edxp_pending_hooks.h>
 #include "edxp_context.h"
 #include "config_manager.h"
 
@@ -43,8 +44,8 @@ namespace edxp {
         return inject_class_loader_;
     }
 
-    void Context::CallOnPostFixupStaticTrampolines(void *class_ptr) {
+    void Context::CallPostFixupStaticTrampolinesCallback(void *class_ptr, jmethodID callback_mid) {
-        if (UNLIKELY(!post_fixup_static_mid_ || !class_linker_class_)) {
+        if (UNLIKELY(!callback_mid || !class_linker_class_)) {
             return;
         }
         if (!class_ptr) {
@@ -55,10 +56,18 @@ namespace edxp {
         art::JNIEnvExt env_ext(env);
         ScopedLocalRef clazz(env, env_ext.NewLocalRefer(class_ptr));
         if (clazz != nullptr) {
-            JNI_CallStaticVoidMethod(env, class_linker_class_, post_fixup_static_mid_, clazz.get());
+            JNI_CallStaticVoidMethod(env, class_linker_class_, callback_mid, clazz.get());
         }
     }
 
+    void Context::CallOnPreFixupStaticTrampolines(void *class_ptr) {
+        CallPostFixupStaticTrampolinesCallback(class_ptr, pre_fixup_static_mid_);
+    }
+
+    void Context::CallOnPostFixupStaticTrampolines(void *class_ptr) {
+        CallPostFixupStaticTrampolinesCallback(class_ptr, post_fixup_static_mid_);
+    }
+
     void Context::LoadDexAndInit(JNIEnv *env, const char *dex_path) {
         if (LIKELY(initialized_)) {
             return;
@@ -97,6 +106,9 @@ namespace edxp {
         env->GetJavaVM(&vm_);
         class_linker_class_ = (jclass) env->NewGlobalRef(
                 FindClassFromLoader(env, kClassLinkerClassName));
+        pre_fixup_static_mid_ = JNI_GetStaticMethodID(env, class_linker_class_,
+                                                      "onPreFixupStaticTrampolines",
+                                                      "(Ljava/lang/Class;)V");
         post_fixup_static_mid_ = JNI_GetStaticMethodID(env, class_linker_class_,
                                                        "onPostFixupStaticTrampolines",
                                                        "(Ljava/lang/Class;)V");
@@ -110,6 +122,7 @@ namespace edxp {
         RegisterArtClassLinker(env);
         RegisterArtHeap(env);
         RegisterEdxpYahfa(env);
+        RegisterPendingHooks(env);
 
         // must call entry class's methods after all native methods registered
         if (LIKELY(entry_class_)) {

edxp-core/src/main/cpp/main/src/edxp_context.h

@@ -22,6 +22,8 @@ namespace edxp {
 
         jobject GetCurrentClassLoader() const;
 
+        void CallOnPreFixupStaticTrampolines(void *class_ptr);
+
         void CallOnPostFixupStaticTrampolines(void *class_ptr);
 
         void PrepareJavaEnv(JNIEnv *env);
@@ -70,6 +72,7 @@ namespace edxp {
         jstring nice_name_ = nullptr;
         JavaVM *vm_ = nullptr;
         jclass class_linker_class_ = nullptr;
+        jmethodID pre_fixup_static_mid_ = nullptr;
         jmethodID post_fixup_static_mid_ = nullptr;
 
         Context() {}
@@ -79,6 +82,8 @@ namespace edxp {
         void LoadDexAndInit(JNIEnv *env, const char *dex_path);
 
         jclass FindClassFromLoader(JNIEnv *env, jobject class_loader, const char *class_name) const;
+
+        void CallPostFixupStaticTrampolinesCallback(void *class_ptr, jmethodID mid);
     };
 
 }

edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.cpp

@@ -0,0 +1,30 @@
+
+#include <nativehelper/jni_macros.h>
+#include <set>
+#include <string>
+#include "jni.h"
+#include "native_util.h"
+#include "edxp_pending_hooks.h"
+
+namespace edxp {
+
+    static std::set<std::string> class_descs_;
+
+    bool IsClassPending(const char *class_desc) {
+        return class_descs_.find(class_desc) != class_descs_.end();
+    }
+
+    static void PendingHooks_recordPendingMethodNative(JNI_START, jstring class_desc) {
+        const char *class_desc_chars = env->GetStringUTFChars(class_desc, JNI_FALSE);
+        class_descs_.insert(class_desc_chars);
+    }
+
+    static JNINativeMethod gMethods[] = {
+            NATIVE_METHOD(PendingHooks, recordPendingMethodNative, "(Ljava/lang/String;)V"),
+    };
+
+    void RegisterPendingHooks(JNIEnv *env) {
+        REGISTER_EDXP_NATIVE_METHODS("de.robv.android.xposed.PendingHooks");
+    }
+
+}

edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.h

@@ -0,0 +1,12 @@
+
+#pragma once
+
+#include "jni.h"
+
+namespace edxp {
+
+    bool IsClassPending(const char *);
+
+    void RegisterPendingHooks(JNIEnv *);
+
+} // namespace edxp

edxp-core/src/main/cpp/main/src/jni/edxp_yahfa.cpp

@@ -39,6 +39,19 @@ namespace edxp {
         setNonCompilable(art_method);
     }
 
+    static jboolean Yahfa_setNativeFlag(JNI_START, jobject member, jboolean is_native) {
+        if (!member) {
+            LOGE("setNativeFlagNative: member is null");
+            return JNI_FALSE;
+        }
+        void *art_method = env->FromReflectedMethod(member);
+        if (!art_method) {
+            LOGE("setNativeFlagNative: art_method is null");
+            return JNI_FALSE;
+        }
+        return (jboolean) setNativeFlag(art_method, is_native);
+    }
+
     static JNINativeMethod gMethods[] = {
             NATIVE_METHOD(Yahfa, init, "(I)V"),
             NATIVE_METHOD(Yahfa, findMethodNative,
@@ -48,6 +61,7 @@ namespace edxp {
             NATIVE_METHOD(Yahfa, ensureMethodCached,
                           "(Ljava/lang/reflect/Method;Ljava/lang/reflect/Method;)V"),
             NATIVE_METHOD(Yahfa, setMethodNonCompilable, "(Ljava/lang/reflect/Member;)V"),
+            NATIVE_METHOD(Yahfa, setNativeFlag, "(Ljava/lang/reflect/Member;Z)Z"),
     };
 
     void RegisterEdxpYahfa(JNIEnv *env) {

xposed-bridge/src/main/java/com/elderdrivers/riru/edxp/hook/HookProvider.java

@@ -25,4 +25,7 @@ public interface HookProvider {
     boolean initXResourcesNative();
 
     boolean removeFinalFlagNative(Class clazz);
+
+    void setNativeFlag(Member hookMethod, boolean isNative);
+
 }

xposed-bridge/src/main/java/de/robv/android/xposed/PendingHooks.java

@@ -1,14 +1,21 @@
 package de.robv.android.xposed;
 
+import com.elderdrivers.riru.edxp.config.EdXpConfigGlobal;
+
 import java.lang.reflect.Member;
+import java.lang.reflect.Modifier;
+import java.util.HashSet;
 import java.util.concurrent.ConcurrentHashMap;
 
 import static de.robv.android.xposed.XposedBridge.hookMethodNative;
 
 public final class PendingHooks {
 
+    // GuardedBy("PendingHooks.class")
     private static final ConcurrentHashMap<Member, XposedBridge.AdditionalHookInfo>
             sPendingHookMethods = new ConcurrentHashMap<>();
+    // GuardedBy("PendingHooks.class")
+    private static final HashSet<Member> sNonNativeMethods = new HashSet<>();
 
     public synchronized static void hookPendingMethod(Class clazz) {
         for (Member member : sPendingHookMethods.keySet()) {
@@ -18,7 +25,33 @@ public final class PendingHooks {
         }
     }
 
-    public synchronized static void recordPendingMethod(Member hookMethod, XposedBridge.AdditionalHookInfo additionalInfo) {
+    public synchronized static void removeNativeFlags(Class clazz) {
-        sPendingHookMethods.put(hookMethod, additionalInfo);
+        for (Member member : sPendingHookMethods.keySet()) {
+            if (member.getDeclaringClass().equals(clazz) && sNonNativeMethods.contains(member)) {
+                EdXpConfigGlobal.getHookProvider().setNativeFlag(member, false);
+            }
+        }
     }
+
+    public synchronized static void recordPendingMethod(Member hookMethod,
+                                                        XposedBridge.AdditionalHookInfo additionalInfo) {
+        if (!Modifier.isNative(hookMethod.getModifiers())) {
+            // record non-native methods for later native flag temporary removing
+            sNonNativeMethods.add(hookMethod);
+        }
+        EdXpConfigGlobal.getHookProvider().setNativeFlag(hookMethod, true);
+        sPendingHookMethods.put(hookMethod, additionalInfo);
+        recordPendingMethodNative("L" +
+                hookMethod.getDeclaringClass().getName().replace(".", "/") + ";");
+    }
+
+    public synchronized void cleanUp() {
+        sPendingHookMethods.clear();
+        // sNonNativeMethods should be cleared very carefully because their
+        // pre-set native flag have to be removed if its hooking is cancelled
+        // before its class is initialized
+//        sNonNativeMethods.clear();
+    }
+
+    private static native void recordPendingMethodNative(String classDesc);
 }