Skip to content
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

[DeviceSanitizer] Support "-fsanitize-ignorelist=" to disable sanitizing on some of kernels #15294

Open
wants to merge 29 commits into
base: sycl
Choose a base branch
from

Conversation

AllanZyne
Copy link
Contributor

@AllanZyne AllanZyne commented Sep 5, 2024

UR: oneapi-src/unified-runtime#2055

We reuse -fsanitize-ignorelist= to support kernel filtering.

Usage: https://clang.llvm.org/docs/SanitizerSpecialCaseList.html

@AllanZyne AllanZyne marked this pull request as ready for review November 7, 2024 08:06
@AllanZyne AllanZyne requested review from a team as code owners November 7, 2024 08:06
@AllanZyne AllanZyne changed the title [DeviceSanitizer] Kernel name filter [DeviceSanitizer] Support "-fsanitize-ignorelist=" to disable sanitizing on some of kernels Nov 7, 2024
@AllanZyne
Copy link
Contributor Author

Hi @intel/dpcpp-sanitizers-review @intel/dpcpp-tools-reviewers @intel/dpcpp-esimd-reviewers, please review, Thanks!

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

not qualified to review the actual devicesan stuff, some high level comments

clang/lib/Driver/OffloadBundler.cpp Show resolved Hide resolved
llvm/include/llvm/SYCLLowerIR/AsanKernelMetadata.h Outdated Show resolved Hide resolved
@@ -50,6 +50,10 @@ static bool instrumentDeviceGlobal(Module &M) {
if (!isDeviceGlobalVariable(G) || !hasDeviceImageScopeProperty(G))
continue;

// Skip instrumenting on "__AsanKernelMetadata" etc.
if (G.getName().starts_with("__Asan"))
Copy link
Contributor

Choose a reason for hiding this comment

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

are there other variables besides __AsanKernelMetadata?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, but we used to use "__Asan" as an internal prefix.

libdevice/sanitizer_utils.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/AddressSanitizer/common/kernel-filter.cpp Outdated Show resolved Hide resolved
@AllanZyne AllanZyne requested a review from a team as a code owner November 8, 2024 02:58
}
Modified |= ModuleSanitizer.instrumentModule();
if (!Modified)
return PreservedAnalyses::all();

GlobalStringMap.clear();
Copy link
Contributor Author

@AllanZyne AllanZyne Nov 8, 2024

Choose a reason for hiding this comment

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

Reminder: GlobalStringMap should be cleared before every "return".

Comment on lines 795 to 796
// Instrument each image scope device globals if the module has been
// instrumented by sanitizer pass.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reminder: change this comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants