Skip to content

Construct SimSprite and initialize pointer members - #17

Open
jquast wants to merge 1 commit into
SimHacker:mainfrom
jquast:up-fix-sprite-unconstructed-string
Open

jquast wants to merge 1 commit into
SimHacker:mainfrom
jquast:up-fix-sprite-unconstructed-string

Conversation

@jquast

@jquast jquast commented Sep 29, 2026 •

Copy link
Copy Markdown

A series of memory violations, each found with the new make asan build:

  • the SimSprite allocated by newPtr in newSprite() is left unconstructed
  • Micropolis::callback, mapBase and mopBase are read before assignment

#12 contains the same fixes, and this PR also Closes #11. It allocates the sprite with new SimSprite(); this constructs it over the pooled allocation. It sets callback in the constructor; this initializes it where it is declared, along with mapBase and mopBase.

Detected by running sim smoke:

$ make asan
$ cd apps/micropolis
$ for i in $(seq 1 12); do echo "run #"$i: $(node_modules/.bin/tsx cli/entry.ts sim smoke --city /cities/kobe.cty --ticks 480 2>&1 | grep RuntimeError); done
run #1: RuntimeError: memory access out of bounds
run #2: RuntimeError: memory access out of bounds
run #3: RuntimeError: memory access out of bounds
run #4: RuntimeError: memory access out of bounds
run #5: RuntimeError: memory access out of bounds
run #6: RuntimeError: memory access out of bounds
run #7: RuntimeError: memory access out of bounds
run #8: RuntimeError: memory access out of bounds
run #9: RuntimeError: memory access out of bounds
run #10: RuntimeError: memory access out of bounds
run #11: RuntimeError: memory access out of bounds
run #12: RuntimeError: memory access out of bounds

Depends on #15 (the loader fix) and #16 (the make asan target) to run the commands above.

newSprite() allocated SimSprite with newPtr (malloc) and then assigned
to its std::string name member while the member held unconstructed
storage, which is undefined behaviour that worked on a zero-filled heap.
Construct the object in place over the existing pooled allocation.
Micropolis::callback, mapBase and mopBase have the same problem: all
three are read before assignment in guarded paths (setCallback deletes
the previous callback; initMapArrays guards on the base pointers), so
give them NULL default initializers.
@jquast
jquast marked this pull request as ready for review September 29, 2026 17:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermittent memory access out of bounds in simTick() — unconstructed std::string in SimSprite

1 participant