Skip to content

Five independent simulator correctness fixes (memory safety, stubbed NimBLE helpers, flash-sleep guard)#200

Draft
faisal-shah wants to merge 1 commit into
InfiniTimeOrg:mainfrom
faisal-shah:sim-fixes-upstream
Draft

Five independent simulator correctness fixes (memory safety, stubbed NimBLE helpers, flash-sleep guard)#200
faisal-shah wants to merge 1 commit into
InfiniTimeOrg:mainfrom
faisal-shah:sim-fixes-upstream

Conversation

@faisal-shah

Copy link
Copy Markdown

Five self-contained fixes found while developing a companion app against InfiniSim. Each is independent — happy to split into separate PRs if you'd prefer, I've kept them together only because they're all small.

Opened as a draft for your feedback on whether these are wanted and in this shape.

1. MoveScreen: stack-buffer-overflow in the screen-slide animation

MoveScreen passes sdl_height as the flush-area height, but the on-stack color_p array only holds buffer_height = sdl_height - abs(height) valid rows. In the "up" case the pointer is also already advanced by sdl_width * abs(height), so monitor_flush reads sdl_width * abs(height) colours past the end of the array.

Found with ASan; it intermittently corrupted adjacent stack frames and crashed the simulator during ordinary app navigation. Fix passes buffer_height as the area height.

2. ble_uuid_cmp: implement it

The stub return 0 means "every UUID is equal", so any service that dispatches on the characteristic UUID matches its first characteristic for every access. Implemented for 16/32/128-bit.

3. os_mbuf_copydata / os_mbuf_append: implement them

Both were no-ops. Firmware written against the normal NimBLE idioms (os_mbuf_copydata to parse a write, os_mbuf_append to produce a read) therefore saw empty payloads and produced empty responses in the simulator only — the code works on hardware. os_mbuf_append also bounds-checks against a capacity the fake-mbuf creator supplies, so an over-long append is rejected rather than overrunning the caller's buffer.

4. SpiNorFlash: fail loudly on access while asleep

On hardware the flash is in deep power-down after Sleep(): reads return garbage, writes are dropped. The simulator served correct data regardless, which hides missing-wakeup bugs that only appear on a real watch. Now aborts with the offending operation. (abort() rather than an exception because the caller is often littlefs C code, which exceptions must not unwind.)

5. -funsigned-char

The firmware targets ARM, where plain char is unsigned; x86 makes it signed. Firmware code that stores byte values in a char — or compares one against a value above 127 — behaves differently in the simulator than on the watch. This makes the sim match the target.


Testing. Builds clean against the vendored InfiniTime submodule and runs. Fixes 1–4 were each found by an actual failure (1 by AddressSanitizer, 2–3 by services silently misbehaving only under the simulator, 4 by flash corruption that reproduced only on hardware).

Relationship to my other PRs. Independent of #198 and #199. It touches CMakeLists.txt, as #198 does, but in a different place (compile options vs. the source list), so they shouldn't conflict.

Five self-contained fixes found while building a companion app against
InfiniSim. Each stands alone; none depends on custom firmware.

1. MoveScreen: fix a stack-buffer-overflow in the screen-slide animation.
   The flush area was given sdl_height rows, but the on-stack colour buffer
   only holds `buffer_height` valid rows — and in the up case the pointer is
   already advanced by sdl_width*abs(height), so monitor_flush read that many
   colours past the end of the array. Found with ASan; it intermittently
   corrupted adjacent stack frames and crashed the sim during app navigation.

2. ble_uuid_cmp: implement it. The stub returned 0 for every pair, i.e. "all
   UUIDs are equal", which silently breaks any service that dispatches on the
   characteristic UUID (the first characteristic always won).

3. os_mbuf_copydata / os_mbuf_append: implement them. Both were no-ops, so
   firmware using the normal NimBLE parsing idioms saw empty payloads and
   produced empty responses in the simulator only.

4. SpiNorFlash: abort on access while asleep. On hardware the chip is in deep
   power-down: reads return garbage and writes are dropped. The sim happily
   served correct data, hiding missing-wakeup bugs that only appear on a real
   watch. Fails loudly instead.

5. -funsigned-char: the firmware targets ARM where `char` is unsigned; x86
   makes it signed, so byte-valued `char`s behave differently in the sim than
   on the watch.
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.

1 participant