[llvm] [hwasan] Omit tag check for null pointers (PR #122206)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 8 21:57:30 PST 2025
https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/122206
>From 57d25733636f24239b1a15775e06ea670f79782f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 9 Jan 2025 00:14:26 +0000
Subject: [PATCH 1/6] [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 9bec782ca8ce97..d0804d8c39b90f 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 dca39fe03fb100..1e0b113707168b 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 <thurston at google.com>
Date: Thu, 9 Jan 2025 00:38:25 +0000
Subject: [PATCH 2/6] 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 d0804d8c39b90f..d4e1f9916ff664 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 2031728c2f33dd..8e966a739c8c02 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<PostDominatorTreeAnalysis>(F);
LoopInfo *LI = FAM.getCachedResult<LoopAnalysis>(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 1e0b113707168b..7aebe2121aeb62 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 = '<stdin>'
source_filename = "<stdin>"
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll b/llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll
index a201174df995b3..95cf6f1544df08 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 <thurston at google.com>
Date: Thu, 9 Jan 2025 01:23:50 +0000
Subject: [PATCH 3/6] 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 8e966a739c8c02..6639fff75faa5e 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 <thurston at google.com>
Date: Thu, 9 Jan 2025 01:29:04 +0000
Subject: [PATCH 4/6] 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 6639fff75faa5e..534abf2eadf965 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 <thurston at google.com>
Date: Thu, 9 Jan 2025 05:54:13 +0000
Subject: [PATCH 5/6] 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 d4e1f9916ff664..9d9d9889b3858e 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 534abf2eadf965..1cb5469b052e75 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 <thurston at google.com>
Date: Thu, 9 Jan 2025 05:57:12 +0000
Subject: [PATCH 6/6] 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 1cb5469b052e75..75a19357ea1bb9 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);
More information about the llvm-commits
mailing list