Add thread-safety to creation of MemCopy and modulo functions.

src/platform-posix.cc

-// Copyright 2009 the V8 project authors. All rights reserved.
+// Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
 //       with the distribution.
@@ skipping 186 matching lines @@
     // If the length is zero, the assignment fails.
     if (str.length() > 0)
       str[str.length() - 1] = '\0';
     return -1;
   } else {
     return n;
   }
 }
 
 
+#if defined(V8_TARGET_ARCH_IA32)
+static OS::MemCopyFunction memcopy_function = NULL;
+static Mutex* memcopy_function_mutex = OS::CreateMutex();
+// Defined in codegen-ia32.cc.
+OS::MemCopyFunction CreateMemCopyFunction();
+
+// Copy memory area to disjoint memory area.
+void OS::MemCopy(void* dest, const void* src, size_t size) {

[Vitaly Repeshko, 2011/03/30 10:17:19]

Why does this have to be in platform files? It'd be nice to avoid repeating it.

[William Hesse, 2011/03/30 11:26:44]

It depends on whether it is an inline function or not.  Since the global statics
need to be declared in a single compilation unit, and v8utils does not have a
.cc file, and MemCopy seems like a platform-specific replacement for the OS
memcpy function, this seemed like the right place for it.

[Vitaly Repeshko, 2011/03/30 12:25:01]

I think creating v8utils.cc is a better solution. Otherwise, we have to write
this code in all platforms that support ia32. Am I missing something?

+  if (memcopy_function == NULL) {
+    ScopedLock lock(memcopy_function_mutex);
+    Isolate::EnsureDefaultIsolate();
+    if (memcopy_function == NULL) {
+      memcopy_function = CreateMemCopyFunction();

[Vitaly Repeshko, 2011/03/30 10:17:19]

You need a memory barrier between function creation and storing it to the global
pointer to make double-checked locking safe. Use MemoryBarrier or Release_Store
from atomicops.

[William Hesse, 2011/03/30 11:26:44]

Done.

[William Hesse, 2011/03/30 11:26:44]

We end the function CreateMemCopyFunction() with a CPU::FlushICache, but that
does nothing, so I have also added a memory barrier there, just before returning
the pointer. 
On 2011/03/30 10:17:19, Vitaly Repeshko wrote:

[Vitaly Repeshko, 2011/03/30 12:25:01]

Well, strictly speaking we need both CPU and compiler memory barriers here.
Having one at the end of CreateMemCopyFunction() means only having the CPU
barrier. The compiler is free to put something in the global pointer before
storing the return value there. (Even though a sane compiler won't do that in
this case.) To prevent it from doing this we need a memory barrier here:
MemCopyFunction tmp = CreateMemCopyFunction();
MemoryBarrier();
memcopy_function = tmp;
--or--
Release_Store(&memcopy_function, CreateMemCopyFunction());

+    }
+  }
+  (*memcopy_function)(dest, src, size);
+#ifdef DEBUG
+  CHECK_EQ(0, memcmp(dest, src, size));
+#endif
+}
+#endif  // V8_TARGET_ARCH_IA32
+
 // ----------------------------------------------------------------------------
 // POSIX string support.
 //
 
 char* OS::StrChr(char* str, int c) {
   return strchr(str, c);
 }
 
 
 void OS::StrNCpy(Vector<char> dest, const char* src, size_t n) {
@@ skipping 172 matching lines @@
   return ntohl(value);
 }
 
 
 Socket* OS::CreateSocket() {
   return new POSIXSocket();
 }
 
 
 } }  // namespace v8::internal