i#4014 dr$sim phys: Make physaddr_t per-thread

The physaddr_t class is not thread-safe and was previously used racily
in the drmemtrace code.  We fix that by creating a separate instance
per thread.  A test with multiple threads is added.

Issue: #4014

clients/drcachesim/tests/phys-threads.templatex

@@ -0,0 +1,56 @@
+
+    -------------------------------------------------------------------
+     Performance for solving AX=B Linear Equation using Jacobi method
+     Running on DynamoRIO
+     Client version .*
+    ...................................................................
+
+     Matrix Size :  64
+     Threads     :  4
+
+
+     Started iteration 1 of the computation...
+
+     Finished computing current solution distance in mode 0.
+     Mode changed to 0.
+
+     Started iteration 2 of the computation...
+
+     Finished computing current solution distance in mode 0.
+     Mode changed to 0.
+
+     Started iteration 3 of the computation...
+
+     Finished computing current solution distance in mode 0.
+     Mode changed to 0.
+
+
+     The Jacobi Method For AX=B .........DONE
+     Total Number Of iterations   :  3
+    ...................................................................
+---- <application exited with code 0> ----
+Cache simulation results:
+Core #0 \(4 thread\(s\)\)
+  L1I stats:
+    Hits:                       *[0-9,\.]*
+    Misses:                     *[0-9,\.]*
+    Compulsory misses:          *[0-9,\.]*
+    Invalidations:              *0
+.*    Miss rate:                *[0-9,\.]*%
+  L1D stats:
+    Hits:                       *[0-9,\.]*
+    Misses:                     *[0-9,\.]*
+    Compulsory misses:          *[0-9,\.]*
+    Invalidations:              *0
+.*    Miss rate:                *[0-9,\.]*%
+Core #1 \(3 thread\(s\).*
+Core #2 \(3 thread\(s\).*
+Core #3 \(3 thread\(s\).*
+LL stats:
+    Hits:                    *[0-9,\.]*
+    Misses:                  *[0-9,\.]*
+    Compulsory misses:       *[0-9,\.]*
+    Invalidations:           *0
+.*    Local miss rate:        *[0-9,.]*%
+    Child hits:              *[0-9,\.]*
+    Total miss rate:                  0[\.,]..%

clients/drcachesim/tracer/physaddr.h

@@ -42,6 +42,8 @@
 #include "hashtable.h"
 #include "../common/trace_entry.h"
 
