[ASan][Driver] Add sanitize-target flag to support enabling ASan in device or host compilation #76127
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
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-clang Author: None (jinge90) ChangesIn offloading scenario, the whole compiling process consists of device and host compilation phase, device and host code will be bundled together. Address sanitizer can be enabled in either one or both compilation phases via fsanitize-target=host|device|both. The meaning of each value is: Full diff: https://github.com/llvm/llvm-project/pull/76127.diff 9 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 0acb5ae134ea24..af44b98e9d046b 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -224,6 +224,10 @@ ENUM_CODEGENOPT(SanitizeAddressUseAfterReturn,
llvm::AsanDetectStackUseAfterReturnMode, 2,
llvm::AsanDetectStackUseAfterReturnMode::Runtime
) ///< Set detection mode for stack-use-after-return.
+ENUM_CODEGENOPT(
+ SanitizeTargetsToEnable, llvm::AsanTargetsToEnable, 2,
+ llvm::AsanTargetsToEnable::Both) ///< Set targets to enable sanitizer
+ /// passes in offloading scenario.
CODEGENOPT(SanitizeAddressPoisonCustomArrayCookie, 1,
0) ///< Enable poisoning operator new[] which is not a replaceable
///< global allocation function in AddressSanitizer
diff --git a/clang/include/clang/Basic/Sanitizers.h b/clang/include/clang/Basic/Sanitizers.h
index c890242269b334..e3fe35ed285dc3 100644
--- a/clang/include/clang/Basic/Sanitizers.h
+++ b/clang/include/clang/Basic/Sanitizers.h
@@ -211,6 +211,10 @@ StringRef AsanDetectStackUseAfterReturnModeToString(
llvm::AsanDetectStackUseAfterReturnMode
AsanDetectStackUseAfterReturnModeFromString(StringRef modeStr);
+llvm::AsanTargetsToEnable
+AsanTargetsToEnableFromString(StringRef asanTargetsStr);
+
+StringRef AsanTargetsToEnableToString(llvm::AsanTargetsToEnable target);
} // namespace clang
#endif // LLVM_CLANG_BASIC_SANITIZERS_H
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 1b02087425b751..b9a3b3b05b48af 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2462,6 +2462,18 @@ def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-unde
"when emitting check metadata.">,
MarshallingInfoInt<CodeGenOpts<"EmitCheckPathComponentsToStrip">, "0", "int">;
+def sanitize_targets_EQ
+ : Joined<["-"], "fsanitize-target=">,
+ MetaVarName<"<target>">,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"Select target to enable sanitizer in offloading scenario, "
+ "valid targets are host, device, both(both host and device "
+ "enabled).">,
+ Group<f_clang_Group>,
+ Values<"host,device,both">,
+ NormalizedValuesScope<"llvm::AsanTargetsToEnable">,
+ NormalizedValues<["Host", "Device", "Both"]>,
+ MarshallingInfoEnum<CodeGenOpts<"SanitizeTargetsToEnable">, "Both">;
} // end -f[no-]sanitize* flags
def funsafe_math_optimizations : Flag<["-"], "funsafe-math-optimizations">,
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 07070ec4fc0653..aaa896a02eb142 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -67,6 +67,8 @@ class SanitizerArgs {
bool HwasanUseAliases = false;
llvm::AsanDetectStackUseAfterReturnMode AsanUseAfterReturn =
llvm::AsanDetectStackUseAfterReturnMode::Invalid;
+ llvm::AsanTargetsToEnable AsanTargetsToEnable =
+ llvm::AsanTargetsToEnable::Both;
std::string MemtagMode;
@@ -79,7 +81,10 @@ class SanitizerArgs {
bool needsStableAbi() const { return StableABI; }
bool needsMemProfRt() const { return NeedsMemProfRt; }
- bool needsAsanRt() const { return Sanitizers.has(SanitizerKind::Address); }
+ bool needsAsanRt() const {
+ bool AsanIsNotDeviceOnly =
+ !(AsanTargetsToEnable == llvm::AsanTargetsToEnable::Device);
+ return Sanitizers.has(SanitizerKind::Address) && AsanIsNotDeviceOnly; }
bool needsHwasanRt() const {
return Sanitizers.has(SanitizerKind::HWAddress);
}
diff --git a/clang/lib/Basic/Sanitizers.cpp b/clang/lib/Basic/Sanitizers.cpp
index 62ccdf8e9bbf28..05eda31c3f7ffd 100644
--- a/clang/lib/Basic/Sanitizers.cpp
+++ b/clang/lib/Basic/Sanitizers.cpp
@@ -112,4 +112,27 @@ AsanDetectStackUseAfterReturnModeFromString(StringRef modeStr) {
.Default(llvm::AsanDetectStackUseAfterReturnMode::Invalid);
}
+llvm::AsanTargetsToEnable
+AsanTargetsToEnableFromString(StringRef asanTargetsStr) {
+ return llvm::StringSwitch<llvm::AsanTargetsToEnable>(asanTargetsStr)
+ .Case("host", llvm::AsanTargetsToEnable::Host)
+ .Case("device", llvm::AsanTargetsToEnable::Device)
+ .Case("both", llvm::AsanTargetsToEnable::Both)
+ .Default(llvm::AsanTargetsToEnable::Invalid);
+}
+
+StringRef AsanTargetsToEnableToString(llvm::AsanTargetsToEnable target) {
+ switch (target) {
+ case llvm::AsanTargetsToEnable::Host:
+ return "host";
+ case llvm::AsanTargetsToEnable::Device:
+ return "device";
+ case llvm::AsanTargetsToEnable::Both:
+ return "both";
+ case llvm::AsanTargetsToEnable::Invalid:
+ return "invalid";
+ }
+ return "invalid";
+}
+
} // namespace clang
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 480410db1021b7..f4006270a13229 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -700,7 +700,24 @@ static void addSanitizers(const Triple &TargetTriple,
DestructorKind));
}
};
- ASanPass(SanitizerKind::Address, false);
+ // AddressSanitizer can be enabled in offloading scenario to detect bugs in
+ // both host and device code. Users may use '-fsanitizer-targets' flag to
+ // enable this only on the host or only on the device. The default behavior
+ // is to enable AddressSanitizer in both host and device compilation.
+ // Currently, we support this in SYCL compiler.
+ bool IgnoreAsanPass = false;
+ if (LangOpts.isSYCL()) {
+ llvm::AsanTargetsToEnable AsanTarget =
+ CodeGenOpts.getSanitizeTargetsToEnable();
+ if ((AsanTarget == llvm::AsanTargetsToEnable::Device) &&
+ LangOpts.SYCLIsHost)
+ IgnoreAsanPass = true;
+ if ((AsanTarget == llvm::AsanTargetsToEnable::Host) &&
+ LangOpts.SYCLIsDevice)
+ IgnoreAsanPass = true;
+ }
+ if (!IgnoreAsanPass)
+ ASanPass(SanitizerKind::Address, false);
ASanPass(SanitizerKind::KernelAddress, true);
auto HWASanPass = [&](SanitizerMask Mask, bool CompileKernel) {
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index ad68c086b71790..8805e4c915cd6e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1010,6 +1010,17 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
AsanUseAfterReturn = parsedAsanUseAfterReturn;
}
+ if (const auto *Arg = Args.getLastArg(options::OPT_sanitize_targets_EQ)) {
+ auto parsedAsanTargetsToEnable =
+ AsanTargetsToEnableFromString(Arg->getValue());
+ if (parsedAsanTargetsToEnable == llvm::AsanTargetsToEnable::Invalid &&
+ DiagnoseErrors) {
+ TC.getDriver().Diag(clang::diag::err_drv_unsupported_option_argument)
+ << Arg->getSpelling() << Arg->getValue();
+ }
+ AsanTargetsToEnable = parsedAsanTargetsToEnable;
+ }
+
} else {
AsanUseAfterScope = false;
// -fsanitize=pointer-compare/pointer-subtract requires -fsanitize=address.
@@ -1138,6 +1149,29 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
GPUSanitize = true;
}
+ // In offloading scenario, the whole compiling process consists of device and
+ // host compilation phase, device and host code will be bundled together.
+ // Address sanitizer can be enabled in either one or both compilation phases
+ // via fsanitize-target=host|device|both. This flag must be passed to both
+ // device and host compilation to indicate whether to enable ASanPass in
+ // current phase. In SYCL, when SYCL compiler's target triple is SPIR, we are
+ // in device compilation phase, if target triple is not SPIR and '-fsycl' is
+ // used, we are in host compilation phase.
+ if (TC.getTriple().isSPIR()) {
+ if (Sanitizers.has(SanitizerKind::Address)) {
+ CmdArgs.push_back(
+ Args.MakeArgString("-fsanitize-target=" +
+ AsanTargetsToEnableToString(AsanTargetsToEnable)));
+ }
+ } else {
+ if (Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false) &&
+ Sanitizers.has(SanitizerKind::Address)) {
+ CmdArgs.push_back(
+ Args.MakeArgString("-fsanitize-target=" +
+ AsanTargetsToEnableToString(AsanTargetsToEnable)));
+ }
+ }
+
// Translate available CoverageFeatures to corresponding clang-cc1 flags.
// Do it even if Sanitizers.empty() since some forms of coverage don't require
// sanitizers.
diff --git a/clang/test/Driver/sycl-sanitize-target.cpp b/clang/test/Driver/sycl-sanitize-target.cpp
new file mode 100644
index 00000000000000..c927d3b4e0f1a8
--- /dev/null
+++ b/clang/test/Driver/sycl-sanitize-target.cpp
@@ -0,0 +1,29 @@
+// UNSUPPORTED: system-windows
+
+// Check whether ASan runtime libraries are linked or not when
+// fsanitize-target=host|device|both. The ASan runtime libraries
+// are NOT needed for device compilation in offloading scenario.
+
+// RUN: %clangxx -fsycl -fsanitize=address %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=SYCL-ASAN-DEFAULT %s
+// SYCL-ASAN-DEFAULT: "{{.*}}/ld"
+// SYCL-ASAN-DEFAULT-SAME: "{{.*}}/libclang_rt.asan{{.*}}.a"
+
+// RUN: %clangxx -fsycl -fsanitize=address -fsanitize-target=host %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=SYCL-ASAN-HOST %s
+// SYCL-ASAN-HOST: "{{.*}}/ld"
+// SYCL-ASAN-HOST-SAME: "{{.*}}/libclang_rt.asan{{.*}}.a"
+
+// RUN: %clangxx -fsycl -fsanitize=address -fsanitize-target=both %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=SYCL-ASAN-BOTH %s
+// SYCL-ASAN-BOTH: "{{.*}}/ld"
+// SYCL-ASAN-BOTH-SAME: "{{.*}}/libclang_rt.asan{{.*}}.a"
+
+// RUN: %clangxx -fsycl -fsanitize=address -fsanitize-target=device %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=SYCL-ASAN-DEVICE %s
+// SYCL-ASAN-DEVICE: "{{.*}}/ld"
+// SYCL-ASAN-DEVICE-NOT: "{{.*}}/libclang_rt.asan{{.*}}.a"
+
+// RUN: not %clangxx -fsycl -fsanitize=address -fsanitize-target=YYY %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=SYCL-ASAN-INVALID %s
+// SYCL-ASAN-INVALID: error: unsupported argument 'YYY'
diff --git a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
index d697b72cde05e9..80984c2bd3e6a3 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
@@ -34,6 +34,14 @@ enum class AsanDetectStackUseAfterReturnMode {
Invalid, ///< Not a valid detect mode.
};
+/// Targets to enable address sanitizer pass in offloading scenario.
+enum class AsanTargetsToEnable {
+ Host, ///< Only enable AddressSanitizerPass for host compilation.
+ Device, ///< Only enable AddressSanitizerPass for device compilation.
+ Both, ///< Enable AddressSanitizerPass for both host and device compilation.
+ Invalid ///< Not a valid detect mode.
+};
+
} // namespace llvm
#endif
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Hi, @vitalybuka and @bader |
…evice or host compilation In offloading scenario, the whole compiling process consists of device and host compilation phase, device and host code will be bundled together. Address sanitizer can be enabled in either one or both compilation phases via fsanitize-target=host|device|both. The meaning of each value is: -fsanitize-target=host AsanPass only enabled in host code -fsanitize-target=device AsanPass only enabled in device code -fsanitize-target=both AsanPass enabled in both host and device code The default value is 'both'.
Signed-off-by: jinge90 <ge.jin@intel.com>
|
Hi, @MaskRay and @AaronBallman |
|
It seems as the build system business to control this logic, not driver. |
|
Hi, @MaskRay and @AaronBallman |
AaronBallman
requested changes
Jan 16, 2024
There was a problem hiding this comment.
I had some conversations with @jinge90 offline and we're now thinking that perhaps a better approach is to have a more general feature for specifying host-only and target-only options instead of having each option split into two. Marking this review as requesting changes so it doesn't get accidentally accepted.
vitalybuka
requested changes
Mar 13, 2024
|
Using general option to pass device specific flags to device compilation should be better approach, so this pr is not needed any more, thanks very much! |
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.
In offloading scenario, the whole compiling process consists of device and host compilation phase, device and host code will be bundled together. Address sanitizer can be enabled in either one or both compilation phases via fsanitize-target=host|device|both. The meaning of each value is:
-fsanitize-target=host AsanPass only enabled in host code
-fsanitize-target=device AsanPass only enabled in device code
-fsanitize-target=both AsanPass enabled in both host and device code
The default value is 'both'.