fsm_compact_states must remap endids, to avoid dangling references.

src/libfsm/endids.c

@@ -880,6 +880,8 @@ fsm_endid_iter_bulk(const struct fsm *fsm,
  return 1;
  }
 
+ const fsm_state_t state_count = fsm_countstates(fsm);
+
  bucket_count = ei->bucket_count;
 
  for (b_i = 0; b_i < bucket_count; b_i++) {
@@ -891,6 +893,7 @@ fsm_endid_iter_bulk(const struct fsm *fsm,
 
  count = b->ids->count;
 
+ assert(b->state < state_count);
  if (!cb(b->state, &b->ids->ids[0], count, opaque)) {
  return 0;
  }
@@ -1026,3 +1029,62 @@ fsm_increndids(struct fsm * fsm, int delta)
  fsm_mapendids(fsm, incr_remap, &delta);
 }
 
+int
+fsm_endid_compact(struct fsm *fsm, fsm_state_t *mapping, size_t mapping_count)
+{
+ /* Don't reallocate unless something has actually changed. */
+ bool changes = false;
+ for (size_t i = 0; i < mapping_count; i++) {
+ if (mapping[i] != i) {
+ changes = true;
+ break;
+ }
+ }
+
+ /* nothing to do */
+ if (!changes) { return 1; }
+
+ struct endid_info *ei = fsm->endid_info;
+
+ struct endid_info_bucket *nbuckets = f_malloc(fsm->alloc,
+ ei->bucket_count * sizeof(nbuckets[0]));
+ if (nbuckets == NULL) {
+ return 0;
+ }
+
+ const uint64_t mask = ei->bucket_count - 1;
+ assert((ei->bucket_count & mask) == 0);
+
+ /* initialize to empty */
+ for (size_t nb_i = 0; nb_i < ei->bucket_count; nb_i++) {
+ nbuckets[nb_i].state = BUCKET_NO_STATE;
+ }
+
+ for (size_t ob_i = 0; ob_i < ei->bucket_count; ob_i++) {
+ const struct endid_info_bucket *ob = &ei->buckets[ob_i];
+ if (ob->state == BUCKET_NO_STATE) { continue; }
+
+ assert(ob->state < mapping_count);
+ const fsm_state_t nstate = mapping[ob->state];
+ if (nstate == FSM_STATE_REMAP_NO_STATE) { continue; }
+
+ const uint64_t hash = hash_id(nstate);
+
+ bool placed = false;
+ for (size_t probes = 0; probes < ei->bucket_count; probes++) {
+ const size_t nb_i = (hash + probes) & mask;
+ struct endid_info_bucket *nb = &nbuckets[nb_i];
+ if (nb->state == BUCKET_NO_STATE) {
+ nb->state = nstate;
+ nb->ids = ob->ids;
+ placed = true;
+ break;
+ }
+ }
+ assert(placed);
+ }
+
+ f_free(fsm->alloc, ei->buckets);
+ ei->buckets = nbuckets;
+ return 1;
+}

src/libfsm/endids.h

@@ -63,4 +63,7 @@ fsm_endid_iter_state(const struct fsm *fsm, fsm_state_t state,
 void
 fsm_endid_dump(FILE *f, const struct fsm *fsm);
 
+int
+fsm_endid_compact(struct fsm *fsm, fsm_state_t *mapping, size_t mapping_count);
+
 #endif

src/libfsm/state.c

@@ -18,6 +18,7 @@
 #include <adt/edgeset.h>
 
 #include "internal.h"
+#include "endids.h"
 
 int
 fsm_addstate(struct fsm *fsm, fsm_state_t *state)
@@ -179,7 +180,7 @@ fsm_compact_states(struct fsm *fsm,
  const fsm_state_t orig_statecount = fsm->statecount;
 
  fsm_state_t *mapping = f_malloc(fsm->alloc,
- fsm->statecount * sizeof(mapping[0]));
+ orig_statecount * sizeof(mapping[0]));
  if (mapping == NULL) {
  return 0;
  }
@@ -254,6 +255,10 @@ fsm_compact_states(struct fsm *fsm,
  }
  }
 
+ /* Remap end metadata */
+ if (!fsm_endid_compact(fsm, mapping, orig_statecount)) {
+ return 0;
+ }
  assert(dst == kept);
  assert(kept == fsm->statecount);
 

tests/endids/endids_trim_must_compact_endids.c

@@ -0,0 +1,64 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include <assert.h>
+
+#include <fsm/fsm.h>
+#include <fsm/print.h>
+
+/* Regression: When fsm_trim called fsm_compact_states it previously
+ * didn't remap endids, so the endid collection could refer to stale
+ * state IDs or past the end of fsm->states[]. */
+int main(void)
+{
+ size_t limit = 100;
+ const fsm_end_id_t end_id = 12345;
+
+ /* Create FSMs with a bunch of garbage states, to increase
+ * the likelihood that the endid's state not being updated
+ * after calling fsm_trim triggers the bug. */
+ for (size_t end_state = 1; end_state < limit; end_state++) {
+ struct fsm *fsm = fsm_new(NULL);
+ assert(fsm != NULL);
+
+ for (size_t i = 0; i < limit; i++) {
+ if (!fsm_addstate(fsm, NULL)) {
+ return EXIT_FAILURE;
+ }
+ }
+
+ const fsm_state_t start_state = end_state - 1;
+ fsm_setstart(fsm, start_state);
+ fsm_setend(fsm, end_state, 1);
+
+ /* add an any edge, so the end state is reachable */
+ if (!fsm_addedge_any(fsm, start_state, end_state)) {
+ return EXIT_FAILURE;
+ }
+
+ if (!fsm_setendid(fsm, end_id)) {
+ return EXIT_FAILURE;
+ }
+
+ /* Call fsm_trim, as fsm_minimise would. */
+ (void)fsm_trim(fsm, FSM_TRIM_START_AND_END_REACHABLE, NULL);
+
+ /* Execute the fsm and check that the endid's state was updated. */
+ fsm_state_t end;
+ const char *input = "x";
+ int ret = fsm_exec(fsm, fsm_sgetc, &input, &end, NULL);
+ assert(ret == 1);
+
+ size_t count = fsm_endid_count(fsm, end);
+ assert(count == 1);
+
+ fsm_end_id_t id_buf[1];
+ if (!fsm_endid_get(fsm, end, 1, id_buf)) {
+ return EXIT_FAILURE;
+ }
+ assert(id_buf[0] == end_id);
+
+ fsm_free(fsm);
+ }
+ return EXIT_SUCCESS;
+}