[llvm] 61ed649 - [SanitizerBinaryMetadata] Do not add to GPU code

[Marco Elver via llvm-commits]

Author: Marco Elver
Date: 2023-03-09T10:15:28+01:00
New Revision: 61ed64954b979df0d5bfdfbe54a7c27e20be9001

URL: https://github.com/llvm/llvm-project/commit/61ed64954b979df0d5bfdfbe54a7c27e20be9001
DIFF: https://github.com/llvm/llvm-project/commit/61ed64954b979df0d5bfdfbe54a7c27e20be9001.diff

LOG: [SanitizerBinaryMetadata] Do not add to GPU code

SanitizerBinaryMetadata should only apply to to host code, and not GPU
code. Recently AMD GPU target code has experimental sanitizer support.

If we're compiling a mixed host/device source file, only add sanitizer
metadata to host code.

Differential Revision: https://reviews.llvm.org/D145519

Added: 
    

Modified: 
    clang/lib/Driver/SanitizerArgs.cpp
    llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 390faef0dbdf1..299d4adf3333a 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1100,13 +1100,16 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
   // NVPTX doesn't currently support sanitizers.  Bailing out here means
   // that e.g. -fsanitize=address applies only to host code, which is what we
   // want for now.
-  //
-  // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
-  if (TC.getTriple().isNVPTX() ||
-      (TC.getTriple().isAMDGPU() &&
-       !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
-                     true)))
+  if (TC.getTriple().isNVPTX())
     return;
+  // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
+  bool GPUSanitize = false;
+  if (TC.getTriple().isAMDGPU()) {
+    if (!Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
+                      true))
+      return;
+    GPUSanitize = true;
+  }
 
   // Translate available CoverageFeatures to corresponding clang-cc1 flags.
   // Do it even if Sanitizers.empty() since some forms of coverage don't require
@@ -1143,20 +1146,22 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
   addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-coverage-ignorelist=",
                         CoverageIgnorelistFiles);
 
-  // Translate available BinaryMetadataFeatures to corresponding clang-cc1
-  // flags. Does not depend on any other sanitizers.
-  const std::pair<int, std::string> BinaryMetadataFlags[] = {
-      std::make_pair(BinaryMetadataCovered, "covered"),
-      std::make_pair(BinaryMetadataAtomics, "atomics"),
-      std::make_pair(BinaryMetadataUAR, "uar")};
-  for (const auto &F : BinaryMetadataFlags) {
-    if (BinaryMetadataFeatures & F.first)
-      CmdArgs.push_back(
-          Args.MakeArgString("-fexperimental-sanitize-metadata=" + F.second));
+  if (!GPUSanitize) {
+    // Translate available BinaryMetadataFeatures to corresponding clang-cc1
+    // flags. Does not depend on any other sanitizers. Unsupported on GPUs.
+    const std::pair<int, std::string> BinaryMetadataFlags[] = {
+        std::make_pair(BinaryMetadataCovered, "covered"),
+        std::make_pair(BinaryMetadataAtomics, "atomics"),
+        std::make_pair(BinaryMetadataUAR, "uar")};
+    for (const auto &F : BinaryMetadataFlags) {
+      if (BinaryMetadataFeatures & F.first)
+        CmdArgs.push_back(
+            Args.MakeArgString("-fexperimental-sanitize-metadata=" + F.second));
+    }
+    addSpecialCaseListOpt(Args, CmdArgs,
+                          "-fexperimental-sanitize-metadata-ignorelist=",
+                          BinaryMetadataIgnorelistFiles);
   }
-  addSpecialCaseListOpt(Args, CmdArgs,
-                        "-fexperimental-sanitize-metadata-ignorelist=",
-                        BinaryMetadataIgnorelistFiles);
 
   if (TC.getTriple().isOSWindows() && needsUbsanRt()) {
     // Instruct the code generator to embed linker directives in the object file

diff  --git a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
index 563d3e4039a9b..a1309a4ec0f45 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -131,6 +131,8 @@ class SanitizerBinaryMetadata {
         IRB(M.getContext()) {
     // FIXME: Make it work with other formats.
     assert(TargetTriple.isOSBinFormatELF() && "ELF only");
+    assert(!(TargetTriple.isNVPTX() || TargetTriple.isAMDGPU()) &&
+           "Device targets are not supported");
   }
 
   bool run();