From 4fcfcb67f36cba5311055f25730c5a1fafa5b531 Mon Sep 17 00:00:00 2001 From: Ivy Tang Date: Fri, 31 Jan 2025 15:12:39 -0800 Subject: [PATCH] patch chained getters Summary: got a non-determinism task since getters can be chained P1722216211 https://fburl.com/dexbolt/z5ynhn3e. patch the chained getters by saving the candidate class names to patch and iterating over them again also use missing_param_annos to not double patch parameters Reviewed By: thezhangwei Differential Revision: D68924809 fbshipit-source-id: 7b334e84a05a2e0527d3e711d14bec30414cee8e --- .../TypedefAnnoPatcher.cpp | 54 ++++++++++++++++--- opt/typedef-anno-checker/TypedefAnnoPatcher.h | 5 ++ test/integ/typedef/TypedefAnnoCheckerTest.cpp | 28 ++++++++++ 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp b/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp index 7e487d406dd..39d5ae7ea87 100644 --- a/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp +++ b/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp @@ -213,11 +213,14 @@ void add_param_annotations(DexMethod* m, // Run usedefs to see if the source of the annotated value is a parameter // If it is, return the index the parameter. If not, return -1 -int find_and_patch_parameter(DexMethod* m, - IRInstruction* insn, - src_index_t arg_index, - DexAnnotationSet* anno_set, - live_range::UseDefChains* ud_chains) { +int find_and_patch_parameter( + DexMethod* m, + IRInstruction* insn, + src_index_t arg_index, + DexAnnotationSet* anno_set, + live_range::UseDefChains* ud_chains, + std::vector>* + missing_param_annos = nullptr) { // Patch missing parameter annotations from accessed fields live_range::Use use_of_id{insn, arg_index}; auto udchains_it = ud_chains->find(use_of_id); @@ -234,7 +237,9 @@ int find_and_patch_parameter(DexMethod* m, if (!is_static(m)) { param_index -= 1; } - add_param_annotations(m, anno_set, param_index); + if (!missing_param_annos) { + add_param_annotations(m, anno_set, param_index); + } return param_index; } param_index += 1; @@ -392,7 +397,31 @@ void TypedefAnnoPatcher::run(const Scope& scope) { patch_enclosed_method(cls); patch_ctor_params_from_synth_cls_fields(cls); } + populate_chained_getters(cls); }); + patch_chained_getters(); +} + +void TypedefAnnoPatcher::populate_chained_getters(DexClass* cls) { + auto class_name = cls->get_deobfuscated_name_or_empty_copy(); + auto class_name_prefix = class_name.substr(0, class_name.find('$')); + if (m_patched_returns.count(class_name_prefix)) { + m_chained_getters.insert(cls); + } +} + +void TypedefAnnoPatcher::patch_chained_getters() { + std::vector sorted_candidates; + for (auto* cls : m_chained_getters) { + sorted_candidates.push_back(cls); + } + std::sort(sorted_candidates.begin(), sorted_candidates.end(), + compare_dexclasses); + for (auto* cls : sorted_candidates) { + for (auto m : cls->get_all_methods()) { + patch_parameters_and_returns(m); + } + } } void TypedefAnnoPatcher::patch_ctor_params_from_synth_cls_fields( @@ -643,7 +672,8 @@ void TypedefAnnoPatcher::patch_synth_cls_fields_from_ctor_param( for (cfg::Block* b : cfg.blocks()) { for (auto& mie : InstructionIterable(b)) { auto* insn = mie.insn; - if (!opcode::is_an_iput(insn->opcode())) { + if (insn->opcode() != OPCODE_IPUT && + insn->opcode() != OPCODE_IPUT_OBJECT) { continue; } DexField* field = insn->get_field()->as_def(); @@ -735,6 +765,11 @@ void TypedefAnnoPatcher::patch_parameters_and_returns( return; } + if (has_typedef_annos(m->get_param_anno(), m_typedef_annos) || + type_inference::get_typedef_anno_from_member(m, m_typedef_annos)) { + return; + } + always_assert_log(code->editable_cfg_built(), "%s has no cfg built", SHOW(m)); auto& cfg = code->cfg(); type_inference::TypeInference inference(cfg, false, m_typedef_annos, @@ -775,5 +810,10 @@ void TypedefAnnoPatcher::patch_parameters_and_returns( anno_set.add_annotation(std::make_unique( DexType::make_type(anno.get()->get_name()), DAV_RUNTIME)); add_annotations(m, &anno_set); + auto class_name = type_class(m->get_class())->str(); + auto class_name_prefix = class_name.substr(0, class_name.size() - 1); + if (!m_patched_returns.count(class_name_prefix)) { + m_patched_returns.insert(class_name_prefix); + } } } diff --git a/opt/typedef-anno-checker/TypedefAnnoPatcher.h b/opt/typedef-anno-checker/TypedefAnnoPatcher.h index ff515798967..fdfe8528d88 100644 --- a/opt/typedef-anno-checker/TypedefAnnoPatcher.h +++ b/opt/typedef-anno-checker/TypedefAnnoPatcher.h @@ -48,8 +48,13 @@ class TypedefAnnoPatcher { void patch_ctor_params_from_synth_cls_fields(DexClass* cls); + void populate_chained_getters(DexClass* cls); + void patch_chained_getters(); + std::unordered_set m_typedef_annos; const method_override_graph::Graph& m_method_override_graph; InsertOnlyConcurrentMap> m_lambda_anno_map; + InsertOnlyConcurrentSet m_patched_returns; + InsertOnlyConcurrentSet m_chained_getters; }; diff --git a/test/integ/typedef/TypedefAnnoCheckerTest.cpp b/test/integ/typedef/TypedefAnnoCheckerTest.cpp index b0936f991a9..aeddaa28fde 100644 --- a/test/integ/typedef/TypedefAnnoCheckerTest.cpp +++ b/test/integ/typedef/TypedefAnnoCheckerTest.cpp @@ -1145,12 +1145,40 @@ TEST_F(TypedefAnnoCheckerTest, TestCompanionObject) { "TypedefAnnoCheckerKtTest;.testCompanionObject:()Ljava/lang/String;") ->as_def(); + auto companion_class = + type_class(DexType::make_type("Lcom/facebook/redextest/" + "TypedefAnnoCheckerKtTest$Companion;")); + companion_class->set_deobfuscated_name( + "Lcom/facebook/redextest/" + "TypedefAnnoCheckerKtTest$Companion;"); + auto enclosing_class = + type_class(DexType::make_type("Lcom/facebook/redextest/" + "TypedefAnnoCheckerKtTest;")); + enclosing_class->set_deobfuscated_name( + "Lcom/facebook/redextest/" + "TypedefAnnoCheckerKtTest;"); + auto method_override_graph = mog::build_graph(scope); run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); EXPECT_TRUE(checker.complete()); + + auto companion_getter = DexMethod::get_method( + "Lcom/facebook/redextest/" + "TypedefAnnoCheckerKtTest$Companion;." + "getCompanion_val:()Ljava/lang/String;") + ->as_def(); + + auto found_annotation = false; + for (auto const& param : + companion_getter->get_anno_set()->get_annotations()) { + if (param->type()->str() == "Linteg/TestStringDef;") { + found_annotation = true; + } + } + EXPECT_TRUE(found_annotation); } TEST_F(TypedefAnnoCheckerTest, TestCompanionVarSetter) {