Conversation
… by training. Not required AFAIK as any custom EP should use the new plugin EP infrastructure.
There was a problem hiding this comment.
Pull request overview
Removes the legacy “custom EP shared library load + arbitrary entry-point symbol” infrastructure (previously used by deprecated training flows) to reduce attack surface and align custom EP usage with the newer plugin EP mechanism.
Changes:
- Removes Python/C++ bindings that supported dynamic EP loading via
shared_lib_pathand custom entry-point symbols. - Deletes the custom EP test shared library/testdata and related unit tests.
- Simplifies training EP provider-option resolution and hashing now that external EP registration is gone.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| orttraining/orttraining/test/python/onnxruntime_test_register_ep.py | Removes training-side Python test that exercised _register_provider_lib custom EP loading. |
| orttraining/orttraining/python/orttraining_python_module.cc | Removes dynamic EP hashing + external EP registration plumbing; uses provider options directly. |
| orttraining/orttraining/python/orttraining_pybind_state.cc | Removes provider-option merge helper tied to external EP registration. |
| orttraining/orttraining/python/orttraining_pybind_common.h | Removes ext_execution_provider_info_map_ and related API surface from training env. |
| onnxruntime/test/testdata/custom_execution_provider_library/version_script.lds | Deletes export control file for the removed test EP shared library. |
| onnxruntime/test/testdata/custom_execution_provider_library/test_model.onnx | Deletes model used solely by the removed dynamic EP loading test. |
| onnxruntime/test/testdata/custom_execution_provider_library/symbols.def | Deletes Windows export list for the removed test EP shared library. |
| onnxruntime/test/testdata/custom_execution_provider_library/my_execution_provider.h | Deletes test EP implementation (header). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_execution_provider.cc | Deletes test EP implementation (source). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_ep_factory.h | Deletes test EP factory API (header). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_ep_factory.cc | Deletes test EP factory + hash entry points (source). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_ep_data_transfer.h | Deletes test EP data transfer stub (header). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_ep_data_transfer.cc | Deletes test EP data transfer stub (source). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_allocator.h | Deletes test EP allocator (header). |
| onnxruntime/test/testdata/custom_execution_provider_library/my_allocator.cc | Deletes test EP allocator (source). |
| onnxruntime/test/testdata/custom_execution_provider_library/exported_symbols.lst | Deletes macOS exported symbols list for removed test EP shared library. |
| onnxruntime/test/python/onnxruntime_test_python.py | Removes Python test that exercised dynamic EP loading via shared_lib_path. |
| onnxruntime/python/onnxruntime_pybind_state_common.h | Removes the default dynamic EP entry-point constant. |
| onnxruntime/python/onnxruntime_pybind_state.cc | Removes dynamic loading path that created EP factories from arbitrary shared libraries/symbols. |
| include/onnxruntime/core/graph/constants.h | Removes public string constants for the legacy provider-option keys. |
| cmake/onnxruntime_unittests.cmake | Removes building/linking of the test_execution_provider shared library used by the deleted tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1317,25 +1299,6 @@ static std::shared_ptr<IExecutionProviderFactory> CreateExecutionProviderFactory | |||
| } | |||
| #endif | |||
|
|
|||
There was a problem hiding this comment.
With the shared_lib_path/entry-point dynamic loading path removed, attempts to use the old API will now fall through to Unknown Provider Type even if the user provided the legacy options. Consider checking for legacy keys like shared_lib_path / provider_factory_entry_point in provider_options_map before throwing, and raise a more actionable error that points users to register_execution_provider_library() (plugin EP infrastructure).
| auto provider_options_it = provider_options_map.find(type); | |
| if (provider_options_it != provider_options_map.end()) { | |
| const auto& provider_options = provider_options_it->second; | |
| if (provider_options.find("shared_lib_path") != provider_options.end() || | |
| provider_options.find("provider_factory_entry_point") != provider_options.end()) { | |
| throw std::runtime_error( | |
| "Execution provider '" + type + | |
| "' was configured with legacy dynamic loading options " | |
| "('shared_lib_path'/'provider_factory_entry_point'), which are no longer supported. " | |
| "Register the execution provider library first via register_execution_provider_library() " | |
| "and then use the provider by name."); | |
| } | |
| } |
There was a problem hiding this comment.
AFAIK there are no users of this so graceful messaging doesn't add value.
| constexpr const char* kCannExecutionProvider = "CANNExecutionProvider"; | ||
| constexpr const char* kAzureExecutionProvider = "AzureExecutionProvider"; | ||
| constexpr const char* kVSINPUExecutionProvider = "VSINPUExecutionProvider"; |
There was a problem hiding this comment.
include/onnxruntime/core/graph/constants.h is a public header; removing the provider-option key constants here is a source-level breaking change for any downstream code that referenced these strings (even if the underlying feature is being removed for security). Consider keeping the constants as deprecated aliases (or moving them to an internal header) and/or adding an explicit migration note pointing users to the plugin EP registration API.
There was a problem hiding this comment.
Removing will ensure any current usage is immediately identified by the breakage.
Description
Remove load of custom EP library with arbitrary symbol name for registration. Was used by training which has been deprecated for a while.
AFAIK not required as any current custom EP should use the new plugin EP infrastructure.
Originally added in #8719
Motivation and Context
Resolve security issue.