From e9177bc99ec04526076341242c28da5c18176395 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Wed, 11 Dec 2024 12:22:05 -0500
Subject: [PATCH 01/11] i#7113 instr decode cache: move module read into
 raw2trace_shared

This is in preparation for further changes that include the module mapper logic in a new library (drmemtrace_instr_decode_cache) that provides an instr decode cache (instr_decode_cache_t). To avoid having to pull in the whole drmemtrace_raw2trace implementation into drmemtrace_instr_decode_cache, which will end up including it in the invariant checker and everything that depends on, we separate out the module mapper into raw2trace_shared which is intended for such cases.

Issue: #7113
---
 clients/drcachesim/tools/opcode_mix.cpp       | 15 ++++++----
 clients/drcachesim/tools/view.cpp             | 28 +++++++++----------
 .../drcachesim/tracer/raw2trace_directory.cpp | 15 ++--------
 .../drcachesim/tracer/raw2trace_shared.cpp    | 25 +++++++++++++++++
 clients/drcachesim/tracer/raw2trace_shared.h  | 12 ++++++++
 5 files changed, 63 insertions(+), 32 deletions(-)

diff --git a/clients/drcachesim/tools/opcode_mix.cpp b/clients/drcachesim/tools/opcode_mix.cpp
index 07c712e09d0..c4ebad04dfd 100644
--- a/clients/drcachesim/tools/opcode_mix.cpp
+++ b/clients/drcachesim/tools/opcode_mix.cpp
@@ -92,13 +92,18 @@ opcode_mix_t::initialize()
         return "";
     // Legacy trace support where binaries are needed.
     // We do not support non-module code for such traces.
-    std::string error = directory_.initialize_module_file(module_file_path_);
-    if (!error.empty())
-        return "Failed to initialize directory: " + error;
+    file_t modfile;
+    char *modfile_bytes;
+    std::string error = read_module_file_bytes(module_file_path_, modfile, modfile_bytes);
+    if (!error.empty()) {
+        return "Failed to read module file: " + error;
+    }
     module_mapper_ =
-        module_mapper_t::create(directory_.modfile_bytes_, nullptr, nullptr, nullptr,
-                                nullptr, knob_verbose_, knob_alt_module_dir_);
+        module_mapper_t::create(modfile_bytes, nullptr, nullptr, nullptr, nullptr,
+                                knob_verbose_, knob_alt_module_dir_);
     module_mapper_->get_loaded_modules();
+    delete[] modfile_bytes;
+    dr_close_file(modfile);
     error = module_mapper_->get_last_error();
     if (!error.empty())
         return "Failed to load binaries: " + error;
diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index 4728c5629d9..da224969823 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -124,24 +124,22 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
     if (module_file_path_.empty()) {
         has_modules_ = false;
     } else {
-        std::string error = directory_.initialize_module_file(module_file_path_);
+        file_t modfile;
+        char *modfile_bytes;
+        std::string error =
+            read_module_file_bytes(module_file_path_, modfile, modfile_bytes);
+        if (error.empty()) {
+            module_mapper_ =
+                module_mapper_t::create(modfile_bytes, nullptr, nullptr, nullptr, nullptr,
+                                        knob_verbose_, knob_alt_module_dir_);
+            module_mapper_->get_loaded_modules();
+            delete[] modfile_bytes;
+            dr_close_file(modfile);
+            error = module_mapper_->get_last_error();
+        }
         if (!error.empty())
             has_modules_ = false;
     }
-    if (!has_modules_) {
-        // Continue but omit disassembly to support cases where binaries are
-        // not available and OFFLINE_FILE_TYPE_ENCODINGS is not present.
-        return "";
-    }
-    // Legacy trace support where binaries are needed.
-    // We do not support non-module code for such traces.
-    module_mapper_ =
-        module_mapper_t::create(directory_.modfile_bytes_, nullptr, nullptr, nullptr,
-                                nullptr, knob_verbose_, knob_alt_module_dir_);
-    module_mapper_->get_loaded_modules();
-    std::string error = module_mapper_->get_last_error();
-    if (!error.empty())
-        return "Failed to load binaries: " + error;
     return "";
 }
 
diff --git a/clients/drcachesim/tracer/raw2trace_directory.cpp b/clients/drcachesim/tracer/raw2trace_directory.cpp
index bddb8c7fa82..007b72317a7 100644
--- a/clients/drcachesim/tracer/raw2trace_directory.cpp
+++ b/clients/drcachesim/tracer/raw2trace_directory.cpp
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2017-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2017-2024 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -55,6 +55,7 @@
 #include "directory_iterator.h"
 #include "dr_api.h"              // Must be after windows.h.
 #include "raw2trace_directory.h" // Includes dr_api.h which must be after windows.h.
+#include "raw2trace_shared.h"
 #include "reader.h"
 #include "raw2trace.h"
 #include "record_file_reader.h"
