validator_ragel: Link into TCB, use under env var

src/trusted/validator_arm/ncvalidate.cc

 /*
  * Copyright (c) 2012 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.
  */
 
 #include "native_client/src/trusted/validator_arm/ncvalidate.h"
 
 
 #include <vector>
@@ skipping 102 matching lines @@
      const NaClCPUFeaturesArm *cpu_features) {
   UNREFERENCED_PARAMETER(sb_kind);
   UNREFERENCED_PARAMETER(guest_addr);
   UNREFERENCED_PARAMETER(data_old);
   UNREFERENCED_PARAMETER(data_new);
   UNREFERENCED_PARAMETER(size);
   UNREFERENCED_PARAMETER(cpu_features);
   return NaClValidationFailedNotImplemented;
 }
 
+NaClValidationStatus NACL_SUBARCH_NAME(ApplyDfaValidator, arm, 32) (

[Nick Bray, 2012/04/19 23:11:08]

Don't add this stub.  Just ifdef out references to ApplyDfaValidator on ARM. 
For example, how would we handle Mark's validator?  The MIPS validator?  Etc. 
There's a combinational explosion as we add more validators and architectures. 
Ultimately this "global name" pattern needs to be replaced with dependency
injection, and every validator entry point will have a different name.

[pasko-google - do not use, 2012/04/20 14:30:38]

Mark's validator exists for 2 platforms out of 3, adding an extra stub in ARM
for it is similarly ugly as #ifdef-ing the code in sel_validate_image that
otherwise operates on platform-independent abstractions.
The MIPS validator would live in ApplyValidator, will define a stub for
DfaValidator and one for Mark's validator.

Under assumption that "validator strategies differ a lot depending on platform"
better would be to define _only_ ApplyValidator function for each platform that
would deal with various blows and whistles like caching, choosing an exact
validator etc. Making so would be quite a huge refactoring. I'd ask to bypass it
now.

Under assumption that "validator strategies are pretty much the same for all
platforms" we add all validation methods for all platforms, some of which are
stubs. This seems to be the original assumption.

Under some assumptions we can implement a hybrid scheme with dynamic dispatch
and caching. Only good for complicated cases. I'll try to avoid that until there
actually is a lot of existing dispatch code that can be regularized. Better
refactor a dumb system than eliminate an overly complex system for the task.
Yes, adding ApplyDfaValidatorCopy, ApplyDfaValidatorCodeReplacement for all is a
bit explosive. I did not think about it yet. 
Probably. Though I would first try to reduce the amount of unneeded global names
(introducing a damn simple new argument?), then among the remaining names decide
whether we actually need runtime dispatch/injection. Not clear to me at this
point.

What I think is clear is that a hybrid approach with some "global names" and
some dynamic validator selection is worse than any of the two strategies alone.
So, looking at this I decided to go with what is common in the codebase, then
refactor, maybe. 

Yeah, increases technical debt. But without clear scope of validator features
planned it is a reasonable approach IMHO.

+    enum NaClSBKind sb_kind,
+    uintptr_t guest_addr,
+    uint8_t *data,
+    size_t size,
+    int stubout_mode,
+    int readonly_text,
+    const NaClCPUFeaturesArm *cpu_features,
+    struct NaClValidationCache *cache) {
+  UNREFERENCED_PARAMETER(sb_kind);
+  UNREFERENCED_PARAMETER(guest_addr);
+  UNREFERENCED_PARAMETER(data);
+  UNREFERENCED_PARAMETER(size);
+  UNREFERENCED_PARAMETER(cpu_features);
+  UNREFERENCED_PARAMETER(stubout_mode);
+  UNREFERENCED_PARAMETER(readonly_text);
+  UNREFERENCED_PARAMETER(cache);
+  return NaClValidationFailedNotImplemented;
+}
+
 EXTERN_C_END