diff --git a/opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp b/opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp index d41804308a..633cf92218 100644 --- a/opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp +++ b/opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp @@ -9,6 +9,7 @@ #include "AnnoUtils.h" #include "ClassUtil.h" +#include "KotlinNullCheckMethods.h" #include "PassManager.h" #include "Resolver.h" #include "Show.h" @@ -56,6 +57,34 @@ bool is_int_or_obj_ref(const type_inference::TypeEnvironment& env, reg_t reg) { : true; } +bool is_null_check(const DexMethod* m) { + for (DexMethodRef* kotlin_null_method : + kotlin_nullcheck_wrapper::get_kotlin_null_assertions()) { + if (kotlin_null_method->get_name() == m->get_name()) { + return true; + } + } + return m->get_deobfuscated_name_or_empty_copy() == + "Lcom/google/common/base/Preconditions;.checkNotNull:(Ljava/lang/" + "Object;)Ljava/lang/Object;"; +} + +bool has_typedef_annos(ParamAnnotations* param_annos, + const std::unordered_set& typedef_annos) { + if (!param_annos) { + return false; + } + for (auto& anno : *param_annos) { + auto& anno_set = anno.second; + auto typedef_anno = type_inference::get_typedef_annotation( + anno_set->get_annotations(), typedef_annos); + if (typedef_anno != boost::none) { + return true; + } + } + return false; +} + DexMethod* resolve_method(DexMethod* caller, IRInstruction* insn) { auto def_method = resolve_method(insn->get_method(), opcode_to_search(insn), caller); @@ -345,13 +374,14 @@ void patch_return_anno_from_get(type_inference::TypeInference& inference, IRInstruction* insn) { always_assert(opcode::is_an_iget(insn->opcode()) || opcode::is_an_sget(insn->opcode())); - auto name = caller->get_deobfuscated_name_or_empty(); - auto pos = name.rfind('$'); - if (pos == std::string::npos) { + auto name = caller->get_simple_deobfuscated_name(); + auto last_dollar = name.find('$'); + if (last_dollar == std::string::npos) { return; } - pos++; - if (!(pos < name.size() && name[pos] >= '0' && name[pos] <= '9')) { + last_dollar++; + if (!(last_dollar < name.size() && name[last_dollar] >= '0' && + name[last_dollar] <= '9')) { return; } auto field_ref = insn->get_field(); @@ -534,7 +564,7 @@ void SynthAccessorPatcher::run(const Scope& scope) { } patch_synth_methods_overriding_annotated_methods(m); if (is_constructor(m)) { - if (m->get_param_anno()) { + if (has_typedef_annos(m->get_param_anno(), m_typedef_annos)) { patch_synth_cls_fields_from_ctor_param(m); } else { if (has_kotlin_default_ctor_marker(m)) { @@ -1367,6 +1397,16 @@ void TypedefAnnoChecker::check_instruction( << ".\n failed instruction: " << show(insn) << "\n\n"; m_error += out.str(); m_good = false; + } else if (env_anno == boost::none && field_anno != boost::none) { + bool good = check_typedef_value(m, field_anno, ud_chains, insn, 0, + inference, envs); + if (!good) { + std::ostringstream out; + out << " Error writing to field " << show(insn->get_field()) + << "in method" << SHOW(m) << "\n\n"; + m_error += out.str(); + TRACE(TAC, 1, "writing to field: %s", SHOW(insn->get_field())); + } } break; } @@ -1566,6 +1606,12 @@ bool TypedefAnnoChecker::check_typedef_value( if (is_parcel_or_json_read(def_method) && is_model_gen(m)) { break; } + // the result of usedef chains on a check cast could resolve to this + // look further up for the real source + if (is_null_check(def_method)) { + check_typedef_value(m, annotation, ud_chains, def, 0, inference, envs); + break; + } std::vector callees; if (mog::is_true_virtual(m_method_override_graph, def_method) && !def_method->get_code()) { @@ -1652,6 +1698,10 @@ bool TypedefAnnoChecker::check_typedef_value( } break; } + case OPCODE_CHECK_CAST: { + check_typedef_value(m, annotation, ud_chains, def, 0, inference, envs); + break; + } default: { std::ostringstream out; out << "TypedefAnnoCheckerPass: the method " << show(m) diff --git a/test/integ/TypedefAnnoCheckerTest.cpp b/test/integ/TypedefAnnoCheckerTest.cpp index cccf2353af..2cfca3bea7 100644 --- a/test/integ/TypedefAnnoCheckerTest.cpp +++ b/test/integ/TypedefAnnoCheckerTest.cpp @@ -1115,8 +1115,15 @@ TEST_F(TypedefAnnoCheckerTest, TestInvalidVarField) { run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); - // See T193638685: Temporarily disabling to unblock SDK 35 - // EXPECT_FALSE(checker.complete()); + EXPECT_FALSE(checker.complete()); + EXPECT_EQ( + checker.error(), + "TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidVarField:()Ljava/lang/String;\n\ + the string value 5 does not have the typedef annotation \n\ + Linteg/TestStringDef; attached to it. \n\ + Check that the value is annotated and exists in the typedef annotation class.\n\ + failed instruction: CONST_STRING \"5\"\n\ + Error writing to field Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.var_field:Ljava/lang/String;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidVarField:()Ljava/lang/String;\n\n"); } TEST_F(TypedefAnnoCheckerTest, TestReturnIntField) { @@ -1182,18 +1189,15 @@ TEST_F(TypedefAnnoCheckerTest, TestInvalidCompanionVarSetter) { run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); - // See T193638685: Temporarily disabling to unblock SDK 35 - // EXPECT_FALSE(checker.complete()); - // EXPECT_EQ( - // checker.error(), - // "TypedefAnnoCheckerPass: in method - // Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidCompanionVarSetter:()Ljava/lang/String;\n\ - // the string value 5 does not have the typedef annotation \n\ - // Linteg/TestStringDef; attached to it. \n\ - // Check that the value is annotated and exists in the typedef annotation - // class.\n\ - // failed instruction: CONST_STRING \"5\"\n\ - // Error caught when returning the faulty value\n\n"); + EXPECT_FALSE(checker.complete()); + EXPECT_EQ( + checker.error(), + "TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidCompanionVarSetter:()Ljava/lang/String;\n\ + the string value 5 does not have the typedef annotation \n\ + Linteg/TestStringDef; attached to it. \n\ + Check that the value is annotated and exists in the typedef annotation class.\n\ + failed instruction: CONST_STRING \"5\"\n\ + Error writing to field Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.companion_var:Ljava/lang/String;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidCompanionVarSetter:()Ljava/lang/String;\n\n"); } TEST_F(TypedefAnnoCheckerTest, TestCompanionIntVarSetter) { @@ -1675,7 +1679,22 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarIntInvalid) { run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); - // EXPECT_FALSE(checker.complete()); + EXPECT_FALSE(checker.complete()); + EXPECT_EQ( + checker.error(), + "TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\ + the int value 7 does not have the typedef annotation \n\ + Linteg/TestIntDef; attached to it. \n\ + Check that the value is annotated and exists in its typedef annotation class.\n\ + failed instruction: CONST v1, 7\n\ + Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\ +\n\ +TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\ + the int value 9 does not have the typedef annotation \n\ + Linteg/TestIntDef; attached to it. \n\ + Check that the value is annotated and exists in its typedef annotation class.\n\ + failed instruction: CONST v1, 9\n\ + Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\n"); } TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarIntDefault) { @@ -1723,7 +1742,22 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarIntDefaultInvalid) { run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); - // EXPECT_FALSE(checker.complete()); + EXPECT_FALSE(checker.complete()); + EXPECT_EQ( + checker.error(), + "TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\ + the int value 7 does not have the typedef annotation \n\ + Linteg/TestIntDef; attached to it. \n\ + Check that the value is annotated and exists in its typedef annotation class.\n\ + failed instruction: CONST v1, 7\n\ + Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\ +\n\ +TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\ + the int value 9 does not have the typedef annotation \n\ + Linteg/TestIntDef; attached to it. \n\ + Check that the value is annotated and exists in its typedef annotation class.\n\ + failed instruction: CONST v1, 9\n\ + Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\n"); } TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarString) { @@ -1771,7 +1805,22 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarStringInvalid) { run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); - // EXPECT_FALSE(checker.complete()); + EXPECT_FALSE(checker.complete()); + EXPECT_EQ( + checker.error(), + "TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\ + the string value seven does not have the typedef annotation \n\ + Linteg/TestStringDef; attached to it. \n\ + Check that the value is annotated and exists in the typedef annotation class.\n\ + failed instruction: CONST_STRING \"seven\"\n\ + Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\ +\n\ +TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\ + the string value eight does not have the typedef annotation \n\ + Linteg/TestStringDef; attached to it. \n\ + Check that the value is annotated and exists in the typedef annotation class.\n\ + failed instruction: CONST_STRING \"eight\"\n\ + Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\n"); } TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarStringDefault) { @@ -1822,5 +1871,20 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarStringDefaultInvalid) { run_patcher(scope, *method_override_graph); auto checker = run_checker(scope, method, *method_override_graph); - // EXPECT_FALSE(checker.complete()); + EXPECT_FALSE(checker.complete()); + EXPECT_EQ( + checker.error(), + "TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\ + the string value seven does not have the typedef annotation \n\ + Linteg/TestStringDef; attached to it. \n\ + Check that the value is annotated and exists in the typedef annotation class.\n\ + failed instruction: CONST_STRING \"seven\"\n\ + Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\ +\n\ +TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\ + the string value eight does not have the typedef annotation \n\ + Linteg/TestStringDef; attached to it. \n\ + Check that the value is annotated and exists in the typedef annotation class.\n\ + failed instruction: CONST_STRING \"eight\"\n\ + Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\n"); }