From f732178cb69f4f5825ad56b1504fd6b9bb417f16 Mon Sep 17 00:00:00 2001 From: Maosu Zhao Date: Tue, 7 May 2024 14:49:15 +0800 Subject: [PATCH 1/9] [DeviceSanitizer] Do sanitize for device globals in AddressSanitizer pass Now UR already implemented API "urProgramGetGlobalVariablePointer", so we can use it to query the size of device globals and remove "__AsanDeviceGlobalCount". --- .../llvm/SYCLLowerIR/SanitizeDeviceGlobal.h | 23 --- llvm/lib/SYCLLowerIR/CMakeLists.txt | 1 - llvm/lib/SYCLLowerIR/SanitizeDeviceGlobal.cpp | 144 ------------------ .../Instrumentation/AddressSanitizer.cpp | 76 +++++++++ .../Transforms/Instrumentation/CMakeLists.txt | 1 + llvm/tools/sycl-post-link/sycl-post-link.cpp | 6 - sycl/plugins/unified_runtime/CMakeLists.txt | 10 +- 7 files changed, 79 insertions(+), 182 deletions(-) delete mode 100644 llvm/include/llvm/SYCLLowerIR/SanitizeDeviceGlobal.h delete mode 100644 llvm/lib/SYCLLowerIR/SanitizeDeviceGlobal.cpp diff --git a/llvm/include/llvm/SYCLLowerIR/SanitizeDeviceGlobal.h b/llvm/include/llvm/SYCLLowerIR/SanitizeDeviceGlobal.h deleted file mode 100644 index a0e7b2999b480..0000000000000 --- a/llvm/include/llvm/SYCLLowerIR/SanitizeDeviceGlobal.h +++ /dev/null @@ -1,23 +0,0 @@ -//===-- SanitizeDeviceGlobal.h - instrument device global for sanitizer ---===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// This pass adds red zone to each image scope device global and record the -// information like size, red zone size and beginning address. The information -// will be used by address sanitizer. -//===----------------------------------------------------------------------===// - -#include "llvm/IR/PassManager.h" - -namespace llvm { - -class SanitizeDeviceGlobalPass - : public PassInfoMixin { -public: - PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM); -}; - -} // namespace llvm diff --git a/llvm/lib/SYCLLowerIR/CMakeLists.txt b/llvm/lib/SYCLLowerIR/CMakeLists.txt index 09dcd42781fcd..80d21f7f8f9ca 100644 --- a/llvm/lib/SYCLLowerIR/CMakeLists.txt +++ b/llvm/lib/SYCLLowerIR/CMakeLists.txt @@ -70,7 +70,6 @@ add_llvm_component_library(LLVMSYCLLowerIR SYCLPropagateAspectsUsage.cpp SYCLPropagateJointMatrixUsage.cpp SYCLUtils.cpp - SanitizeDeviceGlobal.cpp LocalAccessorToSharedMemory.cpp GlobalOffset.cpp diff --git a/llvm/lib/SYCLLowerIR/SanitizeDeviceGlobal.cpp b/llvm/lib/SYCLLowerIR/SanitizeDeviceGlobal.cpp deleted file mode 100644 index 81415b0f6f9dc..0000000000000 --- a/llvm/lib/SYCLLowerIR/SanitizeDeviceGlobal.cpp +++ /dev/null @@ -1,144 +0,0 @@ -//===-- SanitizeDeviceGlobal.cpp - instrument device global for sanitizer -===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// This pass adds red zone to each image scope device global and record the -// information like size, red zone size and beginning address. The information -// will be used by address sanitizer. -// TODO: Do this in AddressSanitizer pass when urProgramGetGlobalVariablePointer -// is implemented. -//===----------------------------------------------------------------------===// - -#include "llvm/SYCLLowerIR/SanitizeDeviceGlobal.h" -#include "llvm/IR/IRBuilder.h" -#include "llvm/SYCLLowerIR/DeviceGlobals.h" - -#define DEBUG_TYPE "SanitizeDeviceGlobal" - -using namespace llvm; - -namespace { - -// Add extra red zone to each image scope device globals if the module has been -// instrumented by sanitizer pass. And record their infomation like size, red -// zone size, beginning address. -static bool instrumentDeviceGlobal(Module &M) { - auto &DL = M.getDataLayout(); - IRBuilder<> IRB(M.getContext()); - SmallVector GlobalsToRemove; - SmallVector NewDeviceGlobals; - SmallVector DeviceGlobalMetadata; - - constexpr uint64_t MaxRZ = 1 << 18; - constexpr uint64_t MinRZ = 32; - - Type *IntTy = Type::getIntNTy(M.getContext(), DL.getPointerSizeInBits()); - - // Device global meta data is described by a structure - // size_t device_global_size - // size_t device_global_size_with_red_zone - // size_t beginning address of the device global - StructType *StructTy = StructType::get(IntTy, IntTy, IntTy); - - for (auto &G : M.globals()) { - // Non image scope device globals are implemented by device USM, and the - // out-of-bounds check for them will be done by sanitizer USM part. So we - // exclude them here. - if (!isDeviceGlobalVariable(G) || !hasDeviceImageScopeProperty(G)) - continue; - - Type *Ty = G.getValueType(); - const uint64_t SizeInBytes = DL.getTypeAllocSize(Ty); - const uint64_t RightRedzoneSize = [&] { - // The algorithm for calculating red zone size comes from - // llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp - uint64_t RZ = 0; - if (SizeInBytes <= MinRZ / 2) { - // Reduce redzone size for small size objects, e.g. int, char[1]. - // Optimize when SizeInBytes is less than or equal to half of MinRZ. - RZ = MinRZ - SizeInBytes; - } else { - // Calculate RZ, where MinRZ <= RZ <= MaxRZ, and RZ ~ 1/4 * - // SizeInBytes. - RZ = std::clamp((SizeInBytes / MinRZ / 4) * MinRZ, MinRZ, MaxRZ); - - // Round up to multiple of MinRZ. - if (SizeInBytes % MinRZ) - RZ += MinRZ - (SizeInBytes % MinRZ); - } - - assert((RZ + SizeInBytes) % MinRZ == 0); - return RZ; - }(); - Type *RightRedZoneTy = ArrayType::get(IRB.getInt8Ty(), RightRedzoneSize); - StructType *NewTy = StructType::get(Ty, RightRedZoneTy); - Constant *NewInitializer = ConstantStruct::get( - NewTy, G.getInitializer(), Constant::getNullValue(RightRedZoneTy)); - - // Create a new global variable with enough space for a redzone. - GlobalVariable *NewGlobal = new GlobalVariable( - M, NewTy, G.isConstant(), G.getLinkage(), NewInitializer, "", &G, - G.getThreadLocalMode(), G.getAddressSpace()); - NewGlobal->copyAttributesFrom(&G); - NewGlobal->setComdat(G.getComdat()); - NewGlobal->setAlignment(Align(MinRZ)); - NewGlobal->copyMetadata(&G, 0); - - Value *Indices2[2]; - Indices2[0] = IRB.getInt32(0); - Indices2[1] = IRB.getInt32(0); - - G.replaceAllUsesWith( - ConstantExpr::getGetElementPtr(NewTy, NewGlobal, Indices2, true)); - NewGlobal->takeName(&G); - GlobalsToRemove.push_back(&G); - NewDeviceGlobals.push_back(NewGlobal); - DeviceGlobalMetadata.push_back(ConstantStruct::get( - StructTy, ConstantInt::get(IntTy, SizeInBytes), - ConstantInt::get(IntTy, SizeInBytes + RightRedzoneSize), - ConstantExpr::getPointerCast(NewGlobal, IntTy))); - } - - if (GlobalsToRemove.empty()) - return false; - - // Create global to record number of device globals - GlobalVariable *NumOfDeviceGlobals = new GlobalVariable( - M, IntTy, false, GlobalValue::ExternalLinkage, - ConstantInt::get(IntTy, NewDeviceGlobals.size()), - "__AsanDeviceGlobalCount", nullptr, GlobalValue::NotThreadLocal, 1); - NumOfDeviceGlobals->setUnnamedAddr(GlobalValue::UnnamedAddr::Local); - - // Create meta data global to record device globals' information - ArrayType *ArrayTy = ArrayType::get(StructTy, NewDeviceGlobals.size()); - Constant *MetadataInitializer = - ConstantArray::get(ArrayTy, DeviceGlobalMetadata); - GlobalVariable *AsanDeviceGlobalMetadata = new GlobalVariable( - M, MetadataInitializer->getType(), false, GlobalValue::ExternalLinkage, - MetadataInitializer, "__AsanDeviceGlobalMetadata", nullptr, - GlobalValue::NotThreadLocal, 1); - AsanDeviceGlobalMetadata->setUnnamedAddr(GlobalValue::UnnamedAddr::Local); - - for (auto *G : GlobalsToRemove) - G->eraseFromParent(); - - return true; -} - -} - -namespace llvm { - -PreservedAnalyses SanitizeDeviceGlobalPass::run(Module &M, - ModuleAnalysisManager &MAM) { - bool Modified = false; - - Modified |= instrumentDeviceGlobal(M); - - return Modified ? PreservedAnalyses::none() : PreservedAnalyses::all(); -} - -} diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 38c37d861aa00..52bc80ce79f67 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -64,6 +64,7 @@ #include "llvm/IR/Use.h" #include "llvm/IR/Value.h" #include "llvm/MC/MCSectionMachO.h" +#include "llvm/SYCLLowerIR/DeviceGlobals.h" #include "llvm/Support/Casting.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" @@ -923,6 +924,7 @@ class ModuleAddressSanitizer { void initializeCallbacks(Module &M); void instrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat); + void instrumentDeviceGlobal(IRBuilder<> &IRB, Module &M); void InstrumentGlobalsCOFF(IRBuilder<> &IRB, Module &M, ArrayRef ExtendedGlobals, ArrayRef MetadataInitializers); @@ -2449,6 +2451,77 @@ Instruction *ModuleAddressSanitizer::CreateAsanModuleDtor(Module &M) { return ReturnInst::Create(*C, AsanDtorBB); } +void ModuleAddressSanitizer::instrumentDeviceGlobal(IRBuilder<> &IRB, + Module &M) { + auto &DL = M.getDataLayout(); + SmallVector GlobalsToRemove; + SmallVector NewDeviceGlobals; + SmallVector DeviceGlobalMetadata; + + Type *IntTy = Type::getIntNTy(M.getContext(), DL.getPointerSizeInBits()); + + // Device global meta data is described by a structure + // size_t device_global_size + // size_t device_global_size_with_red_zone + // size_t beginning address of the device global + StructType *StructTy = StructType::get(IntTy, IntTy, IntTy); + + for (auto &G : M.globals()) { + // Non image scope device globals are implemented by device USM, and the + // out-of-bounds check for them will be done by sanitizer USM part. So we + // exclude them here. + if (!isDeviceGlobalVariable(G) || !hasDeviceImageScopeProperty(G)) + continue; + + Type *Ty = G.getValueType(); + const uint64_t SizeInBytes = DL.getTypeAllocSize(Ty); + const uint64_t RightRedzoneSize = getRedzoneSizeForGlobal(SizeInBytes); + Type *RightRedZoneTy = ArrayType::get(IRB.getInt8Ty(), RightRedzoneSize); + StructType *NewTy = StructType::get(Ty, RightRedZoneTy); + Constant *NewInitializer = ConstantStruct::get( + NewTy, G.getInitializer(), Constant::getNullValue(RightRedZoneTy)); + + // Create a new global variable with enough space for a redzone. + GlobalVariable *NewGlobal = new GlobalVariable( + M, NewTy, G.isConstant(), G.getLinkage(), NewInitializer, "", &G, + G.getThreadLocalMode(), G.getAddressSpace()); + NewGlobal->copyAttributesFrom(&G); + NewGlobal->setComdat(G.getComdat()); + NewGlobal->setAlignment(Align(getMinRedzoneSizeForGlobal())); + NewGlobal->copyMetadata(&G, 0); + + Value *Indices2[2]; + Indices2[0] = IRB.getInt32(0); + Indices2[1] = IRB.getInt32(0); + + G.replaceAllUsesWith( + ConstantExpr::getGetElementPtr(NewTy, NewGlobal, Indices2, true)); + NewGlobal->takeName(&G); + GlobalsToRemove.push_back(&G); + NewDeviceGlobals.push_back(NewGlobal); + DeviceGlobalMetadata.push_back(ConstantStruct::get( + StructTy, ConstantInt::get(IntTy, SizeInBytes), + ConstantInt::get(IntTy, SizeInBytes + RightRedzoneSize), + ConstantExpr::getPointerCast(NewGlobal, IntTy))); + } + + if (GlobalsToRemove.empty()) + return; + + // Create meta data global to record device globals' information + ArrayType *ArrayTy = ArrayType::get(StructTy, NewDeviceGlobals.size()); + Constant *MetadataInitializer = + ConstantArray::get(ArrayTy, DeviceGlobalMetadata); + GlobalVariable *AsanDeviceGlobalMetadata = new GlobalVariable( + M, MetadataInitializer->getType(), false, GlobalValue::AppendingLinkage, + MetadataInitializer, "__AsanDeviceGlobalMetadata", nullptr, + GlobalValue::NotThreadLocal, 1); + AsanDeviceGlobalMetadata->setUnnamedAddr(GlobalValue::UnnamedAddr::Local); + + for (auto *G : GlobalsToRemove) + G->eraseFromParent(); +} + void ModuleAddressSanitizer::InstrumentGlobalsCOFF( IRBuilder<> &IRB, Module &M, ArrayRef ExtendedGlobals, ArrayRef MetadataInitializers) { @@ -2922,6 +2995,9 @@ bool ModuleAddressSanitizer::instrumentModule(Module &M) { auto *MD = M.getOrInsertNamedMetadata("device.sanitizer"); Metadata *MDVals[] = {MDString::get(Ctx, "asan")}; MD->addOperand(MDNode::get(Ctx, MDVals)); + + IRBuilder<> IRB(*C); + instrumentDeviceGlobal(IRB, M); } const uint64_t Priority = GetCtorAndDtorPriority(TargetTriple); diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt index bc9e35c4f763e..ce42a8843c2da 100644 --- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt +++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt @@ -36,6 +36,7 @@ add_llvm_component_library(LLVMInstrumentation Core Demangle MC + SYCLLowerIR Support TargetParser TransformUtils diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index e2bd06e0707bd..84cc54db0ffc6 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -43,7 +43,6 @@ #include "llvm/SYCLLowerIR/ModuleSplitter.h" #include "llvm/SYCLLowerIR/SYCLDeviceRequirements.h" #include "llvm/SYCLLowerIR/SYCLUtils.h" -#include "llvm/SYCLLowerIR/SanitizeDeviceGlobal.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/InitLLVM.h" @@ -1088,11 +1087,6 @@ processInputModule(std::unique_ptr M) { // "llvm.compiler.used" they can be erased safely. Modified |= removeDeviceGlobalFromCompilerUsed(*M.get()); - // Instrument each image scope device globals if the module has been - // instrumented by sanitizer pass. - if (isModuleUsingAsan(*M)) - Modified |= runModulePass(*M); - // Do invoke_simd processing before splitting because this: // - saves processing time (the pass is run once, even though on larger IR) // - doing it before SYCL/ESIMD splitting is required for correctness diff --git a/sycl/plugins/unified_runtime/CMakeLists.txt b/sycl/plugins/unified_runtime/CMakeLists.txt index a4aa203f05365..e6e8e777ff48d 100644 --- a/sycl/plugins/unified_runtime/CMakeLists.txt +++ b/sycl/plugins/unified_runtime/CMakeLists.txt @@ -94,14 +94,8 @@ if(SYCL_PI_UR_USE_FETCH_CONTENT) CACHE PATH "Path to external '${name}' adapter source dir" FORCE) endfunction() - set(UNIFIED_RUNTIME_REPO "https://github.com/oneapi-src/unified-runtime.git") - # commit ebf873fb5996c9ddca32bbb7c9330d3ffe15473c - # Merge: 633ec408 8f375039 - # Author: Kenneth Benzie (Benie) - # Date: Thu May 2 10:56:55 2024 +0100 - # Merge pull request #1535 from przemektmalon/przemek/sampled-image-fetch - # [Bindless][Exp] Add device queries for sampled image fetch - set(UNIFIED_RUNTIME_TAG ebf873fb5996c9ddca32bbb7c9330d3ffe15473c) + set(UNIFIED_RUNTIME_REPO "https://github.com/zhaomaosu/unified-runtime.git") + set(UNIFIED_RUNTIME_TAG simplify-device-global) fetch_adapter_source(level_zero "https://github.com/oneapi-src/unified-runtime.git" From 84079bc288c787d17b9cab0cf7a425cf194207cf Mon Sep 17 00:00:00 2001 From: Maosu Zhao Date: Thu, 9 May 2024 16:21:10 +0800 Subject: [PATCH 2/9] Add lit tests and option --- .../Transforms/Instrumentation/AddressSanitizer.cpp | 11 +++++++++-- .../AddressSanitizer/SPIRV/device_global.ll | 13 +++++++++++++ .../SPIRV/device_global_non_image_scope.ll | 11 +++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll create mode 100644 llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 52bc80ce79f67..0476eefc3dbe3 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -437,6 +437,11 @@ static cl::opt ClOverrideDestructorKind( "Use global destructors")), cl::init(AsanDtorKind::Invalid), cl::Hidden); +// SYCL flags +static cl::opt ClDeviceGlobals("asan-device-globals", + cl::desc("instrument device globals"), + cl::Hidden, cl::init(true)); + // Debug flags. static cl::opt ClDebug("asan-debug", cl::desc("debug"), cl::Hidden, @@ -2996,8 +3001,10 @@ bool ModuleAddressSanitizer::instrumentModule(Module &M) { Metadata *MDVals[] = {MDString::get(Ctx, "asan")}; MD->addOperand(MDNode::get(Ctx, MDVals)); - IRBuilder<> IRB(*C); - instrumentDeviceGlobal(IRB, M); + if (ClDeviceGlobals) { + IRBuilder<> IRB(*C); + instrumentDeviceGlobal(IRB, M); + } } const uint64_t Priority = GetCtorAndDtorPriority(TargetTriple); diff --git a/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll new file mode 100644 index 0000000000000..6fa8c999e4a75 --- /dev/null +++ b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll @@ -0,0 +1,13 @@ +; RUN: opt < %s -passes=asan -asan-instrumentation-with-call-threshold=0 -asan-stack=0 -asan-globals=0 -asan-constructor-kind=none -S | FileCheck %s + +; check that if image scope device globals can be correctly instrumented. + +target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64" +target triple = "spir64-unknown-unknown" + +@dev_global = addrspace(1) global { [4 x i32] } zeroinitializer #0 + +; CHECK: @dev_global = addrspace(1) global { { [4 x i32] }, [16 x i8] } +; CHECK: @__AsanDeviceGlobalMetadata = appending local_unnamed_addr addrspace(1) global [1 x { i64, i64, i64 }] [{ i64, i64, i64 } { i64 16, i64 32, i64 ptrtoint (ptr addrspace(1) @dev_global to i64) }] + +attributes #0 = { "sycl-device-global-size"="16" "sycl-device-image-scope" } diff --git a/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll new file mode 100644 index 0000000000000..d2354d8dfa7be --- /dev/null +++ b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll @@ -0,0 +1,11 @@ +; RUN: opt < %s -passes=asan -asan-instrumentation-with-call-threshold=0 -asan-stack=0 -asan-globals=0 -asan-constructor-kind=none -S | FileCheck %s + +; check if non image scope device globals will not be instrumented. + +target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64" +target triple = "spir64-unknown-unknown" + +@dev_global = addrspace(1) global { ptr addrspace(1), [4 x i32] } zeroinitializer + +; CHECK: @dev_global = addrspace(1) global { ptr addrspace(1), [4 x i32] } +; CHECK-NOT: @__AsanDeviceGlobalMetadata From ef376bb7289591c8ccfb1116872919c067b7500d Mon Sep 17 00:00:00 2001 From: Maosu Zhao Date: Tue, 21 May 2024 16:00:29 +0800 Subject: [PATCH 3/9] Minor updates --- .../Instrumentation/AddressSanitizer.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index d217bb022d936..af9072797134d 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -2650,16 +2650,15 @@ void ModuleAddressSanitizer::instrumentDeviceGlobal(IRBuilder<> &IRB, Module &M) { auto &DL = M.getDataLayout(); SmallVector GlobalsToRemove; - SmallVector NewDeviceGlobals; SmallVector DeviceGlobalMetadata; - Type *IntTy = Type::getIntNTy(M.getContext(), DL.getPointerSizeInBits()); + Type *IntptrTy = Type::getIntNTy(M.getContext(), DL.getPointerSizeInBits()); // Device global meta data is described by a structure // size_t device_global_size // size_t device_global_size_with_red_zone // size_t beginning address of the device global - StructType *StructTy = StructType::get(IntTy, IntTy, IntTy); + StructType *StructTy = StructType::get(IntptrTy, IntptrTy, IntptrTy); for (auto &G : M.globals()) { // Non image scope device globals are implemented by device USM, and the @@ -2693,18 +2692,17 @@ void ModuleAddressSanitizer::instrumentDeviceGlobal(IRBuilder<> &IRB, ConstantExpr::getGetElementPtr(NewTy, NewGlobal, Indices2, true)); NewGlobal->takeName(&G); GlobalsToRemove.push_back(&G); - NewDeviceGlobals.push_back(NewGlobal); DeviceGlobalMetadata.push_back(ConstantStruct::get( - StructTy, ConstantInt::get(IntTy, SizeInBytes), - ConstantInt::get(IntTy, SizeInBytes + RightRedzoneSize), - ConstantExpr::getPointerCast(NewGlobal, IntTy))); + StructTy, ConstantInt::get(IntptrTy, SizeInBytes), + ConstantInt::get(IntptrTy, SizeInBytes + RightRedzoneSize), + ConstantExpr::getPointerCast(NewGlobal, IntptrTy))); } if (GlobalsToRemove.empty()) return; // Create meta data global to record device globals' information - ArrayType *ArrayTy = ArrayType::get(StructTy, NewDeviceGlobals.size()); + ArrayType *ArrayTy = ArrayType::get(StructTy, DeviceGlobalMetadata.size()); Constant *MetadataInitializer = ConstantArray::get(ArrayTy, DeviceGlobalMetadata); GlobalVariable *AsanDeviceGlobalMetadata = new GlobalVariable( From d9c9b9fd1149888455ea6f66ff3051c1daaa5865 Mon Sep 17 00:00:00 2001 From: "Zhao, Maosu" Date: Mon, 3 Jun 2024 04:56:21 -0700 Subject: [PATCH 4/9] Minor fix --- .../Instrumentation/AddressSanitizer.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index af9072797134d..f0f91a4f9d77d 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -1468,12 +1468,23 @@ static bool isUnsupportedAMDGPUAddrspace(Value *Addr) { return false; } +static bool isUnsupportedDeviceGlobal(GlobalVariable *G) { + // Non image scope device globals are implemented by device USM, and the + // out-of-bounds check for them will be done by sanitizer USM part. So we + // exclude them here. + return (!isDeviceGlobalVariable(*G) || !hasDeviceImageScopeProperty(*G)); +} + static bool isUnsupportedSPIRAccess(Value *Addr, Function *Func) { // Skip SPIR-V built-in varibles auto *OrigValue = Addr->stripInBoundsOffsets(); if (OrigValue->getName().starts_with("__spirv_BuiltIn")) return true; + GlobalVariable *GV = dyn_cast(OrigValue); + if (GV && isUnsupportedDeviceGlobal(GV)) + return true; + Type *PtrTy = cast(Addr->getType()->getScalarType()); switch (PtrTy->getPointerAddressSpace()) { case kSpirOffloadPrivateAS: { @@ -2661,10 +2672,7 @@ void ModuleAddressSanitizer::instrumentDeviceGlobal(IRBuilder<> &IRB, StructType *StructTy = StructType::get(IntptrTy, IntptrTy, IntptrTy); for (auto &G : M.globals()) { - // Non image scope device globals are implemented by device USM, and the - // out-of-bounds check for them will be done by sanitizer USM part. So we - // exclude them here. - if (!isDeviceGlobalVariable(G) || !hasDeviceImageScopeProperty(G)) + if (isUnsupportedDeviceGlobal(&G)) continue; Type *Ty = G.getValueType(); From 3ec0ee05852cb6e17a17c1f6e45b90a81e9732d1 Mon Sep 17 00:00:00 2001 From: Maosu Zhao Date: Wed, 5 Jun 2024 17:37:15 +0800 Subject: [PATCH 5/9] Ignore __AsanDeviceGlobalMetadata in OffloadBundler --- clang/lib/Driver/OffloadBundler.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp index 39344379b1fdf..b8ff0c6fdff19 100644 --- a/clang/lib/Driver/OffloadBundler.cpp +++ b/clang/lib/Driver/OffloadBundler.cpp @@ -687,8 +687,11 @@ class ObjectFileHandler final : public FileHandler { return std::move(Err); // If we are dealing with a bitcode file do not add special globals - // llvm.used and llvm.compiler.used to the list of defined symbols. - if (SF->isIR() && (Name == "llvm.used" || Name == "llvm.compiler.used")) + // llvm.used and llvm.compiler.used and __AsanDeviceGlobalMetadata to + // the list of defined symbols. + if (SF->isIR() && + (Name == "llvm.used" || Name == "llvm.compiler.used" || + Name == "__AsanDeviceGlobalMetadata")) continue; // Add symbol name with the target prefix to the buffer. From b1ac2b01c0df130aaa37a60a081eb1297ab22408 Mon Sep 17 00:00:00 2001 From: "Zhao, Maosu" Date: Wed, 4 Sep 2024 22:29:27 -0700 Subject: [PATCH 6/9] Update tag --- sycl/cmake/modules/FetchUnifiedRuntime.cmake | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sycl/cmake/modules/FetchUnifiedRuntime.cmake b/sycl/cmake/modules/FetchUnifiedRuntime.cmake index d3da2e00062b2..326cc5a95ae8c 100644 --- a/sycl/cmake/modules/FetchUnifiedRuntime.cmake +++ b/sycl/cmake/modules/FetchUnifiedRuntime.cmake @@ -116,14 +116,8 @@ if(SYCL_UR_USE_FETCH_CONTENT) CACHE PATH "Path to external '${name}' adapter source dir" FORCE) endfunction() - set(UNIFIED_RUNTIME_REPO "https://github.com/oneapi-src/unified-runtime.git") - # commit b766009add8dc41ad03e2a63be9f6ed40eea4c55 (HEAD, origin/main, origin/HEAD) - # Merge: 608e9184 c3c57284 - # Author: Omar Ahmed - # Date: Tue Aug 27 16:30:04 2024 +0100 - # Merge pull request #2011 from nrspruit/fix_opencl_usm_align_check - # [OpenCL] Fix USM alignment error check to occur always and return nulllptr - set(UNIFIED_RUNTIME_TAG b766009add8dc41ad03e2a63be9f6ed40eea4c55) + set(UNIFIED_RUNTIME_REPO "https://github.com/zhaomaosu/unified-runtime.git") + set(UNIFIED_RUNTIME_TAG simplify-device-global) set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "EXAMPLES") # Due to the use of dependentloadflag and no installer for UMF and hwloc we need From 12bcc7e80479c26a3ff559ffe915a0a2fe841ba9 Mon Sep 17 00:00:00 2001 From: Maosu Zhao Date: Wed, 30 Oct 2024 15:46:30 +0800 Subject: [PATCH 7/9] Update comments --- .../Instrumentation/AddressSanitizer/SPIRV/device_global.ll | 2 +- .../AddressSanitizer/SPIRV/device_global_non_image_scope.ll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll index 6fa8c999e4a75..a30eca4bc75be 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll +++ b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global.ll @@ -1,6 +1,6 @@ ; RUN: opt < %s -passes=asan -asan-instrumentation-with-call-threshold=0 -asan-stack=0 -asan-globals=0 -asan-constructor-kind=none -S | FileCheck %s -; check that if image scope device globals can be correctly instrumented. +; check that image scope device globals can be correctly instrumented. target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64" target triple = "spir64-unknown-unknown" diff --git a/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll index d2354d8dfa7be..735c437c47169 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll +++ b/llvm/test/Instrumentation/AddressSanitizer/SPIRV/device_global_non_image_scope.ll @@ -1,6 +1,6 @@ ; RUN: opt < %s -passes=asan -asan-instrumentation-with-call-threshold=0 -asan-stack=0 -asan-globals=0 -asan-constructor-kind=none -S | FileCheck %s -; check if non image scope device globals will not be instrumented. +; check non image scope device globals will not be instrumented. target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64" target triple = "spir64-unknown-unknown" From 7bdef55d58727168a03e280c7168e4bc086cc280 Mon Sep 17 00:00:00 2001 From: Maosu Zhao Date: Wed, 30 Oct 2024 16:27:34 +0800 Subject: [PATCH 8/9] Update per comment --- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 8cf706f1b48ef..a787961cb547c 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -2782,7 +2782,7 @@ void ModuleAddressSanitizer::instrumentDeviceGlobal(IRBuilder<> &IRB) { SmallVector GlobalsToRemove; SmallVector DeviceGlobalMetadata; - Type *IntptrTy = Type::getIntNTy(M.getContext(), DL.getPointerSizeInBits()); + Type *IntptrTy = M.getDataLayout().getIntPtrType(*C, kSpirOffloadGlobalAS); // Device global meta data is described by a structure // size_t device_global_size From c8bf9a8070c61a907dac83de01ffd12e1a3aaa9c Mon Sep 17 00:00:00 2001 From: Callum Fare Date: Tue, 19 Nov 2024 10:26:38 +0000 Subject: [PATCH 9/9] Bump UR tag --- sycl/cmake/modules/FetchUnifiedRuntime.cmake | 2 +- sycl/cmake/modules/UnifiedRuntimeTag.cmake | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sycl/cmake/modules/FetchUnifiedRuntime.cmake b/sycl/cmake/modules/FetchUnifiedRuntime.cmake index d6a0251d63c86..72841724fa01d 100644 --- a/sycl/cmake/modules/FetchUnifiedRuntime.cmake +++ b/sycl/cmake/modules/FetchUnifiedRuntime.cmake @@ -116,7 +116,7 @@ if(SYCL_UR_USE_FETCH_CONTENT) CACHE PATH "Path to external '${name}' adapter source dir" FORCE) endfunction() - set(UNIFIED_RUNTIME_REPO "https://github.com/zhaomaosu/unified-runtime.git") + set(UNIFIED_RUNTIME_REPO "https://github.com/oneapi-src/unified-runtime.git") include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules/UnifiedRuntimeTag.cmake) set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "EXAMPLES") diff --git a/sycl/cmake/modules/UnifiedRuntimeTag.cmake b/sycl/cmake/modules/UnifiedRuntimeTag.cmake index dbb8df4e07154..3744a4e87ad76 100644 --- a/sycl/cmake/modules/UnifiedRuntimeTag.cmake +++ b/sycl/cmake/modules/UnifiedRuntimeTag.cmake @@ -1,7 +1,7 @@ -# commit dbd168cbed2d2590b47904728cd5762f1c2f4c6b (HEAD, origin/main, origin/HEAD) -# Merge: 694c1b9a 27ad3f7d -# Author: Piotr Balcer -# Date: Mon Oct 28 16:29:45 2024 +0100 -# Merge pull request #2242 from nrspruit/sysman_env_disable -# [L0] Enable Sysman Thru Env by default and have zesInit be optional -set(UNIFIED_RUNTIME_TAG simplify-device-global) +# commit 0ea47d7c70b9a21a3d90612a0a0e7525034e62f7 +# Merge: e3247c23 e36941cb +# Author: Callum Fare +# Date: Tue Nov 19 10:24:08 2024 +0000 +# Merge pull request #1584 from zhaomaosu/simplify-device-global +# [DeviceSanitizer] Remove device global "__AsanDeviceGlobalCount" +set(UNIFIED_RUNTIME_TAG 0ea47d7c70b9a21a3d90612a0a0e7525034e62f7)