Skip to content

Commit

Permalink
Deleting ditto dead code
Browse files Browse the repository at this point in the history
Summary: Moving large number of classes to the longtail module caused a crash. The final step of loading secondary dex files was verifying  the canary class. https://fburl.com/code/98pp1ymx. This canary class’s initializer (P898179371) caused the demand of longtail classes when voltron and fallback loader wasn’t available. It turned out to be dead which we should delete.

Reviewed By: agampe

Differential Revision: D51939733

fbshipit-source-id: 26123a1f4202d61e9c090fd5109b5542a8ece4a1
  • Loading branch information
Jidong Chen authored and facebook-github-bot committed Dec 8, 2023
1 parent 1d16eca commit 4f91636
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 82 deletions.
61 changes: 0 additions & 61 deletions opt/interdex/InterDex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1113,66 +1113,6 @@ void InterDex::add_dexes_from_store(const DexStore& store) {
post_process_dex(m_emitting_state, fodr);
}

void InterDex::set_clinit_methods_if_needed(DexClass* cls) const {
using namespace dex_asm;

if (m_methods_for_canary_clinit_reference.empty()) {
// No methods to call from clinit; don't create clinit.
return;
}

if (cls->get_clinit()) {
// We already created and added a clinit.
return;
}

// Create a clinit static method.
auto proto =
DexProto::make_proto(type::_void(), DexTypeList::make_type_list({}));
DexMethod* clinit =
DexMethod::make_method(cls->get_type(),
DexString::make_string("<clinit>"), proto)
->make_concrete(ACC_STATIC | ACC_CONSTRUCTOR, false);
clinit->set_code(std::make_unique<IRCode>());
cls->add_method(clinit);
clinit->set_deobfuscated_name(show_deobfuscated(clinit));

// Add code to clinit to call the other methods.
auto code = clinit->get_code();
size_t max_size = 0;
for (const auto& method_name : m_methods_for_canary_clinit_reference) {
// No need to do anything if this method isn't present in the build.
if (DexMethodRef* method = DexMethod::get_method(method_name)) {
std::vector<Operand> reg_operands;
int64_t reg = 0;
for (auto* dex_type : *method->get_proto()->get_args()) {
Operand reg_operand = {VREG, reg};
switch (dex_type->get_name()->c_str()[0]) {
case 'J':
case 'D':
// 8 bytes
code->push_back(dasm(OPCODE_CONST_WIDE, {reg_operand, 0_L}));
reg_operands.push_back(reg_operand);
reg += 2;
break;
default:
// 4 or fewer bytes
code->push_back(dasm(OPCODE_CONST, {reg_operand, 0_L}));
reg_operands.push_back(reg_operand);
++reg;
break;
}
}
max_size = std::max(max_size, (size_t)reg);
code->push_back(dasm(OPCODE_INVOKE_STATIC, method, reg_operands.begin(),
reg_operands.end()));
}
}
code->set_registers_size(max_size);
code->push_back(dasm(OPCODE_RETURN_VOID));
code->build_cfg();
}

