Record that the sigframe write recorded is conservative

src/RecordSession.cc

@@ -19,6 +19,7 @@
 #include "PerfCounters.h"
 #include "RecordTask.h"
 #include "TraceeAttentionSet.h"
+#include "TraceStream.h"
 #include "VirtualPerfCounterMonitor.h"
 #include "WaitManager.h"
 #include "core.h"
@@ -1758,7 +1759,8 @@ bool RecordSession::signal_state_changed(RecordTask* t, StepState* step_state) {
       // We record this data even if sigframe_size is zero to simplify replay.
       // Stop recording data if we run off the end of a writable mapping.
       // Our sigframe size is conservative so we need to do this.
-      t->record_remote_writable(t->regs().sp(), sigframe_size);
+      t->record_remote_writable(t->regs().sp(), sigframe_size,
+                                MemWriteSizeValidation::CONSERVATIVE);
 
       // This event is used by the replayer to set up the signal handler frame.
       // But if we don't have a handler, we don't want to record the event

src/RecordTask.cc

@@ -1724,7 +1724,8 @@ bool RecordTask::record_remote_by_local_map(remote_ptr<void> addr,
   return false;
 }
 
-void RecordTask::record_remote(remote_ptr<void> addr, ssize_t num_bytes) {
+void RecordTask::record_remote(remote_ptr<void> addr, ssize_t num_bytes,
+                               MemWriteSizeValidation size_validation) {
   ASSERT(this, num_bytes >= 0);
 
   if (!addr) {
@@ -1750,11 +1751,11 @@ void RecordTask::record_remote(remote_ptr<void> addr, ssize_t num_bytes) {
     ASSERT(this, false) << "Should have recorded " << num_bytes << " bytes from "
                         << addr << ", but failed";
   }
-  trace_writer().write_raw(rec_tid, buf.data(), num_bytes, addr);
+  trace_writer().write_raw(rec_tid, buf.data(), num_bytes, addr, size_validation);
 }
 
-void RecordTask::record_remote_writable(remote_ptr<void> addr,
-                                        ssize_t num_bytes) {
+void RecordTask::record_remote_writable(remote_ptr<void> addr, ssize_t num_bytes,
+                                        MemWriteSizeValidation size_validation) {
   ASSERT(this, num_bytes >= 0);
 
   remote_ptr<void> p = addr;
@@ -1777,7 +1778,7 @@ void RecordTask::record_remote_writable(remote_ptr<void> addr,
   }
   num_bytes = min(num_bytes, p - addr);
 
-  record_remote(addr, num_bytes);
+  record_remote(addr, num_bytes, size_validation);
 }
 
 ssize_t RecordTask::record_remote_fallible(remote_ptr<void> addr,

src/RecordTask.h

@@ -400,7 +400,8 @@ class RecordTask final : public Task {
   void record_local(remote_ptr<T> addr, const T* buf, size_t count = 1) {
     record_local(addr, sizeof(T) * count, buf);
   }
-  void record_remote(remote_ptr<void> addr, ssize_t num_bytes);
+  void record_remote(remote_ptr<void> addr, ssize_t num_bytes,
+                     MemWriteSizeValidation size_validation = MemWriteSizeValidation::EXACT);
   template <typename T> void record_remote(remote_ptr<T> addr) {
     record_remote(addr, sizeof(T));
   }
@@ -417,7 +418,8 @@ class RecordTask final : public Task {
   // Record as much as we can of the bytes in this range. Will record only
   // contiguous mapped-writable data starting at `addr`. rr mappings (e.g. syscallbuf)
   // are treated as non-contiguous with any other mapping.
-  void record_remote_writable(remote_ptr<void> addr, ssize_t num_bytes);
+  void record_remote_writable(remote_ptr<void> addr, ssize_t num_bytes,
+                              MemWriteSizeValidation size_validation = MemWriteSizeValidation::EXACT);
 
   // Simple helper that attempts to use the local mapping to record if one
   // exists

src/ReplayTask.cc

@@ -157,8 +157,14 @@ static size_t write_data_with_holes(ReplayTask* t,
     if (holes_iter != buf.holes.end()) {
       data_end = data_offset + holes_iter->offset - addr_offset;
     }
-    t->write_bytes_helper(buf.addr + addr_offset, data_end - data_offset, buf.data.data() + data_offset,
-                          nullptr);
+    bool ok = true;
+    ssize_t nwritten = t->write_bytes_helper(buf.addr + addr_offset, data_end - data_offset,
+                                             buf.data.data() + data_offset,&ok);
+
+    ASSERT(t, ok || buf.size_validation == MemWriteSizeValidation::CONSERVATIVE)
+        << "Should have written " << buf.data.size() << " bytes to " << (buf.addr + addr_offset)
+        << ", but only wrote " << nwritten;
+
     addr_offset += data_end - data_offset;
     data_offset = data_end;
   }

src/Task.cc

@@ -3213,17 +3213,13 @@ static ssize_t safe_pwrite64(Task* t, const void* buf, ssize_t buf_size,
   return nwritten;
 }
 
-void Task::write_bytes_helper(remote_ptr<void> addr, ssize_t buf_size,
+ssize_t Task::write_bytes_helper(remote_ptr<void> addr, ssize_t buf_size,
                               const void* buf, bool* ok, uint32_t flags) {
-  ASSERT(this, buf_size >= 0) << "Invalid buf_size " << buf_size;
-  if (0 == buf_size) {
-    return;
-  }
-
   ssize_t nwritten = write_bytes_helper_no_notifications(addr, buf_size, buf, ok, flags);
   if (nwritten > 0) {
     vm()->notify_written(addr, nwritten, flags);
   }
+  return nwritten;
 }
 
 ssize_t Task::write_bytes_helper_no_notifications(remote_ptr<void> addr, ssize_t buf_size,

src/Task.h

@@ -816,7 +816,7 @@ class Task {
   /**
    * |flags| is bits from WriteFlags.
    */
-  void write_bytes_helper(remote_ptr<void> addr, ssize_t buf_size,
+  ssize_t write_bytes_helper(remote_ptr<void> addr, ssize_t buf_size,
                           const void* buf, bool* ok = nullptr,
                           uint32_t flags = 0);
   /**

src/TraceStream.cc

@@ -422,6 +422,7 @@ void TraceWriter::write_frame(RecordTask* t, const Event& ev,
     w.setTid(r.rec_tid);
     w.setAddr(r.addr.as_int());
     w.setSize(r.size);
+    w.setSizeIsConservative(r.size_validation == MemWriteSizeValidation::CONSERVATIVE);
     auto holes = w.initHoles(r.holes.size());
     for (size_t j = 0; j < r.holes.size(); ++j) {
       holes[j].setOffset(r.holes[j].offset);
@@ -573,7 +574,8 @@ TraceFrame TraceReader::read_frame(FrameTime skip_before) {
       const auto& hole = holes[j];
       h[j] = { hole.getOffset(), hole.getSize() };
     }
-    raw_recs[i] = { w.getAddr(), (size_t)w.getSize(), i32_to_tid(w.getTid()), h };
+    raw_recs[i] = { w.getAddr(), (size_t)w.getSize(), i32_to_tid(w.getTid()), h,
+                    w.getSizeIsConservative() ? MemWriteSizeValidation::CONSERVATIVE : MemWriteSizeValidation::EXACT };
   }
 
   if (ret.global_time < skip_before) {
@@ -1237,8 +1239,9 @@ KernelMapping TraceReader::read_mapped_region(MappedData* data, bool* found,
 
 void TraceWriter::write_raw_header(pid_t rec_tid, size_t total_len,
                                    remote_ptr<void> addr,
-                                   const std::vector<WriteHole>& holes = std::vector<WriteHole>()) {
-  raw_recs.push_back({ addr, total_len, rec_tid, holes });
+                                   const std::vector<WriteHole>& holes,
+                                   MemWriteSizeValidation size_validation) {
+  raw_recs.push_back({ addr, total_len, rec_tid, holes, size_validation });
 }
 
 void TraceWriter::write_raw_data(const void* d, size_t len) {
@@ -1253,6 +1256,7 @@ bool TraceReader::read_raw_data_for_frame(RawData& d) {
   auto& rec = raw_recs[raw_recs.size() - 1];
   d.rec_tid = rec.rec_tid;
   d.addr = rec.addr;
+  d.size_validation = rec.size_validation;
 
   d.data.resize(rec.size);
   auto hole_iter = rec.holes.begin();
@@ -1285,6 +1289,7 @@ bool TraceReader::read_raw_data_for_frame_with_holes(RawDataWithHoles& d) {
   d.addr = rec.addr;
   d.holes = std::move(rec.holes);
   size_t data_size = rec.size;
+  d.size_validation = rec.size_validation;
   for (auto& h : d.holes) {
     data_size -= h.size;
   }

src/TraceStream.h

@@ -38,6 +38,16 @@ struct WriteHole {
   uint64_t size;
 };
 
+enum class MemWriteSizeValidation {
+  /* A data record where we _know_ that this data was all written, and it
+   * _must_ be replicated in the replayed process in its entirety */
+  EXACT,
+  /* We recorded a range of memory that's a conservative over-estimate of
+    * what was actually written, and it might not replay cleanly in its
+    * entirety in a replayed process */
+  CONSERVATIVE,
+};
+
 /**
  * TraceStream stores all the data common to both recording and
  * replay.  TraceWriter deals with recording-specific logic, and
@@ -56,6 +66,7 @@ class TraceStream {
     size_t size;
     pid_t rec_tid;
     std::vector<WriteHole> holes;
+    MemWriteSizeValidation size_validation;
   };
 
   /**
@@ -224,13 +235,15 @@ class TraceWriter : public TraceStream {
    * 'addr' is the address in the tracee where the data came from/will be
    * restored to.
    */
-  void write_raw(pid_t tid, const void* data, size_t len, remote_ptr<void> addr) {
+  void write_raw(pid_t tid, const void* data, size_t len, remote_ptr<void> addr,
+                 MemWriteSizeValidation size_validation = MemWriteSizeValidation::EXACT) {
     write_raw_data(data, len);
-    write_raw_header(tid, len, addr, std::vector<WriteHole>());
+    write_raw_header(tid, len, addr, std::vector<WriteHole>(), size_validation);
   }
   void write_raw_data(const void* data, size_t len);
   void write_raw_header(pid_t tid, size_t total_len, remote_ptr<void> addr,
-                        const std::vector<WriteHole>& holes);
+                        const std::vector<WriteHole>& holes,
+                        MemWriteSizeValidation size_validation = MemWriteSizeValidation::EXACT);
 
   /**
    * Write a task event (clone or exec record) to the trace.
@@ -344,6 +357,7 @@ class TraceReader : public TraceStream {
     std::vector<uint8_t> data;
     remote_ptr<void> addr;
     pid_t rec_tid;
+    MemWriteSizeValidation size_validation;
   };
 
   /**
@@ -354,6 +368,7 @@ class TraceReader : public TraceStream {
     remote_ptr<void> addr;
     pid_t rec_tid;
     std::vector<WriteHole> holes;
+    MemWriteSizeValidation size_validation;
   };
 
   /**

src/rr_trace.capnp

@@ -244,6 +244,10 @@ struct MemWrite {
   # A list of regions where zeroes are written. These are not
   # present in the compressed data.
   holes @3 :List(WriteHole);
+  # This is set for writes where we don't actually know that the write
+  # will apply in full in the replayee (e.g. for conservative sigframe
+  # captures when handling EV_SIGNAL).
+  sizeIsConservative @4 :Bool;
 }
 
 enum Arch {