Skip to content

fix: prevent panics in shape inference when operands missing#2411

Open
jaweed3 wants to merge 1 commit into
sonos:mainfrom
jaweed3:fix/arity-checks
Open

fix: prevent panics in shape inference when operands missing#2411
jaweed3 wants to merge 1 commit into
sonos:mainfrom
jaweed3:fix/arity-checks

Conversation

@jaweed3

@jaweed3 jaweed3 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Problem

Five ops (Concat, Squeeze, Gemm, Reshape, and the RNN family) index inputs[k] / outputs[k] in their rules() without arity validation. A malformed ONNX with too few operands triggers an index-out-of-bounds panic instead of a graceful error.

Fix

Add check_input_arity / check_output_arity guard at the top of each rules() before any indexing.

Op File Guard
Concat hir/src/ops/array/concat.rs check_input_arity(inputs, 1)?;
Squeeze onnx/src/ops/array/squeeze.rs check_input_arity(inputs, 1)?;
Gemm onnx/src/ops/math/gemm.rs check_input_arity(inputs, 2)?;
Reshape hir/src/ops/array/reshape.rs check_input_arity(inputs, 2)?;
RNN onnx/src/ops/rec/common.rs check_output_arity(outputs, output_count.max(1))?;

Verification

Before: tract concat_poc.onnx dump → panic at concat.rs:37
After: tract concat_poc.onnx dumpError: Wrong input number. Rules expect 1, node has 0.

Closes #2391.

Concat, Squeeze, Gemm, Reshape, and the RNN family all access
inputs[k] or outputs[k] without first checking that at least k+1
operands exist. A malformed ONNX file with too few inputs/outputs
triggers an index-out-of-bounds panic.

Add check_input_arity / check_output_arity guards so these
cases produce a proper TractError instead.

Closes sonos#2391.
@github-actions

Copy link
Copy Markdown

🟡 Bench vs main — no speed regressions · 1 load/memory mover(s)

Reference: main nightly, latest 2026-06-25 (0d old) · PR ddadf4c0d · ran on apple-m1-max, i9-11900kb_rtx-4060, jetson-orin-nx · 651 metrics compared

Speed — evaltime · prefill · decode

no inference-speed regressions

Load & memory (worst first)

Δ metric device main → PR
🔴 +6.1% openelm_270M_q40ef16_541
RSS @ ready · metal
apple-m1-max 299 MB → 318 MB
🟢 3 improvement(s)
Δ metric device main → PR
🟢 -26.7% parakeet_tdt_600m_v3_f32f32_decoder_pass
load+optimize · metal
apple-m1-max 146 ms → 107 ms
🟢 -6.2% llama_3_2_3B_q40ef32_516
load+optimize · metal
apple-m1-max 1.56 s → 1.46 s
🟢 -5.1% llama_3_2_3B_q40ef32_516
load · metal
apple-m1-max 1.36 s → 1.29 s

lower is better except prefill/decode (tok/s) · adaptive thresholds (max(floor, k×noise) vs the series' own history) · single-shot vs nightly reference · full report → run

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reachable panics in ONNX/HIR shape inference: operand indexing without arity validation (multiple ops)

1 participant