[llvm] Reapply^2 "[HWASan] remove incorrectly inferred attributes" (#106622) (PR #106816)

[Florian Mayer via llvm-commits]

https://github.com/fmayer updated https://github.com/llvm/llvm-project/pull/106816

>From 75e830a3486ffc37d7db5128b54afe3ef622979e Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Fri, 30 Aug 2024 18:27:20 -0700
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 .../Instrumentation/HWAddressSanitizer.cpp    | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 69e5835bee8a5e..1cd8e2e41a536c 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -591,6 +591,12 @@ void HWAddressSanitizer::createHwasanCtorComdat() {
   appendToCompilerUsed(M, Dummy);
 }
 
+static bool onlyReadsArgMemory(const Function &F) {
+  return F.onlyAccessesArgMemory() && llvm::all_of(F.args(), [](const auto &A) {
+           return A.onlyReadsMemory();
+  });
+}
+
 /// Module-level initialization.
 ///
 /// inserts a call to __hwasan_init to the module's constructor list.
@@ -598,6 +604,28 @@ void HWAddressSanitizer::initializeModule() {
   LLVM_DEBUG(dbgs() << "Init " << M.getName() << "\n");
   TargetTriple = Triple(M.getTargetTriple());
 
+  for (auto &F : M.functions()) {
+    // Remove memory attributes that are invalid with HWASan.
+    // HWASan checks read from shadow, which invalidates memory(argmem: *)
+    // Short granule checks on function arguments read from the argument memory
+    // (last byte of the granule), which invalidates writeonly.
+    //
+    // This is not only true for sanitized functions, because AttrInfer can
+    // infer those attributes on libc functions, which is not true if those
+    // are instrumented (Android) or intercepted.
+
+    // nobuiltin makes sure later passes don't restore assumptions about
+    // the function.
+    if (F.doesNotAccessMemory() || F.onlyReadsMemory() || onlyReadsArgMemory(F))
+      continue;
+    F.addFnAttr(llvm::Attribute::NoBuiltin);
+    F.removeFnAttr(llvm::Attribute::Memory);
+    // F.setMemoryEffects(F.getMemoryEffects() | MemoryEffects::writeOnly() |
+    // MemoryEffects::readOnly());
+    for (auto &A : F.args())
+      A.removeAttr(llvm::Attribute::WriteOnly);
+  }
+
   // x86_64 currently has two modes:
   // - Intel LAM (default)
   // - pointer aliasing (heap only)

>From d0eaab9c9b2eb451408014c1f071ddd3e1cb6c4e Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Fri, 30 Aug 2024 21:48:45 -0700
Subject: [PATCH 2/3] msg

Created using spr 1.3.4
---
 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 1cd8e2e41a536c..ae96538cb4d482 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -594,7 +594,7 @@ void HWAddressSanitizer::createHwasanCtorComdat() {
 static bool onlyReadsArgMemory(const Function &F) {
   return F.onlyAccessesArgMemory() && llvm::all_of(F.args(), [](const auto &A) {
            return A.onlyReadsMemory();
-  });
+         });
 }
 
 /// Module-level initialization.

>From 19c3293d1d9910a81a6621325f25f63dffe66396 Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Tue, 3 Sep 2024 18:06:54 -0700
Subject: [PATCH 3/3] upd

Created using spr 1.3.4
---
 .../lib/Transforms/Instrumentation/HWAddressSanitizer.cpp | 2 +-
 llvm/test/Instrumentation/HWAddressSanitizer/mem-attr.ll  | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 1d9b39dd693ab6..a7e7f9a570dac7 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -615,7 +615,7 @@ void HWAddressSanitizer::initializeModule() {
     if (!F.doesNotAccessMemory()) {
       bool WritesMemory = !F.onlyReadsMemory();
       bool ReadsMemory = !F.onlyWritesMemory();
-      if (WritesMemory && !ReadsMemory) {
+      if ((WritesMemory && !ReadsMemory) || F.onlyAccessesArgMemory()) {
         F.removeFnAttr(Attribute::Memory);
         Changed = true;
       }
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/mem-attr.ll b/llvm/test/Instrumentation/HWAddressSanitizer/mem-attr.ll
index 919eacb2951f5e..724e6c5a0bdec9 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/mem-attr.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/mem-attr.ll
@@ -11,5 +11,13 @@ entry:
   ret void
 }
 
+; CHECK: define dso_local void @test_readonly(ptr nocapture noundef readonly %p) local_unnamed_addr #0
+define dso_local void @test_readonly(ptr nocapture noundef readonly %p) local_unnamed_addr #1 {
+entry:
+  store i32 42, ptr %p, align 4
+  ret void
+}
+
 ; CHECK: attributes #0 = { nobuiltin sanitize_hwaddress uwtable }
 attributes #0 = { sanitize_hwaddress memory(argmem: write) uwtable }
+attributes #1 = { sanitize_hwaddress memory(argmem: read) uwtable }