Rewrite non-temporal instructions

src/trusted/validator_ragel/dfa_validate_common.c

Index: src/trusted/validator_ragel/dfa_validate_common.c
diff --git a/src/trusted/validator_ragel/dfa_validate_common.c b/src/trusted/validator_ragel/dfa_validate_common.c
index 4972cbd654b3fcd0d884dec0099db63750991802..2b440dd9e55e85ac13422deadeb1c272b2d24a56 100644
--- a/src/trusted/validator_ragel/dfa_validate_common.c
+++ b/src/trusted/validator_ragel/dfa_validate_common.c
@@ -12,6 +12,7 @@
 #include "native_client/src/shared/platform/nacl_check.h"
 #include "native_client/src/trusted/service_runtime/nacl_config.h"
 #include "native_client/src/trusted/validator_ragel/validator.h"
+#include "native_client/src/include/build_config.h"
 
 /* Used as an argument to copy_func when unsupported instruction must be
    replaced with HLTs.  */
@@ -33,27 +34,81 @@ Bool NaClDfaProcessValidationError(const uint8_t *begin, const uint8_t *end,
   return FALSE;
 }
 
+#if NACL_BUILD_SUBARCH == 64
+static Bool IsREX(uint8_t byte) {
+  return byte >= 0x40 && byte <= 0x4f;
+}
+#endif
+
+static Bool RewriteUnsupportedInstruction(const uint8_t *begin,
+                                          const uint8_t *end) {
+  uint8_t *ptr = (uint8_t *) begin;
+  /* We usually only check and rewrite the first few bytes without examining
+   * further because this function is only called when the validator tells us
+   * that it is an 'unsupported instruction' and there are no other validation
+   * failures.
+   */
+#if NACL_BUILD_SUBARCH == 32
+  UNREFERENCED_PARAMETER(end);
+  if (memcmp(begin, "\x0f\xe7", 2) == 0) {
+    /* movntq => movq */

[khimg, 2015/08/08 00:33:11]

Why movntq is not rewritten in 64bit mode?

[ruiq, 2015/08/10 00:57:15]

As the CL description says, we only rewrite non-temporal instructions that are
found in current nexes of the webstore so that we do not break them. For future
nexes, they will fail validation if they contain (other) non-temporal
instructions. 

+    ptr[1] = 0x7f;
+    return TRUE;
+  } else if (memcmp(begin, "\x66\x0f\xe7", 3) == 0) {
+    /* movntdq => movdqa */
+    ptr[2] = 0x7f;
+    return TRUE;
+  }
+#elif NACL_BUILD_SUBARCH == 64
+  if (IsREX(begin[0]) && (begin[1] == 0x0f)) {
+    uint8_t opcode_byte2 = begin[2];
+    switch (opcode_byte2) {
+      case 0x2b:
+        /* movntps => movaps */

[khimg, 2015/08/08 00:33:11]

Why movntps is not rewritten in 32bit mode?

[ruiq, 2015/08/10 00:57:15]

ditto.

+        ptr[2] = 0x29;
+        return TRUE;
+      case 0xc3:
+        /* movnti => mov */

[khimg, 2015/08/08 00:33:11]

Why movnti is not rewritten in 32bit moed?

[ruiq, 2015/08/10 00:57:15]

ditto.

+        ptr[1] = 0x89;
+        memmove(ptr + 2, ptr + 3, end - begin - 3);

[khimg, 2015/08/08 00:33:11]

You can't do that. Use the following approach:
  ptr[2] = 0x89;
  ptr[1] = ptr[0];
  ptr[0] = 0x90;

[ruiq, 2015/08/10 00:57:15]

The nop has to be put at the end because before the non-temporal store
instruction, there can be an instruction that clears the upper 32-bits of a
register. That instruction followed with this non-temporal store will pass the
validation, but not if they are separated. 

+        ptr[end - begin - 1] = 0x90;
+        return TRUE;
+      case 0x18:
+        /* prefetchnta => nop */

[khimg, 2015/08/08 00:33:11]

Why prefetchnta is not rewritten in 32bit mode?
You can replace it with a singular nop:
  ptr[2] = 0x1F;
  ptr[3] &= 0xC7;

[ruiq, 2015/08/10 00:57:15]

ditto.

+        memset(ptr, 0x90, end - begin);
+        return TRUE;
+      default:
+        return FALSE;
+    }
+  } else if (begin[0] == 0x66 && IsREX(begin[1]) &&
+      memcmp(begin + 2, "\x0f\xe7", 2) == 0) {
+    /* movntdq => movdqa */
+    ptr[3] = 0x7f;
+    return TRUE;
+  }
+#endif
+  return FALSE;
+}
+
 Bool NaClDfaStubOutUnsupportedInstruction(const uint8_t *begin,
                                           const uint8_t *end,
                                           uint32_t info,
                                           void *callback_data) {
   struct StubOutCallbackData *data = callback_data;
-  /* Stub-out instructions unsupported on this CPU, but valid on other CPUs.  */
   if ((info & VALIDATION_ERRORS_MASK) == CPUID_UNSUPPORTED_INSTRUCTION) {
+    /* Stub-out instructions unsupported on this CPU, but valid on other CPUs.*/
     data->did_rewrite = 1;
     memset((uint8_t *)begin, NACL_HALT_OPCODE, end - begin);
     return TRUE;
   } else if ((info & VALIDATION_ERRORS_MASK) == UNSUPPORTED_INSTRUCTION) {
-    if (data->flags & NACL_DISABLE_NONTEMPORALS_X86) {
-      return FALSE;
-    } else {
-      /* TODO(ruiq): rewrite instruction. For now, we keep the original
-       * instruction and indicate validation success, which is consistent
-       * with current validation results. */
-      data->did_rewrite = 0;
+    if (!(data->flags & NACL_DISABLE_NONTEMPORALS_X86) &&
+        RewriteUnsupportedInstruction(begin, end)) {
+      data->did_rewrite = 1;
       return TRUE;
     }
+    return FALSE;
   } else {
+    /* Other validation errors the validator cannot fix. */
     return FALSE;
   }
 }