Resurrect Petr's 64-bit dynamic code modification CL:...

src/trusted/validator_x86/nccopycode.c

 /*
  * Copyright 2010 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.
  */
 
 /*
  * nccopycode.c
  * Copies two code streams in a thread-safe way
  *
  */
 
 #include "native_client/src/include/portability.h"
 
 #if NACL_WINDOWS == 1
 #include <windows.h>
 #else
 #include <sys/mman.h>
 #endif
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <string.h>
 #include <assert.h>
+#include "native_client/src/shared/platform/nacl_check.h"
+#if NACL_TARGET_SUBARCH == 32
 #include "native_client/src/trusted/validator_x86/ncdecode.h"
 #include "native_client/src/trusted/validator_x86/ncvalidate.h"
-#include "native_client/src/shared/platform/nacl_check.h"
+#elif NACL_TARGET_SUBARCH == 64
+#include "native_client/src/trusted/validator_x86/nc_inst_iter.h"
+#include "native_client/src/trusted/validator_x86/nc_segment.h"
+#include "native_client/src/trusted/validator_x86/nc_inst_state_internal.h"
+#else
+#error "Unknown Platform"
+#endif
 
 /* x86 HALT opcode */
 static const uint8_t kNaClFullStop = 0xf4;
 
 /*
  * Max size of aligned writes we may issue to code without syncing.
  * 8 is a safe value according to:
  *   [1] Advance Micro Devices Inc. AMD64 Architecture Program-
  *   mers Manual Volume 1: Application Programming, 2009.
  *   [2] Intel Inc. Intel 64 and IA-32 Architectures Software Developers
@@ skipping 112 matching lines @@
   prot = (prot == prot_a ? prot_b : prot_a);
   rv = mprotect(g_squashybuffer, size, prot);
   CHECK(rv == 0);
 #endif
 }
 
 /*
  * Copy a single instruction, avoiding the possibility of other threads
  * executing a partially changed instruction.
  */