@@ -435,17 +436,7 @@ raw2trace_directory_t::open_cpu_schedule_file()
 std::string
 raw2trace_directory_t::read_module_file(const std::string &modfilename)
 {
-    modfile_ = dr_open_file(modfilename.c_str(), DR_FILE_READ);
-    if (modfile_ == INVALID_FILE)
-        return "Failed to open module file " + modfilename;
-    uint64 modfile_size;
-    if (!dr_file_size(modfile_, &modfile_size))
-        return "Failed to get module file size: " + modfilename;
-    size_t modfile_size_ = (size_t)modfile_size;
-    modfile_bytes_ = new char[modfile_size_];
-    if (dr_read_file(modfile_, modfile_bytes_, modfile_size_) < (ssize_t)modfile_size_)
-        return "Didn't read whole module file " + modfilename;
-    return "";
+    return read_module_file_bytes(modfilename, modfile_, modfile_bytes_);
 }
 
 bool
diff --git a/clients/drcachesim/tracer/raw2trace_shared.cpp b/clients/drcachesim/tracer/raw2trace_shared.cpp
index ae6ffa7e3ed..9942677605d 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.cpp
+++ b/clients/drcachesim/tracer/raw2trace_shared.cpp
@@ -143,5 +143,30 @@ drmemtrace_get_timestamp_from_offline_trace(const void *trace, size_t trace_size
     return DRMEMTRACE_SUCCESS;
 }
 
+std::string
+read_module_file_bytes(const std::string &modfilename, file_t &modfile,
+                       char *&modfile_bytes)
+{
+    modfile = dr_open_file(modfilename.c_str(), DR_FILE_READ);
+    if (modfile == INVALID_FILE)
+        return "Failed to open module file " + modfilename;
+    uint64 modfile_size;
+    if (!dr_file_size(modfile, &modfile_size)) {
+        dr_close_file(modfile);
+        modfile = INVALID_FILE;
+        return "Failed to get module file size: " + modfilename;
+    }
+    size_t modfile_size_ = (size_t)modfile_size;
+    modfile_bytes = new char[modfile_size_];
+    if (dr_read_file(modfile, modfile_bytes, modfile_size_) < (ssize_t)modfile_size_) {
+        dr_close_file(modfile);
+        delete[] modfile_bytes;
+        modfile = INVALID_FILE;
+        modfile_bytes = nullptr;
+        return "Didn't read whole module file " + modfilename;
+    }
+    return "";
+}
+
 } // namespace drmemtrace
 } // namespace dynamorio
diff --git a/clients/drcachesim/tracer/raw2trace_shared.h b/clients/drcachesim/tracer/raw2trace_shared.h
index 1fa67a1e3f1..a1ee25a8a37 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.h
+++ b/clients/drcachesim/tracer/raw2trace_shared.h
@@ -154,6 +154,18 @@ class memref_counter_t : public reader_t {
     std::list<trace_entry_t> list_;
 };
 
+/**
+ * Reads the module file at the given \p modfilename path. Returns an
+ * empty string if successful, or the error string if not.
+ *
+ * If successful, the returned \p modfile must be closed using
+ * \p dr_close_file, and the returned \p modefilebytes must be freed
+ * using a delete[].
+ */
+std::string
+read_module_file_bytes(const std::string &modfilename, file_t &modfile,
+                       char *&modfile_bytes);
+
 } // namespace drmemtrace
 } // namespace dynamorio
 

From 88d3fb89275c8f0cfd7a025ca78dc05401aabe37 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Wed, 11 Dec 2024 12:27:54 -0500
Subject: [PATCH 02/11] Preserve error propagation on failure to load binaries
 in view tool

---
 clients/drcachesim/tools/opcode_mix.cpp | 2 +-
 clients/drcachesim/tools/view.cpp       | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/clients/drcachesim/tools/opcode_mix.cpp b/clients/drcachesim/tools/opcode_mix.cpp
index c4ebad04dfd..c7ffa13945d 100644
--- a/clients/drcachesim/tools/opcode_mix.cpp
+++ b/clients/drcachesim/tools/opcode_mix.cpp
@@ -56,7 +56,7 @@
 #include "dr_api.h"
 #include "memref.h"
 #include "raw2trace.h"
-#include "raw2trace_directory.h"
+#include "raw2trace_shared.h"
 #include "reader.h"
 #include "trace_entry.h"
 #include "utils.h"
diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index da224969823..fd58afb6a78 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -53,7 +53,7 @@
 #include "memref.h"
 #include "memtrace_stream.h"
 #include "raw2trace.h"
-#include "raw2trace_directory.h"
+#include "raw2trace_shared.h"
 #include "trace_entry.h"
 #include "utils.h"
 
@@ -136,9 +136,12 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
             delete[] modfile_bytes;
             dr_close_file(modfile);
             error = module_mapper_->get_last_error();
-        }
-        if (!error.empty())
+            if (!error.empty()) {
+                return "Failed to load binaries: " + error;
+            }
+        } else {
             has_modules_ = false;
+        }
     }
     return "";
 }

