Don't be afraid of invoke-interface, -super

Summary: Handling invoke-interface is a no-op for now (more on that in later diffs), but handling invoke-super does find more opportunities. Reviewed By: ssj933 Differential Revision: D49525943 fbshipit-source-id: 26e3177b929ebecf1359a0695eab565f653f4a41
facebook · Oct 17, 2023 · 2c10f8d · 2c10f8d
1 parent f08a7d9
commit 2c10f8d
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 17 deletions.
diff --git a/opt/object-sensitive-dce/ObjectSensitiveDcePass.cpp b/opt/object-sensitive-dce/ObjectSensitiveDcePass.cpp
@@ -65,7 +65,11 @@ class CallGraphStrategy final : public call_graph::BuildStrategy {
       if (opcode::is_an_invoke(insn->opcode())) {
         auto callee =
             resolve_method(insn->get_method(), opcode_to_search(insn), method);
-        if (callee == nullptr || may_be_overridden(callee)) {
+        if (callee == nullptr) {
+          continue;
+        }
+        if (!opcode::is_invoke_super(insn->opcode()) &&
+            may_be_overridden(callee)) {
           continue;
         }
         callsites.emplace_back(callee, insn);
@@ -170,8 +174,7 @@ void ObjectSensitiveDcePass::run_pass(DexStoresVector& stores,
     auto& cfg = code.cfg();
     uv::FixpointIterator used_vars_fp_iter(
         *ptrs_fp_iter_map.at_unsafe(method),
-        build_summary_map(effect_summaries, call_graph, method),
-        cfg);
+        build_summary_map(effect_summaries, call_graph, method), cfg, method);
     used_vars_fp_iter.run(uv::UsedVarsSet());
 
     cfg::CFGMutation mutator(cfg);

diff --git a/opt/object-sensitive-dce/SideEffectSummary.cpp b/opt/object-sensitive-dce/SideEffectSummary.cpp
@@ -172,13 +172,8 @@ void SummaryBuilder::analyze_instruction_effects(
     classify_heap_write(env, insn->src(0), summary);
     break;
   }
-
   case OPCODE_INVOKE_SUPER:
-  case OPCODE_INVOKE_INTERFACE: {
-    TRACE(OSDCE, 3, "Unknown invoke: %s", SHOW(insn));
-    summary->effects |= EFF_UNKNOWN_INVOKE;
-    break;
-  }
+  case OPCODE_INVOKE_INTERFACE:
   case OPCODE_INVOKE_STATIC:
   case OPCODE_INVOKE_DIRECT:
   case OPCODE_INVOKE_VIRTUAL: {

diff --git a/opt/object-sensitive-dce/UsedVarsAnalysis.cpp b/opt/object-sensitive-dce/UsedVarsAnalysis.cpp
@@ -45,8 +45,10 @@ using namespace ir_analyzer;
 FixpointIterator::FixpointIterator(
     const local_pointers::FixpointIterator& pointers_fp_iter,
     side_effects::InvokeToSummaryMap invoke_to_summary_map,
-    const cfg::ControlFlowGraph& cfg)
+    const cfg::ControlFlowGraph& cfg,
+    const DexMethod* method)
     : BaseBackwardsIRAnalyzer<UsedVarsSet>(cfg),
+      m_method(method),
       m_insn_env_map(gen_instruction_environment_map(cfg, pointers_fp_iter)),
       m_invoke_to_summary_map(std::move(invoke_to_summary_map)) {}
 
@@ -173,10 +175,12 @@ bool FixpointIterator::is_required(const IRInstruction* insn,
   case OPCODE_SPUT_SHORT: {
     return true;
   }
+  case OPCODE_INVOKE_SUPER:
+  case OPCODE_INVOKE_INTERFACE:
   case OPCODE_INVOKE_DIRECT:
   case OPCODE_INVOKE_STATIC:
   case OPCODE_INVOKE_VIRTUAL: {
-    auto method = resolve_method(insn->get_method(), opcode_to_search(insn));
+    auto method = resolve_invoke_method(insn, m_method);
     if (method == nullptr) {
       return true;
     }
@@ -206,10 +210,6 @@ bool FixpointIterator::is_required(const IRInstruction* insn,
           return is_used_or_escaping_write(env, used_vars, insn->src(idx));
         });
   }
-  case OPCODE_INVOKE_SUPER:
-  case OPCODE_INVOKE_INTERFACE: {
-    return true;
-  }
   default: {
     if (insn->has_dest()) {
       return used_vars.contains(insn->dest());

diff --git a/opt/object-sensitive-dce/UsedVarsAnalysis.h b/opt/object-sensitive-dce/UsedVarsAnalysis.h
@@ -68,7 +68,8 @@ class FixpointIterator final
  public:
   FixpointIterator(const local_pointers::FixpointIterator& pointers_fp_iter,
                    side_effects::InvokeToSummaryMap invoke_to_summary_map,
-                   const cfg::ControlFlowGraph& cfg);
+                   const cfg::ControlFlowGraph& cfg,
+                   const DexMethod* method = nullptr);
 
   void analyze_instruction(IRInstruction* insn,
                            UsedVarsSet* used_vars) const override;
@@ -91,6 +92,7 @@ class FixpointIterator final
   }
 
  private:
+  const DexMethod* m_method;
   std::unordered_map<const IRInstruction*, local_pointers::Environment>
       m_insn_env_map;
   side_effects::InvokeToSummaryMap m_invoke_to_summary_map;

diff --git a/opt/optimize_enums/EnumTransformer.cpp b/opt/optimize_enums/EnumTransformer.cpp
@@ -1536,7 +1536,7 @@ class EnumTransformer final {
     cfg.calculate_exit_block();
     ptrs::FixpointIterator fp_iter(cfg);
     fp_iter.run(ptrs::Environment());
-    used_vars::FixpointIterator uv_fpiter(fp_iter, summaries, cfg);
+    used_vars::FixpointIterator uv_fpiter(fp_iter, summaries, cfg, ctor);
     uv_fpiter.run(used_vars::UsedVarsSet());
     auto dead_instructions = used_vars::get_dead_instructions(cfg, uv_fpiter);
     for (const auto& insn : dead_instructions) {

diff --git a/test/integ/ObjectSensitiveDceTest.cpp b/test/integ/ObjectSensitiveDceTest.cpp
@@ -60,6 +60,25 @@ TEST_F(ObjectSensitiveDceTest, basic) {
   ASSERT_EQ(it->insn->opcode(), OPCODE_RETURN_VOID);
 }
 
+TEST_F(ObjectSensitiveDceTest, invoke_super) {
+  auto method_ref = DexMethod::get_method(
+      "Lcom/facebook/redextest/ObjectSensitiveDceTest;.invoke_super:()V");
+  EXPECT_NE(method_ref, nullptr);
+  auto method = method_ref->as_def();
+  EXPECT_NE(method, nullptr);
+
+  std::vector<Pass*> passes = {
+      new ObjectSensitiveDcePass(),
+  };
+
+  run_passes(passes);
+
+  auto ii = InstructionIterable(method->get_code());
+  auto it = ii.begin();
+  ASSERT_TRUE(it != ii.end());
+  ASSERT_EQ(it->insn->opcode(), OPCODE_RETURN_VOID);
+}
+
 TEST_F(ObjectSensitiveDceTest, clinit_with_side_effects) {
   auto method_ref = DexMethod::get_method(
       "Lcom/facebook/redextest/"

diff --git a/test/integ/ObjectSensitiveDceTest.java b/test/integ/ObjectSensitiveDceTest.java
@@ -13,6 +13,11 @@ public static void basic() {
     useless.foo();
   }
 
+  public static void invoke_super() {
+    UselessDerived useless = new UselessDerived();
+    useless.foo();
+  }
+
   public static void clinit_with_side_effects() {
     UselessWithClInitWithSideEffects useless = new UselessWithClInitWithSideEffects();
     useless.foo();
@@ -32,6 +37,21 @@ public void foo() {
   }
 }
 
+class UselessBase {
+  int F;
+  public UselessBase() {}
+  public void foo() {
+    F = 42;
+  }
+}
+
+class UselessDerived extends UselessBase {
+  public UselessDerived() {}
+  public void foo() {
+    super.foo();
+  }
+}
+
 class UselessWithClInitWithSideEffects {
   int F;
   public UselessWithClInitWithSideEffects() {}