Bug 1285074 - Make ICacheMap manipulation signal-safe. r=luke
authorSean Stangl <sstangl@mozilla.com>
Fri, 18 Nov 2016 14:29:00 -0500
changeset 323529 39a7161b9628b4634900389b4add67580db9d4f4
parent 323528 d6f6bea2147962d0567accb421d385340210af78
child 323530 957dff87958ea8bda3d0e772fb15db03899a9d83
push id30978
push usercbook@mozilla.com
push dateMon, 21 Nov 2016 14:44:46 +0000
treeherdermozilla-central@0534254e9a40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1285074
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1285074 - Make ICacheMap manipulation signal-safe. r=luke
js/src/jit/arm/Simulator-arm.cpp
js/src/jit/arm/Simulator-arm.h
js/src/jit/mips32/Simulator-mips32.cpp
js/src/jit/mips32/Simulator-mips32.h
js/src/jit/mips64/Simulator-mips64.cpp
js/src/jit/mips64/Simulator-mips64.h
js/src/wasm/WasmSignalHandlers.cpp
--- a/js/src/jit/arm/Simulator-arm.cpp
+++ b/js/src/jit/arm/Simulator-arm.cpp
@@ -1046,32 +1046,48 @@ FlushICacheLocked(Simulator::ICacheMap& 
         size -= bytes_to_flush;
         MOZ_ASSERT((start & CachePage::kPageMask) == 0);
         offset = 0;
     }
     if (size != 0)
         FlushOnePageLocked(i_cache, start, size);
 }
 
-static void
-CheckICacheLocked(Simulator::ICacheMap& i_cache, SimInstruction* instr)
+void
+Simulator::checkICacheLocked(Simulator::ICacheMap& i_cache, SimInstruction* instr)
 {
     intptr_t address = reinterpret_cast<intptr_t>(instr);
     void* page = reinterpret_cast<void*>(address & (~CachePage::kPageMask));
     void* line = reinterpret_cast<void*>(address & (~CachePage::kLineMask));
     int offset = (address & CachePage::kPageMask);
     CachePage* cache_page = GetCachePageLocked(i_cache, page);
     char* cache_valid_byte = cache_page->validityByte(offset);
     bool cache_hit = (*cache_valid_byte == CachePage::LINE_VALID);
     char* cached_line = cache_page->cachedData(offset & ~CachePage::kLineMask);
+
+    // Read all state before considering signal handler effects.
+    int cmpret = 0;
     if (cache_hit) {
         // Check that the data in memory matches the contents of the I-cache.
-        MOZ_ASSERT(memcmp(reinterpret_cast<void*>(instr),
-                          cache_page->cachedData(offset),
-                          SimInstruction::kInstrSize) == 0);
+        cmpret = memcmp(reinterpret_cast<void*>(instr),
+                        cache_page->cachedData(offset),
+                        SimInstruction::kInstrSize);
+    }
+
+    // Check for signal handler interruption between reading state and asserting.
+    // It is safe for the signal to arrive during the !cache_hit path, since it
+    // will be cleared the next time this function is called.
+    if (cacheInvalidatedBySignalHandler_) {
+        i_cache.clear();
+        cacheInvalidatedBySignalHandler_ = false;
+        return;
+    }
+
+    if (cache_hit) {
+        MOZ_ASSERT(cmpret == 0);
     } else {
         // Cache miss. Load memory into the cache.
         memcpy(cached_line, line, CachePage::kLineLength);
         *cache_valid_byte = CachePage::LINE_VALID;
     }
 }
 
 HashNumber
@@ -1123,16 +1139,17 @@ Simulator::Simulator(JSContext* cx)
     pc_modified_ = false;
     icount_ = 0L;
     resume_pc_ = 0;
     break_pc_ = nullptr;
     break_instr_ = 0;
     single_stepping_ = false;
     single_step_callback_ = nullptr;
     single_step_callback_arg_ = nullptr;
+    cacheInvalidatedBySignalHandler_ = false;
     skipCalleeSavedRegsCheck = false;
 
     // Set up architecture state.
     // All registers are initialized to zero to start with.
     for (int i = 0; i < num_registers; i++)
         registers_[i] = 0;
 
     n_flag_ = false;