From 3ed2e8237415d03860b9a3f0256b5fa1f05d3182 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Thu, 12 Dec 2024 10:31:46 -0500
Subject: [PATCH 03/11] Fix cleanup and lifetime of modfile_bytes

---
 clients/drcachesim/CMakeLists.txt       |  2 +-
 clients/drcachesim/tools/opcode_mix.cpp | 10 ++++++----
 clients/drcachesim/tools/opcode_mix.h   |  7 +++----
 clients/drcachesim/tools/view.cpp       | 13 +++++++++----
 clients/drcachesim/tools/view.h         |  8 +++++---
 5 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt
index a67e5b64504..3e682ecdea8 100644
--- a/clients/drcachesim/CMakeLists.txt
+++ b/clients/drcachesim/CMakeLists.txt
@@ -1202,7 +1202,7 @@ if (BUILD_TESTS)
     add_executable(tool.drcacheoff.skip_unit_tests tests/skip_unit_tests.cpp)
     configure_DynamoRIO_standalone(tool.drcacheoff.skip_unit_tests)
     target_link_libraries(tool.drcacheoff.skip_unit_tests drmemtrace_analyzer
-      drmemtrace_view drmemtrace_raw2trace test_helpers)
+      drmemtrace_view drmemtrace_raw2trace test_helpers ${zlib_libs})
     add_win32_flags(tool.drcacheoff.skip_unit_tests)
     use_DynamoRIO_extension(tool.drcacheoff.skip_unit_tests drreg_static)
     use_DynamoRIO_extension(tool.drcacheoff.skip_unit_tests drcovlib_static)
diff --git a/clients/drcachesim/tools/opcode_mix.cpp b/clients/drcachesim/tools/opcode_mix.cpp
index c7ffa13945d..a5ebe0a6981 100644
--- a/clients/drcachesim/tools/opcode_mix.cpp
+++ b/clients/drcachesim/tools/opcode_mix.cpp
@@ -93,16 +93,15 @@ opcode_mix_t::initialize()
     // Legacy trace support where binaries are needed.
     // We do not support non-module code for such traces.
     file_t modfile;
-    char *modfile_bytes;
-    std::string error = read_module_file_bytes(module_file_path_, modfile, modfile_bytes);
+    std::string error =
+        read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
     if (!error.empty()) {
         return "Failed to read module file: " + error;
     }
     module_mapper_ =
-        module_mapper_t::create(modfile_bytes, nullptr, nullptr, nullptr, nullptr,
+        module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr, nullptr,
                                 knob_verbose_, knob_alt_module_dir_);
     module_mapper_->get_loaded_modules();
-    delete[] modfile_bytes;
     dr_close_file(modfile);
     error = module_mapper_->get_last_error();
     if (!error.empty())
@@ -115,6 +114,9 @@ opcode_mix_t::~opcode_mix_t()
     for (auto &iter : shard_map_) {
         delete iter.second;
     }
+    if (modfile_bytes_ != nullptr) {
+        delete[] modfile_bytes_;
+    }
 }
 
 bool
diff --git a/clients/drcachesim/tools/opcode_mix.h b/clients/drcachesim/tools/opcode_mix.h
index d67fd985855..e07e7997b81 100644
--- a/clients/drcachesim/tools/opcode_mix.h
+++ b/clients/drcachesim/tools/opcode_mix.h
@@ -46,7 +46,6 @@
 #include "analysis_tool.h"
 #include "memref.h"
 #include "raw2trace.h"
