Repository: mephi42/memtrace (language: python)

Issue:
Refuse to load mismatching indices
Generate a unique identifier for each trace and copy it to instruction and use-def indices.
Refuse to load instruction and use-def indices with mismatching identifiers.


Discussion / hints:

Fix patch:
diff --git a/memtrace_ext/entries.h b/memtrace_ext/entries.h
index 07bfc93e5..edff313ea 100644
--- a/memtrace_ext/entries.h
+++ b/memtrace_ext/entries.h
@@ -100,6 +100,8 @@ class Tlv : public B {
   }
 };
 
+using TraceId = std::array<std::uint8_t, 16>;
+
 template <Endianness E, typename W, typename B = Overlay>
 class HeaderEntry : public B {
  public:
@@ -118,11 +120,17 @@ class HeaderEntry : public B {
     return RawInt<E, std::uint16_t>(this->GetData() + kRegsSizeOffset)
         .GetValue();
   }
+  const TraceId& GetTraceId() const {
+    return *reinterpret_cast<const TraceId *>(
+        this->GetData() + kTraceIdOffset);
+  }
 
  private:
   static constexpr size_t kMachineTypeOffset = Tlv<E, W>::kFixedLength;
   static constexpr size_t kRegsSizeOffset =
       kMachineTypeOffset + sizeof(std::uint16_t);
+  static constexpr size_t kTraceIdOffset =
+      kRegsSizeOffset + sizeof(std::uint16_t);
 };
 
 template <Endianness E, typename W, typename B = Overlay>
