From b9bec986a40c462c1d57184b32e269bc9cb4ae66 Mon Sep 17 00:00:00 2001
From: Derek Bruening <bruening@google.com>
Date: Wed, 25 May 2022 04:53:56 +0000
Subject: [PATCH] i#5495: Fix drcachesim fork bug (#5500)

Fixes a bug where drcachesim -offline complained about an open file
across a fork.  This bug blocked tracing of SPECCPU perlbench.

Adds a new test of drcachesim -offline with an app that forks.  As is,
this matched the -satisfy_w_xor_x test regex, and I tried to make it
work with that option by fixing a file close bug here.  However, I did
not have time to figure out another bug which I filed as #5499.  I
thus tightened the "fork" regex to exclude this test from
-satisfy_w_xor_x.  I also had to disable the malloc check and issue a
warning for static link offline across fork due to the unsolved #4660.

Issue: #5495, #5499, #4660
Fixes #5495
---
 .../drcachesim/tests/offline-fork.templatex   | 29 +++++++++++++++++++
 clients/drcachesim/tracer/tracer.cpp          | 10 +++++++
 core/heap.c                                   |  2 +-
 suite/tests/CMakeLists.txt                    |  7 ++++-
 4 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 clients/drcachesim/tests/offline-fork.templatex

diff --git a/clients/drcachesim/tests/offline-fork.templatex b/clients/drcachesim/tests/offline-fork.templatex
new file mode 100644
index 00000000000..d4b57c2cf73
--- /dev/null
+++ b/clients/drcachesim/tests/offline-fork.templatex
@@ -0,0 +1,29 @@
+parent is running under DynamoRIO
+parent waiting for child
+child is running under DynamoRIO
+child has exited
+Cache simulation results:
+Core #0 \(1 thread\(s\)\)
+  L1I stats:
+    Hits:                         *[0-9,\.]*
+    Misses:                       *[0-9,\.]*
+    Compulsory misses:            *[0-9,\.]*
+    Invalidations:                *0
+.*    Miss rate:                        [0-3][,\.]..%
+  L1D stats:
+    Hits:                         *[0-9,\.]*
+    Misses:                       *[0-9,\.]*
+    Compulsory misses:            *[0-9,\.]*
+    Invalidations:                *0
+.*   Miss rate:                        [0-9][,\.]..%
+Core #1 \(0 thread\(s\)\)
+Core #2 \(0 thread\(s\)\)
+Core #3 \(0 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-4][,\.]..%
diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp
index c77e0ed6da6..393136571d9 100644
--- a/clients/drcachesim/tracer/tracer.cpp
+++ b/clients/drcachesim/tracer/tracer.cpp
@@ -2897,6 +2897,15 @@ init_offline_dir(void)
 static void
 fork_init(void *drcontext)
 {
+    if (op_offline.get_value()) {
+        /* XXX i#4660: droption references at fork init time use malloc.
+         * We do not fully support static link with fork at this time.
+         */
+        dr_allow_unsafe_static_behavior();
+#    ifdef DRMEMTRACE_STATIC
+        NOTIFY(0, "-offline across a fork is unsafe with statically linked clients\n");
+#    endif
+    }
     /* We use DR_FILE_CLOSE_ON_FORK, and we dumped outstanding data prior to the
      * fork syscall, so we just need to create a new subdir, new module log, and
      * a new initial thread file for offline, or register the new process for
@@ -2908,6 +2917,7 @@ fork_init(void *drcontext)
      */
     data->num_refs = 0;
     if (op_offline.get_value()) {
+        data->file = INVALID_FILE;
         if (!init_offline_dir()) {
             FATAL("Failed to create a subdir in %s\n", op_outdir.get_value().c_str());
         }
diff --git a/core/heap.c b/core/heap.c
index 065f071e649..c6fa65eb7c2 100644
--- a/core/heap.c
+++ b/core/heap.c
@@ -2172,7 +2172,7 @@ vmm_heap_fork_init(dcontext_t *dcontext)
      * os_delete_memory_file(). This may not work on Windows if that function needs to do
      * more.
      */
-    os_close(old_fd);
+    os_close_protected(old_fd);
     return;
 
 vmm_heap_fork_init_failed:
diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt
index d8f2e3e5c62..a386b584515 100644
--- a/suite/tests/CMakeLists.txt
+++ b/suite/tests/CMakeLists.txt
@@ -113,7 +113,7 @@ set(main_run_list
   "SHORT::X86::ONLY::client.events$::-code_api -thread_private -disable_traces"
   "SHORT::X86::LIN::ONLY::client.events$::-code_api -no_early_inject" # only early on ARM
   # XXX i#3556: NYI on Windows and Mac (and not supported on 32-bit).
-  "SHORT::X64::LIN::ONLY::drcache.*\\.simple$|selfmod2|racesys|reachability|fork|file_io|loglevel$::-code_api -satisfy_w_xor_x"
+  "SHORT::X64::LIN::ONLY::drcache.*\\.simple$|selfmod2|racesys|reachability|linux.*fork|file_io|loglevel$::-code_api -satisfy_w_xor_x"
   # maybe this should be SHORT as -coarse_units will eventually be the default?
   "X86::-code_api -opt_memory"       # i#1575: ARM -coarse_units NYI
   "X86::-code_api -opt_speed"        # i#1551: ARM indcall2direct NYI
@@ -3564,6 +3564,11 @@ if (BUILD_CLIENTS)
     # and print out the "---- <application exited with code 0> ----".
     torunonly_drcacheoff(simple ${ci_shared_app} "" "" "")
 
+    if (UNIX)
+      # Test an app with a fork.
+      torunonly_drcacheoff(fork linux.fork "" "" "")
+    endif ()
+
     # Test reading a legacy pre-interleaved file.
     if (ZLIB_FOUND)
       torunonly_api(tool.drcacheoff.legacy "${drcachesim_path}" "offline-legacy.c" ""