[hwasan] Omit tag check for null pointers #122206

thurstond · 2025-01-09T02:16:48Z

If the pointer to be checked is statically known to be zero, the tag check will always pass since:

the tag is zero
shadow memory for address 0 is initialized to 0 and never updated.
We can therefore elide the tag check.

We perform the elision in two places:

the HWASan pass
when lowering the CHECK_MEMACCESS intrinsic. 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. As a last line of defense, we perform elision here too.

This also updates the tests from #122186

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 llvm#122186 Note: the HWASan instrumentation pass will still emit the check.memaccess intrinsic. This patch performs the elision at CodeGen.

llvmbot · 2025-01-09T02:17:20Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-aarch64

Author: Thurston Dang (thurstond)

Changes

If the pointer to be checked is statically known to be zero, the tag check will always pass since:

the tag is zero
shadow memory for address 0 is initialized to 0 and never updated.
We can therefore elide the tag check.

We perform the elision in two places:

the HWASan pass
when lowering the CHECK_MEMACCESS intrinsic. Conceivably the HWASan pass has a "cannot currently statically prove to be null" pointer (and is therefore unable to omit the intrinsic) but later optimization passes convert it into a statically known-null pointer. As a last line of defense, we perform elision here too.

This also updates the tests from #122186

Full diff: https://github.com/llvm/llvm-project/pull/122206.diff

4 Files Affected:

(modified) llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp (+8)
(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+16-4)
(modified) llvm/test/CodeGen/AArch64/hwasan-zero-ptr.ll (+6-5)
(modified) llvm/test/Instrumentation/HWAddressSanitizer/zero-ptr.ll (+1-3)

diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 9bec782ca8ce97..d4e1f9916ff664 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();
+
+  // 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;
+
   bool IsShort =
       ((MI.getOpcode() == AArch64::HWASAN_CHECK_MEMACCESS_SHORTGRANULES) ||
        (MI.getOpcode() ==
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 2031728c2f33dd..534abf2eadf965 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);
@@ -1163,12 +1163,23 @@ void HWAddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) {
 }
 
 bool HWAddressSanitizer::instrumentMemAccess(InterestingMemoryOperand &O,
-                                             DomTreeUpdater &DTU,
-                                             LoopInfo *LI) {
+                                             DomTreeUpdater &DTU, 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, 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)
     return false; // FIXME
 
@@ -1701,8 +1712,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 dca39fe03fb100..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>"
@@ -25,7 +27,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]
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
 ;

vitalybuka · 2025-01-09T03:06:01Z

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

+  // 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)


can this be just

if (Known.isZero())

That's a nice simplification! Done.

vitalybuka · 2025-01-09T03:09:19Z

Can you try to apply and measure code size change after the patch on some non-trivial binary, e.g. clang.

thurstond · 2025-01-09T05:55:34Z

Can you try to apply and measure code size change after the patch on some non-trivial binary, e.g. clang.

Will do.

thurstond · 2025-01-09T20:09:29Z

Can you try to apply and measure code size change after the patch on some non-trivial binary, e.g. clang.

There are no instances of HWASan tag checks for provably-null pointers in clang (when bootstrapping revision b48b99f).

thurstond · 2025-01-09T20:44:48Z

Can you try to apply and measure code size change after the patch on some non-trivial binary, e.g. clang.

Spanner server (Google internal) built with Arm HWASan in opt mode has 14 instances of tag checks for null pointers:

      7 __hwasan_check_x4294967071_18_fixed_4398046511104_short_v2
      7 __hwasan_check_x4294967071_3_fixed_4398046511104_short_v2

vitalybuka · 2025-01-09T20:58:10Z

There are no instances of HWASan tag checks for provably-null pointers in clang (when bootstrapping revision b48b99f).

That's what I though.
The patch is LGTM, as it's relatively simple. But maybe we don't need it.

thurstond · 2025-01-09T21:48:16Z

That's what I though. The patch is LGTM, as it's relatively simple. But maybe we don't need it.

The patch uses KnownBits value analysis, which is beneficial to shareholders.

If the pointer to be checked is statically known to be zero, the tag check will always pass since: 1) the tag is zero 2) shadow memory for address 0 is initialized to 0 and never updated. We can therefore elide the tag check. We perform the elision in two places: 1) the HWASan pass 2) when lowering the CHECK_MEMACCESS intrinsic. 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. As a last line of defense, we perform elision here too. This also updates the tests from llvm#122186

thurstond added 4 commits January 9, 2025 00:14

Omit check during HWASan pass if pointer is statically known to be null

a4c255b

Fix check for known zero value

4c37dd3

clang-format

5042e58

thurstond changed the title ~~Omit tag check for null pointers~~ [hwasan] Omit tag check for null pointers Jan 9, 2025

llvmbot added backend:AArch64 compiler-rt:sanitizer llvm:transforms labels Jan 9, 2025

thurstond requested review from dvyukov and vitalybuka January 9, 2025 02:17

vitalybuka reviewed Jan 9, 2025

View reviewed changes

Use Known.isZero(). Update comment.

1a54db9

thurstond added 2 commits January 9, 2025 05:57

Whitespace

cbb6b22

Update test comment

7f8c862

fmayer approved these changes Jan 9, 2025

View reviewed changes

vitalybuka approved these changes Jan 9, 2025

View reviewed changes

thurstond merged commit 4f42e16 into llvm:main Jan 9, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[hwasan] Omit tag check for null pointers #122206

[hwasan] Omit tag check for null pointers #122206

Uh oh!

thurstond commented Jan 9, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jan 9, 2025 •

edited

Loading

Uh oh!

vitalybuka Jan 9, 2025

Uh oh!

thurstond Jan 9, 2025

Uh oh!

vitalybuka commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

vitalybuka commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

Uh oh!

Uh oh!

[hwasan] Omit tag check for null pointers #122206

[hwasan] Omit tag check for null pointers #122206

Uh oh!

Conversation

thurstond commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vitalybuka Jan 9, 2025

Choose a reason for hiding this comment

Uh oh!

thurstond Jan 9, 2025

Choose a reason for hiding this comment

Uh oh!

vitalybuka commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

vitalybuka commented Jan 9, 2025

Uh oh!

thurstond commented Jan 9, 2025

Uh oh!

Uh oh!

Uh oh!

thurstond commented Jan 9, 2025 •

edited

Loading

llvmbot commented Jan 9, 2025 •

edited

Loading