Repository navigation
Conversation
add_architecture_hint, repro and solution passed caller pointers straight to CStr::from_ptr, so a NULL (e.g. Python None) segfaulted Simics. Check for NULL first and return an error instead.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
What changed in this PR
Prevents UB/segfaults when Simics interface methods receive NULL string pointers by rejecting NULL before calling CStr::from_ptr, and adds a regression test for the reported crash.
Changes:
- Added
util::c_str_arghelper to reject NULL and convert C string args safely (w.r.t. NULL). - Updated interface methods (
add_architecture_hint,repro,solution) to usec_str_arginstead ofCStr::from_ptrdirectly. - Added regression test ensuring
None/NULL string args fail with a clear error rather than segfaulting.
| File | Description |
|---|---|
| tests/null_string_args_latest.rs | Adds regression coverage for NULL string interface argument handling. |
| src/util/mod.rs | Introduces c_str_arg helper to reject NULL prior to C string conversion. |
| src/interfaces/fuzz.rs | Switches string pointer handling in repro/solution to the new helper. |
| src/interfaces/config.rs | Switches add_architecture_hint to the new helper. |
Review effort: Lite
Copilot stopped reviewing on behalf of
Wenzel due to an error
October 23, 2026 13:49
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #123
Summary
add_architecture_hint,reproandsolutionpassed the caller'schar *straight toCStr::from_ptr. A NULL pointer (e.g. PythonNone) is UB and segfaults Simics. Since?only covers the UTF-8 check, which runs after the read, it doesn't catch this.A NULL now returns an error like any other invalid argument. The generated
ffi(expect)wrapper then reports it asFailed to execute FFI method solution: message must not be NULL. Non-NUL-terminated or dangling pointers can't be detected from Rust, so that stays the caller's contract.c_str_argis anunsafe fnwith a# Safetysection.Evidence
New test
tests/null_string_args_latest.rsloads tsffs and callstsffs.iface.fuzz.solution(1, None).Segmentation fault (SIGSEGV) in main thread ("simics")from libc attest.pyline 5. Test FAILED.must not be NULLpanic message. Test ok.cargo clippy --all-targetsis clean.Merge Danger
Door: two-way
Blast Radius: small
Only NULL inputs behave differently. They used to crash Simics with a segfault; now they abort through the existing interface-error panic path with a clear message.