Skip to content

Commit

Permalink
Validate parameters of each gate in a circuit (#1834)
Browse files Browse the repository at this point in the history
This commit adds checks of arguments (A number of qubits, clbits,
and parameters) for each gate to prevent from unexpected memory access
when a user defines wrong custom gate with a number of a standard gate.

* validate parameters of each gate in a circuit
* fix lint error
* fix lint error
  • Loading branch information
hhorii authored Jun 6, 2023
1 parent 91a60b7 commit 747bf6d
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 127 deletions.
61 changes: 6 additions & 55 deletions qiskit_aer/backends/aer_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,62 +636,13 @@ def _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, c
copied = True
params[i] = 0.0

# fmt: off
if name in {
"ccx",
"ccz",
"cp",
"cswap",
"csx",
"cx",
"cy",
"cz",
"delay",
"ecr",
"h",
"id",
"mcp",
"mcphase",
"mcr",
"mcrx",
"mcry",
"mcrz",
"mcswap",
"mcsx",
"mcu",
"mcu1",
"mcu2",
"mcu3",
"mcx",
"mcx_gray",
"mcy",
"mcz",
"p",
"r",
"rx",
"rxx",
"ry",
"ryy",
"rz",
"rzx",
"rzz",
"s",
"sdg",
"swap",
"sx",
"sxdg",
"t",
"tdg",
"u",
"x",
"y",
"z",
"u1",
"u2",
"u3",
"cu",
"cu1",
"cu2",
"cu3",
"ccx", "ccz", "cp", "cswap", "csx", "cx", "cy", "cz", "delay", "ecr", "h",
"id", "mcp", "mcphase", "mcr", "mcrx", "mcry", "mcrz", "mcswap", "mcsx",
"mcu", "mcu1", "mcu2", "mcu3", "mcx", "mcx_gray", "mcy", "mcz", "p", "r",
"rx", "rxx", "ry", "ryy", "rz", "rzx", "rzz", "s", "sdg", "swap", "sx", "sxdg",
"t", "tdg", "u", "x", "y", "z", "u1", "u2", "u3", "cu", "cu1", "cu2", "cu3",
}:
aer_circ.gate(name, qubits, params, [], conditional_reg, label if label else name)
elif name == "measure":
Expand Down
7 changes: 7 additions & 0 deletions releasenotes/notes/check_param_length-eb69cd92825bbca4.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
fixes:
- |
Previously, parameters for gates are not validate in C++. If parameters are shorter than
expected (due to custom gate), segmentaion faults are thrown. This commit adds checks
whether parameter lenght is expceted. This commit will fix issues reported in #1612.
https://github.com/Qiskit/qiskit-aer/issues/1612
1 change: 1 addition & 0 deletions src/framework/circuit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class Circuit {
const int_t cond_regidx = -1, const std::string label = "") {
ops.push_back(Operations::make_gate(name, qubits, params, string_params,
cond_regidx, label));
check_gate_params(ops.back());
}

void diagonal(const reg_t &qubits, const cvector_t &vec,
Expand Down
72 changes: 54 additions & 18 deletions src/framework/operations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,32 +349,72 @@ inline void check_empty_name(const Op &op) {
// Raise an exception if qubits list is empty
inline void check_empty_qubits(const Op &op) {
if (op.qubits.empty())
throw std::invalid_argument(R"(Invalid qobj ")" + op.name +
R"(" instruction ("qubits" is empty).)");
throw std::invalid_argument(R"(Invalid operation ")" + op.name +
R"(" ("qubits" is empty).)");
}

// Raise an exception if params is empty
inline void check_empty_params(const Op &op) {
if (op.params.empty())
throw std::invalid_argument(R"(Invalid qobj ")" + op.name +
R"(" instruction ("params" is empty).)");
throw std::invalid_argument(R"(Invalid operation ")" + op.name +
R"(" ("params" is empty).)");
}

// Raise an exception if qubits is more than expected
inline void check_length_qubits(const Op &op, const size_t size) {
if (op.qubits.size() < size)
throw std::invalid_argument(R"(Invalid operation ")" + op.name +
R"(" ("qubits" is incorrect length).)");
}

