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

[Florian Mayer via llvm-commits]

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

This reverts commit 66927fb95abef9327b453d7213c5df7d641269be.

Filter functions this applies to, which I initially wanted to do in a
follow up to make reverts easier, but turns out without that it gets
really slow


>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] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
 =?UTF-8?q?l=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)