Skip to content

Commit

Permalink
Remove ArrayNullnessDomain from DexTypeDomain
Browse files Browse the repository at this point in the history 
Summary: Dis-engage `ArrayConstNullnessDomain` from `DexTypeDomain`. It is effectively not being used by the analysis. We can clean up the leftover logic.

Reviewed By: ssj933

Differential Revision: D51720366

fbshipit-source-id: 2c92a03d6e70048dba448b93c20ba3b21a94348d
  • Loading branch information
@facebook-github-bot
Wei Zhang (Devinfra) authored and facebook-github-bot committed Dec 4, 2023
1 parent 032fb59 commit b099839
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 127 deletions.
54 changes: 6 additions & 48 deletions libredex/DexTypeEnvironment.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,15 @@ using TypedefAnnotationDomain = SingletonDexTypeDomain;

/*
*
* ArrayConstNullnessDomain X SingletonDexTypeDomain X SmallSetDexTypeDomain
* ConstNullnessDomain X SingletonDexTypeDomain X SmallSetDexTypeDomain
*
*
* When the SmallSetDexTypeDomain has elements, then they represent an exact set
* of non-interface classes (including arrays), or possibly java.lang.Throwable.
*/
class DexTypeDomain final
: public sparta::ReducedProductAbstractDomain<DexTypeDomain,
ArrayConstNullnessDomain,
ConstNullnessDomain,
SingletonDexTypeDomain,
SmallSetDexTypeDomain,
TypedefAnnotationDomain> {
Expand All @@ -272,7 +272,7 @@ class DexTypeDomain final

using BaseType =
sparta::ReducedProductAbstractDomain<DexTypeDomain,
ArrayConstNullnessDomain,
ConstNullnessDomain,
SingletonDexTypeDomain,
SmallSetDexTypeDomain,
TypedefAnnotationDomain>;
Expand Down Expand Up @@ -322,21 +322,14 @@ class DexTypeDomain final
: TypedefAnnotationDomain())) {}

static void reduce_product(
std::tuple<ArrayConstNullnessDomain,
std::tuple<ConstNullnessDomain,
SingletonDexTypeDomain,
SmallSetDexTypeDomain,
TypedefAnnotationDomain>& /* product */) {}

static DexTypeDomain null() { return DexTypeDomain(IS_NULL); }

NullnessDomain get_nullness() const {
auto domain = get<0>();
if (domain.which() == 0) {
return domain.get<ConstNullnessDomain>().get_nullness();
} else {
return domain.get<ArrayNullnessDomain>().get_nullness();
}
}
NullnessDomain get_nullness() const { return get<0>().get_nullness(); }

bool is_null() const { return get_nullness().element() == IS_NULL; }

Expand All @@ -345,42 +338,7 @@ class DexTypeDomain final
bool is_nullable() const { return get_nullness().is_top(); }

boost::optional<ConstantDomain::ConstantType> get_constant() const {
if (auto const_nullness = get<0>().maybe_get<ConstNullnessDomain>()) {
return const_nullness->const_domain().get_constant();
}
return boost::none;
}

ArrayNullnessDomain get_array_nullness() const {
if (auto array_nullness = get<0>().maybe_get<ArrayNullnessDomain>()) {
return *array_nullness;
}
return ArrayNullnessDomain::top();
}

NullnessDomain get_array_element_nullness(
boost::optional<int64_t> idx) const {
if (!ArrayNullnessDomain::is_valid_array_idx(idx)) {
return NullnessDomain::top();
}
return get_array_nullness().get_element(*idx);
}

void set_array_element_nullness(boost::optional<int64_t> idx,
const NullnessDomain& nullness) {
if (!ArrayNullnessDomain::is_valid_array_idx(idx)) {
apply<0>([&](ArrayConstNullnessDomain* d) {
d->apply<ArrayNullnessDomain>([&](ArrayNullnessDomain* array_nullness) {
array_nullness->reset_elements();
});
});
return;
}
apply<0>([&](ArrayConstNullnessDomain* d) {
d->apply<ArrayNullnessDomain>([&](ArrayNullnessDomain* array_nullness) {
array_nullness->set_element(*idx, nullness);
});
});
return get<0>().const_domain().get_constant();
}

const SingletonDexTypeDomain& get_single_domain() const { return get<1>(); }
Expand Down
70 changes: 1 addition & 69 deletions service/type-analysis/LocalTypeAnalyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,52 +96,13 @@ bool RegisterTypeAnalyzer::analyze_aget(const IRInstruction* insn,

always_assert_log(type::is_array(*array_type), "Wrong array type %s in %s",
SHOW(*array_type), SHOW(insn));
auto idx_opt = env->get(insn->src(1)).get_constant();
auto nullness = env->get(insn->src(0)).get_array_element_nullness(idx_opt);
const auto ctype = type::get_array_component_type(*array_type);
auto cls = type_class(ctype);
bool is_type_exact = cls && !cls->is_external() && is_final(cls);
// is_type_exact is to decide whether to populate the
// small-set-dex-type-domain, which should only hold exact (non-interface)
// class (and possibly java.lang.Throwable, but we ignore that here).
env->set(RESULT_REGISTER,
DexTypeDomain(ctype, nullness.element(), is_type_exact));
return true;
}

/*
* Only populating array nullness since we don't model array element types.
*/
bool RegisterTypeAnalyzer::analyze_aput(const IRInstruction* insn,
DexTypeEnvironment* env) {
if (insn->opcode() != OPCODE_APUT_OBJECT) {
return false;
}
boost::optional<int64_t> idx_opt = env->get(insn->src(2)).get_constant();
auto nullness = env->get(insn->src(0)).get_nullness();
env->mutate_reg_environment([&](RegTypeEnvironment* env) {
auto array_reg = insn->src(1);
env->update(array_reg, [&](const DexTypeDomain& domain) {
auto copy = domain;
copy.set_array_element_nullness(idx_opt, nullness);
return copy;
});
});
return true;
}