diff --git a/memtrace_ext/memtrace_ext.cc b/memtrace_ext/memtrace_ext.cc
index aa55bb6cd..2aaab301b 100644
--- a/memtrace_ext/memtrace_ext.cc
+++ b/memtrace_ext/memtrace_ext.cc
@@ -45,8 +45,9 @@ namespace {
 
 constexpr size_t kHostWordSize = sizeof(void*);
 
-void HexDump(std::FILE* f, const std::uint8_t* buf, size_t n) {
-  for (size_t i = 0; i < n; i++) std::fprintf(f, "%02x", buf[i]);
+void HexDump(std::FILE* f, const void* buf, size_t n) {
+  for (size_t i = 0; i < n; i++)
+    std::fprintf(f, "%02x", static_cast<const std::uint8_t *>(buf)[i]);
 }
 
 void ReprDump(std::FILE* f, const std::uint8_t* buf, size_t n) {
@@ -196,6 +197,9 @@ class Dumper {
     std::fprintf(f_, "Machine           : %s\n",
                  GetMachineTypeStr(entry.GetMachineType()));
     std::fprintf(f_, "Regs size         : %d\n", entry.GetRegsSize());
+    std::fprintf(f_, "Trace ID          : ");
+    HexDump(f_, &entry.GetTraceId(), sizeof(TraceId));
+    std::fprintf(f_, "\n");
     return disasmEngine_.Init(entry.GetMachineType(), E, sizeof(W));
   }
 
@@ -573,6 +577,8 @@ class TraceBase {
   virtual size_t GetWordSize() = 0;
   virtual MachineType GetMachineType() = 0;
   virtual std::uint16_t GetRegsSize() = 0;
+  virtual TraceId GetTraceId() = 0;
+  virtual std::vector<std::uint8_t> GetTraceIdPy() = 0;
   virtual boost::python::object Next() = 0;
   virtual int SeekStart() = 0;
   virtual int SeekInsn(std::uint32_t index) = 0;
@@ -652,6 +658,7 @@ struct MemDefSeeker : public DefSeeker<W> {
 };
 
 struct InsnIndexHeader {
+  TraceId traceId;
   std::uint8_t stepShift;
 };
 
@@ -807,6 +814,14 @@ class Trace : public TraceBase {
 
   std::uint16_t GetRegsSize() override { return header_.GetRegsSize(); }
 
+  TraceId GetTraceId() override { return header_.GetTraceId(); }
+
+  std::vector<std::uint8_t> GetTraceIdPy() override {
+    const TraceId& traceId = header_.GetTraceId();
+
+    return std::vector<std::uint8_t>(traceId.begin(), traceId.end());
+  }
+
   boost::python::object Next() override {
     while (true) {
       if (cur_ == end_) boost::python::objects::stop_iteration_error();
@@ -922,6 +937,7 @@ class Trace : public TraceBase {
         mmapIndex_.push_back(MmapIndexEntry{static_cast<size_t>(prev - data_)});
     }
     InsnIndexHeader header;
+    header.traceId = header_.GetTraceId();
     header.stepShift = static_cast<std::uint8_t>(stepShift);
     if ((err = WriteHeader(indexPath.Get("header").c_str(), header)) < 0)
       return err;
@@ -937,6 +953,8 @@ class Trace : public TraceBase {
     InsnIndexHeader header;
     if ((err = ReadHeader(indexPath.Get("header").c_str(), &header)) < 0)
       return err;
+    if (header.traceId != header_.GetTraceId())
+      return -EINVAL;
     if ((err = insnIndex_.Init(indexPath.Get("data").c_str(),
                                InitMode::OpenExisting)) < 0)
       return err;
@@ -1629,6 +1647,7 @@ class UdState {
 };
 
 struct BinaryHeader {
+  TraceId traceId;
   std::uint8_t hostEndianness;
   std::uint8_t hostWordSize;
   MachineType machineType;  // Traced program machine type.
@@ -2131,6 +2150,7 @@ class Ud : public UdBase {
   int DumpBinary() const {
     if (binary_ == nullptr) return 0;
     BinaryHeader header;
+    header.traceId = fullTrace_->GetTraceId();
     header.hostEndianness = static_cast<std::uint8_t>(kHostEndianness);
     header.hostWordSize = static_cast<std::uint8_t>(kHostWordSize);
     header.machineType = machineType_;
@@ -2162,7 +2182,8 @@ UdBase* UdBase::Load(const char* rawPath, std::shared_ptr<TraceBase> trace) {
   BinaryHeader header;
   if (ReadUdHeader(rawPath, &header) < 0) return nullptr;
   if (static_cast<Endianness>(header.hostEndianness) != kHostEndianness ||
-      static_cast<size_t>(header.hostWordSize) != kHostWordSize)
+      static_cast<size_t>(header.hostWordSize) != kHostWordSize ||
+      header.traceId != trace->GetTraceId())
     return nullptr;
   UdBase* ud = nullptr;
   TraceBase::Downcast(trace, [rawPath, header, &ud](auto trace) {
@@ -2318,6 +2339,7 @@ BOOST_PYTHON_MODULE(_memtrace) {
       .def("get_word_size", &TraceBase::GetWordSize)
       .def("get_machine_type", &TraceBase::GetMachineType)
       .def("get_regs_size", &TraceBase::GetRegsSize)
+      .def("get_trace_id", &TraceBase::GetTraceIdPy)
       .def("__iter__", bp::objects::identity_function())
       .def("__next__", &TraceBase::Next)
       .def("seek_start", &TraceBase::SeekStart)
diff --git a/valgrind-tracer/mt_main.c b/valgrind-tracer/mt_main.c
index 997bbccd0..45197d1c3 100644
--- a/valgrind-tracer/mt_main.c
+++ b/valgrind-tracer/mt_main.c
@@ -3,6 +3,7 @@
 #include "pub_core_debuginfo.h"
 #include "pub_core_libcfile.h"
 #include "pub_core_machine.h"
+#include "pub_core_syscall.h"
 #include "pub_tool_aspacehl.h"
 #include "pub_tool_basics.h"
 #include "pub_tool_guest.h"
@@ -107,6 +108,8 @@ typedef struct {
 #define MAX_SYMBOLIC_PC_RANGES 32
 static SymbolicAddrRange symbolicPcRanges[MAX_SYMBOLIC_PC_RANGES];
 static UInt nSymbolicPcRanges;
+static UChar traceId[16];
+static Bool traceIdSpecified;
 
 /* Generic entry header. */
 struct __attribute__((__packed__)) Tlv {
@@ -128,6 +131,7 @@ struct __attribute__((__packed__)) HeaderEntry {
    struct Tlv tlv;
    UShort e_machine;
    UShort regsSize;
+   UChar traceId[16];
 };
 
 /* Used for MT_LOAD, MT_STORE, MT_REG, MT_GET_REG, MT_PUT_REG. */
@@ -350,12 +354,18 @@ static void open_trace_file(void)
    trace = traceStart;
    traceEnd = traceStart + TRACE_BUFFER_SIZE;
 
-   MT_STATIC_ASSERT(sizeof(struct HeaderEntry) == 8);
+   MT_STATIC_ASSERT(sizeof(struct HeaderEntry) == 24);
    entry = (struct HeaderEntry*)trace;
    entry->tlv.tag = MT_MAGIC;
    entry->tlv.length = sizeof(struct HeaderEntry);
    entry->e_machine = VG_ELF_MACHINE;
    entry->regsSize = sizeof(VexGuestArchState);
+   VG_(memset)(entry->traceId, 0, sizeof(entry->traceId));
+   if (traceIdSpecified)
+      VG_(memcpy)(entry->traceId, traceId, sizeof(entry->traceId));
+   else
+      VG_(do_syscall3)(__NR_getrandom,
+                       (UWord)entry->traceId, sizeof(entry->traceId), 0);
    trace += sizeof(struct HeaderEntry);
 
    store_reg_meta();
@@ -754,14 +764,56 @@ static Bool add_pc_range(const HChar* spec)
    return True;
 }
 
+static Int parse_hex_digit(HChar digit) {
+   switch (digit) {
+   case '0': return 0;
+   case '1': return 1;
+   case '2': return 2;
+   case '3': return 3;
+   case '4': return 4;
+   case '5': return 5;
+   case '6': return 6;
+   case '7': return 7;
+   case '8': return 8;
+   case '9': return 9;
+   case 'a': case 'A': return 10;
+   case 'b': case 'B': return 11;
+   case 'c': case 'C': return 12;
+   case 'd': case 'D': return 13;
+   case 'e': case 'E': return 14;
+   case 'f': case 'F': return 15;
+   default: return -1;
+   }
+}
+
+static Bool set_trace_id(const HChar *spec)
+{
+   Int digit;
+   SizeT i;
+
+   for (i = 0; i < sizeof(traceId) * 2; i++) {
+      digit = parse_hex_digit(spec[i]);
+      if (digit == -1)
+         return False;
+
+      traceId[i / 2] |= digit << (4 - (i % 2) * 4);
+   }
+
+   traceIdSpecified = True;
+   return True;
+}
+
 static Bool mt_process_cmd_line_option(const HChar* arg)
 {
    const HChar* tmpStr;
 
    if (VG_STR_CLO(arg, "--pc-range", tmpStr))
       return add_pc_range(tmpStr);
-   else
-      return False;
+
+   if (VG_STR_CLO(arg, "--trace-id", tmpStr))
+      return set_trace_id(tmpStr);
+
+   return False;
 }
 
 static void mt_print_usage(void)
@@ -1045,7 +1097,7 @@ static void mt_pre_clo_init(void)
    VG_(details_name)("Memory Tracer");
    VG_(details_description)("Valgrind tool for tracing memory accesses");
    VG_(details_copyright_author)(
-      "Copyright (C) 2019-2020, and GNU GPL'd, by mephi42.");
+      "Copyright (C) 2019-2024, and GNU GPL'd, by mephi42.");
    VG_(details_bug_reports_to)("https://github.com/mephi42/memtrace");
    VG_(basic_tool_funcs)(mt_post_clo_init,
                          mt_instrument,


Test patch:
diff --git a/test/i386-dump.txt b/test/i386-dump.txt
index da0807025..b9ac7c4b8 100644
--- a/test/i386-dump.txt
+++ b/test/i386-dump.txt
@@ -3,6 +3,7 @@ Word              : I
 Word size         : 4
 Machine           : EM_386
 Regs size         : 352
+Trace ID          : fedcba98765432100123456789abcdef
 [         0] MT_REGMETA uint32_t host_EvC_FAILADDR [0x0]
 [         1] MT_REGMETA uint32_t host_EvC_COUNTER [0x4]
 [         2] MT_REGMETA uint32_t eax [0x8]
diff --git a/test/i386-stats.txt b/test/i386-stats.txt
index 0d9dcaa0e..16931b486 100644
--- a/test/i386-stats.txt
+++ b/test/i386-stats.txt
@@ -1,5 +1,5 @@
 MT_GET_REG count=14 size=224
-MT_HEADER32 count=1 size=8
+MT_HEADER32 count=1 size=24
 MT_INSN count=13 size=240
 MT_INSN_EXEC count=13 size=104
 MT_LOAD count=2 size=32
@@ -7,4 +7,4 @@ MT_MMAP count=4 size=
 MT_PUT_REG count=25 size=400
 MT_REGMETA count=66 size=1184
 MT_STORE count=2 size=32
-total count=140 size=2424
+total count=140 size=2440
diff --git a/test/test.py b/test/test.py
index f7623d1b4..0788c8355 100755
--- a/test/test.py
+++ b/test/test.py
@@ -117,7 +117,7 @@ def disable_aslr(cls):
     @classmethod
     def memtrace_call(cls, fp):
         p = memtrace.tracer.popen(
-            [f"./{cls.get_target()}"],
+            ["--trace-id=fedcba98765432100123456789abcdef", f"./{cls.get_target()}"],
             stdin=fp,
             stdout=subprocess.DEVNULL,
             cwd=cls.workdir.name,
@@ -290,6 +290,11 @@ def test_trace(self) -> None:
                 "Machine           : {}\n".format(trace.get_machine_type()).encode()
             )
             fp.write("Regs size         : {}\n".format(trace.get_regs_size()).encode())
+            fp.write(
+                "Trace ID          : {}\n".format(
+                    bytes(trace.get_trace_id()).hex()
+                ).encode()
+            )
             pydir_bytes = self.pydir.encode()
             workdir_bytes = self.workdir.name.encode()
             testdir_bytes = self.basedir.encode()
diff --git a/test/x86_64-dump.txt b/test/x86_64-dump.txt
index 65c7de439..2f8f27ca8 100644
--- a/test/x86_64-dump.txt
+++ b/test/x86_64-dump.txt
@@ -3,6 +3,7 @@ Word              : Q
 Word size         : 8
 Machine           : EM_X86_64
 Regs size         : 928
+Trace ID          : fedcba98765432100123456789abcdef
 [         0] MT_REGMETA uint64_t host_EvC_FAILADDR [0x0]
 [         1] MT_REGMETA uint32_t host_EvC_COUNTER [0x8]
 [         2] MT_REGMETA uint32_t pad0 [0xc]
diff --git a/test/x86_64-stats.txt b/test/x86_64-stats.txt
index ababa3d87..c9b9bd462 100644
--- a/test/x86_64-stats.txt
+++ b/test/x86_64-stats.txt
@@ -1,5 +1,5 @@
 MT_GET_REG count=10 size=240
-MT_HEADER64 count=1 size=8
+MT_HEADER64 count=1 size=24
 MT_INSN count=11 size=264
 MT_INSN_EXEC count=11 size=88
 MT_LOAD count=1 size=24
@@ -7,4 +7,4 @@ MT_MMAP count=4 size=
 MT_PUT_REG count=22 size=528
 MT_REGMETA count=115 size=2192
 MT_STORE count=1 size=24
-total count=176 size=3640
+total count=176 size=3656