-void CopyInstruction(const struct NCDecoderState *mstate_old,
-                     const struct NCDecoderState *mstate_new) {
-  uint8_t* dst = mstate_old->inst.maddr;
-  uint8_t* src = mstate_new->inst.maddr;
-  int sz = mstate_old->inst.length;
+void CopyInstructionInternal(uint8_t *dst,
+                             uint8_t *src,
+                             uint8_t sz) {
   intptr_t offset = 0;
-  CHECK(mstate_new->inst.length == mstate_old->inst.length);
+  uint8_t *firstbyte_p = dst;
 
   while (sz > 0 && dst[0] == src[0]) {
     /* scroll to first changed byte */
     dst++, src++, sz--;
   }
 
   if (sz == 0) {
     /* instructions are identical, we are done */
     return;
   }
 
   while (sz > 0 && dst[sz-1] == src[sz-1]) {
     /* trim identical bytes at end */
     sz--;
   }
 
   if (sz == 1) {
-    /* we assume a 1-byte change it atomic */
+    /* we assume a 1-byte change is atomic */
     *dst = *src;
   } else if (IsTrustedWrite(dst, sz, 4, &offset)) {
     uint8_t tmp[4];
     memcpy(tmp, dst-offset, sizeof tmp);
     memcpy(tmp+offset, src, sz);
     onestore_memmove4(dst-offset, tmp);
   } else if (IsTrustedWrite(dst, sz, 8, &offset)) {
     uint8_t tmp[8];
     memcpy(tmp, dst-offset, sizeof tmp);
     memcpy(tmp+offset, src, sz);
     onestore_memmove8(dst-offset, tmp);
   } else {
     /* the slow path, first flip first byte to halt*/
-    uint8_t firstbyte = mstate_old->inst.maddr[0];
-    mstate_old->inst.maddr[0] = kNaClFullStop;
+    uint8_t firstbyte = firstbyte_p[0];
+    firstbyte_p[0] = kNaClFullStop;
 
     SerializeAllProcessors();
 
     /* copy the rest of instruction */
-    if (dst == mstate_old->inst.maddr) {
+    if (dst == firstbyte_p) {
       /* but not the first byte! */
       firstbyte = *src;
       dst++, src++, sz--;
     }
     memcpy(dst, src, sz);
 
     SerializeAllProcessors();
 
     /* flip first byte back */
-    mstate_old->inst.maddr[0] = firstbyte;
+    firstbyte_p[0] = firstbyte;
   }
 }
 
+#if NACL_TARGET_SUBARCH == 32
+
+/*
+ * Copy a single instruction, avoiding the possibility of other threads
+ * executing a partially changed instruction.
+ */
+void CopyInstruction(const struct NCDecoderState *mstate_old,
+                     const struct NCDecoderState *mstate_new) {
+  CHECK(mstate_new->inst.length == mstate_old->inst.length);
+
+  CopyInstructionInternal(mstate_old->inst.maddr,
+                          mstate_new->inst.maddr,
+                          mstate_old->inst.length);
+}
+
 int NCCopyCode(uint8_t *dst, uint8_t *src, NaClPcAddress vbase,
                size_t sz, int bundle_size) {
   struct NCValidatorState *vstate;
   vstate = NCValidateInit(vbase, vbase+sz, bundle_size);
   CHECK(NULL != vstate);
   NCDecodeSegmentPair(dst, src, vbase, sz, vstate, CopyInstruction);
   NCValidateFreeState(&vstate);
   return 0;
 }
 
+#elif NACL_TARGET_SUBARCH == 64
+
+int NaClCopyCodeIter(uint8_t *dst, uint8_t *src,
+                     NaClPcAddress vbase, size_t size) {
+  NaClSegment segment_old, segment_new;
+  NaClInstIter *iter_old, *iter_new;
+  NaClInstState *istate_old, *istate_new;
+
+  NaClSegmentInitialize(dst, vbase, size, &segment_old);
+  NaClSegmentInitialize(src, vbase, size, &segment_new);
+
+  iter_old = NaClInstIterCreate(&segment_old);
+  iter_new = NaClInstIterCreate(&segment_new);
+  while (NaClInstIterHasNext(iter_old) &&
+         NaClInstIterHasNext(iter_new)) {
+    istate_old = NaClInstIterGetState(iter_old);

[bsy, 2010/12/15 04:06:50]

does this get pseudo-atomic sequences as a single NaClnstState?  comment on
assumptions made by this code... e.g., this is only used after
NaClValidateCodeReplacement

[elijahtaylor (use chromium), 2010/12/15 21:26:37]

Comment added to clarify what we're doing.

On 2010/12/15 04:06:50, bsy wrote:

+    istate_new = NaClInstIterGetState(iter_new);
+    if (istate_old->length != istate_new->length ||
+        istate_new->vpc != istate_old->vpc) {
+      NaClLog(LOG_ERROR,
+              "Segment replacement: copied instructions misaligned\n");

[bsy, 2010/12/15 04:06:50]

is this precondition violation that should never happen, due to
NaClValidateCodeReplacement?  if so, a comment and a LOG_FATAL would be more
appropriate; if not, then NaClInstIterDestroys are necessary to properly clean
up.

[elijahtaylor (use chromium), 2010/12/15 21:26:37]

Changed to LOG_FATAL and commented as this should be guaranteed not to happen.

On 2010/12/15 04:06:50, bsy wrote:

+      return 1;
+    }
+    CopyInstructionInternal(istate_old->mpc,

[bsy, 2010/12/15 04:06:50]

add a comment here that replacing all affected instructions' first bytes with
HLT, then a SerializeAllProcessors, then the rest of the bytes with the desired
replacement values, another SerializeAllProcessors, then replace the first byte
of each instruction with the desired values would save overhead, rather than
doing 2 calls to SerializeAllProcessors per instruction.

[elijahtaylor (use chromium), 2010/12/15 21:26:37]

Done.

On 2010/12/15 04:06:50, bsy wrote:

+                            istate_new->mpc,
+                            istate_old->length);
+    NaClInstIterAdvance(iter_old);
+    NaClInstIterAdvance(iter_new);
+  }
+
+  CHECK(!NaClInstIterHasNext(iter_old) && !NaClInstIterHasNext(iter_new));
+
+  NaClInstIterDestroy(iter_old);
+  NaClInstIterDestroy(iter_new);
+  return 0;
+}
+
+#else
+#error "Unknown Platform"
+#endif
+