-
Notifications
You must be signed in to change notification settings - Fork 769
[DevSAN] Make device sanitizer kernel metadata has a unique id #18819
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.
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
base: sycl
Are you sure you want to change the base?
Conversation
We decorated kernel metadata global as a device global, if we don't give a unique id to it, device sanitizer will not work in shared libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures that kernel metadata for device sanitizers receive a unique identifier by computing an MD5 hash over aggregated kernel names, which is then appended to a given prefix. The changes update LLVM test expectations and modify ThreadSanitizer, MemorySanitizer, and AddressSanitizer instrumentation passes to leverage the new unique-id generation mechanism.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
llvm/test/Instrumentation/ThreadSanitizer/SPIRV/kernel_metadata.ll | Updated unique-id check with new MD5-based value |
llvm/test/Instrumentation/MemorySanitizer/SPIRV/instrument_global_address_space.ll | Updated unique-id check with new MD5-based value |
llvm/test/Instrumentation/AddressSanitizer/SPIRV/extend_launch_info_arg.ll | Updated unique-id check with new MD5-based value |
llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp | Introduced aggregation of kernel names and integrated computeKernelMetadataUniqueId |
llvm/lib/Transforms/Instrumentation/SPIRVSanitizerCommonUtils.cpp | Added computeKernelMetadataUniqueId helper using MD5 for unique-id generation |
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | Added similar MD5-based unique-id generation for metadata |
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | Added similar MD5-based unique-id generation for metadata |
llvm/include/llvm/Transforms/Instrumentation/SPIRVSanitizerCommonUtils.h | Declared the computeKernelMetadataUniqueId function |
Comments suppressed due to low confidence (2)
llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp:509
- [nitpick] Consider renaming 'KernelNamesBytes' to a more descriptive name like 'AggregatedKernelNamesBytes' to clarify that it aggregates bytes from multiple kernel names.
SmallVector<uint8_t, 256> KernelNamesBytes;
llvm/lib/Transforms/Instrumentation/SPIRVSanitizerCommonUtils.cpp:63
- Consider adding a docstring for 'computeKernelMetadataUniqueId' that explains its purpose, the use of an MD5 hash for generating a unique id, and how the aggregated kernel name bytes contribute to this identifier.
SmallString<128> computeKernelMetadataUniqueId(StringRef Prefix, SmallVectorImpl<uint8_t> &KernelNamesBytes) {
We decorated kernel metadata global as a device global, if we don't give a unique id to it, device sanitizer will not work in shared libraries.