resolve_invoke_method

Summary: There are some invoke-virtual instructions that reference methods whose definition is actually in interface. Thus, for an invoke-virtual instruction, one should first try resolving it with `MethodSearch::Virtual`, and then `MethodSearch::InterfaceVirtual`. We used to have a bunch of places all over Redex where we do this little dance. This diffs consolidates all such dances with a new convenient helper function (which we should probably use in a few more places where we currently give up in such scenarios.) This is a behavior-preserving change. Reviewed By: thezhangwei Differential Revision: D49440605 fbshipit-source-id: 2f77d1e6a4a5d22336a86911d309d7c4fcd0ba05
facebook · Sep 20, 2023 · f2eefa5 · f2eefa5
1 parent c3a909f
commit f2eefa5
Show file tree

Hide file tree

Showing 13 changed files with 105 additions and 143 deletions.
diff --git a/libredex/CallGraph.cpp b/libredex/CallGraph.cpp
@@ -53,7 +53,7 @@ CallSites SingleCalleeStrategy::get_callsites(const DexMethod* method) const {
       code, [&](const IRList::iterator& it) {
         auto insn = it->insn;
         if (opcode::is_an_invoke(insn->opcode())) {
-          auto callee = this->resolve_callee(method, insn);
+          auto callee = resolve_invoke_method(insn, method);
           if (callee == nullptr || is_definitely_virtual(callee)) {
             return editable_cfg_adapter::LOOP_CONTINUE;
           }
@@ -82,11 +82,6 @@ bool SingleCalleeStrategy::is_definitely_virtual(DexMethod* method) const {
   return method->is_virtual() && m_non_virtual.count(method) == 0;
 }
 
-DexMethod* SingleCalleeStrategy::resolve_callee(const DexMethod* caller,
-                                                IRInstruction* invoke) const {
-  return resolve_method(invoke->get_method(), opcode_to_search(invoke), caller);
-}
-
 MultipleCalleeBaseStrategy::MultipleCalleeBaseStrategy(
     const mog::Graph& method_override_graph, const Scope& scope)
     : SingleCalleeStrategy(method_override_graph, scope),
@@ -219,23 +214,6 @@ CompleteCallGraphStrategy::CompleteCallGraphStrategy(
     const mog::Graph& method_override_graph, const Scope& scope)
     : MultipleCalleeBaseStrategy(method_override_graph, scope) {}
 
-static DexMethod* resolve_interface_virtual_callee(const IRInstruction* insn,
-                                                   const DexMethod* caller) {
-  DexMethod* callee = nullptr;
-  if (opcode_to_search(insn) == MethodSearch::Virtual) {
-    callee = resolve_method(insn->get_method(), MethodSearch::InterfaceVirtual,
-                            caller);
-    if (callee == nullptr) {
-      auto insn_method_cls = type_class(insn->get_method()->get_class());
-      if (insn_method_cls != nullptr && !insn_method_cls->is_external()) {
-        TRACE(CALLGRAPH, 1, "Unexpected unresolved insn %s in %s", SHOW(insn),
-              SHOW(caller));
-      }
-    }
-  }
-  return callee;
-}
-
 CallSites CompleteCallGraphStrategy::get_callsites(
     const DexMethod* method) const {
   CallSites callsites;
@@ -247,12 +225,9 @@ CallSites CompleteCallGraphStrategy::get_callsites(
       code, [&](const IRList::iterator& it) {
         auto insn = it->insn;
         if (opcode::is_an_invoke(insn->opcode())) {
-          auto callee = this->resolve_callee(method, insn);
+          auto callee = resolve_invoke_method(insn, method);
           if (callee == nullptr) {
-            callee = resolve_interface_virtual_callee(insn, method);
-            if (callee == nullptr) {
-              return editable_cfg_adapter::LOOP_CONTINUE;
-            }
+            return editable_cfg_adapter::LOOP_CONTINUE;
           }
           if (callee->is_concrete()) {
             callsites.emplace_back(callee, insn);
@@ -332,39 +307,35 @@ MultipleCalleeStrategy::MultipleCalleeStrategy(
   // Gather big overrides true virtual methods.
   ConcurrentSet<const DexMethod*> concurrent_callees;
   ConcurrentSet<const DexMethod*> concurrent_big_overrides;
-  walk::parallel::opcodes(scope, [&](const DexMethod* method,
-                                     IRInstruction* insn) {
-    if (opcode::is_an_invoke(insn->opcode())) {
-      auto callee =
-          resolve_method(insn->get_method(), opcode_to_search(insn), method);
-      if (callee == nullptr) {
-        callee = resolve_interface_virtual_callee(insn, method);
-        if (callee == nullptr) {
-          return;
-        }
-      }
-      if (!callee->is_virtual()) {
-        return;
-      }
-      if (!concurrent_callees.insert(callee)) {
-        return;
-      }
-      const auto& overriding_methods =
-          mog::get_overriding_methods(m_method_override_graph, callee);
-      uint32_t num_override = 0;
-      for (auto overriding_method : overriding_methods) {
-        if (overriding_method->get_code()) {
-          ++num_override;
-        }
-      }
-      if (num_override > big_override_threshold) {
-        concurrent_big_overrides.emplace(callee);
-        for (auto overriding_method : overriding_methods) {
-          concurrent_big_overrides.emplace(overriding_method);
+  walk::parallel::opcodes(
+      scope, [&](const DexMethod* method, IRInstruction* insn) {
+        if (opcode::is_an_invoke(insn->opcode())) {
+          auto callee = resolve_invoke_method(insn, method);
+          if (callee == nullptr) {
+            return;
+          }
+          if (!callee->is_virtual()) {
+            return;
+          }
+          if (!concurrent_callees.insert(callee)) {
+            return;
+          }
+          const auto& overriding_methods =
+              mog::get_overriding_methods(m_method_override_graph, callee);
+          uint32_t num_override = 0;
+          for (auto overriding_method : overriding_methods) {
+            if (overriding_method->get_code()) {
+              ++num_override;
+            }
+          }
+          if (num_override > big_override_threshold) {
+            concurrent_big_overrides.emplace(callee);
+            for (auto overriding_method : overriding_methods) {
+              concurrent_big_overrides.emplace(overriding_method);
+            }
+          }
         }
-      }
-    }
-  });
+      });
   m_big_override = concurrent_big_overrides.move_to_container();
 }
 
@@ -378,12 +349,9 @@ CallSites MultipleCalleeStrategy::get_callsites(const DexMethod* method) const {
       code, [&](const IRList::iterator& it) {
         auto insn = it->insn;
         if (opcode::is_an_invoke(insn->opcode())) {
-          auto callee = this->resolve_callee(method, insn);
+          auto callee = resolve_invoke_method(insn, method);
           if (callee == nullptr) {
-            callee = resolve_interface_virtual_callee(insn, method);
-            if (callee == nullptr) {
-              return editable_cfg_adapter::LOOP_CONTINUE;
-            }
+            return editable_cfg_adapter::LOOP_CONTINUE;
           }
           if (is_definitely_virtual(callee)) {
             // For true virtual callees, add the callee itself and all of its

diff --git a/libredex/CallGraph.h b/libredex/CallGraph.h
@@ -187,8 +187,6 @@ class SingleCalleeStrategy : public BuildStrategy {
 
  protected:
   bool is_definitely_virtual(DexMethod* method) const;
-  virtual DexMethod* resolve_callee(const DexMethod* caller,
-                                    IRInstruction* invoke) const;
 
   const Scope& m_scope;
   std::unordered_set<DexMethod*> m_non_virtual;

diff --git a/libredex/Reachability.cpp b/libredex/Reachability.cpp
@@ -876,20 +876,14 @@ void MethodReferencesGatherer::default_gather_mie(const MethodItemEntry& mie,
       }
     } else if (gather_methods && (opcode::is_invoke_virtual(op) ||
                                   opcode::is_invoke_interface(op))) {
-      auto method_ref = insn->get_method();
-      auto resolved_callee = resolve_method(method_ref, opcode_to_search(insn));
-      if (resolved_callee == nullptr && opcode::is_invoke_virtual(op)) {
-        // There are some invoke-virtual call on methods whose def are
-        // actually in interface.
-        resolved_callee =
-            resolve_method(method_ref, MethodSearch::InterfaceVirtual);
-      }
+      auto resolved_callee = resolve_invoke_method(insn, m_method);
       if (!resolved_callee) {
         // Typically clone() on an array, or other obscure external references
         TRACE(REACH, 2, "Unresolved virtual callee at %s", SHOW(insn));
         refs->unknown_invoke_virtual_targets = true;
         return;
       }
+      auto method_ref = insn->get_method();
       auto base_type = method_ref->get_class();
       refs->base_invoke_virtual_targets_if_class_instantiable[resolved_callee]
           .insert(base_type);

diff --git a/libredex/RefChecker.cpp b/libredex/RefChecker.cpp
@@ -27,17 +27,12 @@ CodeRefs::CodeRefs(const DexMethod* method) {
           always_assert(insn->get_type());
           types_set.insert(insn->get_type());
         } else if (insn->has_method()) {
-          auto callee_ref = insn->get_method();
-          auto callee =
-              resolve_method(callee_ref, opcode_to_search(insn), method);
-          if (!callee && opcode_to_search(insn) == MethodSearch::Virtual) {
-            callee = resolve_method(callee_ref, MethodSearch::InterfaceVirtual,
-                                    method);
-          }
+          auto callee = resolve_invoke_method(insn, method);
           if (!callee) {
             invalid_refs = true;
             return editable_cfg_adapter::LOOP_BREAK;
           }
+          auto callee_ref = insn->get_method();
           if (callee != callee_ref) {
             types_set.insert(callee_ref->get_class());
           }

diff --git a/libredex/Resolver.h b/libredex/Resolver.h
@@ -285,6 +285,50 @@ inline DexMethod* resolve_method(DexMethodRef* method,
   return mdef;
 }
 
+/**
+ * Resolve the method of an invoke instruction. Note that there are some
+ * invoke-virtual call on methods whose def are actually in interface. Thus, for
+ * an invoke-virtual, it first tries MethodSearch::Virtual, and then
+ * MethodSearch::InterfaceVirtual.
+ */
+inline DexMethod* resolve_invoke_method(
+    const IRInstruction* insn,
+    const DexMethod* caller = nullptr,
+    bool* resolved_virtual_to_interface = nullptr) {
+  auto callee_ref = insn->get_method();
+  auto search = opcode_to_search(insn);
+  auto callee = resolve_method(callee_ref, search, caller);
+  if (!callee && search == MethodSearch::Virtual) {
+    callee = resolve_method(callee_ref, MethodSearch::InterfaceVirtual, caller);
+    if (resolved_virtual_to_interface) {
+      *resolved_virtual_to_interface = callee != nullptr;
+    }
+  } else if (resolved_virtual_to_interface) {
+    *resolved_virtual_to_interface = false;
+  }
+  return callee;
+}
+
+inline DexMethod* resolve_invoke_method(
+    const IRInstruction* insn,
+    MethodRefCache& ref_cache,
+    const DexMethod* caller = nullptr,
+    bool* resolved_virtual_to_interface = nullptr) {
+  auto callee_ref = insn->get_method();
+  auto search = opcode_to_search(insn);
+  auto callee = resolve_method(callee_ref, search, ref_cache, caller);
+  if (!callee && search == MethodSearch::Virtual) {
+    callee = resolve_method(callee_ref, MethodSearch::InterfaceVirtual,
+                            ref_cache, caller);
+    if (resolved_virtual_to_interface) {
+      *resolved_virtual_to_interface = callee != nullptr;
+    }
+  } else if (resolved_virtual_to_interface) {
+    *resolved_virtual_to_interface = false;
+  }
+  return callee;
+}
+
 /**
  * Given a scope defined by DexClass, a name and a proto look for the vmethod
  * on the top ancestor. Essentially finds where the method was introduced.

diff --git a/opt/resolve-refs/ResolveRefsPass.cpp b/opt/resolve-refs/ResolveRefsPass.cpp
@@ -250,17 +250,9 @@ void ResolveRefsPass::resolve_method_refs(const DexMethod* caller,
                                           RefStats& stats) {
   always_assert(insn->has_method());
   auto mref = insn->get_method();
-  auto mdef = resolve_method(mref, opcode_to_search(insn), caller);
-  bool resolved_to_interface = false;
-  if (!mdef && opcode_to_search(insn) == MethodSearch::Virtual) {
-    mdef = resolve_method(mref, MethodSearch::InterfaceVirtual, caller);
-    if (mdef) {
-      TRACE(RESO, 4, "InterfaceVirtual resolve to %s in %s", SHOW(mdef),
-            SHOW(insn));
-      const auto* cls = type_class(mdef->get_class());
-      resolved_to_interface = cls && is_interface(cls);
-    }
-  }
+  bool resolved_virtual_to_interface;
+  auto mdef =
+      resolve_invoke_method(insn, caller, &resolved_virtual_to_interface);
   if (!mdef && is_array_clone(insn)) {
     auto* object_array_clone = method::java_lang_Objects_clone();
     TRACE(RESO, 3, "Resolving %s\n\t=>%s", SHOW(mref),
@@ -294,7 +286,9 @@ void ResolveRefsPass::resolve_method_refs(const DexMethod* caller,
   TRACE(RESO, 3, "Resolving %s\n\t=>%s", SHOW(mref), SHOW(mdef));
   insn->set_method(mdef);
   stats.num_mref_simple_resolved++;
-  if (resolved_to_interface && opcode::is_invoke_virtual(insn->opcode())) {
+  if (resolved_virtual_to_interface && cls && is_interface(cls)) {
+    TRACE(RESO, 4, "InterfaceVirtual resolve to %s in %s", SHOW(mdef),
+          SHOW(insn));
     insn->set_opcode(OPCODE_INVOKE_INTERFACE);
     stats.num_resolve_to_interface++;
   }

diff --git a/opt/result-propagation/ResultPropagation.cpp b/opt/result-propagation/ResultPropagation.cpp
@@ -265,12 +265,7 @@ boost::optional<ParamIndex> ReturnParamResolver::get_return_param_index(
     return 0;
   }
 
-  auto callee = resolve_method(method, opcode_to_search(insn), resolved_refs);
-  if (callee == nullptr && opcode == OPCODE_INVOKE_VIRTUAL) {
-    // There are some invoke-virtual call on methods whose def are
-    // actually in interface.
-    callee = resolve_method(method, MethodSearch::InterfaceVirtual);
-  }
+  auto callee = resolve_invoke_method(insn, resolved_refs);
   if (callee == nullptr) {
     return boost::none;
   }

diff --git a/opt/type-analysis/TypeAnalysisCallGraphGenerationPass.cpp b/opt/type-analysis/TypeAnalysisCallGraphGenerationPass.cpp
@@ -78,12 +78,7 @@ class TypeAnalysisBasedStrategy : public MultipleCalleeBaseStrategy {
         if (!opcode::is_an_invoke(insn->opcode())) {
           continue;
         }
-        auto* resolved_callee = this->resolve_callee(method, insn);
-        if (resolved_callee == nullptr &&
-            opcode_to_search(insn) == MethodSearch::Virtual) {
-          resolved_callee = resolve_method(
-              insn->get_method(), MethodSearch::InterfaceVirtual, method);
-        }
+        auto* resolved_callee = resolve_invoke_method(insn, method);
         if (resolved_callee == nullptr) {
           continue;
         }

diff --git a/service/class-merging/ModelMerger.cpp b/service/class-merging/ModelMerger.cpp
@@ -135,27 +135,25 @@ void update_code_type_refs(
         if (!type_reference::proto_has_reference_to(proto, mergeables)) {
           continue;
         }
+        bool resolved_virtual_to_interface;
         const auto meth_def =
-            resolve_method(meth_ref, opcode_to_search(insn), meth);
+            resolve_invoke_method(insn, meth, &resolved_virtual_to_interface);
         // This is a very tricky case where ResolveRefs cannot resolve a
         // MethodRef to MethodDef. It is a invoke-virtual with a MethodRef
         // referencing an interface method implmentation defined in a subclass
         // of the referenced type. To resolve the actual def we need to go
         // through another interface method search. Maybe we should fix it in
         // ResolveRefs.
-        if (meth_def == nullptr) {
-          auto intf_def =
-              resolve_method(meth_ref, MethodSearch::InterfaceVirtual);
-          always_assert(insn->opcode() == OPCODE_INVOKE_VIRTUAL && intf_def);
-          auto new_proto =
-              type_reference::get_new_proto(proto, mergeable_to_merger);
-          DexMethodSpec spec;
-          spec.proto = new_proto;
-          meth_ref->change(spec, true /*rename on collision*/);
-          continue;
-        }
-        not_reached_log("Found mergeable referencing MethodRef %s\n",
-                        SHOW(meth_ref));
+        always_assert_log(resolved_virtual_to_interface,
+                          "Found mergeable referencing MethodRef %s\n",
+                          SHOW(meth_ref));
+        always_assert(insn->opcode() == OPCODE_INVOKE_VIRTUAL);
+        auto new_proto =
+            type_reference::get_new_proto(proto, mergeable_to_merger);
+        DexMethodSpec spec;
+        spec.proto = new_proto;
+        meth_ref->change(spec, true /*rename on collision*/);
+        continue;
       }
       ////////////////////////////////////////
       // Update simple type refs

diff --git a/service/constant-propagation/ConstantPropagationWholeProgramState.cpp b/service/constant-propagation/ConstantPropagationWholeProgramState.cpp
@@ -358,11 +358,7 @@ bool WholeProgramAwareAnalyzer::analyze_invoke(
     return false;
   }
   if (whole_program_state->has_call_graph()) {
-    auto method = resolve_method(insn->get_method(), opcode_to_search(insn));
-    if (method == nullptr && opcode_to_search(insn) == MethodSearch::Virtual) {
-      method =
-          resolve_method(insn->get_method(), MethodSearch::InterfaceVirtual);
-    }
+    auto method = resolve_invoke_method(insn);
     if (method == nullptr) {
       return false;
     }

diff --git a/service/method-inliner/MethodInliner.cpp b/service/method-inliner/MethodInliner.cpp
@@ -489,14 +489,7 @@ void gather_true_virtual_methods(
           continue;
         }
         auto insn_method = insn->get_method();
-        auto callee =
-            resolve_method(insn_method, opcode_to_search(insn), method);
-        if (callee == nullptr) {
-          // There are some invoke-virtual call on methods whose def are
-          // actually in interface.
-          callee = resolve_method(insn->get_method(),
-                                  MethodSearch::InterfaceVirtual);
-        }
+        auto callee = resolve_invoke_method(insn, method);
         if (callee == nullptr) {
           continue;
         }

diff --git a/service/type-analysis/TypeAnalysisRuntimeAssert.cpp b/service/type-analysis/TypeAnalysisRuntimeAssert.cpp
@@ -291,11 +291,7 @@ bool RuntimeAssertTransform::insert_return_value_assert(
   DexMethod* callee = nullptr;
   DexTypeDomain domain = DexTypeDomain::top();
   if (wps.has_call_graph()) {
-    callee = resolve_method(insn->get_method(), opcode_to_search(insn));
-    if (callee == nullptr && opcode_to_search(insn) == MethodSearch::Virtual) {
-      callee =
-          resolve_method(insn->get_method(), MethodSearch::InterfaceVirtual);
-    }
+    callee = resolve_invoke_method(insn);
     if (callee == nullptr || wps.method_is_dynamic(callee)) {
       domain = DexTypeDomain::top();
     }