Skip to content

Commit

Permalink
consistent treatment of native
Browse files Browse the repository at this point in the history
Summary:
It used to be that the non-singleton callgraphs were a bit inconsistent with native methods:
- for non-true virtuals, they would get added to the callgraph
- however, for true virtuals, they would get omitted; instead, all true virtual native methods were added to the dynamic-methods list

Instead, we know always keep around native methods just like those with `get_code()`, and leave it up to whoever inspects the callgraph to do the right thing.

Reviewed By: agampe

Differential Revision: D49800674

fbshipit-source-id: c73cc0d8a5d339b05e8731886a398c10fa8a3c43
  • Loading branch information
Nikolai Tillmann authored and facebook-github-bot committed Oct 17, 2023
1 parent 52b94a1 commit f6dd675
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 27 deletions.
37 changes: 12 additions & 25 deletions libredex/CallGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,22 @@ MultipleCalleeBaseStrategy::MultipleCalleeBaseStrategy(
m_method_override_graph(method_override_graph) {}

const std::vector<const DexMethod*>&
MultipleCalleeBaseStrategy::get_ordered_overriding_methods_with_code(
MultipleCalleeBaseStrategy::get_ordered_overriding_methods_with_code_or_native(
const DexMethod* method) const {
auto res = m_overriding_methods_cache.get(method);
if (res) {
return *res;
}
return init_ordered_overriding_methods_with_code(
return init_ordered_overriding_methods_with_code_or_native(
method, mog::get_overriding_methods(m_method_override_graph, method));
}

const std::vector<const DexMethod*>&
MultipleCalleeBaseStrategy::init_ordered_overriding_methods_with_code(
MultipleCalleeBaseStrategy::init_ordered_overriding_methods_with_code_or_native(
const DexMethod* method,
std::vector<const DexMethod*> overriding_methods) const {
std20::erase_if(overriding_methods, [](auto* m) { return !m->get_code(); });
std20::erase_if(overriding_methods,
[](auto* m) { return !m->get_code() && !is_native(m); });
std::sort(overriding_methods.begin(), overriding_methods.end(),
compare_dexmethods);
return *m_overriding_methods_cache
Expand Down Expand Up @@ -185,19 +186,6 @@ RootAndDynamic MultipleCalleeBaseStrategy::get_roots() const {
}
}
}
if (is_native(method)) {
dynamic_methods.emplace(method);
const auto& overriding_methods =
mog::get_overriding_methods(m_method_override_graph, method, true);
for (auto m : overriding_methods) {
dynamic_methods.emplace(m);
}
const auto& overiden_methods =
mog::get_overridden_methods(m_method_override_graph, method, true);
for (auto m : overiden_methods) {
dynamic_methods.emplace(m);
}
}
}
return root_and_dynamic;
}
Expand All @@ -221,16 +209,15 @@ CallSites CompleteCallGraphStrategy::get_callsites(
if (callee == nullptr) {
return editable_cfg_adapter::LOOP_CONTINUE;
}
if (callee->is_concrete()) {
if (callee->get_code() || is_native(callee)) {
callsites.emplace_back(callee, insn);
}
if (opcode::is_invoke_virtual(insn->opcode()) ||
opcode::is_invoke_interface(insn->opcode())) {
const auto& overriding_methods =
get_ordered_overriding_methods_with_code(callee);

for (auto m : overriding_methods) {
callsites.emplace_back(m, insn);
get_ordered_overriding_methods_with_code_or_native(callee);
for (auto overriding_method : overriding_methods) {
callsites.emplace_back(overriding_method, insn);
}
}
}
Expand Down Expand Up @@ -310,7 +297,7 @@ MultipleCalleeStrategy::MultipleCalleeStrategy(
}
}
if (num_override <= big_override_threshold) {
init_ordered_overriding_methods_with_code(
init_ordered_overriding_methods_with_code_or_native(
callee, std::move(overriding_methods));
} else {
m_big_override.emplace(callee);
Expand Down Expand Up @@ -343,11 +330,11 @@ CallSites MultipleCalleeStrategy::get_callsites(const DexMethod* method) const {
if (m_big_override.count_unsafe(callee)) {
return editable_cfg_adapter::LOOP_CONTINUE;
}
if (callee->get_code()) {
if (callee->get_code() || is_native(callee)) {
callsites.emplace_back(callee, insn);
}
const auto& overriding_methods =
get_ordered_overriding_methods_with_code(callee);
get_ordered_overriding_methods_with_code_or_native(callee);
for (auto overriding_method : overriding_methods) {
callsites.emplace_back(overriding_method, insn);
}
Expand Down
5 changes: 3 additions & 2 deletions libredex/CallGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,12 @@ class MultipleCalleeBaseStrategy : public SingleCalleeStrategy {
RootAndDynamic get_roots() const override;

protected:
const std::vector<const DexMethod*>& get_ordered_overriding_methods_with_code(
const std::vector<const DexMethod*>&
get_ordered_overriding_methods_with_code_or_native(
const DexMethod* method) const;

const std::vector<const DexMethod*>&
init_ordered_overriding_methods_with_code(
init_ordered_overriding_methods_with_code_or_native(
const DexMethod* method, std::vector<const DexMethod*>) const;

const method_override_graph::Graph& m_method_override_graph;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ class WholeProgramStateAccessor {
}
for (const DexMethod* callee : callees) {
if (!callee->get_code()) {
always_assert(is_native(callee));
return ConstantValue::top();
}
}
Expand Down
4 changes: 4 additions & 0 deletions service/type-analysis/WholeProgramState.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ class WholeProgramState {
}
DexTypeDomain ret = DexTypeDomain::bottom();
for (const DexMethod* callee : callees) {
if (!callee->get_code()) {
always_assert(is_native(callee));
return DexTypeDomain::top();
}
auto val = m_method_partition.get(callee);
ret.join_with(val);
}
Expand Down

0 comments on commit f6dd675

Please sign in to comment.