From 2c10f8d6167d2a1030769c5f2144a89b02240287 Mon Sep 17 00:00:00 2001 From: Nikolai Tillmann Date: Tue, 17 Oct 2023 16:30:31 -0700 Subject: [PATCH] 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 --- .../ObjectSensitiveDcePass.cpp | 9 ++++++--- .../SideEffectSummary.cpp | 7 +------ opt/object-sensitive-dce/UsedVarsAnalysis.cpp | 12 +++++------ opt/object-sensitive-dce/UsedVarsAnalysis.h | 4 +++- opt/optimize_enums/EnumTransformer.cpp | 2 +- test/integ/ObjectSensitiveDceTest.cpp | 19 ++++++++++++++++++ test/integ/ObjectSensitiveDceTest.java | 20 +++++++++++++++++++ 7 files changed, 56 insertions(+), 17 deletions(-) diff --git a/opt/object-sensitive-dce/ObjectSensitiveDcePass.cpp b/opt/object-sensitive-dce/ObjectSensitiveDcePass.cpp index a79bcdf528..19dba1a959 100644 --- 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 index 9880411802..b47cb18dcb 100644 --- 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 index 836c4e93fc..5bfdbae823 100644 --- 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(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 index 042e06afcf..ef173d7227 100644 --- 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 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 index e11c422b71..aca5750700 100644 --- 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 index 752d7764e4..4073309f4f 100644 --- 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 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 index c620d0d15d..08eb6953e1 100644 --- 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() {}