// Raise an exception if params is empty
inline void check_length_params(const Op &op, const size_t size) {
if (op.params.size() != size)
throw std::invalid_argument(
R"(Invalid qobj ")" + op.name +
R"(" instruction ("params" is incorrect length).)");
if (op.params.size() < size)
throw std::invalid_argument(R"(Invalid operation ")" + op.name +
R"(" ("params" is incorrect length).)");
}

// Raise an exception if qubits list contains duplications
inline void check_duplicate_qubits(const Op &op) {
auto cpy = op.qubits;
std::unique(cpy.begin(), cpy.end());
if (cpy != op.qubits)
throw std::invalid_argument(R"(Invalid qobj ")" + op.name +
R"(" instruction ("qubits" are not unique).)");
throw std::invalid_argument(R"(Invalid operation ")" + op.name +
R"(" ("qubits" are not unique).)");
}

inline void check_gate_params(const Op &op) {
const stringmap_t<std::tuple<int_t, int_t>> param_tables(
{{"u1", {1, 1}}, {"u2", {1, 2}}, {"u3", {1, 3}},
{"u", {1, 3}}, {"U", {1, 3}}, {"CX", {2, 0}},
{"cx", {2, 0}}, {"cz", {2, 0}}, {"cy", {2, 0}},
{"cp", {2, 1}}, {"cu1", {2, 1}}, {"cu2", {2, 2}},
{"cu3", {2, 3}}, {"swap", {2, 0}}, {"id", {0, 0}},
{"p", {1, 1}}, {"x", {1, 0}}, {"y", {1, 0}},
{"z", {1, 0}}, {"h", {1, 0}}, {"s", {1, 0}},
{"sdg", {1, 0}}, {"t", {1, 0}}, {"tdg", {1, 0}},
{"r", {1, 2}}, {"rx", {1, 1}}, {"ry", {1, 1}},
{"rz", {1, 1}}, {"rxx", {2, 1}}, {"ryy", {2, 1}},
{"rzz", {2, 1}}, {"rzx", {2, 1}}, {"ccx", {3, 0}},
{"cswap", {3, 0}}, {"mcx", {1, 0}}, {"mcy", {1, 0}},
{"mcz", {1, 0}}, {"mcu1", {1, 1}}, {"mcu2", {1, 2}},
{"mcu3", {1, 3}}, {"mcswap", {2, 0}}, {"mcphase", {1, 1}},
{"mcr", {1, 1}}, {"mcrx", {1, 1}}, {"mcry", {1, 1}},
{"mcrz", {1, 1}}, {"sx", {1, 0}}, {"sxdg", {1, 0}},
{"csx", {2, 0}}, {"mcsx", {1, 0}}, {"csxdg", {2, 0}},
{"mcsxdg", {1, 0}}, {"delay", {1, 0}}, {"pauli", {1, 0}},
{"mcx_gray", {1, 0}}, {"cu", {2, 4}}, {"mcu", {1, 4}},
{"mcp", {1, 1}}, {"ecr", {2, 0}}});

auto it = param_tables.find(op.name);
if (it == param_tables.end()) {
std::stringstream msg;
msg << "Invalid gate name :\"" << op.name << "\"." << std::endl;
throw std::invalid_argument(msg.str());
} else {
check_length_qubits(op, std::get<0>(it->second));
check_length_params(op, std::get<1>(it->second));
}
}

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -1155,12 +1195,8 @@ Op input_to_op_gate(const inputdata_t &input) {
check_empty_name(op);
check_empty_qubits(op);
check_duplicate_qubits(op);
if (op.name == "u1")
check_length_params(op, 1);
else if (op.name == "u2")
check_length_params(op, 2);
else if (op.name == "u3")
check_length_params(op, 3);
check_gate_params(op);

return op;
}

Expand Down Expand Up @@ -1586,8 +1622,8 @@ Op input_to_op_save_expval(const inputdata_t &input, bool variance) {
}