bool RegisterTypeAnalyzer::analyze_array_length(const IRInstruction* insn,
DexTypeEnvironment* env) {
auto array_nullness = env->get(insn->src(0)).get_array_nullness();
if (array_nullness.is_top()) {
env->set(RESULT_REGISTER, DexTypeDomain::top());
return true;
}
if (auto array_length = array_nullness.get_length()) {
env->set(RESULT_REGISTER, DexTypeDomain(*array_length));
} else {
env->set(RESULT_REGISTER, DexTypeDomain::top());
}
env->set(RESULT_REGISTER, DexTypeDomain(ctype, NN_TOP, is_type_exact));
return true;
}

Expand Down Expand Up @@ -348,35 +309,6 @@ bool RegisterTypeAnalyzer::analyze_invoke(const IRInstruction* insn,
// Note we don't need to take care of the RESULT_REGISTER update from this
// point. The remaining cases are already taken care by the
// WholeProgramAwareAnalyzer::analyze_invoke.
//
// When passed through a call, we need to reset the elements of an
// ArrayNullnessDomain. The domain passed to the callee is a copy and can be
// written over there. That means that the local ArrayNullnessDomain stored in
// the caller environment might be out of date.
//
// E.g., a newly allocated array in a caller environment has its elements
// initially as UNINITIALIED. The array elements can be updated by a callee
// which has access to the array. At that point, the updated element is no
// longer UNINITIALIED. However, the change is not propagated to the caller
// environment. Reference: T107422148, T123970364
for (auto src : insn->srcs()) {
auto type_domain = env->get(src);
auto array_nullness = type_domain.get_array_nullness();
auto dex_type = type_domain.get_dex_type();

if (!array_nullness.is_top() && array_nullness.get_length() &&
*array_nullness.get_length() > 0 && dex_type) {
env->mutate_reg_environment([&](RegTypeEnvironment* env) {
env->transform([&](const DexTypeDomain& domain) {
auto dex_type_local = domain.get_dex_type();
if (dex_type_local && *dex_type == *dex_type_local) {
return DexTypeDomain(*dex_type_local, NN_TOP, false);
}
return domain;
});
});
}
}
return false;
}

Expand Down
5 changes: 0 additions & 5 deletions service/type-analysis/LocalTypeAnalyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ class RegisterTypeAnalyzer final

static bool analyze_aget(const IRInstruction* insn, DexTypeEnvironment* env);

static bool analyze_aput(const IRInstruction* insn, DexTypeEnvironment* env);

static bool analyze_array_length(const IRInstruction* insn,
DexTypeEnvironment* env);

static bool analyze_binop_lit(const IRInstruction* insn,
DexTypeEnvironment* env);

Expand Down
4 changes: 0 additions & 4 deletions test/integ/GlobalTypeAnalysisTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ TEST_F(GlobalTypeAnalysisTest, PrimitiveArrayTest) {
EXPECT_TRUE(rtype.is_not_null());
EXPECT_EQ(rtype.get_single_domain(),
SingletonDexTypeDomain(get_type_simple("[B")));
EXPECT_TRUE(rtype.get_array_nullness().is_top());
}

TEST_F(GlobalTypeAnalysisTest, InstanceSensitiveCtorTest) {
Expand Down Expand Up @@ -309,7 +308,6 @@ TEST_F(GlobalTypeAnalysisTest, ArrayNullnessEscapeTest) {
EXPECT_EQ(rtype.get_single_domain(),
SingletonDexTypeDomain(
get_type_simple("Lcom/facebook/redextest/TestM$A;")));
EXPECT_TRUE(rtype.get_array_nullness().is_top());
}

TEST_F(GlobalTypeAnalysisTest, ArrayNullnessEscape2Test) {
Expand All @@ -329,7 +327,6 @@ TEST_F(GlobalTypeAnalysisTest, ArrayNullnessEscape2Test) {
EXPECT_EQ(rtype.get_single_domain(),
SingletonDexTypeDomain(
get_type_simple("Lcom/facebook/redextest/TestN$A;")));
EXPECT_TRUE(rtype.get_array_nullness().is_top());

auto dance2 = get_method("TestN;.danceWithArray2", "",
"Lcom/facebook/redextest/TestN$A;");
Expand All @@ -340,7 +337,6 @@ TEST_F(GlobalTypeAnalysisTest, ArrayNullnessEscape2Test) {
EXPECT_EQ(rtype.get_single_domain(),
SingletonDexTypeDomain(
get_type_simple("Lcom/facebook/redextest/TestN$A;")));
EXPECT_TRUE(rtype.get_array_nullness().is_top());
}

TEST_F(GlobalTypeAnalysisTest, MultipleCalleeTest) {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/type-analysis/DexTypeEnvironmentTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ TEST_F(DexTypeEnvironmentTest, ConstNullnessDomainTest) {

c1.join_with(nl);
EXPECT_TRUE(c1.is_top());
EXPECT_TRUE(c1.get<0>().get<ConstNullnessDomain>().const_domain().is_top());
EXPECT_TRUE(c1.get<0>().const_domain().is_top());
EXPECT_TRUE(c1.get_nullness().is_top());
EXPECT_TRUE(c1.is_nullable());
}
Expand Down

0 comments on commit b099839

Please sign in to comment.