Skip to content

Conversation

@jphamata
Copy link

Summary

This PR refactors FFI functions (qk_transpile_layout_initial_layout, qk_transpile_layout_output_permutation and qk_transpile_layout_final_layout) in transpile_layout.rs to avoid creating mutable references from raw pointers. The previous implementation used from_raw_parts_mut, which is not sound when the pointed memory may be uninitialized. This PR switches to ptr.add(i).write(...) loops.

Details and comments

Fixes #15370

@jphamata jphamata requested a review from a team as a code owner November 26, 2025 05:52
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Nov 26, 2025
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@CLAassistant
Copy link

CLAassistant commented Nov 26, 2025

CLA assistant check
All committers have signed the CLA.

@debasmita2102
Copy link

Thanks for the refactor improving memory safety in layout writing. I noticed there are no new or modified tests in this PR related to these changes. Could you please confirm if existing tests fully cover the affected code paths? If not, are there plans to add tests to verify these memory safety improvements and prevent regressions? This would help ensure the robustness of this fix.

@jphamata
Copy link
Author

Hi! Just to update, all existing tests that cover transpile_layout.rs passed (tox -e rust), confirming that the refactor didn’t change behavior. I also ran Miri, and the only undefined behavior it detected comes from another module (neighbors.rs), not from the code modified in this PR. So I believe the current coverage is sufficient, but I can look into adding a specific test if needed.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 19693981737

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.005%) to 88.251%

Files with Coverage Reduction New Missed Lines %
crates/transpiler/src/passes/unitary_synthesis.rs 1 92.57%
crates/qasm2/src/lex.rs 4 92.8%
crates/circuit/src/parameter/symbol_expr.rs 5 73.19%
Totals Coverage Status
Change from base Build 19684338744: 0.005%
Covered Lines: 94406
Relevant Lines: 106975

💛 - Coveralls

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

Labels

Community PR PRs from contributors that are not 'members' of the Qiskit repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve memory safety when writing to uninitialized buffers in C API

5 participants