Strip pending hook (#6)

.github/workflows/build.yml

@@ -149,6 +149,7 @@ jobs:
           force-avd-creation: false
           emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
           disable-animations: false
+          avd-name: ${{ matrix.api-level }}_${{ matrix.arch }}
       - name: run tests
         uses: yujincheng08/android-emulator-runner@release/v2
         with:
@@ -159,6 +160,7 @@ jobs:
           force-avd-creation: false
           emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
           disable-animations: true
+          avd-name: ${{ matrix.api-level }}_${{ matrix.arch }}
       - name: Upload outputs
         if: always()
         uses: actions/upload-artifact@v2

lsplant/src/main/jni/art/instrumentation.hpp

@@ -6,37 +6,47 @@
 namespace lsplant::art {
 
 class Instrumentation {
+    inline static ArtMethod *MaybeUseBackupMethod(ArtMethod *art_method, const void *quick_code) {
+        std::shared_lock lk(hooked_methods_lock_);
+        if (auto found = hooked_methods_.find(art_method);
+            found != hooked_methods_.end() && art_method->GetEntryPoint() != quick_code)
+            [[unlikely]] {
+            LOGD("Propagate update method code %p for hooked method %s to its backup", quick_code,
+                 art_method->PrettyMethod().c_str());
+            return found->second.second;
+        }
+        return art_method;
+    }
+
     CREATE_MEM_HOOK_STUB_ENTRY(
         "_ZN3art15instrumentation15Instrumentation21UpdateMethodsCodeImplEPNS_9ArtMethodEPKv", void,
         UpdateMethodsCodeImpl,
-        (Instrumentation * thiz, ArtMethod *art_method, const void *quick_code), {
+        (Instrumentation * thiz, ArtMethod *art_method, const void *quick_code),
-            if (IsHooked(art_method)) [[unlikely]] {
+        { backup(thiz, MaybeUseBackupMethod(art_method, quick_code), quick_code); });
-                LOGD("Skip update method code for hooked method %s",
-                     art_method->PrettyMethod().c_str());
-                return;
-            } else {
-                backup(thiz, art_method, quick_code);
-            }
-        });
 
     CREATE_MEM_HOOK_STUB_ENTRY(
         "_ZN3art15instrumentation15Instrumentation17UpdateMethodsCodeEPNS_9ArtMethodEPKv", void,
         UpdateMethodsCode, (Instrumentation * thiz, ArtMethod *art_method, const void *quick_code),
         {
-            if (IsHooked(art_method)) [[unlikely]] {
+            if (UpdateMethodsCodeImpl.backup) {
-                LOGD("Skip update method code for hooked method %s",
+                UpdateMethodsCodeImpl.backup(thiz, MaybeUseBackupMethod(art_method, quick_code),
-                     art_method->PrettyMethod().c_str());
+                                             quick_code);
-                return;
             } else {
-                backup(thiz, art_method, quick_code);
+                backup(thiz, MaybeUseBackupMethod(art_method, quick_code), quick_code);
             }
         });
 
 public:
     static bool Init(const HookHandler &handler) {
-        if (!HookSyms(handler, UpdateMethodsCodeImpl, UpdateMethodsCode)) {
+        int sdk_int = GetAndroidApiLevel();
+        if (!HookSyms(handler, UpdateMethodsCode)) {
             return false;
         }
+        if (sdk_int >= __ANDROID_API_N__) [[likely]] {
+            if (!HookSyms(handler, UpdateMethodsCodeImpl)) {
+                return false;
+            }
+        }
         return true;
     }
 };

lsplant/src/main/jni/art/runtime/art_method.hpp

@@ -6,7 +6,7 @@
 namespace lsplant::art {
 
 class ArtMethod {
-    CREATE_MEM_FUNC_SYMBOL_ENTRY(std::string, PrettyMethod, ArtMethod *thiz, bool with_signature) {
+    CREATE_FUNC_SYMBOL_ENTRY(std::string, PrettyMethod, ArtMethod *thiz, bool with_signature) {
         if (thiz == nullptr) [[unlikely]]
             return "null";
         else if (PrettyMethodSym) [[likely]]
@@ -212,6 +212,10 @@ public:
             return false;
         }
 
+        if (!RETRIEVE_FUNC_SYMBOL(PrettyMethod, "_ZN3art9ArtMethod12PrettyMethodEPS0_b")) {
+            RETRIEVE_FUNC_SYMBOL(PrettyMethod, "_ZN3art12PrettyMethodEPNS_9ArtMethodEb");
+        }
+
         if (sdk_int <= __ANDROID_API_O__) [[unlikely]] {
             auto abstract_method_error = JNI_FindClass(env, "java/lang/AbstractMethodError");
             if (!abstract_method_error) {

lsplant/src/main/jni/art/runtime/class_linker.hpp

@@ -15,51 +15,10 @@ private:
         }
     }
 
-    [[gnu::always_inline]] static void MaybeDelayHook(mirror::Class *clazz) {
-        const auto *class_def = clazz->GetClassDef();
-        bool should_intercept = class_def && IsPending(class_def);
-        if (should_intercept) [[unlikely]] {
-            LOGD("Pending hook for %p (%s)", clazz, clazz->GetDescriptor().c_str());
-            OnPending(class_def);
-        }
-    }
-
-    CREATE_MEM_HOOK_STUB_ENTRY(
-        "_ZN3art11ClassLinker22FixupStaticTrampolinesENS_6ObjPtrINS_6mirror5ClassEEE", void,
-        FixupStaticTrampolines, (ClassLinker * thiz, mirror::Class *clazz), {
-            backup(thiz, clazz);
-            MaybeDelayHook(clazz);
-        });
-
-    CREATE_MEM_HOOK_STUB_ENTRY("_ZN3art11ClassLinker22FixupStaticTrampolinesEPNS_6mirror5ClassE",
-                               void, FixupStaticTrampolinesRaw,
-                               (ClassLinker * thiz, mirror::Class *clazz), {
-                                   backup(thiz, clazz);
-                                   MaybeDelayHook(clazz);
-                               });
-
-    CREATE_MEM_HOOK_STUB_ENTRY(
-        "_ZN3art11ClassLinker22FixupStaticTrampolinesEPNS_6ThreadENS_6ObjPtrINS_6mirror5ClassEEE",
-        void, FixupStaticTrampolinesWithThread,
-        (ClassLinker * thiz, Thread *self, mirror::Class *clazz), {
-            backup(thiz, self, clazz);
-            MaybeDelayHook(clazz);
-        });
-
-    CREATE_MEM_HOOK_STUB_ENTRY(
-        "_ZN3art11ClassLinker20MarkClassInitializedEPNS_6ThreadENS_6HandleINS_6mirror5ClassEEE",
-        void *, MarkClassInitialized, (ClassLinker * thiz, Thread *self, uint32_t *clazz_ptr), {
-            void *result = backup(thiz, self, clazz_ptr);
-            auto clazz = reinterpret_cast<mirror::Class *>(*clazz_ptr);
-            MaybeDelayHook(clazz);
-            return result;
-        });
-
     CREATE_HOOK_STUB_ENTRY(
         "_ZN3art11ClassLinker30ShouldUseInterpreterEntrypointEPNS_9ArtMethodEPKv", bool,
         ShouldUseInterpreterEntrypoint, (ArtMethod * art_method, const void *quick_code), {
-            if (quick_code != nullptr && (IsHooked(art_method) || IsPending(art_method)))
+            if (quick_code != nullptr && IsHooked(art_method)) [[unlikely]] {
-                [[unlikely]] {
                 return false;
             }
             return backup(art_method, quick_code);
@@ -71,7 +30,7 @@ private:
 
     CREATE_HOOK_STUB_ENTRY("_ZN3art11interpreter29ShouldStayInSwitchInterpreterEPNS_9ArtMethodE",
                            bool, ShouldStayInSwitchInterpreter, (ArtMethod * art_method), {
-                               if (IsHooked(art_method) || IsPending(art_method)) [[unlikely]] {
+                               if (IsHooked(art_method)) [[unlikely]] {
                                    return false;
                                }
                                return backup(art_method);
@@ -88,22 +47,6 @@ public:
             }
         }
 
-        if (sdk_int >= __ANDROID_API_R__) {
-            // In android R, FixupStaticTrampolines won't be called unless it's marking it as
-            // visiblyInitialized.
-            // So we miss some calls between initialized and visiblyInitialized.
-            // Therefore we hook the new introduced MarkClassInitialized instead
-            // This only happens on non-x86 devices
-            if (!HookSyms(handler, MarkClassInitialized) ||
-                !HookSyms(handler, FixupStaticTrampolinesWithThread, FixupStaticTrampolines)) {
-                return false;
-            }
-        } else {
-            if (!HookSyms(handler, FixupStaticTrampolines, FixupStaticTrampolinesRaw)) {
-                return false;
-            }
-        }
-
         if (!RETRIEVE_MEM_FUNC_SYMBOL(
                 SetEntryPointsToInterpreter,
                 "_ZNK3art11ClassLinker27SetEntryPointsToInterpreterEPNS_9ArtMethodE")) {

lsplant/src/main/jni/common.hpp

@@ -75,20 +75,11 @@ namespace art {
 
 namespace {
 // target, backup
-inline std::unordered_map<art::ArtMethod *, jobject> hooked_methods_;
+inline std::unordered_map<art::ArtMethod *, std::pair<jobject, art::ArtMethod *>> hooked_methods_;
 inline std::shared_mutex hooked_methods_lock_;
 
 inline std::list<std::pair<art::ArtMethod *, art::ArtMethod *>> jit_movements_;
 inline std::shared_mutex jit_movements_lock_;
-
-inline std::unordered_map<
-    const art::dex::ClassDef *,
-    std::list<std::tuple<art::ArtMethod *, art::ArtMethod *, art::ArtMethod *>>>
-    pending_classes_;
-inline std::shared_mutex pending_classes_lock_;
-
-inline std::unordered_set<const art::ArtMethod *> pending_methods_;
-inline std::shared_mutex pending_methods_lock_;
 }  // namespace
 
 inline bool IsHooked(art::ArtMethod *art_method) {
@@ -96,24 +87,14 @@ inline bool IsHooked(art::ArtMethod *art_method) {
     return hooked_methods_.contains(art_method);
 }
 
-inline bool IsPending(art::ArtMethod *art_method) {
-    std::shared_lock lk(pending_methods_lock_);
-    return pending_methods_.contains(art_method);
-}
-
-inline bool IsPending(const art::dex::ClassDef *class_def) {
-    std::shared_lock lk(pending_classes_lock_);
-    return pending_classes_.contains(class_def);
-}
-
 inline std::list<std::pair<art::ArtMethod *, art::ArtMethod *>> GetJitMovements() {
     std::unique_lock lk(jit_movements_lock_);
     return std::move(jit_movements_);
 }
 
-inline void RecordHooked(art::ArtMethod *target, jobject backup) {
+inline void RecordHooked(art::ArtMethod *target, jobject reflected_backup, art::ArtMethod* backup) {
     std::unique_lock lk(hooked_methods_lock_);
-    hooked_methods_.emplace(target, backup);
+    hooked_methods_.emplace(target, std::make_pair(reflected_backup, backup));
 }
 
 inline void RecordJitMovement(art::ArtMethod *target, art::ArtMethod *backup) {
@@ -121,37 +102,4 @@ inline void RecordJitMovement(art::ArtMethod *target, art::ArtMethod *backup) {
     jit_movements_.emplace_back(target, backup);
 }
 
-inline void RecordPending(const art::dex::ClassDef *class_def, art::ArtMethod *target,
-                          art::ArtMethod *hook, art::ArtMethod *backup) {
-    {
-        std::unique_lock lk(pending_methods_lock_);
-        pending_methods_.emplace(target);
-    }
-    std::unique_lock lk(pending_classes_lock_);
-    pending_classes_[class_def].emplace_back(std::make_tuple(target, hook, backup));
-}
-
-void OnPending(art::ArtMethod *target, art::ArtMethod *hook, art::ArtMethod *backup);
-
-inline void OnPending(const art::dex::ClassDef *class_def) {
-    {
-        std::shared_lock lk(pending_classes_lock_);
-        if (!pending_classes_.contains(class_def)) return;
-    }
-    typename decltype(pending_classes_)::value_type::second_type set;
-    {
-        std::unique_lock lk(pending_classes_lock_);
-        auto it = pending_classes_.find(class_def);
-        if (it == pending_classes_.end()) return;
-        set = std::move(it->second);
-        pending_classes_.erase(it);
-    }
-    for (auto &[target, hook, backup] : set) {
-        {
-            std::unique_lock mlk(pending_methods_lock_);
-            pending_methods_.erase(target);
-        }
-        OnPending(target, hook, backup);
-    }
-}
 }  // namespace lsplant

lsplant/src/main/jni/lsplant.cc

@@ -459,7 +459,8 @@ bool DoHook(ArtMethod *target, ArtMethod *hook, ArtMethod *backup) {
     ScopedGCCriticalSection section(art::Thread::Current(), art::gc::kGcCauseDebugger,
                                     art::gc::kCollectorTypeDebugger);
     ScopedSuspendAll suspend("LSPlant Hook", false);
-    LOGV("Hooking: target = %p, hook = %p, backup = %p", target, hook, backup);
+    LOGV("Hooking: target = %s(%p), hook = %s(%p), backup = %s(%p)", target->PrettyMethod().c_str(),
+         target, hook->PrettyMethod().c_str(), hook, backup->PrettyMethod().c_str(), backup);
 
     if (auto *trampoline = GenerateTrampolineFor(hook); !trampoline) {
         LOGE("Failed to generate trampoline");
@@ -504,13 +505,6 @@ bool DoUnHook(ArtMethod *target, ArtMethod *backup) {
 
 }  // namespace
 
-void OnPending(art::ArtMethod *target, art::ArtMethod *hook, art::ArtMethod *backup) {
-    LOGD("On pending hook for %p", target);
-    if (!DoHook(target, hook, backup)) {
-        LOGE("Pending hook failed");
-    }
-}
-
 inline namespace v1 {
 
 using ::lsplant::IsHooked;
@@ -542,7 +536,7 @@ using ::lsplant::IsHooked;
     auto *target = ArtMethod::FromReflectedMethod(env, target_method);
     bool is_static = target->IsStatic();
 
-    if (IsHooked(target) || IsPending(target)) {
+    if (IsHooked(target)) {
         LOGW("Skip duplicate hook");
         return nullptr;
     }
@@ -587,24 +581,9 @@ using ::lsplant::IsHooked;
 
     JNI_SetStaticObjectField(env, built_class, hooker_field, hooker_object);
 
-    if (is_static && !Class::IsInitialized(env, target_class.get())) {
-        auto *miror_class = Class::FromReflectedClass(env, target_class);
-        if (!miror_class) {
-            LOGE("Failed to decode target class");
-            return nullptr;
-        }
-        const auto *class_def = miror_class->GetClassDef();
-        if (!class_def) {
-            LOGE("Failed to get target class def");
-            return nullptr;
-        }
-        LOGD("Record pending hook for %p", target);
-        RecordPending(class_def, target, hook, backup);
-        return JNI_NewGlobalRef(env, reflected_backup);
-    }
     if (DoHook(target, hook, backup)) {
         jobject global_backup = JNI_NewGlobalRef(env, reflected_backup);
-        RecordHooked(target, global_backup);
+        RecordHooked(target, global_backup, backup);
         if (!is_proxy) [[likely]] {
             RecordJitMovement(target, backup);
         }
@@ -621,17 +600,11 @@ using ::lsplant::IsHooked;
     }
     auto *target = ArtMethod::FromReflectedMethod(env, target_method);
     jobject reflected_backup = nullptr;
-    {
+    art::ArtMethod *backup = nullptr;
-        std::unique_lock lk(pending_methods_lock_);
-        if (auto it = pending_methods_.find(target); it != pending_methods_.end()) {
-            pending_methods_.erase(it);
-            return true;
-        }
-    }
     {
         std::unique_lock lk(hooked_methods_lock_);
         if (auto it = hooked_methods_.find(target); it != hooked_methods_.end()) {
-            reflected_backup = it->second;
+            std::tie(reflected_backup, backup) = it->second;
             hooked_methods_.erase(it);
         }
     }
@@ -639,7 +612,6 @@ using ::lsplant::IsHooked;
         LOGE("Unable to unhook a method that is not hooked");
         return false;
     }
-    auto *backup = ArtMethod::FromReflectedMethod(env, reflected_backup);
     env->DeleteGlobalRef(reflected_backup);
     return DoUnHook(target, backup);
 }
@@ -654,9 +626,6 @@ using ::lsplant::IsHooked;
     if (std::shared_lock lk(hooked_methods_lock_); hooked_methods_.contains(art_method)) {
         return true;
     }
-    if (std::shared_lock lk(pending_methods_lock_); pending_methods_.contains(art_method)) {
-        return true;
-    }
     return false;
 }
 
@@ -670,8 +639,7 @@ using ::lsplant::IsHooked;
         std::shared_lock lk(hooked_methods_lock_);
         auto it = hooked_methods_.find(art_method);
         if (it != hooked_methods_.end()) {
-            auto *reflected_backup = it->second;
+            art_method = it->second.second;
-            art_method = ArtMethod::FromReflectedMethod(env, reflected_backup);
         }
     }
     if (!art_method) {

test/src/androidTest/java/org/lsposed/lsplant/UnitTest.java

@@ -97,4 +97,23 @@ public class UnitTest {
         Assert.assertTrue(hooker.unhook());
         Assert.assertEquals(o, test.manyParametersMethod(a, b, c, d, e, f, g, h, e, f));
     }
+
+    @Test
+    public void t05_uninitializedStaticMethod() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException, ClassNotFoundException {
+        var uninitializedClass = Class.forName("org.lsposed.lsplant.LSPTest$NeedInitialize", false, LSPTest.class.getClassLoader());
+        var staticMethod = uninitializedClass.getDeclaredMethod("staticMethod");
+        var callStaticMethod = uninitializedClass.getDeclaredMethod("callStaticMethod");
+        var staticMethodReplacement = Replacement.class.getDeclaredMethod("staticMethodReplacement", Hooker.MethodCallback.class);
+
+        Hooker hooker = Hooker.hook(staticMethod, staticMethodReplacement, null);
+        Assert.assertNotNull(hooker);
+        for (int i = 0; i < 1000; ++i) {
+            Assert.assertTrue("Iter " + i, (Boolean) callStaticMethod.invoke(null));
+            Assert.assertFalse("Iter " + i, (boolean) hooker.backup.invoke(null));
+        }
+
+        Assert.assertTrue(hooker.unhook());
+        Assert.assertFalse((Boolean) callStaticMethod.invoke(null));
+    }
+
 }

test/src/main/java/org/lsposed/lsplant/LSPTest.java

@@ -25,4 +25,29 @@ public class LSPTest {
     String manyParametersMethod(String a, boolean b, byte c, short d, int e, long f, float g, double h, Integer i, Long j) {
         return a + b + c + d + e + f + g + h + i + j;
     }
+
+
+    public static class NeedInitialize {
+        static int x;
+
+        static {
+            x = 0;
+        }
+
+        static boolean staticMethod() {
+            try {
+                return x != 0;
+            } catch (Throwable e) {
+                return false;
+            }
+        }
+
+        static boolean callStaticMethod() {
+            try {
+                return staticMethod();
+            } catch (Throwable e) {
+                return false;
+            }
+        }
+    }
 }