// Creates a canary class if necessary. (In particular, the primary dex never
// has a canary class.) This method should be called after flush_out_dex when
// beginning a new dex. As canary classes are added in the end without checks,
Expand All @@ -1189,7 +1129,6 @@ DexClass* InterDex::get_canary_cls(EmittingState& emitting_state,
static std::mutex canary_mutex;
std::lock_guard<std::mutex> lock_guard(canary_mutex);
canary_cls = create_canary(dexnum);
set_clinit_methods_if_needed(canary_cls);
}
MethodRefs clazz_mrefs;
FieldRefs clazz_frefs;
Expand Down
6 changes: 0 additions & 6 deletions opt/interdex/InterDex.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ class InterDex {
const ReserveRefsInfo& reserve_refs,
const XStoreRefs* xstore_refs,
int min_sdk,
std::vector<std::string> methods_for_canary_clinit_reference,
const init_classes::InitClassesWithSideEffects&
init_classes_with_side_effects,
bool transitively_close_interdex_order,
Expand All @@ -80,8 +79,6 @@ class InterDex {
m_original_scope(original_scope),
m_scope(build_class_scope(m_dexen)),
m_xstore_refs(xstore_refs),
m_methods_for_canary_clinit_reference(
std::move(methods_for_canary_clinit_reference)),
m_transitively_close_interdex_order(transitively_close_interdex_order),
m_minimize_cross_dex_refs_explore_alternatives(
minimize_cross_dex_refs_explore_alternatives),
Expand Down Expand Up @@ -189,8 +186,6 @@ class InterDex {
void post_process_dex(EmittingState& emitting_state,
const FlushOutDexResult&) const;

void set_clinit_methods_if_needed(DexClass* cls) const;

/**
* Stores in m_interdex_order a list of coldstart types. It will only contain:
* * classes that still exist in the current scope
Expand Down Expand Up @@ -237,7 +232,6 @@ class InterDex {
std::vector<DexType*> m_interdex_types;
const XStoreRefs* m_xstore_refs;
size_t m_current_classes_when_emitting_remaining{0};
std::vector<std::string> m_methods_for_canary_clinit_reference;

size_t m_transitive_closure_added{0};
size_t m_transitive_closure_moved{0};
Expand Down
25 changes: 10 additions & 15 deletions opt/interdex/InterDexPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ void InterDexPass::bind_config() {
bind("can_touch_coldstart_extended_cls", false,
m_can_touch_coldstart_extended_cls);
bind("expect_order_list", false, m_expect_order_list);
bind("methods_for_canary_clinit_reference", {},
m_methods_for_canary_clinit_reference,
"If set, canary classes will have a clinit generated which call the "
"specified methods, if they exist");

bind("transitively_close_interdex_order", m_transitively_close_interdex_order,
m_transitively_close_interdex_order);
Expand Down Expand Up @@ -153,15 +149,15 @@ void InterDexPass::run_pass(
bool force_single_dex = conf.get_json_config().get("force_single_dex", false);
mgr.set_metric("config.force_single_dex", force_single_dex);

InterDex interdex(
original_scope, dexen, mgr.asset_manager(), conf, plugins,
m_linear_alloc_limit, m_static_prune, m_normal_primary_dex,
m_keep_primary_order, force_single_dex, m_order_interdex, m_emit_canaries,
m_minimize_cross_dex_refs, m_fill_last_coldstart_dex,
m_minimize_cross_dex_refs_config, refs_info, &xstore_refs,
mgr.get_redex_options().min_sdk, m_methods_for_canary_clinit_reference,
init_classes_with_side_effects, m_transitively_close_interdex_order,
m_minimize_cross_dex_refs_explore_alternatives, cache);
InterDex interdex(original_scope, dexen, mgr.asset_manager(), conf, plugins,
m_linear_alloc_limit, m_static_prune, m_normal_primary_dex,
m_keep_primary_order, force_single_dex, m_order_interdex,
m_emit_canaries, m_minimize_cross_dex_refs,
m_fill_last_coldstart_dex, m_minimize_cross_dex_refs_config,
refs_info, &xstore_refs, mgr.get_redex_options().min_sdk,
init_classes_with_side_effects,
m_transitively_close_interdex_order,
m_minimize_cross_dex_refs_explore_alternatives, cache);

if (m_expect_order_list) {
always_assert_log(
Expand Down Expand Up @@ -251,8 +247,7 @@ void InterDexPass::run_pass_on_nonroot_store(
false /* emit canaries */, false /* minimize_cross_dex_refs */,
/* fill_last_coldstart_dex=*/false, cross_dex_refs_config, refs_info,
&xstore_refs, mgr.get_redex_options().min_sdk,
m_methods_for_canary_clinit_reference, init_classes_with_side_effects,
m_transitively_close_interdex_order,
init_classes_with_side_effects, m_transitively_close_interdex_order,
m_minimize_cross_dex_refs_explore_alternatives, cache);

interdex.run_on_nonroot_store();
Expand Down

0 comments on commit 4f91636

Please sign in to comment.