[SYCL] Limit the directories that are searched when loading dependencies of the plugins #12336
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
againull
changed the title
Currently the default search order is used when loading dependencies of the plugins (these depencies include the Level Zero loader and the ICD loader) and that list includes current directory and some other directories which are not considered safe. This patch limits the list of directories when loading the dependencies of the plugins. See: https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa for reference.
againull
force-pushed
the
limit_search_dir
branch
from
8be7cc3 to
3f1c180
Compare
Comment on lines
137
to
150
| // When searching for dependencies of the OpenCL plugin (which includes ICD | ||
| // loader) limit the list of directories to %windows%\system32 and the | ||
| // directory that contains the loaded DLL (OpenCL plugin). ICD loader is | ||
| // located in the same directory as OpenCL plugin. Standard library | ||
| // dependencies are in the system directory. | ||
| dllMap.emplace(ocl_path, LoadLibraryEx(ocl_path.wstring().c_str(), NULL, | ||
| LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | | ||
| LOAD_LIBRARY_SEARCH_SYSTEM32)); | ||
|
|
||
| // When searching for dependencies of the Level Zero plugin (which includes | ||
| // level zero loader) limit the list of directories to %windows%\system32. | ||
| // In this case we know that the Level Zero Loader is located in the system | ||
| // directory. Standard library dependencies are in the system directory. | ||
| auto l0_path = LibSYCLDir / __SYCL_LEVEL_ZERO_PLUGIN_NAME; |
There was a problem hiding this comment.
Please merge comments into a single one.
| auto l0_path = LibSYCLDir / __SYCL_LEVEL_ZERO_PLUGIN_NAME; | ||
| dllMap.emplace(l0_path, LoadLibraryEx(l0_path.wstring().c_str(), NULL, NULL)); | ||
| dllMap.emplace(l0_path, LoadLibraryEx(l0_path.wstring().c_str(), NULL, | ||
| LOAD_LIBRARY_SEARCH_SYSTEM32)); |
There was a problem hiding this comment.
While we don't seem to need LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR here, I think it would make sense to use it still for consistency and to unify code.
There was a problem hiding this comment.
Yes, makes sense, fixed.
againull
requested review from
a team and
ldrumm
and removed request for
a team
|
Included @intel/llvm-reviewers-cuda to get opinion on this change regarding cuda plugin and probably hip plugin. |
Co-authored-by: aelovikov-intel <andrei.elovikov@intel.com>
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
againull
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
approved these changes
Jan 10, 2024
againull
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the default search order is used when loading dependencies of the plugins (these dependencies include the Level Zero loader and the ICD loader for opencl and level zero plugins respectively) and that list includes current directory and some other directories which are not considered safe. This patch limits the list of directories when loading the dependencies of the plugins. See: https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa for reference.