Refactor the process of choosing validators.

src/trusted/validator/ncvalidate.h

Index: src/trusted/validator/ncvalidate.h
diff --git a/src/trusted/validator/ncvalidate.h b/src/trusted/validator/ncvalidate.h
index a46e6960814f49ed0fbd0036ab1a21303d1fc7bf..32d153583f4abdc6d0c6dbbdaaf063866dc9c4e2 100644
--- a/src/trusted/validator/ncvalidate.h
+++ b/src/trusted/validator/ncvalidate.h
@@ -7,26 +7,7 @@
 #ifndef NATIVE_CLIENT_SRC_TRUSTED_VALIDATOR_X86_NCVALIDATE_H__
 #define NATIVE_CLIENT_SRC_TRUSTED_VALIDATOR_X86_NCVALIDATE_H__
 
-/* Defines the API to the native client validation code.
- *
- * Note that the API is designed to use the NACL_SUBARCH_NAME
- * macro. This macro encapsulates the target architecture the
- * code is being compiled for into the name of the function.
- * This lets the API call different functions based on the
- * target architecture the code is being compiled for. It also allows
- * the maintainers of the compiled libraries to change the implementation
- * without having to update the API callers.
- *
- * The macro NACL_SUBARCH_NAME(name, arch, subarch) prepends "NaCl_"
- * to the function name, and suffixes it with the architecture and
- * corresponding subarchitecture (with underscores separating it).
- * Hence, for example, NACL_SUBARCH_NAME(ApplyValidator, x86, 64)
- * would generate the function name NaCL_ApplyValidator_x86_64.
- */
-/* TODO(bradchen): Cleanup the interface to the validator. There
- * are too many combinations of different calls and input parameters,
- * making this interface hard to understand and hard to modify.
- */
+/* Defines the API to the native client validation code. */
 
 #include "native_client/src/include/nacl_base.h"
 #include "native_client/src/shared/utils/types.h"
@@ -52,11 +33,10 @@ typedef enum NaClValidationStatus {
   NaClValidationFailedSegmentationIssue
 } NaClValidationStatus;
 
-/* Applies the validator, as defined by sel_ldr. That is, run the
- * validator where performance is critical.
+/* Function type for applying a Validator, as defined by sel_ldr. That is, run
+ * the validator where performance is critical.
+ *
  * Parameters are:
- *    local_cpu: True if local cpu rules should be applied.
- *           Otherwise, assume no cpu specific rules.
  *    guest_addr - The virtual pc to assume with the beginning address of the
  *           code segment. Typically, this is the corresponding addresss that
  *           will be used by objdump.
@@ -86,47 +66,44 @@ extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyValidator,
     const NaClCPUFeatures   *cpu_features,
     struct NaClValidationCache *cache);
 
-/* Applies the DFA-based validator as in the ApplyValidator case described
- * above.  The interface of this new validator must remain the same as of the
- * ApplyValidator.  Less arguments will be ignored as the new validator
- * implements features, such as stubout mode.
- */
-extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyDfaValidator,
-                                              NACL_TARGET_ARCH,
-                                              NACL_TARGET_SUBARCH)(
-    uintptr_t               guest_addr,
-    uint8_t                 *data,
-    size_t                  size,
-    int                     stubout_mode,
-    int                     readonly_text,
-    const NaClCPUFeatures   *cpu_features,
+typedef NaClValidationStatus (*NaClValidateFunc) (
+    uintptr_t guest_addr,
+    uint8_t *data,
+    size_t size,
+    int stubout_mode,
+    int readonly_text,
+    const NaClCPUFeatures *cpu_features,
     struct NaClValidationCache *cache);
 
-/* Applies the validator, as used in a command-line tool to report issues.
- * Note: This is intentionally separated from ApplyValidator, since it need
- * not be performance critical.
+/* Function type for applying a Validator to copy code from an existing code
+ * segment to a new code segment.
+ *
+ * Note: Current implementations use the instruction decoders, which may
+ * require that the code segment match native client rules.
+ *
  * Parameters are:
- *    local_cpu: True if local cpu rules should be applied.
- *           Otherwise, assume no cpu specific rules.
  *    guest_addr - The virtual pc to assume with the beginning address of the
  *           code segment. Typically, this is the corresponding addresss that
  *           will be used by objdump.
- *    data - The contents of the code segment to be validated.
- *    size - The size of the code segment to be validated.
+ *    data_old - The contents of the original code segment.
+ *    data_new - The addres of the new code segment for which the original
+ *           code segment should be copied into.
+ *    size - The size of the passed code segments.
  *    cpu_features - The CPU features to support while validating.
  */
-extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyValidatorVerbosely,
-                                              NACL_TARGET_ARCH,
-                                              NACL_TARGET_SUBARCH)(
-    uintptr_t               guest_addr,
-    uint8_t                 *data,
-    size_t                  size,
-    const NaClCPUFeatures   *cpu_features);
+typedef NaClValidationStatus (*NaClValidateCopyFunc) (
+    uintptr_t guest_addr,
+    uint8_t *data_old,
+    uint8_t *data_new,
+    size_t size,
+    const NaClCPUFeatures *cpu_features);
 
