Add validation logic on user-provided inputs

[mhucka]

b/510433099

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the robustness of the qsim circuit parser by initializing qubit index variables and adding validation for qubit counts and ID parsing. It replaces ignored return values from absl::SimpleAtoi with proper error handling that returns an InvalidArgument status. The reviewer identified a critical missing piece across all modified functions: the lack of bounds checking for parsed qubit indices against the total number of qubits (num_qubits). Without these checks, the code remains vulnerable to underflow or invalid index calculations. The feedback provides specific code suggestions to ensure all qubit indices are validated to be within the range [0, num_qubits - 1].

[gemini-code-assist]

Code Review

This pull request improves the robustness of the qsim circuit parser by initializing qubit index variables and adding validation for qubit counts and parsing results. While these changes are a good step, the reviewer identified several critical missing checks: qubit indices are not validated against the total number of qubits (num_qubits), which could lead to unsigned underflow or out-of-bounds memory access. Additionally, for multi-qubit gates, the feedback recommends ensuring that qubit indices are distinct and, where signed integers are used, checking for negative values to prevent invalid memory indexing.

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the robustness of the circuit parser in circuit_parser_qsim.cc by adding comprehensive validation for qubit identifiers across various gate and noise channel parsing functions. The changes ensure that qubit IDs are present, successfully parsed as integers, and fall within the valid range of available qubits to prevent out-of-bounds access. A review comment correctly identified a missing range check in the SingleEigenGate function that could lead to an unsigned integer underflow during qubit index calculation.

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request significantly improves the robustness of the circuit parser by adding comprehensive validation for qubit counts, parsing errors, and boundary checks. The reviewer recommends extending this validation to ensure that two-qubit gates (such as FsimGate and PhasedISwapGate) are applied to distinct qubits, preventing physically invalid operations that could lead to undefined behavior in the simulator backend.

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds comprehensive input validation for qubit IDs in circuit_parser_qsim.cc, checking for parsing errors, out-of-bounds indices, and duplicate qubits in multi-qubit gates. The reviewer suggests extending this validation to control qubits to prevent duplicates and overlaps with target qubits. Furthermore, the reviewer recommends using unsigned int consistently for all qubit IDs to ensure type safety and simplify the validation logic by eliminating redundant checks for negative values.

[gemini-code-assist]

The validation logic for control qubits should also check for duplicate control qubits and ensure that control qubits do not overlap with the target qubits of the operation. A qubit cannot act as both a control and a target in the same operation.

    if (!absl::SimpleAtoi(tok, &tmp) || tmp >= num_qubits) {
      return Status(absl::StatusCode::kInvalidArgument,
                    "Invalid control qubit: " + std::string(tok));
    }
    for (const auto& q : op.qubits()) {
      unsigned int target_id;
      if (absl::SimpleAtoi(q.id(), &target_id) && target_id == tmp) {
        return Status(absl::StatusCode::kInvalidArgument,
                      "Control qubit overlaps with target qubit: " + std::string(tok));
      }
    }
    for (const auto& existing_q : *control_qubits) {
      if (existing_q == num_qubits - tmp - 1) {
        return Status(absl::StatusCode::kInvalidArgument,
                      "Duplicate control qubit: " + std::string(tok));
      }
    }

[gemini-code-assist]

Use unsigned int for qubit IDs to be consistent with other gate functions and to avoid signed/unsigned comparison issues and potential overflow when num_qubits is large.

  unsigned int q0 = 0;

[gemini-code-assist]

With q0 as unsigned int, the check for q0 < 0 is redundant, and the comparison with num_qubits is safer and more consistent.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }

[gemini-code-assist]

Use unsigned int for qubit IDs to be consistent with other gate functions and to avoid signed/unsigned comparison issues.

  unsigned int q0 = 0, q1 = 0;

[gemini-code-assist]

With q0 and q1 as unsigned int, the checks for negative values are redundant, and the comparisons with num_qubits are safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }
  if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 >= num_qubits || q0 == q1) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid or duplicate qubit id: " + op.qubits(1).id());
  }

[gemini-code-assist]

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }

[gemini-code-assist]

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

[gemini-code-assist]

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }

[gemini-code-assist]

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

[gemini-code-assist]

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }