Author: Amit Kumar Pandey Date: 2025-01-30T16:28:03+05:30 New Revision: b68b4f64a2bd2e0a22375cf89a4d655fc3667e11
URL: https://github.com/llvm/llvm-project/commit/b68b4f64a2bd2e0a22375cf89a4d655fc3667e11 DIFF: https://github.com/llvm/llvm-project/commit/b68b4f64a2bd2e0a22375cf89a4d655fc3667e11.diff LOG: [Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking. (#123922) ASan bitcode linking is currently available for HIPAMD,OpenMP and OpenCL. Moving sanitizer specific common parts of logic to appropriate API's so as to reduce code redundancy and maintainability. Added: Modified: clang/lib/Driver/ToolChains/AMDGPU.cpp clang/lib/Driver/ToolChains/AMDGPU.h clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp clang/lib/Driver/ToolChains/HIPAMD.cpp clang/lib/Driver/ToolChains/ROCm.h clang/test/Driver/hip-sanitize-options.hip clang/test/Driver/rocm-device-libs.cl Removed: ################################################################################ diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index a8061ffd9321f5..83f486611bc946 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -950,6 +950,11 @@ void ROCMToolChain::addClangTargetOptions( ABIVer)) return; + std::tuple<bool, const SanitizerArgs> GPUSan( + DriverArgs.hasFlag(options::OPT_fgpu_sanitize, + options::OPT_fno_gpu_sanitize, true), + getSanitizerArgs(DriverArgs)); + bool Wave64 = isWave64(DriverArgs, Kind); // TODO: There are way too many flags that change this. Do we need to check @@ -965,21 +970,19 @@ void ROCMToolChain::addClangTargetOptions( DriverArgs.hasArg(options::OPT_cl_fp32_correctly_rounded_divide_sqrt); // Add the OpenCL specific bitcode library. - llvm::SmallVector<std::string, 12> BCLibs; - BCLibs.push_back(RocmInstallation->getOpenCLPath().str()); + llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs; + BCLibs.emplace_back(RocmInstallation->getOpenCLPath().str()); // Add the generic set of libraries. BCLibs.append(RocmInstallation->getCommonBitcodeLibs( DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt, - FastRelaxedMath, CorrectSqrt, ABIVer, false)); + FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, false)); - if (getSanitizerArgs(DriverArgs).needsAsanRt()) { - CC1Args.push_back("-mlink-bitcode-file"); - CC1Args.push_back( - DriverArgs.MakeArgString(RocmInstallation->getAsanRTLPath())); - } - for (StringRef BCFile : BCLibs) { - CC1Args.push_back("-mlink-builtin-bitcode"); + for (auto [BCFile, Internalize] : BCLibs) { + if (Internalize) + CC1Args.push_back("-mlink-builtin-bitcode"); + else + CC1Args.push_back("-mlink-bitcode-file"); CC1Args.push_back(DriverArgs.MakeArgString(BCFile)); } } @@ -1002,18 +1005,35 @@ bool RocmInstallationDetector::checkCommonBitcodeLibs( return true; } -llvm::SmallVector<std::string, 12> +llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> RocmInstallationDetector::getCommonBitcodeLibs( const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, bool Wave64, bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, bool FastRelaxedMath, - bool CorrectSqrt, DeviceLibABIVersion ABIVer, bool isOpenMP = false) const { - llvm::SmallVector<std::string, 12> BCLibs; - - auto AddBCLib = [&](StringRef BCFile) { BCLibs.push_back(BCFile.str()); }; + bool CorrectSqrt, DeviceLibABIVersion ABIVer, + const std::tuple<bool, const SanitizerArgs> &GPUSan, + bool isOpenMP = false) const { + llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs; + + auto GPUSanEnabled = [GPUSan]() { return std::get<bool>(GPUSan); }; + auto AddBCLib = [&](ToolChain::BitCodeLibraryInfo BCLib, + bool Internalize = true) { + BCLib.ShouldInternalize = Internalize; + BCLibs.emplace_back(BCLib); + }; + auto AddSanBCLibs = [&]() { + if (GPUSanEnabled()) { + auto SanArgs = std::get<const SanitizerArgs>(GPUSan); + if (SanArgs.needsAsanRt()) + AddBCLib(getAsanRTLPath(), false); + } + }; + AddSanBCLibs(); AddBCLib(getOCMLPath()); if (!isOpenMP) AddBCLib(getOCKLPath()); + else if (GPUSanEnabled() && isOpenMP) + AddBCLib(getOCKLPath(), false); AddBCLib(getDenormalsAreZeroPath(DAZ)); AddBCLib(getUnsafeMathPath(UnsafeMathOpt || FastRelaxedMath)); AddBCLib(getFiniteOnlyPath(FiniteOnly || FastRelaxedMath)); @@ -1027,7 +1047,7 @@ RocmInstallationDetector::getCommonBitcodeLibs( return BCLibs; } -llvm::SmallVector<std::string, 12> +llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, const std::string &GPUArch, bool isOpenMP) const { @@ -1044,6 +1064,10 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, // If --hip-device-lib is not set, add the default bitcode libraries. // TODO: There are way too many flags that change this. Do we need to check // them all? + std::tuple<bool, const SanitizerArgs> GPUSan( + DriverArgs.hasFlag(options::OPT_fgpu_sanitize, + options::OPT_fno_gpu_sanitize, true), + getSanitizerArgs(DriverArgs)); bool DAZ = DriverArgs.hasFlag(options::OPT_fgpu_flush_denormals_to_zero, options::OPT_fno_gpu_flush_denormals_to_zero, getDefaultDenormsAreZeroForTarget(Kind)); @@ -1061,7 +1085,7 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, return RocmInstallation->getCommonBitcodeLibs( DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt, - FastRelaxedMath, CorrectSqrt, ABIVer, isOpenMP); + FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, isOpenMP); } bool AMDGPUToolChain::shouldSkipSanitizeOption( diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index a9b4552a1f91a4..aad6bc75dffafc 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -142,7 +142,7 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { Action::OffloadKind DeviceOffloadKind) const override; // Returns a list of device library names shared by diff erent languages - llvm::SmallVector<std::string, 12> + llvm::SmallVector<BitCodeLibraryInfo, 12> getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, const std::string &GPUArch, bool isOpenMP = false) const; diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp index 3f0b3f2d86b3ed..2b8917106afc14 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp @@ -9,7 +9,7 @@ #include "AMDGPUOpenMP.h" #include "AMDGPU.h" #include "CommonArgs.h" -#include "ToolChains/ROCm.h" +#include "ROCm.h" #include "clang/Basic/DiagnosticDriver.h" #include "clang/Driver/Compilation.h" #include "clang/Driver/Driver.h" @@ -159,11 +159,6 @@ AMDGPUOpenMPToolChain::getDeviceLibs(const llvm::opt::ArgList &Args) const { if (Args.hasArg(options::OPT_nogpulib)) return {}; - if (!RocmInstallation->hasDeviceLibrary()) { - getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0; - return {}; - } - StringRef GpuArch = getProcessorFromTargetID( getTriple(), Args.getLastArgValue(options::OPT_march_EQ)); diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp index ccee065b590647..158a2520759846 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.cpp +++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp @@ -382,7 +382,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { llvm::sys::path::append(Path, BCName); FullName = Path; if (llvm::sys::fs::exists(FullName)) { - BCLibs.push_back(FullName); + BCLibs.emplace_back(FullName); return; } } @@ -396,28 +396,11 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { StringRef GpuArch = getGPUArch(DriverArgs); assert(!GpuArch.empty() && "Must have an explicit GPU arch."); - // If --hip-device-lib is not set, add the default bitcode libraries. - if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize, - options::OPT_fno_gpu_sanitize, true) && - getSanitizerArgs(DriverArgs).needsAsanRt()) { - auto AsanRTL = RocmInstallation->getAsanRTLPath(); - if (AsanRTL.empty()) { - unsigned DiagID = getDriver().getDiags().getCustomDiagID( - DiagnosticsEngine::Error, - "AMDGPU address sanitizer runtime library (asanrtl) is not found. " - "Please install ROCm device library which supports address " - "sanitizer"); - getDriver().Diag(DiagID); - return {}; - } else - BCLibs.emplace_back(AsanRTL, /*ShouldInternalize=*/false); - } - // Add the HIP specific bitcode library. - BCLibs.push_back(RocmInstallation->getHIPPath()); + BCLibs.emplace_back(RocmInstallation->getHIPPath()); // Add common device libraries like ocml etc. - for (StringRef N : getCommonDeviceLibNames(DriverArgs, GpuArch.str())) + for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str())) BCLibs.emplace_back(N); // Add instrument lib. @@ -426,7 +409,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { if (InstLib.empty()) return BCLibs; if (llvm::sys::fs::exists(InstLib)) - BCLibs.push_back(InstLib); + BCLibs.emplace_back(InstLib); else getDriver().Diag(diag::err_drv_no_such_file) << InstLib; } diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h index dceb0ab0366933..681c242b0678e2 100644 --- a/clang/lib/Driver/ToolChains/ROCm.h +++ b/clang/lib/Driver/ToolChains/ROCm.h @@ -13,6 +13,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Driver/Driver.h" #include "clang/Driver/Options.h" +#include "clang/Driver/SanitizerArgs.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringMap.h" #include "llvm/Option/ArgList.h" @@ -173,12 +174,11 @@ class RocmInstallationDetector { /// Get file paths of default bitcode libraries common to AMDGPU based /// toolchains. - llvm::SmallVector<std::string, 12> - getCommonBitcodeLibs(const llvm::opt::ArgList &DriverArgs, - StringRef LibDeviceFile, bool Wave64, bool DAZ, - bool FiniteOnly, bool UnsafeMathOpt, - bool FastRelaxedMath, bool CorrectSqrt, - DeviceLibABIVersion ABIVer, bool isOpenMP) const; + llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> getCommonBitcodeLibs( + const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, + bool Wave64, bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, + bool FastRelaxedMath, bool CorrectSqrt, DeviceLibABIVersion ABIVer, + const std::tuple<bool, const SanitizerArgs> &GPUSan, bool isOpenMP) const; /// Check file paths of default bitcode libraries common to AMDGPU based /// toolchains. \returns false if there are invalid or missing files. bool checkCommonBitcodeLibs(StringRef GPUArch, StringRef LibDeviceFile, diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip index d94cbdacdaeb3a..8de0ee9e18426b 100644 --- a/clang/test/Driver/hip-sanitize-options.hip +++ b/clang/test/Driver/hip-sanitize-options.hip @@ -18,7 +18,7 @@ // RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \ // RUN: %s 2>&1 | FileCheck -check-prefixes=RDC %s -// RUN: not %clang -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \ +// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -mcode-object-version=5 --offload-arch=gfx900:xnack+ \ // RUN: -fsanitize=address -fgpu-sanitize \ // RUN: -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \ // RUN: %s 2>&1 | FileCheck -check-prefixes=FAIL %s @@ -52,15 +52,15 @@ // CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}} // CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}} -// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]" +// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]" // NORDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}} // NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}} // RDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}} -// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]" +// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]" // RDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}} -// FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer +// FAIL: error: cannot find ROCm device library for ABI version 5; provide its path via '--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build without ROCm device library // XNACK-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' // XNACK-DAG: warning: ignoring '-fsanitize=address' option for offload arch 'gfx900:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead diff --git a/clang/test/Driver/rocm-device-libs.cl b/clang/test/Driver/rocm-device-libs.cl index 6837e219dc35de..f9766e6fa4d996 100644 --- a/clang/test/Driver/rocm-device-libs.cl +++ b/clang/test/Driver/rocm-device-libs.cl @@ -145,8 +145,8 @@ // RUN: 2>&1 | FileCheck --check-prefixes=NOASAN %s // COMMON: "-triple" "amdgcn-amd-amdhsa" -// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc" // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc" +// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc" // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc" // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ockl.bc" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
