Skip to content

fix: CoreML EP crash when loading model with external data file #28043

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

Closed
Scottcjn wants to merge 1 commit into from
Closed

fix: CoreML EP crash when loading model with external data file #28043

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions onnxruntime/core/framework/tensorprotoutils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,12 @@ Status TensorProtoWithExternalDataToTensorProto(
++data;
}
} else {
// Load the external data into memory
// Load the external data into memory.
// model_path is the full model file path (e.g., "/path/to/model.onnx").
// ReadExternalDataForTensor expects the parent directory, matching the
// pattern used by UnpackInitializerData() at line ~2572.
Comment on lines +362 to +363
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

The new comment hard-codes a reference to a specific line number in UnpackInitializerData ("line ~2572"). Line-number references tend to go stale quickly and make the comment misleading over time. Consider rewording to reference the function/behavior instead (e.g., that UnpackInitializerData passes model_path.parent_path()) without citing a line number.

Suggested change
// ReadExternalDataForTensor expects the parent directory, matching the
// pattern used by UnpackInitializerData() at line ~2572.
// ReadExternalDataForTensor expects the parent directory, matching how
// UnpackInitializerData() passes model_path.parent_path().

Copilot uses AI. Check for mistakes.
std::vector<uint8_t> unpacked_data;
ORT_RETURN_IF_ERROR(ReadExternalDataForTensor(ten_proto, model_path, unpacked_data));
ORT_RETURN_IF_ERROR(ReadExternalDataForTensor(ten_proto, model_path.parent_path(), unpacked_data));
Comment on lines +360 to +365
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

This fix changes how external-data paths are resolved for TensorProtoWithExternalDataToTensorProto when a model is loaded from a file path, but there doesn't appear to be a unit test covering this specific scenario (external data in file + model_path points to the .onnx file, not a directory). Please add a regression test that creates a temp directory containing a dummy model path and an external data file, then verifies TensorProtoWithExternalDataToTensorProto successfully inlines the external data when given the model file path.

Copilot uses AI. Check for mistakes.

// Set the raw data in the new tensor
onnxruntime::utils::SetRawDataInTensorProto(result, unpacked_data.data(), unpacked_data.size());
Expand Down
Loading