-#include "raw2trace_directory.h"
 #include "trace_entry.h"
 
 namespace dynamorio {
@@ -185,9 +184,9 @@ class opcode_mix_t : public analysis_tool_t {
     std::string module_file_path_;
     std::unique_ptr<module_mapper_t> module_mapper_;
     std::mutex mapper_mutex_;
-    // We reference directory.modfile_bytes throughout operation, so its lifetime
-    // must match ours.
-    raw2trace_directory_t directory_;
+    // XXX: Perhaps module_mapper_t should be made to own the cleanup of
+    // modfile_bytes_.
+    char *modfile_bytes_ = nullptr;
 
     std::unordered_map<int, shard_data_t *> shard_map_;
     // This mutex is only needed in parallel_shard_init.  In all other accesses to
diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index fd58afb6a78..74e44617ff6 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -125,15 +125,13 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
         has_modules_ = false;
     } else {
         file_t modfile;
-        char *modfile_bytes;
         std::string error =
-            read_module_file_bytes(module_file_path_, modfile, modfile_bytes);
+            read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
         if (error.empty()) {
             module_mapper_ =
-                module_mapper_t::create(modfile_bytes, nullptr, nullptr, nullptr, nullptr,
+                module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr, nullptr,
                                         knob_verbose_, knob_alt_module_dir_);
             module_mapper_->get_loaded_modules();
-            delete[] modfile_bytes;
             dr_close_file(modfile);
             error = module_mapper_->get_last_error();
             if (!error.empty()) {
@@ -663,5 +661,12 @@ view_t::print_results()
     return true;
 }
 
+view_t::~view_t()
+{
+    if (modfile_bytes_ != nullptr) {
+        delete[] modfile_bytes_;
+    }
+}
+
 } // namespace drmemtrace
 } // namespace dynamorio
diff --git a/clients/drcachesim/tools/view.h b/clients/drcachesim/tools/view.h
index 30d5f033e14..4ba6d4102ed 100644
--- a/clients/drcachesim/tools/view.h
+++ b/clients/drcachesim/tools/view.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2018-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2018-2024 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -47,7 +47,6 @@
 #include "memref.h"
 #include "memtrace_stream.h"
 #include "raw2trace.h"
-#include "raw2trace_directory.h"
 
 namespace dynamorio {
 namespace drmemtrace {
@@ -61,6 +60,7 @@ class view_t : public analysis_tool_t {
     view_t(const std::string &module_file_path, uint64_t skip_refs, uint64_t sim_refs,
            const std::string &syntax, unsigned int verbose,
            const std::string &alt_module_dir = "");
+    virtual ~view_t();
     std::string
     initialize_stream(memtrace_stream_t *serial_stream) override;
     bool
@@ -136,7 +136,9 @@ class view_t : public analysis_tool_t {
     // std::optional here.
     std::string module_file_path_;
     std::unique_ptr<module_mapper_t> module_mapper_;
-    raw2trace_directory_t directory_;
+    // XXX: Perhaps module_mapper_t should be made to own the cleanup of
+    // modfile_bytes_.
+    char *modfile_bytes_ = nullptr;
 
     unsigned int knob_verbose_;
     int trace_version_;

From 36ac48d8a1653053c38405f7bd5f130ac781eb8e Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Thu, 12 Dec 2024 10:46:00 -0500
Subject: [PATCH 04/11] Fix merge issue.

---
 clients/drcachesim/tracer/raw2trace_shared.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clients/drcachesim/tracer/raw2trace_shared.cpp b/clients/drcachesim/tracer/raw2trace_shared.cpp
index 9585e40c455..18ec44da3bc 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.cpp
+++ b/clients/drcachesim/tracer/raw2trace_shared.cpp
@@ -187,6 +187,9 @@ read_module_file_bytes(const std::string &modfilename, file_t &modfile,
         modfile = INVALID_FILE;
         modfile_bytes = nullptr;
         return "Didn't read whole module file " + modfilename;
+    }
+    return "";
+}
 
 // The output range is really a segment and not the whole module.
 app_pc

From b96dbb553febd9cf117d910ad2875d7fbfdbecbd Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Thu, 12 Dec 2024 10:49:41 -0500
Subject: [PATCH 05/11] Fix clang-format

---
 clients/drcachesim/tools/view.cpp            | 4 ++--
 clients/drcachesim/tracer/raw2trace_shared.h | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index 74e44617ff6..901276dbc8c 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -129,8 +129,8 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
             read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
         if (error.empty()) {
             module_mapper_ =
-                module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr, nullptr,
-                                        knob_verbose_, knob_alt_module_dir_);
+                module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr,
+                                        nullptr, knob_verbose_, knob_alt_module_dir_);
             module_mapper_->get_loaded_modules();
             dr_close_file(modfile);
             error = module_mapper_->get_last_error();
diff --git a/clients/drcachesim/tracer/raw2trace_shared.h b/clients/drcachesim/tracer/raw2trace_shared.h
index d66298be1ca..8f0dac3a8b2 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.h
+++ b/clients/drcachesim/tracer/raw2trace_shared.h
@@ -171,7 +171,6 @@ std::string
 read_module_file_bytes(const std::string &modfilename, file_t &modfile,
                        char *&modfile_bytes);
 
-
 struct module_t {
     module_t(const char *path, app_pc orig, byte *map, size_t offs, size_t size,
              size_t total_size, bool external = false)

From 2d91616680fd724fab7dc34669169e8bd0273339 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Thu, 12 Dec 2024 11:19:52 -0500
Subject: [PATCH 06/11] Revive lost comments.

---
 clients/drcachesim/tools/view.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index 901276dbc8c..0a2604afaa8 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -128,6 +128,8 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
         std::string error =
             read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
         if (error.empty()) {
+            // Legacy trace support where binaries are needed.
+            // We do not support non-module code for such traces.
             module_mapper_ =
                 module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr,
                                         nullptr, knob_verbose_, knob_alt_module_dir_);
@@ -138,6 +140,8 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
                 return "Failed to load binaries: " + error;
             }
         } else {
+            // Continue but omit disassembly to support cases where binaries are
+            // not available and OFFLINE_FILE_TYPE_ENCODINGS is not present.
             has_modules_ = false;
         }
     }

From 76e0db16d477b2d78e69beb0bda711df9e1ed400 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Thu, 12 Dec 2024 11:24:15 -0500
Subject: [PATCH 07/11] Make diff simpler

---
 clients/drcachesim/tools/view.cpp | 33 ++++++++++++++++---------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index 0a2604afaa8..0e030e937b8 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -127,24 +127,25 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
         file_t modfile;
         std::string error =
             read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
-        if (error.empty()) {
-            // Legacy trace support where binaries are needed.
-            // We do not support non-module code for such traces.
-            module_mapper_ =
-                module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr,
-                                        nullptr, knob_verbose_, knob_alt_module_dir_);
-            module_mapper_->get_loaded_modules();
-            dr_close_file(modfile);
-            error = module_mapper_->get_last_error();
-            if (!error.empty()) {
-                return "Failed to load binaries: " + error;
-            }
-        } else {
-            // Continue but omit disassembly to support cases where binaries are
-            // not available and OFFLINE_FILE_TYPE_ENCODINGS is not present.
+        if (!error.empty())
             has_modules_ = false;
-        }
+        else
+            dr_close_file(modfile);
+    }
+    if (!has_modules_) {
+        // Continue but omit disassembly to support cases where binaries are
+        // not available and OFFLINE_FILE_TYPE_ENCODINGS is not present.
+        return "";
     }
