i#5538 memtrace seek, part 11: Fix omitted header problems

Fixes a bug where reader_t's detection of duplicated timestamp,cpuid
headers at the start of a chunk assumed single-threaded mode.  We
switch to using a simple per-tid chunk footer trigger.

Adds a test to view_test via a new serial mock which takes in
trace_entry_t and allows testing of the interleaving code.  Tests both
proper chunk header elision as well as replicating the bug where
elision should not happen.

The test revealed a separate bug in the view tool where the version
and filetype ordinals, for delaying, were not updated on new threads.
That is fixed here as well as otherwise the new tests fail.

Issue: #5538

clients/drcachesim/CMakeLists.txt

@@ -1,5 +1,5 @@
 # **********************************************************
-# Copyright (c) 2015-2022 Google, Inc.    All rights reserved.
+# Copyright (c) 2015-2023 Google, Inc.    All rights reserved.
 # **********************************************************
 
 # Redistribution and use in source and binary forms, with or without
@@ -699,7 +699,7 @@ if (BUILD_TESTS)
     add_test(NAME tool.drcachesim.invariant_checker_test
              COMMAND tool.drcachesim.invariant_checker_test)
 
-    add_executable(tool.drcacheoff.view_test tests/view_test.cpp)
+    add_executable(tool.drcacheoff.view_test tests/view_test.cpp reader/file_reader.cpp)
     configure_DynamoRIO_standalone(tool.drcacheoff.view_test)
     add_win32_flags(tool.drcacheoff.view_test)
     target_link_libraries(tool.drcacheoff.view_test drmemtrace_view drmemtrace_raw2trace)

clients/drcachesim/reader/file_reader.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2016-2022 Google, Inc.  All rights reserved.
+ * Copyright (c) 2016-2023 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -361,10 +361,12 @@ template <typename T> class file_reader_t : public reader_t {
         return true;
     }
 
+    // Protected for access by mock_file_reader_t.
+    std::vector<T> input_files_;
+
 private:
     std::string input_path_;
     std::vector<std::string> input_path_list_;
-    std::vector<T> input_files_;
     trace_entry_t entry_copy_;
     // The current thread we're processing is "index".  If it's set to input_files_.size()
     // that means we need to pick a new thread.

clients/drcachesim/reader/reader.cpp

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2016-2022 Google, Inc.  All rights reserved.
+ * Copyright (c) 2016-2023 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -262,21 +262,19 @@ reader_t::process_input_entry()
         // and use them to start post-seek iteration.
         if (chunk_instr_count_ > 0 &&
             cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_TIMESTAMP &&
-            cur_instr_count_ / chunk_instr_count_ !=
-                last_timestamp_instr_count_ / chunk_instr_count_) {
+            skip_chunk_header_.find(cur_tid_) != skip_chunk_header_.end()) {
             VPRINT(this, 2, "skipping start-of-chunk dup timestamp\n");
-            skip_next_cpu_ = true;
-        } else if (cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_CPU_ID &&
-                   skip_next_cpu_) {
+        } else if (chunk_instr_count_ > 0 &&
+                   cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_CPU_ID &&
+                   skip_chunk_header_.find(cur_tid_) != skip_chunk_header_.end()) {
             VPRINT(this, 2, "skipping start-of-chunk dup cpu\n");
-            skip_next_cpu_ = false;
+            skip_chunk_header_.erase(cur_tid_);
         } else if (cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_RECORD_ORDINAL) {
             // Not exposed to tools.
         } else {
             have_memref = true;
         }
         if (cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_TIMESTAMP) {
-            last_timestamp_instr_count_ = cur_instr_count_;
             // Today, a skipped memref is just a duplicate of one that we've
             // already seen, so this condition is not really needed. But to
             // be future-proof, we want to avoid looking at timestamps that
@@ -295,6 +293,8 @@ reader_t::process_input_entry()
             page_size_ = cur_ref_.marker.marker_value;
         else if (cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT)
             chunk_instr_count_ = cur_ref_.marker.marker_value;
+        else if (cur_ref_.marker.marker_type == TRACE_MARKER_TYPE_CHUNK_FOOTER)
+            skip_chunk_header_.insert(cur_tid_);
         break;
     default:
         ERRMSG("Unknown trace entry type %s (%d)\n", trace_type_names[input_entry_->type],

clients/drcachesim/reader/reader.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2015-2022 Google, Inc.  All rights reserved.
+ * Copyright (c) 2015-2023 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -40,6 +40,7 @@
 #include <assert.h>
 #include <iterator>
 #include <unordered_map>
+#include <unordered_set>
 // For exporting we avoid "../common" and rely on -I.
 #include "memref.h"
 #include "memtrace_stream.h"
@@ -204,7 +205,7 @@ class reader_t : public std::iterator<std::input_iterator_tag, memref_t>,
     uint64_t cur_ref_count_ = 0;
     int64_t suppress_ref_count_ = -1;
     uint64_t cur_instr_count_ = 0;
-    uint64_t last_timestamp_instr_count_ = 0;
+    std::unordered_set<memref_tid_t> skip_chunk_header_;
     uint64_t last_timestamp_ = 0;
     trace_entry_t *input_entry_ = nullptr;
     // Remember top-level headers for the memtrace_stream_t interface.
@@ -228,7 +229,6 @@ class reader_t : public std::iterator<std::input_iterator_tag, memref_t>,
     addr_t prev_instr_addr_ = 0;
     int bundle_idx_ = 0;
     std::unordered_map<memref_tid_t, memref_pid_t> tid2pid_;
-    bool skip_next_cpu_ = false;
     bool expect_no_encodings_ = true;
     encoding_info_t last_encoding_;
     std::unordered_map<addr_t, encoding_info_t> encodings_;