Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18847
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ⏳ 24 Pending, 1 Unrelated FailureAs of commit 2f2a920 with merge base 651f2f2 ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Reverts a runtime-time validation in the XNNPACK compiler when creating the XNNPACK subgraph, restoring the prior behavior of passing the serialized num_externs directly to xnn_create_subgraph.
Changes:
- Removed the
num_externs <= xvalues->size()guard inXNNCompiler::compileModel. - Passed
flatbuffer_graph->num_externs()directly asexternal_value_idswhen creating the XNNPACK subgraph.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| xnn_subgraph_t subgraph_ptr = nullptr; | ||
| status = xnn_create_subgraph( | ||
| /*external_value_ids=*/num_externs, | ||
| /*external_value_ids=*/flatbuffer_graph->num_externs(), | ||
| /*flags=*/0, | ||
| &subgraph_ptr); |
There was a problem hiding this comment.
Removing the num_externs <= xvalues->size() guard means a malformed/hostile flatbuffer can set num_externs to a very large value, potentially causing excessive allocation/slowdown (or OOM) inside xnn_create_subgraph. Consider adding a safer sanity check instead: e.g., scan xvalues to compute max_external_id actually referenced (and validate external_id < num_externs for any external value), then pass max_external_id + 1 (or a bounded value) to xnn_create_subgraph and return InvalidProgram on inconsistency.
Internal failures on: pytorch#18799 `num_externs <= num_values` is not the right check. We should scan xvalues to find `num_externs` and use that, provided they are valid. Will put up separate PR for the change. Co-authored-by: Github Executorch <github_executorch@arm.com>
Internal failures on: #18799
num_externs <= num_valuesis not the right check. We should scan xvalues to findnum_externsand use that, provided they are valid. Will put up separate PR for the change.