A mechanism to identify/forbid/"rewrite" non-temporal instructions (and other)

src/trusted/validator/validation_pnacl_mode_test.cc

Index: src/trusted/validator/validation_pnacl_mode_test.cc
diff --git a/src/trusted/validator/validation_pnacl_mode_test.cc b/src/trusted/validator/validation_pnacl_mode_test.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1a193b91dfdb1cca401e4022e11cef007057c66b
--- /dev/null
+++ b/src/trusted/validator/validation_pnacl_mode_test.cc
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2015 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 "gtest/gtest.h"
+
+#include "native_client/src/shared/platform/nacl_log.h"
+#include "native_client/src/shared/utils/types.h"
+#include "native_client/src/trusted/validator/ncvalidate.h"
+
+#define CODE_SIZE 32
+#define NOP 0x90
+#define MOVNTI_CODE_SIZE 8
+
+// mov %edi,%edi; movnti %rax,0x68(%r15,%rdi,1)
+const char* movnti_code = "\x89\xff\x49\x0f\xc3\x44\x3f\x68";
+
+class ValidationPNaClModeTests : public ::testing::Test {
+ protected:
+  NaClValidationMetadata *metadata_ptr;
+  const struct NaClValidatorInterface *validator;
+  NaClCPUFeatures *cpu_features;
+
+  unsigned char code_buffer[CODE_SIZE];
+
+  void SetUp() {
+    metadata_ptr = NULL;
+    validator = NaClCreateValidator();
+    cpu_features = (NaClCPUFeatures *) malloc(validator->CPUFeatureSize);
+    EXPECT_NE(cpu_features, (NaClCPUFeatures *) NULL);
+    memset(cpu_features, 0, validator->CPUFeatureSize);
+    validator->SetAllCPUFeatures(cpu_features);
+    memset(code_buffer, NOP, sizeof(code_buffer));
+  }
+
+  NaClValidationStatus Validate(int pnacl_mode) {
+    return validator->Validate(0, code_buffer, 32,
+                               FALSE,  /* stubout_mode */
+                               pnacl_mode,
+                               FALSE,  /* readonly_test */
+                               cpu_features,
+                               metadata_ptr,
+                               NULL);
+  }
+
+  void TearDown() {
+    free(cpu_features);
+  }
+};
+
+TEST_F(ValidationPNaClModeTests, movnti_non_pnacl_mode) {
+  memcpy(code_buffer, movnti_code, MOVNTI_CODE_SIZE);
+  NaClValidationStatus status = Validate(0);
+  // In non-pnacl mode: use the original validation rule, i.e., allow
+  // non-temporal writes. In future, we will do rewriting.

[gdeepti, 2015/07/28 03:08:57]

Add a todo for rewriting. 

[ruiq, 2015/07/28 04:58:55]

There is a TODO in the place where we should do rewriting
(src/trusted/validator_ragel/dfa_validate_common.c). My thought was that the
tests are for current code, would it be a bit misleading if we add a TODO which
is essentially "TOTEST"?

+  EXPECT_EQ(NaClValidationSucceeded, status);
+  // Code should not change.
+  EXPECT_EQ(0, memcmp(code_buffer, movnti_code, MOVNTI_CODE_SIZE));

[gdeepti, 2015/07/28 03:08:58]

As we are not rewriting yet, seems redundant to check it here. Especially since
we should be rewriting in non PNaCl mode. Perhaps remove this and add details to
the TODO for the rewriting check that would be needed.  

[ruiq, 2015/07/28 04:58:55]

This is a sanity check. It was added because the code is modifiable
(readonly_test = FALSE), and we want to make sure it's not modified in the user
callback (current validation does rewrite some CPU unsupported instructions to
HALT). Would it be better to keep this to capture possible mistakes, as it is
not that expensive?

+}
+
+TEST_F(ValidationPNaClModeTests, movnti_pnacl_mode) {
+  memcpy(code_buffer, movnti_code, MOVNTI_CODE_SIZE);
+  NaClValidationStatus status = Validate(1);
+  // In pnacl mode: forbid non-temporal writes.
+  EXPECT_EQ(NaClValidationFailed, status);
+  // Code should not change.
+  EXPECT_EQ(0, memcmp(code_buffer, movnti_code, MOVNTI_CODE_SIZE));
+}
+
+int main(int argc, char *argv[]) {
+  // The IllegalInst test touches the log mutex deep inside the validator.
+  // This causes an SEH exception to be thrown on Windows if the mutex is not
+  // initialized.
+  // http://code.google.com/p/nativeclient/issues/detail?id=1696
+  NaClLogModuleInit();
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}