@@ -4642,17 +4659,17 @@ Simulator::decodeSpecialCondition(SimIns
 }
 
 // Executes the current instruction.
 void
 Simulator::instructionDecode(SimInstruction* instr)
 {
     if (Simulator::ICacheCheckingEnabled) {
         AutoLockSimulatorCache als(this);
-        CheckICacheLocked(icache(), instr);
+        checkICacheLocked(icache(), instr);
     }
 
     pc_modified_ = false;
 
     static const uint32_t kSpecialCondition = 15 << 28;
     if (instr->conditionField() == kSpecialCondition) {
         decodeSpecialCondition(instr);
     } else if (conditionallyExecute(instr)) {
--- a/js/src/jit/arm/Simulator-arm.h
+++ b/js/src/jit/arm/Simulator-arm.h
@@ -26,16 +26,18 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #ifndef jit_arm_Simulator_arm_h
 #define jit_arm_Simulator_arm_h
 
 #ifdef JS_SIMULATOR_ARM
 
+#include "mozilla/Atomics.h"
+
 #include "jit/arm/Architecture-arm.h"
 #include "jit/arm/disasm/Disasm-arm.h"
 #include "jit/IonTypes.h"
 #include "threading/Thread.h"
 #include "vm/MutexIDs.h"
 
 namespace js {
 namespace jit {
@@ -340,20 +342,40 @@ class Simulator
     void decodeVCVTBetweenFloatingPointAndIntegerFrac(SimInstruction* instr);
 
     // Support for some system functions.
     void decodeType7CoprocessorIns(SimInstruction* instr);
 
     // Executes one instruction.
     void instructionDecode(SimInstruction* instr);
 
+  private:
+    // ICache checking.
+    struct ICacheHasher {
+        typedef void* Key;
+        typedef void* Lookup;
+        static HashNumber hash(const Lookup& l);
+        static bool match(const Key& k, const Lookup& l);
+    };
+
+  public:
+    typedef HashMap<void*, CachePage*, ICacheHasher, SystemAllocPolicy> ICacheMap;
+
   public:
     static bool ICacheCheckingEnabled;
     static void FlushICache(void* start, size_t size);
 
+    // Jitcode may be rewritten from a signal handler, but is prevented from
+    // calling FlushICache() because the signal may arrive within the critical
+    // area of an AutoLockSimulatorCache. This flag instructs the Simulator
+    // to remove all cache entries the next time it checks, avoiding false negatives.
+    mozilla::Atomic<bool, mozilla::ReleaseAcquire> cacheInvalidatedBySignalHandler_;
+
+    void checkICacheLocked(ICacheMap& i_cache, SimInstruction* instr);
+
     static int64_t StopSimAt;
 
     // For testing the MoveResolver code, a MoveResolver is set up, and
     // the VFP registers are loaded with pre-determined values,
     // then the sequence of code is simulated.  In order to test this with the
     // simulator, the callee-saved registers can't be trashed. This flag
     // disables that feature.
     bool skipCalleeSavedRegsCheck;
@@ -446,28 +468,16 @@ class Simulator
     StopCountAndDesc watched_stops_[kNumOfWatchedStops];
 
   public:
     int64_t icount() {
         return icount_;
     }
 
   private:
-    // ICache checking.
-    struct ICacheHasher {
-        typedef void* Key;
-        typedef void* Lookup;
-        static HashNumber hash(const Lookup& l);
-        static bool match(const Key& k, const Lookup& l);
-    };
-
-  public:
-    typedef HashMap<void*, CachePage*, ICacheHasher, SystemAllocPolicy> ICacheMap;
-
-  private:
     // This lock creates a critical section around 'redirection_' and
     // 'icache_', which are referenced both by the execution engine
     // and by the off-thread compiler (see Redirection::Get in the cpp file).
     Mutex cacheLock_;
 #ifdef DEBUG
     mozilla::Maybe<Thread::Id> cacheLockHolder_;
 #endif
 
--- a/js/src/jit/mips32/Simulator-mips32.cpp
+++ b/js/src/jit/mips32/Simulator-mips32.cpp
@@ -1188,32 +1188,48 @@ FlushICacheLocked(Simulator::ICacheMap& 
         MOZ_ASSERT((start & CachePage::kPageMask) == 0);
         offset = 0;
     }
     if (size != 0) {
         FlushOnePageLocked(i_cache, start, size);
     }
 }
 
-static void
-CheckICacheLocked(Simulator::ICacheMap& i_cache, SimInstruction* instr)
+void
+Simulator::checkICacheLocked(Simulator::ICacheMap& i_cache, SimInstruction* instr)
 {
     intptr_t address = reinterpret_cast<intptr_t>(instr);
     void* page = reinterpret_cast<void*>(address & (~CachePage::kPageMask));
     void* line = reinterpret_cast<void*>(address & (~CachePage::kLineMask));
     int offset = (address & CachePage::kPageMask);
     CachePage* cache_page = GetCachePageLocked(i_cache, page);
     char* cache_valid_byte = cache_page->validityByte(offset);
     bool cache_hit = (*cache_valid_byte == CachePage::LINE_VALID);
     char* cached_line = cache_page->cachedData(offset & ~CachePage::kLineMask);
+
+    // Read all state before considering signal handler effects.
+    int cmpret = 0;
     if (cache_hit) {
         // Check that the data in memory matches the contents of the I-cache.
-        MOZ_ASSERT(memcmp(reinterpret_cast<void*>(instr),
-                          cache_page->cachedData(offset),
-                          SimInstruction::kInstrSize) == 0);
+        cmpret = memcmp(reinterpret_cast<void*>(instr),
+                        cache_page->cachedData(offset),
+                        SimInstruction::kInstrSize);
+    }
+
+    // Check for signal handler interruption between reading state and asserting.
+    // It is safe for the signal to arrive during the !cache_hit path, since it
+    // will be cleared the next time this function is called.
+    if (cacheInvalidatedBySignalHandler_) {
+        i_cache.clear();
+        cacheInvalidatedBySignalHandler_ = false;
+        return;
+    }
+
+    if (cache_hit) {
+        MOZ_ASSERT(cmpret == 0);
     } else {
         // Cache miss.  Load memory into the cache.
         memcpy(cached_line, line, CachePage::kLineLength);
         *cache_valid_byte = CachePage::LINE_VALID;
     }
 }
 
 HashNumber
@@ -1236,17 +1252,18 @@ Simulator::FlushICache(void* start_addr,
     if (Simulator::ICacheCheckingEnabled) {
         Simulator* sim = Simulator::Current();
         AutoLockSimulatorCache als(sim);
         js::jit::FlushICacheLocked(sim->icache(), start_addr, size);
     }
 }
 
 Simulator::Simulator()
-  : cacheLock_(mutexid::SimulatorCacheLock)
+  : cacheLock_(mutexid::SimulatorCacheLock),
+    cacheInvalidatedBySignalHandler_(false)
 {
     // Set up simulator support first. Some of this information is needed to
     // setup the architecture state.
 
     // Note, allocation and anything that depends on allocated memory is
     // deferred until init(), in order to handle OOM properly.
 
     stack_ = nullptr;
@@ -3295,17 +3312,17 @@ Simulator::decodeTypeJump(SimInstruction
 }
 
 // Executes the current instruction.
 void
 Simulator::instructionDecode(SimInstruction* instr)
 {
     if (Simulator::ICacheCheckingEnabled) {
         AutoLockSimulatorCache als(this);
-        CheckICacheLocked(icache(), instr);
+        checkICacheLocked(icache(), instr);
     }
     pc_modified_ = false;
 
     switch (instr->instructionType()) {
       case SimInstruction::kRegisterType:
         decodeTypeRegister(instr);
         break;
       case SimInstruction::kImmediateType:
--- a/js/src/jit/mips32/Simulator-mips32.h
+++ b/js/src/jit/mips32/Simulator-mips32.h
@@ -26,16 +26,18 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #ifndef jit_mips32_Simulator_mips32_h
 #define jit_mips32_Simulator_mips32_h
 
 #ifdef JS_SIMULATOR_MIPS32
 
+#include "mozilla/Atomics.h"
+
 #include "jit/IonTypes.h"
 #include "threading/Thread.h"
 #include "vm/MutexIDs.h"
 
 namespace js {
 namespace jit {
 
 class Simulator;
@@ -383,16 +385,25 @@ class Simulator {
     Mutex cacheLock_;
 #ifdef DEBUG
     mozilla::Maybe<Thread::Id> cacheLockHolder_;
 #endif
 
     Redirection* redirection_;
     ICacheMap icache_;
 
+  private:
+    // Jitcode may be rewritten from a signal handler, but is prevented from
+    // calling FlushICache() because the signal may arrive within the critical
+    // area of an AutoLockSimulatorCache. This flag instructs the Simulator
+    // to remove all cache entries the next time it checks, avoiding false negatives.
+    mozilla::Atomic<bool, mozilla::ReleaseAcquire> cacheInvalidatedBySignalHandler_;
+
+    void checkICacheLocked(ICacheMap& i_cache, SimInstruction* instr);
+
   public:
     ICacheMap& icache() {
         // Technically we need the lock to access the innards of the
         // icache, not to take its address, but the latter condition
         // serves as a useful complement to the former.
         MOZ_ASSERT(cacheLockHolder_.isSome());
         return icache_;
     }
--- a/js/src/jit/mips64/Simulator-mips64.cpp
+++ b/js/src/jit/mips64/Simulator-mips64.cpp
@@ -1202,32 +1202,48 @@ FlushICacheLocked(Simulator::ICacheMap& 
         size -= bytes_to_flush;
         MOZ_ASSERT((start & CachePage::kPageMask) == 0);
         offset = 0;
     }
     if (size != 0)
         FlushOnePageLocked(i_cache, start, size);
 }
 
-static void
-CheckICacheLocked(Simulator::ICacheMap& i_cache, SimInstruction* instr)
+void
+Simulator::checkICacheLocked(Simulator::ICacheMap& i_cache, SimInstruction* instr)
 {
     intptr_t address = reinterpret_cast<intptr_t>(instr);
     void* page = reinterpret_cast<void*>(address & (~CachePage::kPageMask));
     void* line = reinterpret_cast<void*>(address & (~CachePage::kLineMask));
     int offset = (address & CachePage::kPageMask);
     CachePage* cache_page = GetCachePageLocked(i_cache, page);
     char* cache_valid_byte = cache_page->validityByte(offset);
     bool cache_hit = (*cache_valid_byte == CachePage::LINE_VALID);
     char* cached_line = cache_page->cachedData(offset & ~CachePage::kLineMask);
+
+    // Read all state before considering signal handler effects.
+    int cmpret = 0;
     if (cache_hit) {
         // Check that the data in memory matches the contents of the I-cache.
-        MOZ_ASSERT(memcmp(reinterpret_cast<void*>(instr),
-                          cache_page->cachedData(offset),
-                          SimInstruction::kInstrSize) == 0);
+        cmpret = memcmp(reinterpret_cast<void*>(instr),
+                        cache_page->cachedData(offset),
+                        SimInstruction::kInstrSize);
+    }
+
+    // Check for signal handler interruption between reading state and asserting.
+    // It is safe for the signal to arrive during the !cache_hit path, since it
+    // will be cleared the next time this function is called.
+    if (cacheInvalidatedBySignalHandler_) {
+        i_cache.clear();
+        cacheInvalidatedBySignalHandler_ = false;
+        return;
+    }
+
+    if (cache_hit) {
+        MOZ_ASSERT(cmpret == 0);
     } else {
         // Cache miss.  Load memory into the cache.
         memcpy(cached_line, line, CachePage::kLineLength);
         *cache_valid_byte = CachePage::LINE_VALID;
     }
 }
 
 HashNumber
@@ -1250,17 +1266,18 @@ Simulator::FlushICache(void* start_addr,
     if (Simulator::ICacheCheckingEnabled) {
         Simulator* sim = Simulator::Current();
         AutoLockSimulatorCache als(sim);
         js::jit::FlushICacheLocked(sim->icache(), start_addr, size);
     }
 }
 
 Simulator::Simulator()
-  : cacheLock_(mutexid::SimulatorCacheLock)
+  : cacheLock_(mutexid::SimulatorCacheLock),
+    cacheInvalidatedBySignalHandler_(false)
 {
     // Set up simulator support first. Some of this information is needed to
     // setup the architecture state.
 
     // Note, allocation and anything that depends on allocated memory is
     // deferred until init(), in order to handle OOM properly.
 
     stack_ = nullptr;
@@ -3619,17 +3636,17 @@ Simulator::decodeTypeJump(SimInstruction
 }
 
 // Executes the current instruction.
 void
 Simulator::instructionDecode(SimInstruction* instr)
 {
     if (Simulator::ICacheCheckingEnabled) {
         AutoLockSimulatorCache als(this);
-        CheckICacheLocked(icache(), instr);
+        checkICacheLocked(icache(), instr);
     }
     pc_modified_ = false;
 
     switch (instr->instructionType()) {
       case SimInstruction::kRegisterType:
         decodeTypeRegister(instr);
         break;
       case SimInstruction::kImmediateType:
--- a/js/src/jit/mips64/Simulator-mips64.h
+++ b/js/src/jit/mips64/Simulator-mips64.h
@@ -27,16 +27,18 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #ifndef jit_mips64_Simulator_mips64_h
 #define jit_mips64_Simulator_mips64_h
 
 #ifdef JS_SIMULATOR_MIPS64
 
+#include "mozilla/Atomics.h"
+
 #include "jit/IonTypes.h"
 #include "threading/Thread.h"
 #include "vm/MutexIDs.h"
 
 namespace js {
 namespace jit {
 
 class Simulator;
@@ -399,16 +401,25 @@ class Simulator {
     Mutex cacheLock_;
 #ifdef DEBUG
     mozilla::Maybe<Thread::Id> cacheLockHolder_;
 #endif
 
     Redirection* redirection_;
     ICacheMap icache_;
 
+  private:
+    // Jitcode may be rewritten from a signal handler, but is prevented from
+    // calling FlushICache() because the signal may arrive within the critical
+    // area of an AutoLockSimulatorCache. This flag instructs the Simulator
+    // to remove all cache entries the next time it checks, avoiding false negatives.
+    mozilla::Atomic<bool, mozilla::ReleaseAcquire> cacheInvalidatedBySignalHandler_;
+
+    void checkICacheLocked(ICacheMap& i_cache, SimInstruction* instr);
+
   public:
     ICacheMap& icache() {
         // Technically we need the lock to access the innards of the
         // icache, not to take its address, but the latter condition
         // serves as a useful complement to the former.
         MOZ_ASSERT(cacheLockHolder_.isSome());
         return icache_;
     }
--- a/js/src/wasm/WasmSignalHandlers.cpp
+++ b/js/src/wasm/WasmSignalHandlers.cpp
@@ -1268,17 +1268,28 @@ RedirectJitCodeToInterruptCheck(JSRuntim
 // SIGVTALRM a relative of SIGALRM, so intended for user code, but, unlike
 // SIGALRM, not used anywhere else in Mozilla.
 static const int sInterruptSignal = SIGVTALRM;
 
 static void
 JitInterruptHandler(int signum, siginfo_t* info, void* context)
 {
     if (JSRuntime* rt = RuntimeForCurrentThread()) {
+
+#if defined(JS_SIMULATOR_ARM) || defined(JS_SIMULATOR_MIPS32) || defined(JS_SIMULATOR_MIPS64)
+        bool prevICacheCheckingState = Simulator::ICacheCheckingEnabled;
+        Simulator::ICacheCheckingEnabled = false;
+#endif
+
         RedirectJitCodeToInterruptCheck(rt, (CONTEXT*)context);
+
+#if defined(JS_SIMULATOR_ARM) || defined(JS_SIMULATOR_MIPS32) || defined(JS_SIMULATOR_MIPS64)
+        Simulator::ICacheCheckingEnabled = prevICacheCheckingState;
+#endif // JS_SIMULATOR
+
         rt->finishHandlingJitInterrupt();
     }
 }
 #endif
 
 static bool sTriedInstallSignalHandlers = false;
 static bool sHaveSignalHandlers = false;