i#2039 trace trim, part 6: Enable for AArchXX (#5770)

Since the stability issues with drbbdup on larger apps seem limited to
x86, we enable -align_endpoints by default for AArchXX.

This revealed a drbbdup issue on AArch64:
OP_isb must end a bb.  Added this to drbbdup's list of special
opcodes, and to the bb event docs where it was missing.
This fixes drcacheoff.gencode test failures.

Adds more synch to the burst_replaceall test to ensure all
threads are scheduled to avoid empty threads which mess up the
expected output with -align_endpoints.

Relaxes the burst_threadfilter template to allow empty unscheduled
threads.  Relaxes the burst_replace template to allow lazy file opening.

Issue: #2039

clients/drcachesim/CMakeLists.txt

@@ -41,6 +41,25 @@ else ()
   set(os_name "unix")
   # Ditto.
   add_definitions(-DUNIX)
+  if (LINUX)
+    add_definitions(-DLINUX)
+  elseif (APPLE)
+    add_definitions(-DMACOS)
+  endif ()
+endif ()
+# Ditto here, in particular for options.cpp.
+if (X86)
+  if (X64)
+    add_definitions(-DX86_64)
+  else ()
+    add_definitions(-DX86_32)
+  endif ()
+elseif (AARCHXX)
+  if (X64)
+    add_definitions(-DARM_64)
+  else ()
+    add_definitions(-DARM_32)
+  endif ()
 endif ()
 
 # GCC 6+ has a warning for an ABI change due to a bug introduced in GCC 5:
@@ -844,6 +863,7 @@ add_executable(drcachesim_ops
 set_target_properties(drcachesim_ops PROPERTIES COMPILE_FLAGS "${ORIG_CMAKE_CXX_FLAGS}")
 add_win32_flags(drcachesim_ops)
 use_DynamoRIO_extension(drcachesim_ops droption)
+add_dependencies(drcachesim_ops api_headers)
 
 # We then have to insert it into the doxygen files at build time:
 set(srcdoc ${CMAKE_CURRENT_SOURCE_DIR}/docs/drcachesim.dox.in)

clients/drcachesim/common/options.cpp

@@ -33,6 +33,7 @@
 /* shared options for both the frontend and the client */
 
 #include <string>
+#include "dr_api.h" // For IF_X86_ELSE.
 #include "droption.h"
 #include "options.h"
 
@@ -288,8 +289,8 @@ droption_t<bytesize_t> op_max_global_trace_refs(
 droption_t<bool> op_align_endpoints(
     // XXX i#2039,i#5686: Make this true by default (and maybe remove it altogether) once
     // robustness issues with drbbdup are fixed (restore state for scatter/gather and
-    // other libs; yet-undiagnosed other state restore issues).
-    DROPTION_SCOPE_CLIENT, "align_endpoints", false,
+    // other libs; yet-undiagnosed other state restore issues) on x86.
+    DROPTION_SCOPE_CLIENT, "align_endpoints", IF_X86_ELSE(false, true),
     "Nop tracing when partially attached or detached",
     "When using attach/detach to trace a burst, the attach and detach processes are "
     "staggered, with the set of threads producing trace data incrementally growing or "

clients/drcachesim/tests/burst_replaceall.cpp

@@ -62,6 +62,8 @@
 static void *finished;
 static constexpr int num_threads = 3;
 static void *started[num_threads];
+static void *attached;
+static void *post_attach[num_threads];
 #endif
 
 bool
@@ -252,6 +254,8 @@ void *
 {
     uintptr_t i = reinterpret_cast<uintptr_t>(arg);
     signal_cond_var(started[i]);
+    wait_cond_var(attached);
+    signal_cond_var(post_attach[i]);
     wait_cond_var(finished);
     return 0;
 }
@@ -273,8 +277,10 @@ main(int argc, const char *argv[])
     uintptr_t thread[num_threads];
 #    endif
     finished = create_cond_var();
+    attached = create_cond_var();
     for (uint i = 0; i < num_threads; i++) {
         started[i] = create_cond_var();
+        post_attach[i] = create_cond_var();
 #    ifdef UNIX
         pthread_create(&thread[i], NULL, thread_func, (void *)(uintptr_t)i);
 #    else
@@ -308,6 +314,13 @@ main(int argc, const char *argv[])
         if (i == iter_start) {
             std::cerr << "pre-DR start\n";
             dr_app_start();
+#ifndef WINDOWS // XXX i#2040: Disabled until Windows static limits are fixed.
+            // Ensure our threads are actually scheduled during our burst window to avoid
+            // missing threads from -align_endpoints.
+            signal_cond_var(attached);
+            for (uint i = 0; i < num_threads; i++)
+                wait_cond_var(post_attach[i]);
+#endif
         }
         if (i >= iter_start && i <= iter_stop)
             assert(dr_app_running_under_dynamorio());

clients/drcachesim/tests/offline-burst_replace.templatex

@@ -6,8 +6,7 @@ create dir .*
 open file .*
 open file .*
 open file .*
-open file .*
-pre-DR start
+pre-DR start.*
 0: writing [0-9]+ bytes .*
 1: writing [0-9]+ bytes .*
 restore the write file function

clients/drcachesim/tests/offline-burst_threadfilter.templatex

@@ -21,7 +21,7 @@ Total counts:
     *[0-9]* total data stores
     *[0-9]* total icache flushes
     *[0-9]* total dcache flushes
-           4 total threads
+           [1-4] total threads
     *[0-9]* total scheduling markers
     *[0-9]* total transfer markers
            0 total function id markers

core/lib/dr_events.h

@@ -248,6 +248,8 @@ DR_API
  * - There can only be one far branch (call, jump, or return) in
  * a basic block, and it must be the final instruction in the
  * block.
+ * - The AArch64 instruction ISB must be the final instruction in the
+ * block.
  * - The exit control-flow of a block ending in a system call or
  * int instruction cannot be changed, nor can instructions be inserted
  * after the system call or int instruction itself, unless

ext/drbbdup/drbbdup.c

@@ -232,7 +232,7 @@ drbbdup_is_special_instr(instr_t *instr)
 {
     return instr != NULL &&
         (instr_is_syscall(instr) || instr_is_cti(instr) || instr_is_ubr(instr) ||
-         instr_is_interrupt(instr));
+         instr_is_interrupt(instr) IF_AARCH64(|| instr_get_opcode(instr) == OP_isb));
 }
 
 /****************************************************************************