- Use the simulator to do atomic operations that could also

runtime/vm/simulator_arm.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include <setjmp.h>
 #include <stdlib.h>
 
 #include "vm/globals.h"
 #if defined(TARGET_ARCH_ARM)
 
@@ skipping 660 matching lines @@
     offset += len;
   }
   ASSERT(result != NULL);
   result[offset] = '\0';
   return result;
 }
 
 
 // Synchronization primitives support.
 Mutex* Simulator::exclusive_access_lock_ = NULL;
-Simulator::AddressTag Simulator::exclusive_access_state_[kNumAddressTags];
-int Simulator::next_address_tag_;
+Simulator::AddressTag Simulator::exclusive_access_state_[kNumAddressTags] =
+    {{NULL, 0}};
+int Simulator::next_address_tag_ = 0;
 
 
-void Simulator::SetExclusiveAccess(uword addr) {
-  Isolate* isolate = Isolate::Current();
-  ASSERT(isolate != NULL);
-  int i = 0;
-  while ((i < kNumAddressTags) &&
-         (exclusive_access_state_[i].isolate != isolate)) {
-    i++;
-  }
-  if (i == kNumAddressTags) {
-    i = next_address_tag_;
-    if (++next_address_tag_ == kNumAddressTags) next_address_tag_ = 0;
-    exclusive_access_state_[i].isolate = isolate;
-  }
-  exclusive_access_state_[i].addr = addr;
+void Simulator::InitOnce() {
+  // Setup exclusive access state lock.
+  exclusive_access_lock_ = new Mutex();
 }
 
 
-bool Simulator::HasExclusiveAccessAndOpen(uword addr) {
-  Isolate* isolate = Isolate::Current();
-  ASSERT(isolate != NULL);
-  bool result = false;
-  for (int i = 0; i < kNumAddressTags; i++) {
-    if (exclusive_access_state_[i].isolate == isolate) {
-      if (exclusive_access_state_[i].addr == addr) {
-        result = true;
-      }
-      exclusive_access_state_[i].addr = NULL;
-      continue;
-    }
-    if (exclusive_access_state_[i].addr == addr) {
-      exclusive_access_state_[i].addr = NULL;
-    }
-  }
-  return result;
-}
-
-
-void Simulator::InitOnce() {
-  // Setup exclusive access state.
-  exclusive_access_lock_ = new Mutex();
-  for (int i = 0; i < kNumAddressTags; i++) {
-    exclusive_access_state_[i].isolate = NULL;
-    exclusive_access_state_[i].addr = NULL;
-  }
-  next_address_tag_ = 0;
-}
-
-
 Simulator::Simulator() {
   // Setup simulator support first. Some of this information is needed to
   // setup the architecture state.
   // We allocate the stack here, the size is computed as the sum of
   // the size specified by the user and the buffer space needed for
   // handling stack overflow exceptions. To be safe in potential
   // stack underflows we also add some underflow buffer space.
   stack_ = new char[(Isolate::GetSpecifiedStackSize() +
                      Isolate::kStackSizeBuffer +
                      kSimulatorStackUnderflowSize)];
@@ skipping 362 matching lines @@
 
 void Simulator::WriteB(uword addr, uint8_t value) {
   static StatsCounter counter_write_b("Simulated byte writes");
   counter_write_b.Increment();
   uint8_t* ptr = reinterpret_cast<uint8_t*>(addr);
   *ptr = value;
 }
 
 
 // Synchronization primitives support.
+void Simulator::SetExclusiveAccess(uword addr) {
+  Isolate* isolate = Isolate::Current();
+  ASSERT(isolate != NULL);
+  int i = 0;
+  // Find an entry for this isolate in the exclusive access state.
+  while ((i < kNumAddressTags) &&
+         (exclusive_access_state_[i].isolate != isolate)) {
+    i++;
+  }
+  // Round-robin replacement of previously used entries.
+  if (i == kNumAddressTags) {
+    i = next_address_tag_;
+    if (++next_address_tag_ == kNumAddressTags) {
+      next_address_tag_ = 0;
+    }
+    exclusive_access_state_[i].isolate = isolate;

[zra, 2014/10/27 14:53:12]

Should we assert here that exclusive_access_state_[i].addr == 0? Or instead
search for an entry where that's the case?

[Ivan Posva, 2014/10/27 17:58:24]

You cannot assert this. As it is perfectly legal to:
ldrex Rt, some_addr
then branch out if Rt is not the expected value and then later
ldrex Rt, some_other_addr
At this point exclusive_access_state's addr for this isolate is not zero and
you'd get a wrong assertion failure.
The CPU the simulator is modeling has only one reservation slot to be very
conservative. Thus you need to invalidate the existing reservation if it exists
for this isolate.

[zra, 2014/10/27 18:09:17]

What happens when you run out of address tags?

+  }
+  // Remember the address being reserved.
+  exclusive_access_state_[i].addr = addr;
+}
+
+
+bool Simulator::HasExclusiveAccessAndOpen(uword addr) {
+  Isolate* isolate = Isolate::Current();
+  ASSERT(isolate != NULL);
+  ASSERT(addr != 0);
+  bool result = false;
+  for (int i = 0; i < kNumAddressTags; i++) {
+    if (exclusive_access_state_[i].isolate == isolate) {
+      // Check whether the current isolates address reservation matches.

[zra, 2014/10/27 14:53:12]

isolate's

[Ivan Posva, 2014/10/27 17:58:23]

Done.

+      if (exclusive_access_state_[i].addr == addr) {
+        result = true;
+      }
+      exclusive_access_state_[i].addr = 0;
+    } else if (exclusive_access_state_[i].addr == addr) {
+      // Other isolates with matching address lose their reservations.
+      exclusive_access_state_[i].addr = 0;
+    }
+  }
+  return result;
+}
+
+
 void Simulator::ClearExclusive() {
-  // This lock is initialized in Simulator::InitOnce().
   MutexLocker ml(exclusive_access_lock_);
-  // Set exclusive access to open state for this isolate.
-  HasExclusiveAccessAndOpen(NULL);
+  // Remove the reservation for this isolate.
+  SetExclusiveAccess(NULL);
 }
 
 
 intptr_t Simulator::ReadExclusiveW(uword addr, Instr* instr) {
-  // This lock is initialized in Simulator::InitOnce().
   MutexLocker ml(exclusive_access_lock_);
   SetExclusiveAccess(addr);
   return ReadW(addr, instr);
 }
 
 
 intptr_t Simulator::WriteExclusiveW(uword addr, intptr_t value, Instr* instr) {
-  // This lock is initialized in Simulator::InitOnce().
   MutexLocker ml(exclusive_access_lock_);
   bool write_allowed = HasExclusiveAccessAndOpen(addr);
   if (write_allowed) {
     WriteW(addr, value, instr);
     return 0;  // Success.
   }
   return 1;  // Failure.
 }
 
 
 uword Simulator::CompareExchange(uword* address,
                                  uword compare_value,
                                  uword new_value) {
   // This lock is initialized in Simulator::InitOnce().

[koda, 2014/10/27 14:05:10]

Why keep this comment but remove the other two instances?

[Ivan Posva, 2014/10/27 17:58:24]

Done. Oversight, thanks for noticing.

   MutexLocker ml(exclusive_access_lock_);
+  // We do not check for a reservation as it would be guaranteed to be found. No

[koda, 2014/10/27 14:05:10]

Why not ASSERT this then?

[Ivan Posva, 2014/10/27 17:58:23]

Maybe the comment needs rephrasing: What it is supposed to convey that since we
hold the lock the load would get the reservation and nobody else can get it and
steal it while the lock is held.

[koda, 2014/10/27 18:32:48]

Then you should assert that the current isolate holds the lock when
SetExclusiveAccess/HasExclusiveAccessAndOpen are called, and document it in the
header.

[Ivan Posva, 2014/10/27 19:21:08]

Done.

+  // other isolate is able to make a reservation while we hold the lock.
   uword value = *address;
   if (value == compare_value) {
     *address = new_value;
     // Same effect on exclusive access state as a successful STREX.
     HasExclusiveAccessAndOpen(reinterpret_cast<uword>(address));
   } else {
     // Same effect on exclusive access state as an LDREX.
     SetExclusiveAccess(reinterpret_cast<uword>(address));
   }
   return value;
@@ skipping 2709 matching lines @@
   set_register(kExceptionObjectReg, bit_cast<int32_t>(raw_exception));
   set_register(kStackTraceObjectReg, bit_cast<int32_t>(raw_stacktrace));
   buf->Longjmp();
 }
 
 }  // namespace dart
 
 #endif  // !defined(HOST_ARCH_ARM)
 
 #endif  // defined TARGET_ARCH_ARM