Merge "[veridex] Add a --target-sdk-version to avoid false positives." into pi-dev

4 files changed, 119 insertions, 17 deletions

diff --git a/tools/veridex/flow_analysis.cc b/tools/veridex/flow_analysis.cc
index e2833bf01d..154c60f6ac 100644
--- a/tools/veridex/flow_analysis.cc
+++ b/tools/veridex/flow_analysis.cc
@@ -112,7 +112,12 @@ void VeriFlowAnalysis::UpdateRegister(uint32_t dex_register, const VeriClass* cl
       RegisterValue(RegisterSource::kNone, DexFileReference(nullptr, 0), cls);
 }
 
-const RegisterValue& VeriFlowAnalysis::GetRegister(uint32_t dex_register) {
+void VeriFlowAnalysis::UpdateRegister(uint32_t dex_register, int32_t value, const VeriClass* cls) {
+  current_registers_[dex_register] =
+      RegisterValue(RegisterSource::kConstant, value, DexFileReference(nullptr, 0), cls);
+}
+
+const RegisterValue& VeriFlowAnalysis::GetRegister(uint32_t dex_register) const {
   return current_registers_[dex_register];
 }
 
@@ -131,6 +136,49 @@ RegisterValue VeriFlowAnalysis::GetFieldType(uint32_t field_index) {
   return RegisterValue(RegisterSource::kField, DexFileReference(&dex_file, field_index), cls);
 }
 
+int VeriFlowAnalysis::GetBranchFlags(const Instruction& instruction) const {
+  switch (instruction.Opcode()) {
+    #define IF_XX(cond, op) \
+    case Instruction::IF_##cond: { \
+      RegisterValue lhs = GetRegister(instruction.VRegA()); \
+      RegisterValue rhs = GetRegister(instruction.VRegB()); \
+      if (lhs.IsConstant() && rhs.IsConstant()) { \
+        if (lhs.GetConstant() op rhs.GetConstant()) { \
+          return Instruction::kBranch; \
+        } else { \
+          return Instruction::kContinue; \
+        } \
+      } \
+      break; \
+    } \
+    case Instruction::IF_##cond##Z: { \
+      RegisterValue val = GetRegister(instruction.VRegA()); \
+      if (val.IsConstant()) { \
+        if (val.GetConstant() op 0) { \
+          return Instruction::kBranch; \
+        } else { \
+          return Instruction::kContinue; \
+        } \
+      } \
+      break; \
+    }
+
+    IF_XX(EQ, ==);
+    IF_XX(NE, !=);
+    IF_XX(LT, <);
+    IF_XX(LE, <=);
+    IF_XX(GT, >);
+    IF_XX(GE, >=);
+
+    #undef IF_XX
+
+    default:
+      break;
+  }
+
+  return Instruction::FlagsOf(instruction.Opcode());
+}
+
 void VeriFlowAnalysis::AnalyzeCode() {
   std::vector<uint32_t> work_list;
   work_list.push_back(0);
@@ -149,16 +197,17 @@ void VeriFlowAnalysis::AnalyzeCode() {
       ProcessDexInstruction(inst);
       SetVisited(dex_pc);
 
-      int opcode_flags = Instruction::FlagsOf(inst.Opcode());
-      if ((opcode_flags & Instruction::kContinue) != 0) {
-        if ((opcode_flags & Instruction::kBranch) != 0) {
+      int branch_flags = GetBranchFlags(inst);
+
+      if ((branch_flags & Instruction::kContinue) != 0) {
+        if ((branch_flags & Instruction::kBranch) != 0) {
           uint32_t branch_dex_pc = dex_pc + inst.GetTargetOffset();
           if (MergeRegisterValues(branch_dex_pc)) {
             work_list.push_back(branch_dex_pc);
           }
         }
         dex_pc += inst.SizeInCodeUnits();
-      } else if ((opcode_flags & Instruction::kBranch) != 0) {
+      } else if ((branch_flags & Instruction::kBranch) != 0) {
         dex_pc += inst.GetTargetOffset();
         DCHECK(IsBranchTarget(dex_pc));
       } else {
@@ -178,12 +227,30 @@ void VeriFlowAnalysis::AnalyzeCode() {
 
 void VeriFlowAnalysis::ProcessDexInstruction(const Instruction& instruction) {
   switch (instruction.Opcode()) {
-    case Instruction::CONST_4:
-    case Instruction::CONST_16:
-    case Instruction::CONST:
+    case Instruction::CONST_4: {
+      int32_t register_index = instruction.VRegA();
+      int32_t value = instruction.VRegB_11n();
+      UpdateRegister(register_index, value, VeriClass::integer_);
+      break;
+    }
+    case Instruction::CONST_16: {
+      int32_t register_index = instruction.VRegA();
+      int32_t value = instruction.VRegB_21s();
+      UpdateRegister(register_index, value, VeriClass::integer_);
+      break;
+    }
+
+    case Instruction::CONST: {
+      int32_t register_index = instruction.VRegA();
+      int32_t value = instruction.VRegB_31i();
+      UpdateRegister(register_index, value, VeriClass::integer_);
+      break;
+    }
+
     case Instruction::CONST_HIGH16: {
       int32_t register_index = instruction.VRegA();
-      UpdateRegister(register_index, VeriClass::integer_);
+      int32_t value = instruction.VRegB_21h();
+      UpdateRegister(register_index, value, VeriClass::integer_);
       break;
     }
 
@@ -268,6 +335,8 @@ void VeriFlowAnalysis::ProcessDexInstruction(const Instruction& instruction) {
     case Instruction::RETURN: {
       break;
     }
+
+    // If operations will be handled when looking at the control flow.
     #define IF_XX(cond) \
     case Instruction::IF_##cond: break; \
     case Instruction::IF_##cond##Z: break
@@ -279,6 +348,8 @@ void VeriFlowAnalysis::ProcessDexInstruction(const Instruction& instruction) {
     IF_XX(GT);
     IF_XX(GE);
 
+    #undef IF_XX
+
     case Instruction::GOTO:
     case Instruction::GOTO_16:
     case Instruction::GOTO_32: {
@@ -495,7 +566,13 @@ void VeriFlowAnalysis::ProcessDexInstruction(const Instruction& instruction) {
     case Instruction::SGET_BYTE:
     case Instruction::SGET_CHAR:
     case Instruction::SGET_SHORT: {
-      UpdateRegister(instruction.VRegA_22c(), GetFieldType(instruction.VRegC_22c()));
+      uint32_t dest_reg = instruction.VRegA_21c();
+      uint16_t field_index = instruction.VRegB_21c();
+      if (VeriClass::sdkInt_ != nullptr && resolver_->GetField(field_index) == VeriClass::sdkInt_) {
+        UpdateRegister(dest_reg, gTargetSdkVersion, VeriClass::integer_);
+      } else {
+        UpdateRegister(dest_reg, GetFieldType(instruction.VRegC_22c()));
+      }
       break;
     }
 
diff --git a/tools/veridex/flow_analysis.h b/tools/veridex/flow_analysis.h
index 62c9916a61..fc093600c3 100644
--- a/tools/veridex/flow_analysis.h
+++ b/tools/veridex/flow_analysis.h
@@ -35,6 +35,7 @@ enum class RegisterSource {
   kMethod,
   kClass,
   kString,
+  kConstant,
   kNone
 };
 
@@ -44,28 +45,33 @@ enum class RegisterSource {
 class RegisterValue {
  public:
   RegisterValue() : source_(RegisterSource::kNone),
-                    parameter_index_(0),
+                    value_(0),
                     reference_(nullptr, 0),
                     type_(nullptr) {}
   RegisterValue(RegisterSource source, DexFileReference reference, const VeriClass* type)
-      : source_(source), parameter_index_(0), reference_(reference), type_(type) {}
+      : source_(source), value_(0), reference_(reference), type_(type) {}
 
   RegisterValue(RegisterSource source,
-                uint32_t parameter_index,
+                uint32_t value,
                 DexFileReference reference,
                 const VeriClass* type)
-      : source_(source), parameter_index_(parameter_index), reference_(reference), type_(type) {}
+      : source_(source), value_(value), reference_(reference), type_(type) {}
 
   RegisterSource GetSource() const { return source_; }
   DexFileReference GetDexFileReference() const { return reference_; }
   const VeriClass* GetType() const { return type_; }
   uint32_t GetParameterIndex() const {
     CHECK(IsParameter());
-    return parameter_index_;
+    return value_;
+  }
+  uint32_t GetConstant() const {
+    CHECK(IsConstant());
+    return value_;
   }
   bool IsParameter() const { return source_ == RegisterSource::kParameter; }
   bool IsClass() const { return source_ == RegisterSource::kClass; }
   bool IsString() const { return source_ == RegisterSource::kString; }
+  bool IsConstant() const { return source_ == RegisterSource::kConstant; }
 
   std::string ToString() const {
     switch (source_) {
@@ -91,7 +97,7 @@ class RegisterValue {
 
  private:
   RegisterSource source_;
-  uint32_t parameter_index_;
+  uint32_t value_;
   DexFileReference reference_;
   const VeriClass* type_;
 };
@@ -137,12 +143,15 @@ class VeriFlowAnalysis {
       uint32_t dex_register, RegisterSource kind, VeriClass* cls, uint32_t source_id);
   void UpdateRegister(uint32_t dex_register, const RegisterValue& value);
   void UpdateRegister(uint32_t dex_register, const VeriClass* cls);
+  void UpdateRegister(uint32_t dex_register, int32_t value, const VeriClass* cls);
   void ProcessDexInstruction(const Instruction& inst);
   void SetVisited(uint32_t dex_pc);
   RegisterValue GetFieldType(uint32_t field_index);
 
+  int GetBranchFlags(const Instruction& instruction) const;
+
  protected:
-  const RegisterValue& GetRegister(uint32_t dex_register);
+  const RegisterValue& GetRegister(uint32_t dex_register) const;
   RegisterValue GetReturnType(uint32_t method_index);
 
   VeridexResolver* resolver_;
diff --git a/tools/veridex/veridex.cc b/tools/veridex/veridex.cc
index dc7ea94032..bcd4815a38 100644
--- a/tools/veridex/veridex.cc
+++ b/tools/veridex/veridex.cc
@@ -25,6 +25,7 @@
 #include "precise_hidden_api_finder.h"
 #include "resolver.h"
 
+#include <cstdlib>
 #include <sstream>
 
 namespace art {
@@ -62,6 +63,7 @@ VeriMethod VeriClass::getMethod_ = nullptr;
 VeriMethod VeriClass::getDeclaredMethod_ = nullptr;
 VeriMethod VeriClass::getClass_ = nullptr;
 VeriMethod VeriClass::loadClass_ = nullptr;
+VeriField VeriClass::sdkInt_ = nullptr;
 
 struct VeridexOptions {
   const char* dex_file = nullptr;
@@ -70,6 +72,7 @@ struct VeridexOptions {
   const char* light_greylist = nullptr;
   const char* dark_greylist = nullptr;
   bool precise = true;
+  int target_sdk_version = 28; /* P */
 };
 
 static const char* Substr(const char* str, int index) {
@@ -91,6 +94,7 @@ static void ParseArgs(VeridexOptions* options, int argc, char** argv) {
   static const char* kDarkGreylistOption = "--dark-greylist=";
   static const char* kLightGreylistOption = "--light-greylist=";
   static const char* kImprecise = "--imprecise";
+  static const char* kTargetSdkVersion = "--target-sdk-version=";
 
   for (int i = 0; i < argc; ++i) {
     if (StartsWith(argv[i], kDexFileOption)) {
@@ -105,6 +109,8 @@ static void ParseArgs(VeridexOptions* options, int argc, char** argv) {
       options->light_greylist = Substr(argv[i], strlen(kLightGreylistOption));
     } else if (strcmp(argv[i], kImprecise) == 0) {
       options->precise = false;
+    } else if (StartsWith(argv[i], kTargetSdkVersion)) {
+      options->target_sdk_version = atoi(Substr(argv[i], strlen(kTargetSdkVersion)));
     }
   }
 }
@@ -124,6 +130,7 @@ class Veridex {
   static int Run(int argc, char** argv) {
     VeridexOptions options;
     ParseArgs(&options, argc, argv);
+    gTargetSdkVersion = options.target_sdk_version;
 
     std::vector<std::string> boot_content;
     std::vector<std::string> app_content;
@@ -200,6 +207,11 @@ class Veridex {
     VeriClass::loadClass_ = boot_resolvers[0]->LookupDeclaredMethodIn(
         *VeriClass::class_loader_, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;");
 
+    VeriClass* version = type_map["Landroid/os/Build$VERSION;"];
+    if (version != nullptr) {
+      VeriClass::sdkInt_ = boot_resolvers[0]->LookupFieldIn(*version, "SDK_INT", "I");
+    }
+
     std::vector<std::unique_ptr<VeridexResolver>> app_resolvers;
     Resolve(app_dex_files, resolver_map, type_map, &app_resolvers);
 
diff --git a/tools/veridex/veridex.h b/tools/veridex/veridex.h
index 9c0a158174..31ddbf439e 100644
--- a/tools/veridex/veridex.h
+++ b/tools/veridex/veridex.h
@@ -24,6 +24,8 @@
 
 namespace art {
 
+static int gTargetSdkVersion = 1000;  // Will be initialized after parsing options.
+
 /**
  * Abstraction for fields defined in dex files. Currently, that's a pointer into their
  * `encoded_field` description.
@@ -86,6 +88,8 @@ class VeriClass {
   static VeriMethod getClass_;
   static VeriMethod loadClass_;
 
+  static VeriField sdkInt_;
+
  private:
   Primitive::Type kind_;
   uint8_t dimensions_;