Fix rewind buffer corruption bug

`RewindState` has a `RewindResult` which it checks when it is going to
truncat the rewind buffer (after a successful rewind.) If the
`RewindResult`'s info pointer is not set, `rewind_truncate_to` assumes
that the rewind was not successful, so it doesn't truncate.

The bug was that a local `RewindResult` was used instead of the one in
`RewindState`, so the info pointer was never set, so the rewind buffer
was never truncated. After a rewind, `rewind_append` would append states
that were out of order. This would break the binary search of the rewind
info.

To find the bug, I ended up adding a sanity check function for the
rewind buffer. So I decided to keep it.

Author: binji

src/host.c

@@ -315,12 +315,12 @@ void host_begin_rewind(Host* host) {
 Result host_rewind_to_cycles(Host* host, Cycles cycles) {
   assert(host->rewind_state.rewinding);
 
-  RewindResult result;
-  CHECK(SUCCESS(rewind_to_cycles(host->rewind_buffer, cycles, &result)));
+  RewindResult* result = &host->rewind_state.rewind_result;
+  CHECK(SUCCESS(rewind_to_cycles(host->rewind_buffer, cycles, result)));
 
   struct Emulator* e = host_get_emulator(host);
-  CHECK(SUCCESS(emulator_read_state(e, &result.file_data)));
-  assert(emulator_get_cycles(e) == result.info->cycles);
+  CHECK(SUCCESS(emulator_read_state(e, &result->file_data)));
+  assert(emulator_get_cycles(e) == result->info->cycles);
 
   host->rewind_state.current =
       joypad_find_state(host->joypad_buffer, emulator_get_cycles(e));
@@ -343,9 +343,11 @@ void host_end_rewind(Host* host) {
   assert(host->rewind_state.rewinding);
 
   if (host->rewind_state.rewind_result.info) {
-    rewind_truncate_to(host->rewind_buffer, &host->rewind_state.rewind_result);
+    struct Emulator* e = host_get_emulator(host);
+    rewind_truncate_to(host->rewind_buffer, e,
+                       &host->rewind_state.rewind_result);
     joypad_truncate_to(host->joypad_buffer, host->rewind_state.current);
-    host->last_cycles = emulator_get_cycles(host_get_emulator(host));
+    host->last_cycles = emulator_get_cycles(e);
   }
 
   memset(&host->rewind_state, 0, sizeof(host->rewind_state));

src/rewind.c

@@ -11,6 +11,8 @@
 
 #include "emulator.h"
 
+#define SANITY_CHECK 0
+
 #define INVALID_CYCLES (~0ULL)
 
 #define GET_CYCLES(x) ((x).cycles)
@@ -85,6 +87,8 @@
     }                                      \
   } while (0)
 
+static void rewind_sanity_check(RewindBuffer*, struct Emulator*);
+
 RewindBuffer* rewind_new(const RewindInit* init, struct Emulator* e) {
   RewindBuffer* buffer = malloc(sizeof(RewindBuffer));
   ZERO_MEMORY(*buffer);
@@ -285,6 +289,8 @@ void rewind_append(RewindBuffer* buf, struct Emulator* e) {
   /* Update stats. */
   buf->total_kind_bytes[kind] += new_info->size;
   buf->total_uncompressed_bytes += buf->last_state.size;
+
+  rewind_sanity_check(buf, e);
 }
 
 Result rewind_to_cycles(RewindBuffer* buf, Cycles cycles,
@@ -359,7 +365,8 @@ Result rewind_to_cycles(RewindBuffer* buf, Cycles cycles,
   return OK;
 }
 
-void rewind_truncate_to(RewindBuffer* buffer, RewindResult* result) {
+void rewind_truncate_to(RewindBuffer* buffer, struct Emulator* e,
+                        RewindResult* result) {
   /* Remove data from rewind buffer that are now invalid. */
   int info_range_index = result->info_range_index;
   RewindInfo* info = result->info;
@@ -371,6 +378,8 @@ void rewind_truncate_to(RewindBuffer* buffer, RewindResult* result) {
     info_range[0].begin = info_range[0].end;
     data_range[0].end = data_range[0].begin;
   }
+
+  rewind_sanity_check(buffer, e);
 }
 
 static Bool is_rewind_range_empty(RewindInfoRange* r) {
@@ -431,3 +440,80 @@ RewindStats rewind_get_stats(RewindBuffer* buffer) {
 
   return stats;
 }
+
+void rewind_sanity_check(RewindBuffer* buffer, struct Emulator* e) {
+#if SANITY_CHECK
+  assert(buffer->data_range[0].begin <= buffer->data_range[0].end);
+  assert(buffer->data_range[0].end <= buffer->data_range[1].begin);
+  assert((void*)buffer->data_range[0].end <=
+         (void*)buffer->info_range[1].begin);
+  assert(buffer->info_range[1].end <= buffer->info_range[0].begin);
+  assert(buffer->info_range[0].begin <= buffer->info_range[0].end);
+
+  Bool has_base = FALSE;
+  FileData base;
+  FileData diff;
+  FileData temp;
+  emulator_init_state_file_data(&base);
+  emulator_init_state_file_data(&diff);
+  emulator_init_state_file_data(&temp);
+
+  Result result = emulator_write_state(e, &temp);
+  assert(SUCCESS(result));
+
+  void* last_data_end = NULL;
+  Cycles last_cycles = 0;
+
+  int i;
+  for (i = 0; i < 2; ++i) {
+    RewindDataRange* data_range = &buffer->data_range[i];
+    RewindInfoRange* info_range = &buffer->info_range[i];
+
+    RewindInfo* info;
+    for (info = info_range->end - 1; info >= info_range->begin; --info) {
+      void* data_end = info->data + info->size;
+      if (last_data_end) {
+        assert(last_data_end < (void*)data_end);
+      }
+      last_data_end = data_end;
+    }
+  }
+
+  for (i = 1; i >= 0; --i) {
+    RewindDataRange* data_range = &buffer->data_range[i];
+    RewindInfoRange* info_range = &buffer->info_range[i];
+
+    RewindInfo* info;
+    for (info = info_range->end - 1; info >= info_range->begin; --info) {
+      assert(last_cycles <= info->cycles);
+      last_cycles = info->cycles;
+
+      FileData* fd = NULL;
+      if (info->kind == RewindInfoKind_Base) {
+        has_base = TRUE;
+        decode_rle(info->data, info->size, base.data, base.data + base.size);
+        fd = &base;
+      } else {
+        assert(info->kind == RewindInfoKind_Diff);
+        if (has_base) {
+          decode_diff(info->data, info->size, base.data, diff.data,
+                      diff.data + diff.size);
+          fd = &diff;
+        }
+      }
+
+      if (fd) {
+        emulator_read_state(e, fd);
+        assert(info->cycles == emulator_get_cycles(e));
+      }
+    }
+  }
+
+  result = emulator_read_state(e, &temp);
+  assert(SUCCESS(result));
+
+  file_data_delete(&temp);
+  file_data_delete(&diff);
+  file_data_delete(&base);
+#endif /* SANITY_CHECK */
+}

src/rewind.h

@@ -104,7 +104,7 @@ RewindBuffer* rewind_new(const RewindInit*, struct Emulator*);
 void rewind_delete(RewindBuffer*);
 void rewind_append(RewindBuffer*, struct Emulator*);
 Result rewind_to_cycles(RewindBuffer*, Cycles, RewindResult*);
-void rewind_truncate_to(RewindBuffer*, RewindResult*);
+void rewind_truncate_to(RewindBuffer*, struct Emulator*, RewindResult*);
 Cycles rewind_get_oldest_cycles(RewindBuffer*);
 Cycles rewind_get_newest_cycles(RewindBuffer*);
 RewindStats rewind_get_stats(RewindBuffer*);