+    // Legacy trace support where binaries are needed.
+    // We do not support non-module code for such traces.
+    module_mapper_ =
+        module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr, nullptr,
+                                knob_verbose_, knob_alt_module_dir_);
+    module_mapper_->get_loaded_modules();
+    std::string error = module_mapper_->get_last_error();
+    if (!error.empty())
+        return "Failed to load binaries: " + error;
     return "";
 }
 

From d200b03fb77de0eb50400c3a42fa07d4172f859b Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Sat, 14 Dec 2024 23:56:49 -0500
Subject: [PATCH 08/11] Delete initialize_module_file completely as it is
 unused

---
 api/docs/release.dox                              |  5 +++++
 clients/drcachesim/tracer/raw2trace_directory.cpp | 12 ------------
 clients/drcachesim/tracer/raw2trace_directory.h   | 15 ++++-----------
 clients/drcachesim/tracer/raw2trace_shared.h      |  9 +++++++++
 make/DynamoRIOConfig.cmake.in                     |  2 +-
 5 files changed, 19 insertions(+), 24 deletions(-)
 mode change 100755 => 100644 make/DynamoRIOConfig.cmake.in

diff --git a/api/docs/release.dox b/api/docs/release.dox
index 5359fe8d8d3..88f90ce4168 100644
--- a/api/docs/release.dox
+++ b/api/docs/release.dox
@@ -134,6 +134,11 @@ changes:
    is the encoding of the removed instruction up to a pointer's length. Added
    #OFFLINE_FILE_VERSION_RETIRED_INSTRUCTIONS_ONLY to increase the trace version
    for drmemtraces with uncompleted instructions removed.
+ - Moved module file read logic into read_module_file_bytes() in raw2trace_shared.h.
+   Also removed raw2trace_directory_t::initialize_module_file() and marked
+   raw2trace_directory_t::modfile_bytes_ as a private data member since the
+   read_module_file_bytes() can be directly used without having to pull in the whole
+   raw2trace_directory_t.
 
 Further non-compatibility-affecting changes include:
  - Added X64 Linux support to dr_create_memory_dump(). This API has the same
diff --git a/clients/drcachesim/tracer/raw2trace_directory.cpp b/clients/drcachesim/tracer/raw2trace_directory.cpp
index 007b72317a7..27ed4a59b09 100644
--- a/clients/drcachesim/tracer/raw2trace_directory.cpp
+++ b/clients/drcachesim/tracer/raw2trace_directory.cpp
@@ -433,12 +433,6 @@ raw2trace_directory_t::open_cpu_schedule_file()
 #endif
 }
 
