Skip to content

Restrict unsafe blocks to single statements where possible. #15806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Cryoris wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Restrict unsafe blocks to single statements where possible. #15806

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c271174
Restrict `unsafe` blocks to single statements where possible.
Cryoris Mar 13, 2026
3e58fda
Reduce unsafe scopes further.
raynelfss Apr 28, 2026
7fd5489
Merge pull request #44 from raynelfss/cryoris-contrib-unsafe
Cryoris Apr 28, 2026
72033f6
Merge branch 'main' into unsafe-surgeon-mode
Cryoris Apr 29, 2026
19d5a1f
Merge branch 'unsafe-surgeon-mode' of github.com:Cryoris/qiskit-terra…
Cryoris Apr 29, 2026
3d9fe91
More precise safety docs
Cryoris Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 46 additions & 81 deletions crates/cext/src/circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::circuit_library::pbc::{CPauliProductMeasurement, CPauliProductRotatio
use crate::dag::COperationKind;
use crate::exit_codes::ExitCode;
use crate::pointers::{const_ptr_as_ref, mut_ptr_as_ref};
use crate::transpiler::target::parse_params;

use nalgebra::{Matrix2, Matrix4};
use ndarray::{Array2, ArrayView2};
Expand Down Expand Up @@ -407,53 +408,14 @@ pub unsafe extern "C" fn qk_circuit_gate(
) -> ExitCode {
// SAFETY: Per documentation, the pointer is non-null and aligned.
let circuit = unsafe { mut_ptr_as_ref(circuit) };
// SAFETY: Per the documentation the qubits and params pointers are arrays of num_qubits()
// and num_params() elements respectively.
unsafe {
let qargs: &[Qubit] = match gate.num_qubits() {
0 => &[],
1 => &[Qubit(*qubits.wrapping_add(0))],
2 => &[
Qubit(*qubits.wrapping_add(0)),
Qubit(*qubits.wrapping_add(1)),
],
3 => &[
Qubit(*qubits.wrapping_add(0)),
Qubit(*qubits.wrapping_add(1)),
Qubit(*qubits.wrapping_add(2)),
],
4 => &[
Qubit(*qubits.wrapping_add(0)),
Qubit(*qubits.wrapping_add(1)),
Qubit(*qubits.wrapping_add(2)),
Qubit(*qubits.wrapping_add(3)),
],
// There are no ``QkGate``s > 4 qubits
_ => unreachable!(),
};
let params: &[Param] = match gate.num_params() {
0 => &[],
1 => &[(*params.wrapping_add(0)).into()],
2 => &[
(*params.wrapping_add(0)).into(),
(*params.wrapping_add(1)).into(),
],
3 => &[
(*params.wrapping_add(0)).into(),
(*params.wrapping_add(1)).into(),
(*params.wrapping_add(2)).into(),
],
4 => &[
(*params.wrapping_add(0)).into(),
(*params.wrapping_add(1)).into(),
(*params.wrapping_add(2)).into(),
(*params.wrapping_add(3)).into(),
],
// There are no ``QkGate``s that take > 4 params
_ => unreachable!(),
};
circuit.push_standard_gate(gate, params, qargs).unwrap()
}
// SAFETY: Per documentation, qubits is readable for num_qubits elements of type u32
let qargs: &[Qubit] =
unsafe { ::std::slice::from_raw_parts(qubits as *const Qubit, gate.num_qubits() as usize) };

// SAFETY: Per documentation, the params points is compatible with the gate and safe to read.
let params = unsafe { parse_params(gate, params) };

circuit.push_standard_gate(gate, &params, qargs).unwrap();
ExitCode::Success
}

Expand Down Expand Up @@ -1308,31 +1270,33 @@ pub unsafe extern "C" fn qk_circuit_inst_pauli_product_measurement(
#[unsafe(no_mangle)]
pub unsafe extern "C" fn qk_circuit_instruction_clear(inst: *mut CInstruction) {
// SAFETY: Loading the data from pointers contained in a CInstruction. These should only be
// created by rust code and are constructed from Vecs internally or CStrings.
unsafe {
let inst = mut_ptr_as_ref(inst);
if inst.num_qubits > 0 && !inst.qubits.is_null() {
let qubits = std::slice::from_raw_parts_mut(inst.qubits, inst.num_qubits as usize);
let _: Box<[u32]> = Box::from_raw(qubits as *mut [u32]);
inst.qubits = std::ptr::null_mut();
}
inst.num_qubits = 0;
if inst.num_clbits > 0 && !inst.clbits.is_null() {
let clbits = std::slice::from_raw_parts_mut(inst.clbits, inst.num_clbits as usize);
let _: Box<[u32]> = Box::from_raw(clbits as *mut [u32]);
inst.clbits = std::ptr::null_mut();
}
inst.num_clbits = 0;
if inst.num_params > 0 && !inst.params.is_null() {
let params = std::slice::from_raw_parts_mut(inst.params, inst.num_params as usize);
let _ = Box::from_raw(params as *mut [f64]);
inst.params = std::ptr::null_mut();
}
inst.num_params = 0;
if !inst.name.is_null() {
let _ = CString::from_raw(inst.name);
inst.name = std::ptr::null_mut();
}
// created by Rust code and are constructed from Vecs internally or CStrings.
// This safety comment holds for all unsafe instructions in this function.
let inst = unsafe { mut_ptr_as_ref(inst) };
if inst.num_qubits > 0 && !inst.qubits.is_null() {
let qubits =
unsafe { std::slice::from_raw_parts_mut(inst.qubits, inst.num_qubits as usize) };
let _: Box<[u32]> = unsafe { Box::from_raw(qubits as *mut [u32]) };
inst.qubits = std::ptr::null_mut();
}
inst.num_qubits = 0;
if inst.num_clbits > 0 && !inst.clbits.is_null() {
let clbits =
unsafe { std::slice::from_raw_parts_mut(inst.clbits, inst.num_clbits as usize) };
let _: Box<[u32]> = unsafe { Box::from_raw(clbits as *mut [u32]) };
inst.clbits = std::ptr::null_mut();
}
inst.num_clbits = 0;
if inst.num_params > 0 && !inst.params.is_null() {
let params =
unsafe { std::slice::from_raw_parts_mut(inst.params, inst.num_params as usize) };
let _ = unsafe { Box::from_raw(params as *mut [f64]) };
inst.params = std::ptr::null_mut();
}
inst.num_params = 0;
if !inst.name.is_null() {
let _ = unsafe { CString::from_raw(inst.name) };
inst.name = std::ptr::null_mut();
}
}

Expand All @@ -1352,19 +1316,20 @@ pub unsafe extern "C" fn qk_opcounts_clear(op_counts: *mut OpCounts) {
if op_counts.len > 0 && !op_counts.data.is_null() {
// SAFETY: We load the box from a slice pointer created from
// the raw parts from the OpCounts::data attribute.
unsafe {
let slice: Box<[OpCount]> = Box::from_raw(std::ptr::slice_from_raw_parts_mut(
let slice: Box<[OpCount]> = unsafe {
Box::from_raw(std::ptr::slice_from_raw_parts_mut(
op_counts.data,
op_counts.len,
));
// free the allocated strings in each OpCount
for count in slice.iter() {
if !count.name.is_null() {
let _ = CString::from_raw(count.name as *mut c_char);
}
))
};
// free the allocated strings in each OpCount
for count in slice.iter() {
if !count.name.is_null() {
// SAFETY: Rust constructed this string, so we are fine to free it here
let _ = unsafe { CString::from_raw(count.name as *mut c_char) };
}
// the variable vec goes out of bounds and is freed too
}
// the variable vec goes out of bounds and is freed too
}
op_counts.len = 0;
op_counts.data = std::ptr::null_mut();
Expand Down
2 changes: 2 additions & 0 deletions crates/cext/src/circuit_library/pbc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ pub unsafe extern "C" fn qk_pauli_product_rotation_clear(inst: *mut CPauliProduc

// SAFETY: The user guarantees the instruction is coherent, i.e. the Z and X arrays are
// readable for the correct length and `angle` is a valid, non-null Param pointer.
// Note that every single line here is unsafe, hence we wrap it in a single block.
unsafe {
let x = ::std::slice::from_raw_parts_mut(inst.x, inst.len);
let _: Box<[bool]> = Box::from_raw(x);
Expand Down Expand Up @@ -152,6 +153,7 @@ pub unsafe extern "C" fn qk_pauli_product_measurement_clear(inst: *mut CPauliPro

// SAFETY: The user guarantees the instruction is coherent, i.e. the Z and X arrays are
// readable for the correct length and `angle` is a valid, non-null Param pointer.
// Note that every single line here is unsafe, hence we wrap it in a single block.
unsafe {
let x = ::std::slice::from_raw_parts_mut(inst.x, inst.len);
let _: Box<[bool]> = Box::from_raw(x);
Expand Down
106 changes: 35 additions & 71 deletions crates/cext/src/dag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
use anyhow::Error;
use hashbrown::HashMap;
use num_complex::Complex64;
use smallvec::smallvec;
use smallvec::SmallVec;

use crate::exit_codes::ExitCode;
use crate::transpiler::target::parse_params;
use qiskit_circuit::bit::{ClassicalRegister, QuantumRegister};
use qiskit_circuit::circuit_data::CircuitData;
use qiskit_circuit::dag_circuit::{DAGCircuit, NodeIndex, NodeType};
Expand Down Expand Up @@ -503,76 +504,39 @@ pub unsafe extern "C" fn qk_dag_apply_gate(
) -> u32 {
// SAFETY: Per documentation, the pointer is to valid data.
let dag = unsafe { mut_ptr_as_ref(dag) };
// SAFETY: Per the documentation the qubits and params pointers are arrays of num_qubits()
// and num_params() elements respectively.
unsafe {
let qargs: &[Qubit] = match gate.num_qubits() {
0 => &[],
1 => &[Qubit(*qubits.wrapping_add(0))],
2 => &[
Qubit(*qubits.wrapping_add(0)),
Qubit(*qubits.wrapping_add(1)),
],
3 => &[
Qubit(*qubits.wrapping_add(0)),
Qubit(*qubits.wrapping_add(1)),
Qubit(*qubits.wrapping_add(2)),
],
4 => &[
Qubit(*qubits.wrapping_add(0)),
Qubit(*qubits.wrapping_add(1)),
Qubit(*qubits.wrapping_add(2)),
Qubit(*qubits.wrapping_add(3)),
],
// There are no ``QkGate``s > 4 qubits
_ => panic!(),
};
let params = match gate.num_params() {
0 => None,
1 => Some(smallvec![(*params.wrapping_add(0)).into()]),
2 => Some(smallvec![
(*params.wrapping_add(0)).into(),
(*params.wrapping_add(1)).into(),
]),
3 => Some(smallvec![
(*params.wrapping_add(0)).into(),
(*params.wrapping_add(1)).into(),
(*params.wrapping_add(2)).into(),
]),
4 => Some(smallvec![
(*params.wrapping_add(0)).into(),
(*params.wrapping_add(1)).into(),
(*params.wrapping_add(2)).into(),
(*params.wrapping_add(3)).into(),
]),
// There are no ``QkGate``s that take > 4 params
_ => panic!(),
};
let new_node = if front {
dag.apply_operation_front(
gate.into(),
qargs,
&[],
params.map(Parameters::Params),
None,
#[cfg(feature = "cache_pygates")]
None,
)
.unwrap()
} else {
dag.apply_operation_back(
gate.into(),
qargs,
&[],
params.map(Parameters::Params),
None,
#[cfg(feature = "cache_pygates")]
None,
)
.unwrap()
};
new_node.index() as u32
}
// SAFETY: Per the documentation the qubits pointer is an arrays of num_qubits() elements
Copy link
Copy Markdown
Contributor

@gadial gadial Mar 15, 2026

Choose a reason for hiding this comment

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

Suggested change
// SAFETY: Per the documentation the qubits pointer is an arrays of num_qubits() elements
// SAFETY: Per the documentation the qubits pointer is an array of num_qubits() elements

let qargs: &[Qubit] =
unsafe { ::std::slice::from_raw_parts(qubits as *const Qubit, gate.num_qubits() as usize) };
let params: Option<SmallVec<[Param; 3]>> = if gate.num_params() == 0 {
None
} else {
// SAFETY: Per the documentation, params is readable for num_params elements of f64
Some(unsafe { parse_params(gate, params) })
};
let new_node = if front {
dag.apply_operation_front(
gate.into(),
qargs,
&[],
params.map(Parameters::Params),
None,
#[cfg(feature = "cache_pygates")]
None,
)
.unwrap()
} else {
dag.apply_operation_back(
gate.into(),
qargs,
&[],
params.map(Parameters::Params),
None,
#[cfg(feature = "cache_pygates")]
None,
)
.unwrap()
};
new_node.index() as u32
}

/// @ingroup QkDag
Expand Down
3 changes: 2 additions & 1 deletion crates/cext/src/transpiler/neighbors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ pub struct CNeighbors {
#[unsafe(no_mangle)]
pub unsafe extern "C" fn qk_neighbors_is_all_to_all(neighbors: *const CNeighbors) -> bool {
// SAFETY: per documentation, `neighbors` points to a valid initialized `CNeighbors`.
unsafe { (*neighbors).neighbors.is_null() && (*neighbors).partition.is_null() }
let neighbors = unsafe { const_ptr_as_ref(neighbors) };
Copy link
Copy Markdown
Member

@mtreinish mtreinish Apr 29, 2026

Choose a reason for hiding this comment

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

Doesn't this change the requirements for neighbors? This function asserts alignment while it is not documented as being required for this function.

Copy link
Copy Markdown
Collaborator Author

@Cryoris Cryoris Apr 29, 2026

Choose a reason for hiding this comment

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

Hmm.. isn't that an oversight from before, though? I thought that the pointer needed to be aligned to be dereferenced safely.

neighbors.neighbors.is_null() && neighbors.partition.is_null()
}

/// @ingroup QkNeighbors
Expand Down
Loading
⚡