+// This class is not thread-safe: the caller should create a separate instance
+// per thread.
 class physaddr_t {
 public:
     physaddr_t();
@@ -52,10 +54,11 @@ class physaddr_t {
     virtual2physical(addr_t virt);
 
 private:
-    // Assumed to be single-threaded
 #ifdef LINUX
     addr_t last_vpage_;
     addr_t last_ppage_;
+    // XXX: An app with thousands of threads might hit open file limits.
+    // Sharing the descriptor would require locks, however.
     int fd_;
     // We would use std::unordered_map, but that is not compatible with
     // statically linking drmemtrace into an app.

clients/drcachesim/tracer/tracer.cpp

@@ -161,6 +161,8 @@ typedef struct {
     byte *buf_lz4;
 #endif
     bool has_thread_header;
+    // The physaddr_t class is designed to be per-thread.
+    physaddr_t physaddr;
 } per_thread_t;
 
 #define MAX_NUM_DELAY_INSTRS 32
@@ -193,8 +195,7 @@ static uint64 num_refs_racy; /* racy global memory reference count */
 static volatile bool exited_process;
 
 /* virtual to physical translation */
-static bool have_phys;
-static physaddr_t physaddr;
+static std::atomic<bool> have_phys;
 
 static drmgr_priority_t pri_pre_bbdup = { sizeof(drmgr_priority_t),
                                           DRMGR_PRIORITY_NAME_MEMTRACE, NULL, NULL,
@@ -974,7 +975,7 @@ memtrace(void *drcontext, bool skip_size_cap)
                     type != TRACE_TYPE_THREAD && type != TRACE_TYPE_THREAD_EXIT &&
                     type != TRACE_TYPE_PID) {
                     addr_t virt = instru->get_entry_addr(mem_ref);
-                    addr_t phys = physaddr.virtual2physical(virt);
+                    addr_t phys = data->physaddr.virtual2physical(virt);
                     DR_ASSERT(type != TRACE_TYPE_INSTR_BUNDLE);
                     if (phys != 0)
                         instru->set_entry_addr(mem_ref, phys);
@@ -2608,6 +2609,14 @@ init_thread_in_process(void *drcontext)
     }
 #endif
 
+    if (op_use_physical.get_value()) {
+        if (!data->physaddr.init()) {
+            have_phys = false;
+            NOTIFY(0, "Unable to open pagemap: using virtual addresses for thread T%d.\n",
+                   dr_get_thread_id(drcontext));
+        }
+    }
+
     set_local_window(drcontext, -1);
     if (has_tracing_windows())
         set_local_window(drcontext, tracing_window.load(std::memory_order_acquire));
@@ -2662,7 +2671,7 @@ event_thread_init(void *drcontext)
 {
     per_thread_t *data = (per_thread_t *)dr_thread_alloc(drcontext, sizeof(per_thread_t));
     DR_ASSERT(data != NULL);
-    memset(data, 0, sizeof(*data));
+    *data = {};
     data->file = INVALID_FILE;
     drmgr_set_tls_field(drcontext, tls_idx, data);
 
@@ -2768,6 +2777,7 @@ event_thread_exit(void *drcontext)
         if (data->reserve_buf != NULL)
             dr_raw_mem_free(data->reserve_buf, max_buf_size);
     }
+    data->~per_thread_t();
     dr_thread_free(drcontext, data, sizeof(per_thread_t));
 }
 
@@ -2990,6 +3000,11 @@ drmemtrace_client_main(client_id_t id, int argc, const char *argv[])
     }
 #endif
 
+    if (op_use_physical.get_value()) {
+        // This will be set to false if any thread fails.
+        have_phys = true;
+    }
+
     if (op_offline.get_value()) {
         void *placement;
         if (!init_offline_dir()) {
@@ -3112,11 +3127,6 @@ drmemtrace_client_main(client_id_t id, int argc, const char *argv[])
     /* make it easy to tell, by looking at log file, which client executed */
     dr_log(NULL, DR_LOG_ALL, 1, "drcachesim client initializing\n");
 
-    if (op_use_physical.get_value()) {
-        have_phys = physaddr.init();
-        if (!have_phys)
-            NOTIFY(0, "Unable to open pagemap: using virtual addresses.\n");
-    }
 #ifdef HAS_SNAPPY
     if (op_offline.get_value() && snappy_enabled()) {
         /* Unfortunately libsnappy allocates memory but does not parameterize its

suite/tests/CMakeLists.txt

@@ -3323,8 +3323,15 @@ if (BUILD_CLIENTS)
       "-config_file ${config_files_dir}/cores-1-levels-3-with-missfile.conf"
       "")
 
-    if (NOT WIN32) # No physaddr access on Windows.
+    if (LINUX) # Physaddr access limited to Linux.
+      # XXX: We should verify that we're actually getting physical addresses:
+      # but A) there may not be pagemap access for automated tests and B) we
+      # would need an analyzer to go examine addresses, or to use the view tool.
+      # For now these are just sanity tests that flipping on the option doesn't
+      # cause outright failure.
       torunonly_drcachesim(phys ${ci_shared_app} "-use_physical" "")
+      torunonly_drcachesim(phys-threads client.annotation-concurrency "-use_physical"
+        "${annotation_test_args_shorter}")
     endif ()
 
     set(test_mode_flag "-test_mode")