Fix TypedefAnnoChecker

Summary: Fix typos in the checker and let ZERO be one of the valid types to check on. Add resolve_invoke_method inside the checker

Reviewed By: thezhangwei

Differential Revision: D50100218

fbshipit-source-id: e1a43bd35eef9bad2e954461e03c43385ef2d64e

opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp

@@ -12,18 +12,20 @@
 #include "AnnoUtils.h"
 #include "IROpcode.h"
 #include "PassManager.h"
+#include "Resolver.h"
 #include "Show.h"
 #include "Trace.h"
 #include "TypeUtil.h"
 #include "Walkers.h"
 
-constexpr const char* ACCESS_PREFIX = "acccess$";
-constexpr const char* DEFAULT_SUFFIX = "$defaul";
+constexpr const char* ACCESS_PREFIX = "access$";
+constexpr const char* DEFAULT_SUFFIX = "$default";
 
 namespace {
 bool is_int(const type_inference::TypeEnvironment* env, reg_t reg) {
   return env->get_type(reg).element() == IRType::INT ||
-         env->get_type(reg).element() == IRType::CONST;
+         env->get_type(reg).element() == IRType::CONST ||
+         env->get_type(reg).element() == IRType::ZERO;
 }
 
 bool is_string(const type_inference::TypeEnvironment* env, reg_t reg) {
@@ -65,7 +67,7 @@ void TypedefAnnoChecker::run(DexMethod* m) {
     }
   }
   if (!m_good) {
-    TRACE(TAC, 1, "%s", SHOW(cfg));
+    TRACE(TAC, 1, "%s\n%s", SHOW(m), SHOW(cfg));
   }
 }
 