-/* Applies validator to new code segment, assuming that it was updated
- * from the previously validated old code segment. Assumes that instruction
- * sizes are the same. Only allows changes in branches that don't change
- * instruction sizes.
+/* Function type for applying a Validator on small updates to previously
+ * validated code segments.
+ *
+ * Assumes that instruction sizes are the same. Only allows changes in branches
+ * that don't change instruction sizes.
+ *
  * Parameters are:
  *    guest_addr - The virtual pc to assume with the beginning address of the
  *           code segment. Typically, this is the corresponding addresss that
@@ -136,39 +113,49 @@ extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyValidatorVerbosely,
  *    size - The size of the passed code segments.
  *    cpu_features - The CPU features to support while validating.
  */
-extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyValidatorCodeReplacement,
-                                              NACL_TARGET_ARCH,
-                                              NACL_TARGET_SUBARCH)(
-      uintptr_t             guest_addr,
-      uint8_t               *data_old,
-      uint8_t               *data_new,
-      size_t                size,
-      const NaClCPUFeatures *cpu_features);
-
-/* Runs the validator to copy code from an existing code segment to a new
- * code segment.
- *
- * Note: Current implementations uses the instruction decoders, which may
- * require that the code segment match native client rules.
+typedef NaClValidationStatus (*NaClValidateCodeReplacementFunc) (
+    uintptr_t guest_addr,
+    uint8_t *data_old,
+    uint8_t *data_new,
+    size_t size,
+    const NaClCPUFeatures *cpu_features);
+
+/* The full set of validator API. */
+struct NaClValidatorInterface {
+  NaClValidateFunc Validate;
+  NaClValidateCopyFunc ValidateCopy;

[Nick Bray, 2012/04/27 22:21:36]

Nit: swap position of Copy and Replacement?  (Copy is less important that the
replacement function.)  Veeeery nitish.  it bothers me, but I realize I am being
way too picky.  Please ignore me.

[pasko-google - do not use, 2012/05/12 12:18:40]

no need to apologize :)
Replace is important for JITs.
Copy is important for dynamic linking.
JIT can work without dynamic linking and vice versa. But dynamic linking is more
fundamental.

[pasko-google - do not use, 2012/05/17 10:20:15]

I was wrong. ValidateCodeReplacement and CopyCode are used only together (one
after another), and only in dynamic code modification. nacl-glibc does not need
them. The validation goes first, and  then if it succeeds CodeCopy comes in.
This suggests me to put ValidateCodeReplacement before CopyCode in the validator
interface. Maybe in a separate change?

+  NaClValidateCodeReplacementFunc ValidateCodeReplacement;
+};
+
+/* Known Validator API initializers. */
+extern void NaClValidatorInit_x86_64(struct NaClValidatorInterface **val);
+extern void NaClValidatorInit_x86_32(struct NaClValidatorInterface **val);
+extern void NaClDfaValidatorInit_x86_32(struct NaClValidatorInterface **val);
+extern void NaClDfaValidatorInit_x86_64(struct NaClValidatorInterface **val);
+extern void NaClValidatorInitArm(struct NaClValidatorInterface **val);
+
+/* Make a messy choice for validating functions and record it in NaClApp. */
+extern void NaClSelectValidator(struct NaClValidatorInterface **val);
+
+/* Applies the validator, as used in a command-line tool to report issues.
+ * Note: This is intentionally separated from ApplyValidator, since it need
+ * not be performance critical.
  *
  * Parameters are:
  *    guest_addr - The virtual pc to assume with the beginning address of the
  *           code segment. Typically, this is the corresponding addresss that
  *           will be used by objdump.
- *    data_old - The contents of the original code segment.
- *    data_new - The addres of the new code segment for which the original
- *           code segment should be copied into.
- *    size - The size of the passed code segments.
+ *    data - The contents of the code segment to be validated.
+ *    size - The size of the code segment to be validated.
  *    cpu_features - The CPU features to support while validating.
  */
-extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyValidatorCopy,
+extern NaClValidationStatus NACL_SUBARCH_NAME(ApplyValidatorVerbosely,
                                               NACL_TARGET_ARCH,
                                               NACL_TARGET_SUBARCH)(
-    uintptr_t             guest_addr,
-    uint8_t               *data_old,
-    uint8_t               *data_new,
-    size_t                size,
-    const NaClCPUFeatures *cpu_features);
+    uintptr_t               guest_addr,
+    uint8_t                 *data,
+    size_t                  size,
+    const NaClCPUFeatures   *cpu_features);
 
 EXTERN_C_END