Intial Thumb2 Sandbox (naclrev 6680)

src/trusted/validator_arm/validator.h

Index: src/trusted/validator_arm/validator.h
diff --git a/src/trusted/validator_arm/validator.h b/src/trusted/validator_arm/validator.h
index e3af968786851d5d8bbcd32b65ea8d6a82f75e0c..0a0c0479c36d2001c05460293209e055f1fc6d53 100644
--- a/src/trusted/validator_arm/validator.h
+++ b/src/trusted/validator_arm/validator.h
@@ -1,14 +1,19 @@
 /*
- * Copyright 2009 The Native Client Authors.  All rights reserved.
+ * Copyright 2011 The Native Client Authors.  All rights reserved.
  * Use of this source code is governed by a BSD-style license that can
  * be found in the LICENSE file.
- * Copyright 2009, Google Inc.
+ * Copyright 2011, Google Inc.
  */
 
 #ifndef NATIVE_CLIENT_SRC_TRUSTED_VALIDATOR_ARM_V2_VALIDATOR_H
 #define NATIVE_CLIENT_SRC_TRUSTED_VALIDATOR_ARM_V2_VALIDATOR_H
 
 /*
+ * MRM DEBUG Header, clean this out

[bsy, 2011/09/16 18:42:21]

mrm

+ */
+#include <stdio.h>
+
+/*
  * The SFI validator, and some utility classes it uses.
  */
 
@@ -51,12 +56,38 @@ class Bundle {
     // Note that all Bundles are currently assumed to be the same size.
     return virtual_base_ != other.virtual_base_;
   }
+  bool operator==(const Bundle &other) const {
+    return virtual_base_ == other.virtual_base_;
+  }
 
  private:
   uint32_t virtual_base_;
   uint32_t size_;
 };
 
+class SfiValidator;
+
+// A possible result from a validator pattern.
+enum PatternMatchMode {
+  // The pattern does not apply to the instructions it was given.
+  NO_MATCH_MODE,
+  // The pattern matches, and is safe; do not allow jumps to split it.
+  PATTERN_SAFE_MODE,
+  // The pattern matches, and has detected a problem.
+  PATTERN_UNSAFE_MODE
+};
+
+// Full result from a pattern. Describes whether it matched, and if it did,
+// specifies how long the matching pattern was.
+typedef struct {
+  PatternMatchMode pm_mode;
+  int8_t size;
+} PatternMatch;
+
+/* Pattern matching type */
+typedef PatternMatch (*Pattern)(const SfiValidator &,
+                                  DecodedInstruction insns[],
+                                  ProblemSink *out);
 
 /*
  * The SFI validator itself.  The validator is controlled by the following
@@ -83,7 +114,8 @@ class SfiValidator {
                uint32_t code_region_bytes,
                uint32_t data_region_bytes,
                nacl_arm_dec::RegisterList read_only_registers,
-               nacl_arm_dec::RegisterList data_address_registers);
+               nacl_arm_dec::RegisterList data_address_registers,
+               bool thumb);
 
   /*
    * The main validator entry point.  Validates the provided CodeSegments,
@@ -100,8 +132,9 @@ class SfiValidator {
    */
   bool is_data_address_register(nacl_arm_dec::Register) const;
 