// Check edge case of all coefficients being empty
// In this case the operator had all coefficients zero, or sufficiently close
// to zero that they were all truncated.
// In this case the operator had all coefficients zero, or sufficiently
// close to zero that they were all truncated.
if (op.expval_params.empty()) {
std::string pauli(op.qubits.size(), 'I');
op.expval_params.emplace_back(pauli, 0., 0.);
Expand Down
2 changes: 1 addition & 1 deletion src/simulators/statevector/statevector_state.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const Operations::OpSet StateOpSet(
"sdg", "t", "tdg", "r", "rx", "ry", "rz",
"rxx", "ryy", "rzz", "rzx", "ccx", "cswap", "mcx",
"mcy", "mcz", "mcu1", "mcu2", "mcu3", "mcswap", "mcphase",
"mcr", "mcrx", "mcry", "mcry", "sx", "sxdg", "csx",
"mcr", "mcrx", "mcry", "mcrz", "sx", "sxdg", "csx",
"mcsx", "csxdg", "mcsxdg", "delay", "pauli", "mcx_gray", "cu",
"mcu", "mcp", "ecr"});

Expand Down
28 changes: 27 additions & 1 deletion test/terra/backends/aer_simulator/test_circuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from ddt import ddt
import numpy as np
from qiskit import ClassicalRegister, QuantumCircuit, QuantumRegister, assemble
from qiskit.circuit import CircuitInstruction
from qiskit.circuit.gate import Gate
from qiskit.circuit.library.standard_gates import HGate
from test.terra.reference import ref_algorithms

from test.terra.backends.simulator_test_case import SimulatorTestCase, supported_methods
Expand Down Expand Up @@ -237,3 +238,28 @@ def test_floating_shots(self):
shots = int(shots)
self.assertSuccess(result)
self.assertEqual(sum([result.get_counts()[key] for key in result.get_counts()]), shots)

def test_invalid_parameters(self):
"""Test gates with invalid parameter length."""

backend = self.backend()

class Custom(Gate):
def __init__(self, label=None):
super().__init__("p", 1, [], label=label)

def _define(self):
q = QuantumRegister(1, "q")
qc = QuantumCircuit(q, name=self.name)
qc._append(HGate(), [q[0]], [])
self.definition = qc

qc = QuantumCircuit(1)
qc.append(Custom(), [0])
qc.measure_all()

try:
backend.run(qc).result()
self.fail("do not reach here")
except Exception as e:
self.assertTrue('"params" is incorrect length' in repr(e))
59 changes: 7 additions & 52 deletions test/terra/backends/aer_simulator/test_standard_gates.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,59 +20,14 @@
from qiskit import transpile
import qiskit.quantum_info as qi

# fmt: off
from qiskit.circuit.library.standard_gates import (
CXGate,
CYGate,
CZGate,
DCXGate,
HGate,
IGate,
SGate,
SXGate,
SXdgGate,
SdgGate,
SwapGate,
XGate,
YGate,
ZGate,
TGate,
TdgGate,
iSwapGate,
C3XGate,
C4XGate,
CCXGate,
CHGate,
CSXGate,
CSwapGate,
CPhaseGate,
CRXGate,
CRYGate,
CRZGate,
CUGate,
CU1Gate,
CU3Gate,
CUGate,
PhaseGate,
RC3XGate,
RCCXGate,
RGate,
RXGate,
RXXGate,
RYGate,
RYYGate,
RZGate,
RZXGate,
RZZGate,
UGate,
U1Gate,
U2Gate,
U3Gate,
UGate,
MCXGate,
MCPhaseGate,
MCXGrayCode,
ECRGate,
)
CXGate, CYGate, CZGate, DCXGate, HGate, IGate, SGate, SXGate, SXdgGate,
SdgGate, SwapGate, XGate, YGate, ZGate, TGate, TdgGate, iSwapGate, C3XGate,
C4XGate, CCXGate, CHGate, CSXGate, CSwapGate, CPhaseGate, CRXGate, CRYGate,
CRZGate, CUGate, CU1Gate, CU3Gate, CUGate, PhaseGate, RC3XGate, RCCXGate, RGate,
RXGate, RXXGate, RYGate, RYYGate, RZGate, RZXGate, RZZGate, UGate, U1Gate, U2Gate,
U3Gate, UGate, MCXGate, MCPhaseGate, MCXGrayCode, ECRGate)


CLIFFORD_GATES = [
Expand Down

0 comments on commit 747bf6d

Please sign in to comment.