@@ -88,10 +90,12 @@ void TypedefAnnoChecker::check_instruction(
   case OPCODE_INVOKE_STATIC:
   case OPCODE_INVOKE_INTERFACE: {
     DexMethodRef* ref_method = insn->get_method();
-    if (!ref_method->is_def()) {
+    bool resolved_virtual_to_interface;
+    auto def_method =
+        resolve_invoke_method(insn, m, &resolved_virtual_to_interface);
+    if (!def_method) {
       return;
     }
-    DexMethod* def_method = ref_method->as_def();
     // some methods are called through their access or defaul
     // counterpart, which do not retain the typedef annotation.
     // In these cases, we will skip the checker
@@ -143,22 +147,23 @@ void TypedefAnnoChecker::check_instruction(
                      type_inference::TypeDomain(IRType::INT)) {
         std::ostringstream out;
         out << "TypedefAnnoCheckerPass: the annotation " << SHOW(annotation)
-            << " annotates a parameter with an incompatible type or a "
-               "non-constant parameter in method "
-            << SHOW(m) << " while trying to invoke the method "
-            << SHOW(def_method) << " .";
+            << " annotates a parameter with an incompatible type "
+            << SHOW(env.get_type(reg))
+            << " or a non-constant parameter in method " << SHOW(m)
+            << " while trying to invoke the method " << SHOW(def_method)
+            << " .";
         m_error = out.str();
         m_good = false;
       } else if (env_anno == boost::none) {
         // TypeInference didn't infer anything
-        TRACE(TAC, 1, "invoke method: %s", SHOW(def_method));
         check_typedef_value(m, annotation, ud_chains, insn, param_index,
                             inference, envs);
         if (!m_good) {
           std::ostringstream out;
           out << " This error occured while trying to invoke the method "
               << SHOW(def_method) << " .";
           m_error += out.str();
+          TRACE(TAC, 1, "invoke method: %s", SHOW(def_method));
         }
       }
     }
@@ -323,22 +328,10 @@ void TypedefAnnoChecker::check_typedef_value(
     case OPCODE_INVOKE_DIRECT:
     case OPCODE_INVOKE_STATIC:
     case OPCODE_INVOKE_INTERFACE: {
-      if (def->get_method()->is_def()) {
-        boost::optional<const DexType*> anno =
-            inference->get_typedef_anno_from_member(
-                def->get_method()->as_def());
-        if (anno == boost::none || anno != annotation) {
-          std::ostringstream out;
-          out << "TypedefAnnoCheckerPass: the method "
-              << SHOW(def->get_method()->as_def())
-              << " needs to return a value with the anotation "
-              << SHOW(annotation)
-              << " and include it in it's method signature.";
-          m_good = false;
-          m_error += out.str();
-          return;
-        }
-      } else {
+      bool resolved_virtual_to_interface;
+      auto def_method =
+          resolve_invoke_method(def, m, &resolved_virtual_to_interface);
+      if (!def_method) {
         std::ostringstream out;
         out << "TypedefAnnoCheckerPass: in the method " << SHOW(m)
             << " , the source of the value with annotation " << SHOW(annotation)
@@ -348,6 +341,18 @@ void TypedefAnnoChecker::check_typedef_value(
         m_error += out.str();
         return;
       }
+      boost::optional<const DexType*> anno =
+          inference->get_typedef_anno_from_member(def_method);
+      if (anno == boost::none || anno != annotation) {
+        std::ostringstream out;
+        out << "TypedefAnnoCheckerPass: the method "
+            << SHOW(def->get_method()->as_def())
+            << " needs to return a value with the anotation "
+            << SHOW(annotation) << " and include it in it's method signature.";
+        m_good = false;
+        m_error += out.str();
+        return;
+      }
       break;
     }
     case OPCODE_XOR_INT_LIT: {

test/integ/TypedefAnnoCheckerTest.cpp

@@ -814,3 +814,44 @@ TEST_F(TypedefAnnoCheckerTest, TestXORIfElse) {
 
   EXPECT_TRUE(checker.complete());
 }
+
+TEST_F(TypedefAnnoCheckerTest, TestXORIfElseZero) {
+  auto scope = build_class_scope(stores);
+  auto method = DexMethod::get_method(
+                    "Lcom/facebook/redextest/"
+                    "TypedefAnnoCheckerTest;.testXORIfElseZero:()I")
+                    ->as_def();
+
+  IRCode* code = method->get_code();
+  code->build_cfg();
+  auto& cfg = code->cfg();
+
+  type_inference::TypeInference inference(cfg);
+  inference.run(method);
+
+  auto block = cfg.blocks().at(0);
+  auto env = inference.get_entry_state_at(block);
+  for (auto& mie : InstructionIterable(block)) {
+    auto insn = mie.insn;
+    if (insn->opcode() == OPCODE_XOR_INT_LIT) {
+      EXPECT_EQ(env.get_type(insn->src(0)),
+                type_inference::TypeDomain(IRType::ZERO));
+    }
+    inference.analyze_instruction(insn, &env);
+  }
+
+  ConcurrentMap<const DexClass*, std::unordered_set<const DexString*>>
+      strdef_constants;
+  ConcurrentMap<const DexClass*, std::unordered_set<uint64_t>> intdef_constants;
+  TypedefAnnoCheckerPass pass = TypedefAnnoCheckerPass(get_config());
+  for (auto cls : scope) {
+    gather_typedef_values(pass, cls, strdef_constants, intdef_constants);
+  }
+
+  TypedefAnnoChecker checker =
+      TypedefAnnoChecker(strdef_constants, intdef_constants, get_config());
+  checker.run(method);
+  code->clear_cfg();
+
+  EXPECT_TRUE(checker.complete());
+}

test/integ/TypedefAnnoCheckerTest.java

@@ -175,4 +175,10 @@ void testStringField(@TestStringDef String val) {
     int res = flag ? TestIntDef.ZERO : TestIntDef.ONE;
     return res;
   }
+
+  static @TestIntDef int testXORIfElseZero() {
+    boolean flag = false;
+    int res = flag ? TestIntDef.ZERO : TestIntDef.ONE;
+    return res;
+  }
 }