Skip to content

fix(simics7): replace set-bookmark with snapshot for repro mode#354

Open
Wenzel wants to merge 2 commits into
mainfrom
worktree-fix-issue-287-simics7-bookmark
Open

fix(simics7): replace set-bookmark with snapshot for repro mode#354
Wenzel wants to merge 2 commits into
mainfrom
worktree-fix-issue-287-simics7-bookmark

Conversation

@Wenzel
Copy link
Copy Markdown
Contributor

@Wenzel Wenzel commented Jun 4, 2026

Summary

  • Gates the set-bookmark call with #[cfg(simics_version = "6")] — the command was removed in Simics 7.70+
  • On Simics 7, repro mode reuses the existing tsffs-origin-snapshot (already saved at harness start)
  • Adds REPRO_RESTORE_COMMAND constant to display the correct restore command per version in log messages
  • Updates docs to explain the version difference

Test plan

  • Verify cargo check passes on Simics 7 (no set-bookmark error)
  • Verify repro mode works on Simics 6 (bookmark flow unchanged)
  • Confirm log message prints correct command per Simics version

Fixes #287

🤖 Generated with Claude Code

Simics 7.70+ removed the `set-bookmark` / `reverse-to` CLI commands
that relied on reverse-execution micro-checkpoints. TSFFS repro mode
was calling `set-bookmark start` unconditionally, causing a SimExc_General
error on Simics 7.

This change:
- Gates the `set-bookmark` call with `#[cfg(simics_version = "6")]`
- On Simics 7, repro mode reuses the existing `tsffs-origin-snapshot`
  that is already saved at harness start
- Updates log messages to display version-appropriate restore commands:
  - Simics 6: "reverse-to start"
  - Simics 7: "restore-snapshot tsffs-origin-snapshot"
- Updates documentation to explain the difference between versions

Fixes #287

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates TSFFS repro-mode behavior to avoid using the removed Simics 7 set-bookmark CLI command by relying on the existing origin snapshot, while also making the “restore” instruction shown to users version-aware.

Changes:

  • Gate the repro set-bookmark start call behind #[cfg(simics_version = "6")] to avoid Simics 7 runtime CLI errors.
  • Introduce Tsffs::REPRO_RESTORE_COMMAND and use it in “Stopped for repro” log messages to show the correct restore command per Simics version.
  • Update documentation to explain the Simics 6 (bookmark) vs Simics 7 (snapshot) repro workflow.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/lib.rs Adds version-gated restore-command constant and gates set-bookmark for Simics 6 only.
src/haps/mod.rs Updates repro stop log lines to print the version-appropriate restore command.
docs/src/tutorials/edk2-uefi/reproducing-runs.md Documents Simics 6 vs 7 repro restore procedure and updates sample output.
docs/src/fuzzing/analyzing-results.md Clarifies repro “restore point” behavior differs between Simics 6 (bookmark) and Simics 7 (snapshot).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib.rs Outdated
Comment on lines +671 to +675
/// `set-bookmark`/`reverse-to`.
#[cfg(simics_version = "6")]
pub const REPRO_RESTORE_COMMAND: &'static str = "reverse-to start";
#[cfg(simics_version = "7")]
pub const REPRO_RESTORE_COMMAND: &'static str = "restore-snapshot tsffs-origin-snapshot";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Replaced the const with a repro_restore_command() method that constructs the Simics 7 variant via Self::SNAPSHOT_NAME, so it can't drift.

Comment on lines +113 to +116
SIMICS output:

```txt
[tsffs info] Stopped for repro. Restore to start bookmark with 'reverse-to start'
[tsffs info] Stopped for repro. Restore origin state with 'restore-snapshot tsffs-origin-snapshot'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The block is now labelled as Simics 7 output with a note that Simics 6 shows reverse-to start instead.

- Replace REPRO_RESTORE_COMMAND const with repro_restore_command() method
  so the Simics 7 variant is built from SNAPSHOT_NAME and cannot drift
- Label the GDB stub example output block as Simics 7 to avoid confusing
  Simics 6 users who will see a different restore command

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

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.

Simics 7: repro relies on unavailable set-bookmark CLI command

2 participants