Move validator_x86_XX.rl out of unreviewed.

src/trusted/validator_ragel/validator_internal.h

Index: src/trusted/validator_ragel/validator_internal.h
===================================================================
--- src/trusted/validator_ragel/validator_internal.h	(revision 9944)
+++ src/trusted/validator_ragel/validator_internal.h	(working copy)
@@ -14,7 +14,7 @@
 
 #include "native_client/src/shared/platform/nacl_check.h"
 #include "native_client/src/shared/utils/types.h"
-#include "native_client/src/trusted/validator_ragel/unreviewed/decoding.h"
+#include "native_client/src/trusted/validator_ragel/decoding.h"
 #include "native_client/src/trusted/validator_ragel/unreviewed/validator.h"
 
 /* Maximum set of R-DFA allowable CPUID features.  */
@@ -190,13 +190,6 @@
 #define SET_MODRM_BASE(N) base = (N)
 #define SET_MODRM_INDEX(N) index = (N)
 
-enum {
-  REX_B = 1,
-  REX_X = 2,
-  REX_R = 4,
-  REX_W = 8
-};
-
 enum operand_kind {
   OperandSandboxIrrelevant = 0,
   /*
@@ -322,7 +315,23 @@
   return TRUE;
 }
 
+/*
+ * Mark the given address as valid jump target address.
+ */
+static FORCEINLINE void MakeJumpTargetValid(size_t address,
+                                            bitmap_word *valid_targets) {
+  BitmapSetBit(valid_targets, address);
+}
 
+/*
+ * Mark the given address as invalid jump target address.
+ */
+static FORCEINLINE void MakeInvalidJumpTarget(size_t address,
+                                              bitmap_word *valid_targets) {
+  BitmapClearBit(valid_targets, address);
+}
+
+
 static INLINE Bool ProcessInvalidJumpTargets(
     const uint8_t *data,
     size_t size,
@@ -413,8 +422,8 @@
 }
 
 
-static INLINE void process_0_operands(enum OperandName *restricted_register,
-                                      uint32_t *instruction_info_collected) {
+static INLINE void Process0Operands(enum OperandName *restricted_register,
+                                    uint32_t *instruction_info_collected) {
   /* Restricted %rsp or %rbp must be processed by appropriate nacl-special
    * instruction, not with regular instruction.  */
   if (*restricted_register == REG_RSP) {
@@ -425,10 +434,10 @@
   *restricted_register = NO_REG;
 }
 
-static INLINE void process_1_operand(enum OperandName *restricted_register,
-                                     uint32_t *instruction_info_collected,
-                                     uint8_t rex_prefix,
-                                     uint32_t operand_states) {
+static INLINE void Process1Operand(enum OperandName *restricted_register,
+                                   uint32_t *instruction_info_collected,
+                                   uint8_t rex_prefix,
+                                   uint32_t operand_states) {
   /* Restricted %rsp or %rbp must be processed by appropriate nacl-special
    * instruction, not with regular instruction.  */
   if (*restricted_register == REG_RSP) {
@@ -452,7 +461,7 @@
   }
 }
 
-static INLINE void process_1_operand_zero_extends(
+static INLINE void Process1OperandZeroExtends(
     enum OperandName *restricted_register,
     uint32_t *instruction_info_collected, uint8_t rex_prefix,
     uint32_t operand_states) {
@@ -481,10 +490,10 @@
   }
 }
 
-static INLINE void process_2_operands(enum OperandName *restricted_register,
-                                      uint32_t *instruction_info_collected,
-                                      uint8_t rex_prefix,
-                                      uint32_t operand_states) {
+static INLINE void Process2Operands(enum OperandName *restricted_register,
+                                    uint32_t *instruction_info_collected,
+                                    uint8_t rex_prefix,
+                                    uint32_t operand_states) {
   /* Restricted %rsp or %rbp must be processed by appropriate nacl-special
    * instruction, not with regular instruction.  */
   if (*restricted_register == REG_RSP) {
@@ -517,7 +526,7 @@
   }
 }
 
-static INLINE void process_2_operands_zero_extends(
+static INLINE void Process2OperandsZeroExtends(
      enum OperandName *restricted_register,
      uint32_t *instruction_info_collected,
      uint8_t rex_prefix, uint32_t operand_states) {
@@ -562,4 +571,87 @@
   }
 }
 
+/*
+ * This action redefines the range of the superinstruction to include the
+ * preceding sandboxing sequence then invalidates jump targets on the
+ * interior of the superinstructions and finally clears “the restricted
+ * register” variable.
+ */
+static INLINE void ProcessNaclCallOrJmp(
+    uint32_t *instruction_info_collected, const uint8_t **instruction_start,
+    const uint8_t *current_position, const uint8_t *data,
+    bitmap_word *valid_targets, Bool and_have_rex, Bool add_stores_to_reg) {
+  /* Expand the range.  Only expand by 6 bytes: this covers “add” (second
+   * instruction) and “and” WITHOUT it's REX prefix.
+   */
+  *instruction_start -= 6;
+  /* Compare registers used by “and”, “add”, and “call” or “jmp”.  */
+  if (RMFromModRM((*instruction_start)[1]) !=
+      (add_stores_to_reg ? RegFromModRM((*instruction_start)[5]) :
+                           RMFromModRM((*instruction_start)[5])) ||
+      RMFromModRM((*instruction_start)[1]) != RMFromModRM(current_position[0]))
+    *instruction_info_collected |= UNRECOGNIZED_INSTRUCTION;
+  /* Make start of the second and third instruction not valid targets. */
+  MakeInvalidJumpTarget((*instruction_start - data) + 3, valid_targets);
+  MakeInvalidJumpTarget((*instruction_start - data) + 6, valid_targets);
+  /* Expand the range again.  This time to cover REX prefix (if any).  */
+  if (and_have_rex) --*instruction_start;
+}
+static INLINE void ProcessNaclCallOrJmpAddToRMNoRex(

[Brad Chen, 2012/10/22 21:29:05]

Nit: empty line between function decls.

[khim, 2013/03/08 17:59:53]

Done.

+    uint32_t *instruction_info_collected, const uint8_t **instruction_start,
+    const uint8_t *current_position, const uint8_t *data,
+    bitmap_word *valid_targets) {
+  ProcessNaclCallOrJmp(instruction_info_collected, instruction_start,
+                       current_position, data, valid_targets, FALSE, FALSE);

[Brad Chen, 2012/10/22 21:29:05]

As noted elsewhere I'm not happy with these booleans used like this as I can't
tell from the call-site what effect they have. Can you please find a way to make
things more readable? Options: use a enum instead of a bool, incorporate bool
into procedure name, or use comments.

[khim, 2013/03/08 17:59:53]

Reworked the procedures to exclude bools and added comments to clarify the work
of the procedures. I think I'll declare this the final attempt to simplify that
code. If it's still not clear then I'm ready do declare defeat and scrap it
along with the rest of the validator.

+}
+static INLINE void ProcessNaclCallOrJmpAddToRegNoRex(
+    uint32_t *instruction_info_collected, const uint8_t **instruction_start,
+    const uint8_t *current_position, const uint8_t *data,
+    bitmap_word *valid_targets) {
+  ProcessNaclCallOrJmp(instruction_info_collected, instruction_start,
+                       current_position, data, valid_targets, FALSE, TRUE);
+}
+static INLINE void ProcessNaclCallOrJmpAddToRMWithRex(
+    uint32_t *instruction_info_collected, const uint8_t **instruction_start,
+    const uint8_t *current_position, const uint8_t *data,
+    bitmap_word *valid_targets) {
+  ProcessNaclCallOrJmp(instruction_info_collected, instruction_start,
+                       current_position, data, valid_targets, TRUE, FALSE);
+}
+static INLINE void ProcessNaclCallOrJmpAddToRegWithRex(
+    uint32_t *instruction_info_collected, const uint8_t **instruction_start,
+    const uint8_t *current_position, const uint8_t *data,
+    bitmap_word *valid_targets) {
+  ProcessNaclCallOrJmp(instruction_info_collected, instruction_start,
+                       current_position, data, valid_targets, TRUE, TRUE);
+}
+
+/*
+ * This action redefines the range of the superinstruction to include the
+ * preceding sandboxing sequence then invalidates jump targets on the interior
+ * of the superinstruction.
+ */
+static INLINE void SandboxRxiSuperInst(
+    const uint8_t **instruction_start, const uint8_t *data,
+    bitmap_word *valid_targets, Bool mov_have_rex) {
+  /* Expand the range.  Only expand by 6 bytes: this covers “lea” (second
+   * instruction) and “mov” WITHOUT it's REX prefix.
+   */
+  *instruction_start -= 6;
+  MakeInvalidJumpTarget((*instruction_start - data) + 2, valid_targets);
+  MakeInvalidJumpTarget((*instruction_start - data) + 6, valid_targets);
+  /* Expand the range again.  This time to cover REX prefix (if any).  */
+  if (mov_have_rex) --*instruction_start;
+}
+static INLINE void SandboxRxiSuperInstNoRexOnMov(
+    const uint8_t **instruction_start, const uint8_t *data,
+    bitmap_word *valid_targets) {
+  SandboxRxiSuperInst(instruction_start, data, valid_targets, FALSE);
+}
+static INLINE void SandboxRxiSuperInstWithRexOnMov(
+    const uint8_t **instruction_start, const uint8_t *data,
+    bitmap_word *valid_targets) {
+  SandboxRxiSuperInst(instruction_start, data, valid_targets, TRUE);
+}
+
 #endif  /* NATIVE_CLIENT_SRC_TRUSTED_VALIDATOR_RAGEL_VALIDATOR_INTERNAL_H_ */