-  uint32_t data_address_mask() const { return data_address_mask_; }
-  uint32_t code_address_mask() const { return code_address_mask_; }
+  uint32_t data_address_mask()   const { return data_address_mask_; }
+  uint32_t code_address_mask()   const { return code_address_mask_; }
+  uint32_t code_address_ormask() const { return code_address_ormask_; }
 
   nacl_arm_dec::RegisterList read_only_registers() const {
     return read_only_registers_;
@@ -117,9 +150,12 @@ class SfiValidator {
    * Change masks: this is useful for debugging and cannot be completely
    *               controlled with constructor arguments
    */
-  void change_masks(uint32_t code_address_mask, uint32_t data_address_mask) {
-    code_address_mask_ = code_address_mask;
-    data_address_mask_ = data_address_mask;
+  void change_masks(uint32_t code_address_mask,
+                    uint32_t code_address_ormask,
+                    uint32_t data_address_mask) {
+    code_address_mask_   = code_address_mask;
+    code_address_ormask_ = code_address_ormask;
+    data_address_mask_   = data_address_mask;
   }
 
  private:
@@ -139,15 +175,6 @@ class SfiValidator {
       AddressSet *branches, AddressSet *critical);
 
   /*
-   * Factor of validate_fallthrough, above.  Checks a single instruction using
-   * the instruction patterns defined in the .cc file, with two possible
-   * results:
-   *   1. No patterns matched, or all were safe: nothing happens.
-   *   2. Patterns matched and were unsafe: problems get sent to 'out'.
-   */
-  bool apply_patterns(const DecodedInstruction &, ProblemSink *out);
-
-  /*
    * Factor of validate_fallthrough, above.  Checks a pair of instructions using
    * the instruction patterns defined in the .cc file, with three possible
    * results:
@@ -156,8 +183,9 @@ class SfiValidator {
    *      'critical' for use by the second pass.
    *   3. Patterns matched and were unsafe: problems get sent to 'out'.
    */
-  bool apply_patterns(const DecodedInstruction &first,
-      const DecodedInstruction &second, AddressSet *critical, ProblemSink *out);
+  bool apply_patterns(DecodedInstruction insns[],
+      const Pattern patterns[], unsigned int nPatterns,
+      AddressSet *critical, ProblemSink *out);
 
   /*
    * Validates all branches found by a previous pass, checking destinations.
@@ -171,14 +199,25 @@ class SfiValidator {
   uint32_t bytes_per_bundle_;
   uint32_t data_address_mask_;
   uint32_t code_address_mask_;
+  uint32_t code_address_ormask_;
   uint32_t code_region_bytes_;
   // Registers which cannot be modified by untrusted code.
   nacl_arm_dec::RegisterList read_only_registers_;
   // Registers which must always contain a valid data region address.
   nacl_arm_dec::RegisterList data_address_registers_;
+  // thumb_ is always either 1 or 0
+  uint32_t thumb_;

[bsy, 2011/09/16 18:42:21]

this is C++ code, and bool is used elsewhere.  why not make this a bool?

   const nacl_arm_dec::DecoderState *decode_state_;
 };
 
+/*
+ * Defines the maximal pattern size to be made available to the pattern
+ * matcher.
+ * I know 5 seems a little big, but it is required for the IT matcher.
+ * 16 is a hack, it is a magic number to make sure that the second pattern
+ * matching phase interacts properly with breakpoint.
+ */
+const uint8_t kMaxPattern = 5;
 
 /*
  * A facade that combines an Instruction with its address and a ClassDecoder.
@@ -208,9 +247,9 @@ class DecodedInstruction {
    * take it into account.  The SfiValidator reasons on this separately.
    */
   bool always_precedes(const DecodedInstruction &other) const {
-    return inst_.condition() == other.inst_.condition();
+    return condition() == other.condition();
   }
-
+  // TODO(mrm) These predicates will all be able to lie without an IT predicator

[bsy, 2011/09/16 18:42:21]

"

   /*
    * Checks that 'this' always follows 'other' -- meaning that if 'other'
    * executes, 'this' is guaranteed to follow.  This is important if 'other'
@@ -220,7 +259,7 @@ class DecodedInstruction {
    * take it into account.  The SfiValidator reasons on this separately.
    */
   bool always_follows(const DecodedInstruction &other) const {
-    return inst_.condition() == other.inst_.condition()
+    return condition() == condition()
         && !other.defines(nacl_arm_dec::kRegisterFlags);
   }
 
@@ -230,8 +269,8 @@ class DecodedInstruction {
    * adjacent for this simple check to be meaningful.
    */
   bool is_conditional_on(const DecodedInstruction &other) const {
-    return inst_.condition() == nacl_arm_dec::Instruction::EQ
-        && other.inst_.condition() == nacl_arm_dec::Instruction::AL
+    return condition() == nacl_arm_dec::Instruction::EQ
+        && other.condition() == nacl_arm_dec::Instruction::AL
         && other.defines(nacl_arm_dec::kRegisterFlags);
   }
 
@@ -239,6 +278,40 @@ class DecodedInstruction {
   nacl_arm_dec::SafetyLevel safety() const { return safety_; }
   nacl_arm_dec::RegisterList defs() const { return defs_; }
 
+  /* Passes through queries about the it status */

[bsy, 2011/09/16 18:42:21]

what is "it"?  that's a horrible acronym/name/whatever.

it's the real thing?

iterator?

+  nacl_arm_dec::ITCond it() const {
+    return decoder_->it_sequence(inst_);
+  }
+
+  /* Passes through queries about it safety */
+  nacl_arm_dec::ITSafety it_safe() const {
+    return decoder_->it_safe(inst_);
+  }
+
+  // Calculates the size of the current instruction. It is here instead of
+  // ClassDecoder because it needs the vaddr.
+  nacl_arm_dec::Instruction::Condition condition() const {
+    if (vaddr_ & 1) {
+      if (condition_ != nacl_arm_dec::Instruction::UNCONDITIONAL)
+        return condition_;
+      return decoder_->condition(inst_);
+    } else {
+      return inst_.condition();
+    }
+  }
+  uint8_t size() const {
+    if (vaddr_ & 1) {
+      switch (inst_.bits(15, 11)) {
+        case 0x1D:            // 0b11101
+        case 0x1E:            // 0b11110
+        case 0x1F: return 4;  // 0b11111
+        default:   return 2;
+      }
+    } else {
+      return 4;
+    }
+  }
+
   // The methods below pull values from ClassDecoder on demand.
   bool is_relative_branch() const {
     return decoder_->is_relative_branch(inst_);
@@ -260,6 +333,10 @@ class DecodedInstruction {
     return decoder_->base_address_register(inst_);
   }
 
+  bool sets_bits(uint32_t mask) const {
+    return decoder_->sets_bits(inst_, mask);
+  }
+
   bool clears_bits(uint32_t mask) const {
     return decoder_->clears_bits(inst_, mask);
   }
@@ -285,6 +362,15 @@ class DecodedInstruction {
     return defs().contains_all(rl);
   }
 
+  /*
+   *  Overrides the conditional for this insn, if thumb mode is active.
+   *  It should be used only once on any given insn, as it should originate
+   *  only from within an IT block, and IT blocks must not overlap.
+   */
+  void set_condition(nacl_arm_dec::Instruction::Condition condition) {
+    condition_ = condition;
+  }
+
  private:
   uint32_t vaddr_;
   nacl_arm_dec::Instruction inst_;
@@ -292,6 +378,7 @@ class DecodedInstruction {
 
   nacl_arm_dec::SafetyLevel safety_;
   nacl_arm_dec::RegisterList defs_;
+  nacl_arm_dec::Instruction::Condition condition_;
 };
 
 
@@ -314,14 +401,22 @@ class CodeSegment {
 
   const nacl_arm_dec::Instruction operator[](uint32_t address) const {
     const uint8_t *element = &base_[address - start_addr_];
-    return nacl_arm_dec::Instruction(
-        *reinterpret_cast<const uint32_t *>(element));
+    // Special case to allow addresses two from the end to be read
+    if ((address - start_addr_) + 2 < size_) {
+      return nacl_arm_dec::Instruction(
+          *reinterpret_cast<const uint32_t *>(element));
+    } else if ((address - start_addr_) + 2 == size_) {
+      return nacl_arm_dec::Instruction(((uint32_t)
+          (*reinterpret_cast<const uint16_t *>(element))));
+    } else {
+      fprintf(stderr, "Out of bounds access to code segment: %x\n", address);
+      exit(1);
+    }
   }
 
   bool operator<(const CodeSegment &other) const {
     return start_addr_ < other.start_addr_;
   }
-
  private:
   const uint8_t *base_;
   uint32_t start_addr_;
@@ -380,6 +475,10 @@ class ProblemSink {
 
 // An instruction is unsafe -- more information in the SafetyLevel.
 const char * const kProblemUnsafe = "kProblemUnsafe";
+// An instruction runs off the end of a segment
+const char * const kProblemStraddlesSegment = "kProblemStraddlesSegment";
+// An IT instruction is unconditional, meaning the condition is uninvertable.
+const char * const kProblemUnconditionalIT = "kProblemUnconditionalIT";
 // A branch would break a pseudo-operation pattern.
 const char * const kProblemBranchSplitsPattern = "kProblemBranchSplitsPattern";
 // A branch targets an invalid code address (out of segment).