Skip to content

Commit

Permalink
Support relaxed inits in definitely-assigned-ifields analysis
Browse files Browse the repository at this point in the history
Summary: Fields in classes where base constructor on new-instance is called may remain unassigned (0).

Reviewed By: beicy

Differential Revision: D56045539

fbshipit-source-id: de34eba4638b87893b05866ebddd9478f923b710
  • Loading branch information
Nikolai Tillmann authored and facebook-github-bot committed Apr 12, 2024
1 parent b92e8fb commit 0e68911
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 0 deletions.
33 changes: 33 additions & 0 deletions service/constant-propagation/DefinitelyAssignedIFields.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "ConstantPropagationAnalysis.h"
#include "IRCode.h"
#include "IRInstruction.h"
#include "Lazy.h"
#include "LiveRange.h"
#include "Resolver.h"
#include "StlUtil.h"
#include "Timer.h"
Expand Down Expand Up @@ -321,8 +323,39 @@ std::unordered_set<const DexField*> get_definitely_assigned_ifields(
})
.first;
};
ConcurrentSet<const DexType*> classes_with_relaxed_invoke_init;
walk::code(scope, [&](DexMethod*, IRCode& code) {
auto ii = InstructionIterable(code.cfg());
if (std::none_of(ii.begin(), ii.end(), [](auto& mie) {
return opcode::is_new_instance(mie.insn->opcode());
})) {
return;
}
live_range::MoveAwareChains chains(
code.cfg(), /* ignore_unreachable */ false,
[&](auto* insn) { return opcode::is_new_instance(insn->opcode()); });
for (auto& [def, uses] : chains.get_def_use_chains()) {
always_assert(opcode::is_new_instance(def->opcode()));
for (auto& use : uses) {
if (opcode::is_invoke_direct(use.insn->opcode()) &&
method::is_init(use.insn->get_method())) {
auto resolved =
resolve_method(use.insn->get_method(), MethodSearch::Direct);
if (resolved == nullptr || resolved->get_class() != def->get_type()) {
classes_with_relaxed_invoke_init.insert(def->get_type());
}
}
}
}
});
ConcurrentSet<const DexField*> res;
walk::parallel::classes(scope, [&](DexClass* cls) {
if (classes_with_relaxed_invoke_init.count(cls->get_type())) {
// All constructors, if any, of this class may be skipped.
// TODO: Analyze all new-instance occurrences to see if the fields still
// get initialized before being used.
return;
}
const auto ctors = cls->get_ctors();
if (ctors.empty()) {
// Actually, without a constructor, all fields *are* definitely assigned.
Expand Down
69 changes: 69 additions & 0 deletions test/unit/constant-propagation/IPConstantPropagationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2036,6 +2036,75 @@ TEST_F(InterproceduralConstantPropagationTest,
EXPECT_CODE_EQ(m->get_code(), expected_code.get());
}

TEST_F(InterproceduralConstantPropagationTest,
constantFieldAfterInit_relaxed_init) {
auto cls_ty = DexType::make_type("LFoo;");
ClassCreator creator(cls_ty);
creator.set_super(type::java_lang_Throwable());

auto field_f = DexField::make_field("LFoo;.f:I")->make_concrete(ACC_PUBLIC);
creator.add_field(field_f);

auto init = assembler::method_from_string(R"(
(method (public constructor) "LFoo;.<init>:()V"
(
(load-param-object v0)
(const v1 42)
(iput v1 v0 "LFoo;.f:I")
(return-void)
)
)
)");
init->rstate.set_root(); // Make this an entry point
creator.add_method(init);

auto m = assembler::method_from_string(R"(
(method (public static) "LFoo;.baz:(LFoo;)I"
(
(new-instance "LFoo;")
(move-result-pseudo-object v0)
(invoke-direct (v0) "Ljava/lang/Object;.<init>:()V")
(iget v0 "LFoo;.f:I")
(move-result-pseudo v0)
(return v0)
)
)
)");
m->rstate.set_root(); // Make this an entry point
creator.add_method(m);

Scope scope{creator.create()};
walk::code(scope, [](DexMethod*, IRCode& code) {
code.build_cfg();
code.cfg().calculate_exit_block();
});

InterproceduralConstantPropagationPass::Config config;
config.max_heap_analysis_iterations = 2;

auto fp_iter = InterproceduralConstantPropagationPass(config).analyze(
scope, &m_immut_analyzer_state, &m_api_level_analyzer_state);
auto& wps = fp_iter->get_whole_program_state();
// 0 is included in the numeric interval as no actual constructor was ever
// called
EXPECT_EQ(wps.get_field_value(field_f), SignedConstantDomain(0, 42));

InterproceduralConstantPropagationPass(config).run(make_simple_stores(scope));

auto expected_code = assembler::ircode_from_string(R"(
(
(new-instance "LFoo;")
(move-result-pseudo-object v0)
(invoke-direct (v0) "Ljava/lang/Object;.<init>:()V")
(iget v0 "LFoo;.f:I")
(move-result-pseudo v0)
(return v0)
)
)");
m->get_code()->clear_cfg();
EXPECT_CODE_EQ(m->get_code(), expected_code.get());
}

TEST_F(InterproceduralConstantPropagationTest,
constantFieldAfterInit_read_before_write) {
auto cls_ty = DexType::make_type("LFoo;");
Expand Down

0 comments on commit 0e68911

Please sign in to comment.