-std::string
-raw2trace_directory_t::read_module_file(const std::string &modfilename)
-{
-    return read_module_file_bytes(modfilename, modfile_, modfile_bytes_);
-}
-
 bool
 raw2trace_directory_t::is_window_subdir(const std::string &dir)
 {
@@ -594,12 +588,6 @@ raw2trace_directory_t::initialize(const std::string &indir, const std::string &o
     return open_thread_files();
 }
 
-std::string
-raw2trace_directory_t::initialize_module_file(const std::string &module_file_path)
-{
-    return read_module_file(module_file_path);
-}
-
 std::string
 raw2trace_directory_t::initialize_funclist_file(
     const std::string &funclist_file_path,
diff --git a/clients/drcachesim/tracer/raw2trace_directory.h b/clients/drcachesim/tracer/raw2trace_directory.h
index 1d490c6c4c7..86431afcf30 100644
--- a/clients/drcachesim/tracer/raw2trace_directory.h
+++ b/clients/drcachesim/tracer/raw2trace_directory.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2017-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2017-2024 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -60,8 +60,8 @@ namespace drmemtrace {
 class raw2trace_directory_t {
 public:
     raw2trace_directory_t(unsigned int verbosity = 0)
-        : modfile_bytes_(nullptr)
-        , encoding_file_(INVALID_FILE)
+        : encoding_file_(INVALID_FILE)
+        , modfile_bytes_(nullptr)
         , modfile_(INVALID_FILE)
         , indir_("")
         , outdir_("")
@@ -78,11 +78,6 @@ class raw2trace_directory_t {
     initialize(const std::string &indir, const std::string &outdir,
                const std::string &compress = DEFAULT_TRACE_COMPRESSION_TYPE,
                const std::string &syscall_template_file = "");
-    // Use this instead of initialize() to only fill in modfile_bytes, for
-    // constructing a module_mapper_t.  Returns "" on success or an error message on
-    // failure.
-    std::string
-    initialize_module_file(const std::string &module_file_path);
     // Use this instead of initialize() to only read the funcion map file.
     // Returns "" on success or an error message on failure.
     // On success, pushes the parsed entries from the file into "entries".
@@ -99,7 +94,6 @@ class raw2trace_directory_t {
     static bool
     is_window_subdir(const std::string &dir);
 
-    char *modfile_bytes_;
     file_t encoding_file_;
     std::vector<std::istream *> in_files_;
     std::vector<std::ostream *> out_files_;
@@ -115,8 +109,6 @@ class raw2trace_directory_t {
     std::string
     trace_suffix();
     std::string
-    read_module_file(const std::string &modfilename);
-    std::string
     open_thread_files();
     std::string
     open_thread_log_file(const char *basename);
@@ -130,6 +122,7 @@ class raw2trace_directory_t {
     std::string
     open_kthread_files();
 #endif
+    char *modfile_bytes_;
     file_t modfile_;
     std::string kernel_indir_;
     std::string indir_;
diff --git a/clients/drcachesim/tracer/raw2trace_shared.h b/clients/drcachesim/tracer/raw2trace_shared.h
index 8f0dac3a8b2..23d498995e9 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.h
+++ b/clients/drcachesim/tracer/raw2trace_shared.h
@@ -163,6 +163,15 @@ class memref_counter_t : public reader_t {
  * Reads the module file at the given \p modfilename path. Returns an
  * empty string if successful, or the error string if not.
  *
+ * This uses the DR file APIs to read the module file. Generally we
+ * attempt to isolate implementation that uses DR file APIs to
+ * raw2trace_directory_t, and have raw2trace and raw2trace_shared
+ * deal only with file streams, but we make an exception here. This
+ * is acceptable because no current users of read_module_file_bytes
+ * need to read it from a stream. Also, this is a simple convenience
+ * routine that only reads the file without any module file specific
+ * logic.
+ *
  * If successful, the returned \p modfile must be closed using
  * \p dr_close_file, and the returned \p modefilebytes must be freed
  * using a delete[].
diff --git a/make/DynamoRIOConfig.cmake.in b/make/DynamoRIOConfig.cmake.in
old mode 100755
new mode 100644
index d7597d86e93..0a76e94fd36
--- a/make/DynamoRIOConfig.cmake.in
+++ b/make/DynamoRIOConfig.cmake.in
@@ -1039,7 +1039,7 @@ function (configure_DynamoRIO_client target)
         # -dT is preferred, available on ld 2.18+: we could check for it
         set(LD_SCRIPT_OPTION "-T")
         set(PREFERRED_BASE_FLAGS "-Xlinker ${LD_SCRIPT_OPTION} -Xlinker \"${LD_SCRIPT}\"")
-      endif (LINKER_IS_GNU_GOLD)
+      endif (LINKER_IS_GNU_GOLD OR LINKER_IS_LLVM_LLD)
     else (APPLE)
       set(PREFERRED_BASE_FLAGS "/base:${PREFERRED_BASE} /dynamicbase:no")
     endif (APPLE)

From 57b3ecefb8e095793c2c9ff5934cb273d1b8ae1f Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Sun, 15 Dec 2024 00:00:29 -0500
Subject: [PATCH 09/11] Rename to read_module_file

---
 api/docs/release.dox                              | 7 +++----
 clients/drcachesim/tools/opcode_mix.cpp           | 5 ++---
 clients/drcachesim/tools/view.cpp                 | 3 +--
 clients/drcachesim/tracer/raw2trace_directory.cpp | 2 +-
 clients/drcachesim/tracer/raw2trace_directory.h   | 6 +++---
 clients/drcachesim/tracer/raw2trace_shared.cpp    | 5 ++---
 clients/drcachesim/tracer/raw2trace_shared.h      | 5 ++---
 7 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/api/docs/release.dox b/api/docs/release.dox
index 88f90ce4168..cfb15132e3d 100644
--- a/api/docs/release.dox
+++ b/api/docs/release.dox
@@ -134,10 +134,9 @@ changes:
    is the encoding of the removed instruction up to a pointer's length. Added
    #OFFLINE_FILE_VERSION_RETIRED_INSTRUCTIONS_ONLY to increase the trace version
    for drmemtraces with uncompleted instructions removed.
- - Moved module file read logic into read_module_file_bytes() in raw2trace_shared.h.
-   Also removed raw2trace_directory_t::initialize_module_file() and marked
-   raw2trace_directory_t::modfile_bytes_ as a private data member since the
-   read_module_file_bytes() can be directly used without having to pull in the whole
+ - Moved module file read logic into read_module_file() in raw2trace_shared.h.
+   Also removed raw2trace_directory_t::initialize_module_file() since the
+   read_module_file() can be directly used without having to pull in the whole
    raw2trace_directory_t.
 
 Further non-compatibility-affecting changes include:
diff --git a/clients/drcachesim/tools/opcode_mix.cpp b/clients/drcachesim/tools/opcode_mix.cpp
index a5ebe0a6981..6ff25819d31 100644
--- a/clients/drcachesim/tools/opcode_mix.cpp
+++ b/clients/drcachesim/tools/opcode_mix.cpp
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2017-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2017-2024 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -93,8 +93,7 @@ opcode_mix_t::initialize()
     // Legacy trace support where binaries are needed.
     // We do not support non-module code for such traces.
     file_t modfile;
-    std::string error =
-        read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
+    std::string error = read_module_file(module_file_path_, modfile, modfile_bytes_);
     if (!error.empty()) {
         return "Failed to read module file: " + error;
     }
diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp
index 0e030e937b8..264b2797f78 100644
--- a/clients/drcachesim/tools/view.cpp
+++ b/clients/drcachesim/tools/view.cpp
@@ -125,8 +125,7 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
         has_modules_ = false;
     } else {
         file_t modfile;
-        std::string error =
-            read_module_file_bytes(module_file_path_, modfile, modfile_bytes_);
+        std::string error = read_module_file(module_file_path_, modfile, modfile_bytes_);
         if (!error.empty())
             has_modules_ = false;
         else
diff --git a/clients/drcachesim/tracer/raw2trace_directory.cpp b/clients/drcachesim/tracer/raw2trace_directory.cpp
index 27ed4a59b09..6e1451493bf 100644
--- a/clients/drcachesim/tracer/raw2trace_directory.cpp
+++ b/clients/drcachesim/tracer/raw2trace_directory.cpp
@@ -537,7 +537,7 @@ raw2trace_directory_t::initialize(const std::string &indir, const std::string &o
     }
     std::string modfilename =
         modfile_dir + std::string(DIRSEP) + DRMEMTRACE_MODULE_LIST_FILENAME;
-    std::string err = read_module_file(modfilename);
+    std::string err = read_module_file(modfilename, modfile_, modfile_bytes_);
     if (!err.empty())
         return err;
 
diff --git a/clients/drcachesim/tracer/raw2trace_directory.h b/clients/drcachesim/tracer/raw2trace_directory.h
index 86431afcf30..c8de4f30928 100644
--- a/clients/drcachesim/tracer/raw2trace_directory.h
+++ b/clients/drcachesim/tracer/raw2trace_directory.h
@@ -60,8 +60,8 @@ namespace drmemtrace {
 class raw2trace_directory_t {
 public:
     raw2trace_directory_t(unsigned int verbosity = 0)
-        : encoding_file_(INVALID_FILE)
-        , modfile_bytes_(nullptr)
+        : modfile_bytes_(nullptr)
+        , encoding_file_(INVALID_FILE)
         , modfile_(INVALID_FILE)
         , indir_("")
         , outdir_("")
@@ -94,6 +94,7 @@ class raw2trace_directory_t {
     static bool
     is_window_subdir(const std::string &dir);
 
+    char *modfile_bytes_;
     file_t encoding_file_;
     std::vector<std::istream *> in_files_;
     std::vector<std::ostream *> out_files_;
@@ -122,7 +123,6 @@ class raw2trace_directory_t {
     std::string
     open_kthread_files();
 #endif
-    char *modfile_bytes_;
     file_t modfile_;
     std::string kernel_indir_;
     std::string indir_;
diff --git a/clients/drcachesim/tracer/raw2trace_shared.cpp b/clients/drcachesim/tracer/raw2trace_shared.cpp
index 18ec44da3bc..ca2b17e82b2 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.cpp
+++ b/clients/drcachesim/tracer/raw2trace_shared.cpp
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2016-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2016-2024 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -167,8 +167,7 @@ drmemtrace_get_timestamp_from_offline_trace(const void *trace, size_t trace_size
 }
 
 std::string
-read_module_file_bytes(const std::string &modfilename, file_t &modfile,
-                       char *&modfile_bytes)
+read_module_file(const std::string &modfilename, file_t &modfile, char *&modfile_bytes)
 {
     modfile = dr_open_file(modfilename.c_str(), DR_FILE_READ);
     if (modfile == INVALID_FILE)
diff --git a/clients/drcachesim/tracer/raw2trace_shared.h b/clients/drcachesim/tracer/raw2trace_shared.h
index 23d498995e9..47952f9e6f5 100644
--- a/clients/drcachesim/tracer/raw2trace_shared.h
+++ b/clients/drcachesim/tracer/raw2trace_shared.h
@@ -167,7 +167,7 @@ class memref_counter_t : public reader_t {
  * attempt to isolate implementation that uses DR file APIs to
  * raw2trace_directory_t, and have raw2trace and raw2trace_shared
  * deal only with file streams, but we make an exception here. This
- * is acceptable because no current users of read_module_file_bytes
+ * is acceptable because no current users of read_module_file
  * need to read it from a stream. Also, this is a simple convenience
  * routine that only reads the file without any module file specific
  * logic.
@@ -177,8 +177,7 @@ class memref_counter_t : public reader_t {
  * using a delete[].
  */
 std::string
-read_module_file_bytes(const std::string &modfilename, file_t &modfile,
-                       char *&modfile_bytes);
+read_module_file(const std::string &modfilename, file_t &modfile, char *&modfile_bytes);
 
 struct module_t {
     module_t(const char *path, app_pc orig, byte *map, size_t offs, size_t size,

From 5a98802449ee3602c73cc1a6b8b36afb73e3c3f2 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Sun, 15 Dec 2024 00:21:02 -0500
Subject: [PATCH 10/11] cleanup cmake lib dependences

---
 clients/drcachesim/CMakeLists.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt
index 3e682ecdea8..942e49847b8 100644
--- a/clients/drcachesim/CMakeLists.txt
+++ b/clients/drcachesim/CMakeLists.txt
@@ -161,9 +161,11 @@ add_exported_library(drmemtrace_reuse_distance STATIC tools/reuse_distance.cpp)
 add_exported_library(drmemtrace_histogram STATIC tools/histogram.cpp)
 add_exported_library(drmemtrace_reuse_time STATIC tools/reuse_time.cpp)
 add_exported_library(drmemtrace_basic_counts STATIC tools/basic_counts.cpp)
-add_exported_library(drmemtrace_opcode_mix STATIC tools/opcode_mix.cpp)
+add_exported_library(drmemtrace_opcode_mix STATIC
+                     tools/opcode_mix.cpp tracer/raw2trace_shared.cpp)
 add_exported_library(drmemtrace_syscall_mix STATIC tools/syscall_mix.cpp)
-add_exported_library(drmemtrace_view STATIC tools/view.cpp)
+add_exported_library(drmemtrace_view STATIC
+                     tools/view.cpp tracer/raw2trace_shared.cpp)
 add_exported_library(drmemtrace_func_view STATIC tools/func_view.cpp)
 add_exported_library(drmemtrace_invariant_checker STATIC tools/invariant_checker.cpp)
 add_exported_library(drmemtrace_schedule_stats STATIC tools/schedule_stats.cpp)
@@ -1202,7 +1204,7 @@ if (BUILD_TESTS)
     add_executable(tool.drcacheoff.skip_unit_tests tests/skip_unit_tests.cpp)
     configure_DynamoRIO_standalone(tool.drcacheoff.skip_unit_tests)
     target_link_libraries(tool.drcacheoff.skip_unit_tests drmemtrace_analyzer
-      drmemtrace_view drmemtrace_raw2trace test_helpers ${zlib_libs})
+      drmemtrace_view test_helpers ${zlib_libs})
     add_win32_flags(tool.drcacheoff.skip_unit_tests)
     use_DynamoRIO_extension(tool.drcacheoff.skip_unit_tests drreg_static)
     use_DynamoRIO_extension(tool.drcacheoff.skip_unit_tests drcovlib_static)

From 4bc8f8adfbe41fcf4f868edf9e36922e35c44959 Mon Sep 17 00:00:00 2001
From: Abhinav Anil Sharma <sharmaabhinav@google.com>
Date: Sun, 15 Dec 2024 01:37:56 -0500
Subject: [PATCH 11/11] Release note clarification

---
 api/docs/release.dox | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/api/docs/release.dox b/api/docs/release.dox
index 07158fbc5ec..5b8faf839ed 100644
--- a/api/docs/release.dox
+++ b/api/docs/release.dox
@@ -134,8 +134,8 @@ changes:
    is the encoding of the removed instruction up to a pointer's length. Added
    #OFFLINE_FILE_VERSION_RETIRED_INSTRUCTIONS_ONLY to increase the trace version
    for drmemtraces with uncompleted instructions removed.
- - Moved module file read logic into read_module_file() in raw2trace_shared.h.
-   Also removed raw2trace_directory_t::initialize_module_file() since the
+ - Moved module file read logic into read_module_file() in raw2trace_shared, and
+   removed raw2trace_directory_t::initialize_module_file() since the
    read_module_file() can be directly used without having to pull in the whole
    raw2trace_directory_t.