From 57d25733636f24239b1a15775e06ea670f79782f Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 00:14:26 +0000 Subject: [PATCH 1/7] [hwasan] Optimize lowering of AArch64 check.memaccess for null pointer If the pointer to be checked is statically known to be zero, the tag check will pass since: 1) the tag is zero 2) shadow memory for address 0 is initialized to 0. We therefore elide the check when lowering. This also updates the test in https://github.com/llvm/llvm-project/pull/122186 Note: the HWASan instrumentation pass will still emit the check.memaccess intrinsic. This patch performs the elision at CodeGen. --- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 8 ++++++++ llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll | 1 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index 9bec782ca8ce9..d0804d8c39b90 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -605,6 +605,14 @@ void AArch64AsmPrinter::LowerKCFI_CHECK(const MachineInstr &MI) { void AArch64AsmPrinter::LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI) { Register Reg = MI.getOperand(0).getReg(); + + // If the pointer is statically known to be zero, it has a zero tag and the + // tag check will pass since the shadow memory corresponding to address 0 + // is initialized to zero and never updated. We can therefore elide the tag + // check. + if (Reg == AArch64::XZR) + return; + bool IsShort = ((MI.getOpcode() == AArch64::HWASAN_CHECK_MEMACCESS_SHORTGRANULES) || (MI.getOpcode() == diff --git a/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll b/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll index dca39fe03fb10..1e0b113707168 100644 --- a/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll +++ b/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll @@ -25,7 +25,6 @@ define void @test_store_to_zeroptr() #0 { ; CHECK-NEXT: str x30, [sp, #-16]! // 8-byte Folded Spill ; CHECK-NEXT: .cfi_def_cfa_offset 16 ; CHECK-NEXT: .cfi_offset w30, -16 -; CHECK-NEXT: bl __hwasan_check_x4294967071_19_fixed_0_short_v2 ; CHECK-NEXT: mov x8, xzr ; CHECK-NEXT: mov w9, #42 // =0x2a ; CHECK-NEXT: str x9, [x8] From a4c255bee3c362ca736c845dffb618a859e8b5d6 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 00:38:25 +0000 Subject: [PATCH 2/7] Omit check during HWASan pass if pointer is statically known to be null --- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 8 ++++---- .../Instrumentation/HWAddressSanitizer.cpp | 14 +++++++++++--- llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll | 10 ++++++---- .../Instrumentation/HWAddressSanitizer/zero-ptr.ll | 4 +--- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index d0804d8c39b90..d4e1f9916ff66 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -606,10 +606,10 @@ void AArch64AsmPrinter::LowerKCFI_CHECK(const MachineInstr &MI) { void AArch64AsmPrinter::LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI) { Register Reg = MI.getOperand(0).getReg(); - // If the pointer is statically known to be zero, it has a zero tag and the - // tag check will pass since the shadow memory corresponding to address 0 - // is initialized to zero and never updated. We can therefore elide the tag - // check. + // The HWASan pass won't emit a CHECK_MEMACCESS intrinsic with a pointer + // statically known to be zero. However, conceivably the HWASan pass has a + // "maybe non-zero" pointer but later optimization passes convert it into + // a null pointer. As a last line of defense, we perform elision here too. if (Reg == AArch64::XZR) return; diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index 2031728c2f33d..8e966a739c8c0 100644 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -348,7 +348,7 @@ class HWAddressSanitizer { bool ignoreMemIntrinsic(OptimizationRemarkEmitter &ORE, MemIntrinsic *MI); void instrumentMemIntrinsic(MemIntrinsic *MI); bool instrumentMemAccess(InterestingMemoryOperand &O, DomTreeUpdater &DTU, - LoopInfo *LI); + LoopInfo *LI, const DataLayout &DL); bool ignoreAccessWithoutRemark(Instruction *Inst, Value *Ptr); bool ignoreAccess(OptimizationRemarkEmitter &ORE, Instruction *Inst, Value *Ptr); @@ -1164,11 +1164,18 @@ void HWAddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) { bool HWAddressSanitizer::instrumentMemAccess(InterestingMemoryOperand &O, DomTreeUpdater &DTU, - LoopInfo *LI) { + LoopInfo *LI, const DataLayout &DL) { Value *Addr = O.getPtr(); LLVM_DEBUG(dbgs() << "Instrumenting: " << O.getInsn() << "\n"); + // If the pointer is statically known to be zero, it has a zero tag and the + // tag check will pass since the shadow memory corresponding to address 0 + // is initialized to zero and never updated. We can therefore elide the tag + // check. + if (!llvm::isKnownNonZero(Addr, DL)) + return false; + if (O.MaybeMask) return false; // FIXME @@ -1701,8 +1708,9 @@ void HWAddressSanitizer::sanitizeFunction(Function &F, PostDominatorTree *PDT = FAM.getCachedResult(F); LoopInfo *LI = FAM.getCachedResult(F); DomTreeUpdater DTU(DT, PDT, DomTreeUpdater::UpdateStrategy::Lazy); + const DataLayout &DL = F.getDataLayout(); for (auto &Operand : OperandsToInstrument) - instrumentMemAccess(Operand, DTU, LI); + instrumentMemAccess(Operand, DTU, LI, DL); DTU.flush(); if (ClInstrumentMemIntrinsics && !IntrinToInstrument.empty()) { diff --git a/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll b/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll index 1e0b113707168..7aebe2121aeb6 100644 --- a/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll +++ b/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll @@ -1,11 +1,13 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5 ; RUN: llc -filetype asm -o - %s | FileCheck %s -; This shows that when dereferencing a null pointer, HWASan will call -; __hwasan_check_x4294967071_19_fixed_0_short_v2 -; (N.B. 4294967071 == 2**32 - 239 + 14 == 2**32 - X0 + XZR +; This shows that HWASan will elide the tag check when lowering the memaccess +; intrinsic for null pointers. +; N.B. The HWASan pass will normally omit the memaccess intrinsic if the +; pointer is already statically known to be null. ; -; The source was generated from llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll. +; The source was generated from llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll +; with the memaccess deliberately retained. ; ModuleID = '' source_filename = "" diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll b/llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll index a201174df995b..95cf6f1544df0 100644 --- a/llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll +++ b/llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll @@ -2,7 +2,7 @@ ; RUN: opt < %s -passes=hwasan -S | FileCheck %s ; RUN: opt < %s -passes=hwasan -hwasan-recover=0 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=ABORT-ZERO-BASED-SHADOW -; This shows that HWASan will emit a memaccess check when dereferencing a null +; This shows that HWASan omits the memaccess check when dereferencing a null ; pointer. ; The output is used as the source for llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll. @@ -15,7 +15,6 @@ define void @test_store_to_zeroptr() sanitize_hwaddress { ; CHECK-NEXT: entry: ; CHECK-NEXT: [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr @__hwasan_shadow) ; CHECK-NEXT: [[B:%.*]] = inttoptr i64 0 to ptr -; CHECK-NEXT: call void @llvm.hwasan.check.memaccess.shortgranules(ptr [[DOTHWASAN_SHADOW]], ptr [[B]], i32 19) ; CHECK-NEXT: store i64 42, ptr [[B]], align 8 ; CHECK-NEXT: ret void ; @@ -24,7 +23,6 @@ define void @test_store_to_zeroptr() sanitize_hwaddress { ; ABORT-ZERO-BASED-SHADOW-NEXT: entry: ; ABORT-ZERO-BASED-SHADOW-NEXT: [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr null) ; ABORT-ZERO-BASED-SHADOW-NEXT: [[B:%.*]] = inttoptr i64 0 to ptr -; ABORT-ZERO-BASED-SHADOW-NEXT: call void @llvm.hwasan.check.memaccess.shortgranules.fixedshadow(ptr [[B]], i32 19, i64 0) ; ABORT-ZERO-BASED-SHADOW-NEXT: store i64 42, ptr [[B]], align 8 ; ABORT-ZERO-BASED-SHADOW-NEXT: ret void ; From 4c37dd3ee0c14172dad47fab315aa60ddbf9557c Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 01:23:50 +0000 Subject: [PATCH 3/7] Fix check for known zero value --- .../Instrumentation/HWAddressSanitizer.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index 8e966a739c8c0..6639fff75faa5 100644 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1169,11 +1169,15 @@ bool HWAddressSanitizer::instrumentMemAccess(InterestingMemoryOperand &O, LLVM_DEBUG(dbgs() << "Instrumenting: " << O.getInsn() << "\n"); - // If the pointer is statically known to be zero, it has a zero tag and the - // tag check will pass since the shadow memory corresponding to address 0 - // is initialized to zero and never updated. We can therefore elide the tag - // check. - if (!llvm::isKnownNonZero(Addr, DL)) + // If the pointer is statically known to be zero, the tag check will pass + // since: + // 1) it has a zero tag + // 2) the shadow memory corresponding to address 0 is initialized to zero and + // never updated. + // We can therefore elide the tag check. + llvm::KnownBits Known(DL.getPointerTypeSizeInBits(Addr->getType())); + llvm::computeKnownBits(Addr, Known, DL); + if (Known.getMinValue() == 0 && Known.getMaxValue() == 0) return false; if (O.MaybeMask) From 5042e5877c9dd006a44d890c8814b840256a4d05 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 01:29:04 +0000 Subject: [PATCH 4/7] clang-format --- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index 6639fff75faa5..534abf2eadf96 100644 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1163,8 +1163,8 @@ void HWAddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) { } bool HWAddressSanitizer::instrumentMemAccess(InterestingMemoryOperand &O, - DomTreeUpdater &DTU, - LoopInfo *LI, const DataLayout &DL) { + DomTreeUpdater &DTU, LoopInfo *LI, + const DataLayout &DL) { Value *Addr = O.getPtr(); LLVM_DEBUG(dbgs() << "Instrumenting: " << O.getInsn() << "\n"); From 1a54db93f358b917c4216a3e50a8a6798a48d38d Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 05:54:13 +0000 Subject: [PATCH 5/7] Use Known.isZero(). Update comment. --- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 7 ++++--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index d4e1f9916ff66..9d9d9889b3858 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -607,9 +607,10 @@ void AArch64AsmPrinter::LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI) { Register Reg = MI.getOperand(0).getReg(); // The HWASan pass won't emit a CHECK_MEMACCESS intrinsic with a pointer - // statically known to be zero. However, conceivably the HWASan pass has a - // "maybe non-zero" pointer but later optimization passes convert it into - // a null pointer. As a last line of defense, we perform elision here too. + // statically known to be zero. However, conceivably, the HWASan pass may + // encounter a "cannot currently statically prove to be null" pointer (and is + // therefore unable to omit the intrinsic) that later optimization passes + // convert into a statically known-null pointer. if (Reg == AArch64::XZR) return; diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index 534abf2eadf96..1cb5469b052e7 100644 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1177,7 +1177,7 @@ bool HWAddressSanitizer::instrumentMemAccess(InterestingMemoryOperand &O, // We can therefore elide the tag check. llvm::KnownBits Known(DL.getPointerTypeSizeInBits(Addr->getType())); llvm::computeKnownBits(Addr, Known, DL); - if (Known.getMinValue() == 0 && Known.getMaxValue() == 0) + if (Known.isZero()) return false; if (O.MaybeMask) From cbb6b220f63224a5d60b0f01da2286d7b4964a9f Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 05:57:12 +0000 Subject: [PATCH 6/7] Whitespace --- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index 1cb5469b052e7..75a19357ea1bb 100644 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1173,7 +1173,7 @@ bool HWAddressSanitizer::instrumentMemAccess(InterestingMemoryOperand &O, // since: // 1) it has a zero tag // 2) the shadow memory corresponding to address 0 is initialized to zero and - // never updated. + // never updated. // We can therefore elide the tag check. llvm::KnownBits Known(DL.getPointerTypeSizeInBits(Addr->getType())); llvm::computeKnownBits(Addr, Known, DL); From 7f8c862f0953b6f59eb703810b74d85139cc172a Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 9 Jan 2025 05:59:09 +0000 Subject: [PATCH 7/7] Update test comment --- llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll b/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll index 7aebe2121aeb6..b2eaa31007c35 100644 --- a/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll +++ b/llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll @@ -1,10 +1,11 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5 ; RUN: llc -filetype asm -o - %s | FileCheck %s -; This shows that HWASan will elide the tag check when lowering the memaccess -; intrinsic for null pointers. +; This shows that CodeGen for AArch64 will elide the tag check when lowering +; the HWASan memaccess intrinsic for null pointers. +; ; N.B. The HWASan pass will normally omit the memaccess intrinsic if the -; pointer is already statically known to be null. +; pointer is already statically known to be null. ; ; The source was generated from llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll ; with the memaccess deliberately retained.