[llvm] [IR] Fix User use-after-destroy by zapping in ~User (PR #170575)

[Reid Kleckner via llvm-commits]

https://github.com/rnk updated https://github.com/llvm/llvm-project/pull/170575

>From 682545aade48d3c53e7e8a30add575993f4065c5 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rkleckner at nvidia.com>
Date: Thu, 4 Dec 2025 00:02:00 +0000
Subject: [PATCH 1/2] [IR] Fix User use-after-destroy by zapping in ~User

First, this moves the removal of operands from use lists from
`User::operator delete` to `User::~User`. This is straightforward, and
nothing blocks that.

However, the second complication is that `User::operator delete` needs
to recover the start of the allocation, and it needs to recover that
information somehow without examining the fields of the `User` object.
The natural way to handle this is for the destructor to return an
adjusted `this` pointer, and that's in fact how deleting destructors are
often implemented, but it requires making assumptions about the C++ ABI.
Instead, it seems practical to store the information into the operand
memory, to the left of `this`, and to reload the start of the allocation
from `((void**)this)[-1]` after the destructor runs. The downside is
that zero-operand Users such as `ret void` must allocate more memory. I
have not yet gathered any real data on the memory usage impact, but in
general, I believe there are very few zero-operand Users, so the impact
should be minimal. I'm open to other creative suggestions for how to
avoid this overhead, but I don't see any other good way to do it.

This makes LLVM more compatible with bug finding tools like MSan, GCC
`-flifetime-dse`, and forthcoming enhancements to Clang itself through
`dead_on_return` annotations.

Fixes issue #24952
---
 llvm/lib/IR/User.cpp | 65 ++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/llvm/lib/IR/User.cpp b/llvm/lib/IR/User.cpp
index 9bb7c1298593a..e8c11b5ce82f6 100644
--- a/llvm/lib/IR/User.cpp
+++ b/llvm/lib/IR/User.cpp
@@ -144,19 +144,24 @@ void *User::allocateFixedOperandUser(size_t Size, unsigned Us,
   assert(DescBytesToAllocate % sizeof(void *) == 0 &&
          "We need this to satisfy alignment constraints for Uses");
 
-  uint8_t *Storage = static_cast<uint8_t *>(
-      ::operator new(Size + sizeof(Use) * Us + DescBytesToAllocate));
-  Use *Start = reinterpret_cast<Use *>(Storage + DescBytesToAllocate);
-  Use *End = Start + Us;
-  User *Obj = reinterpret_cast<User *>(End);
+  size_t LeadingSize = DescBytesToAllocate + sizeof(Use) * Us;
+
+  // Ensure we allocate at least one pointer's worth of space before the main
+  // user allocation. We use this memory to pass information from the destructor
+  // to the deletion operator, so it can recover the true allocation start.
+  LeadingSize = std::max(LeadingSize, sizeof(void *));
+
+  uint8_t *Storage = static_cast<uint8_t *>(::operator new(LeadingSize + Size));
+  User *Obj = reinterpret_cast<User *>(Storage + LeadingSize);
+  Use *Operands = reinterpret_cast<Use *>(Obj) - Us;
   Obj->NumUserOperands = Us;
   Obj->HasHungOffUses = false;
   Obj->HasDescriptor = DescBytes != 0;
-  for (; Start != End; Start++)
-    new (Start) Use(Obj);
+  for (unsigned I = 0; I < Us; ++I)
+    new (&Operands[I]) Use(Obj);
 
   if (DescBytes != 0) {
-    auto *DescInfo = reinterpret_cast<DescriptorInfo *>(Storage + DescBytes);
+    auto *DescInfo = reinterpret_cast<DescriptorInfo *>(Operands) - 1;
     DescInfo->SizeInBytes = DescBytes;
   }
 
@@ -189,31 +194,39 @@ void *User::operator new(size_t Size, HungOffOperandsAllocMarker) {
 //                         User operator delete Implementation
 //===----------------------------------------------------------------------===//
 
-// Repress memory sanitization, due to use-after-destroy by operator
-// delete. Bug report 24578 identifies this issue.
-LLVM_NO_SANITIZE_MEMORY_ATTRIBUTE void User::operator delete(void *Usr) {
+User::~User() {
   // Hung off uses use a single Use* before the User, while other subclasses
   // use a Use[] allocated prior to the user.
-  User *Obj = static_cast<User *>(Usr);
-  if (Obj->HasHungOffUses) {
-    assert(!Obj->HasDescriptor && "not supported!");
+  void *AllocStart = nullptr;
+  if (HasHungOffUses) {
+    assert(!HasDescriptor && "not supported!");
 
-    Use **HungOffOperandList = static_cast<Use **>(Usr) - 1;
+    Use **HungOffOperandList = reinterpret_cast<Use **>(this) - 1;
     // drop the hung off uses.
-    Use::zap(*HungOffOperandList, *HungOffOperandList + Obj->NumUserOperands,
+    Use::zap(*HungOffOperandList, *HungOffOperandList + NumUserOperands,
              /* Delete */ true);
-    ::operator delete(HungOffOperandList);
-  } else if (Obj->HasDescriptor) {
-    Use *UseBegin = static_cast<Use *>(Usr) - Obj->NumUserOperands;
-    Use::zap(UseBegin, UseBegin + Obj->NumUserOperands, /* Delete */ false);
+    AllocStart = HungOffOperandList;
+  } else if (HasDescriptor) {
+    Use *UseBegin = reinterpret_cast<Use *>(this) - NumUserOperands;
+    Use::zap(UseBegin, UseBegin + NumUserOperands, /* Delete */ false);
 
     auto *DI = reinterpret_cast<DescriptorInfo *>(UseBegin) - 1;
-    uint8_t *Storage = reinterpret_cast<uint8_t *>(DI) - DI->SizeInBytes;
-    ::operator delete(Storage);
-  } else {
-    Use *Storage = static_cast<Use *>(Usr) - Obj->NumUserOperands;
-    Use::zap(Storage, Storage + Obj->NumUserOperands,
+    AllocStart = reinterpret_cast<uint8_t *>(DI) - DI->SizeInBytes;
+  } else if (NumUserOperands > 0) {
+    Use *Storage = reinterpret_cast<Use *>(this) - NumUserOperands;
+    Use::zap(Storage, Storage + NumUserOperands,
              /* Delete */ false);
-    ::operator delete(Storage);
+    AllocStart = Storage;
+  } else {
+    // Handle the edge case where there are no operands and no descriptor.
+    AllocStart = (void **)(this) - 1;
   }
+
+  // Operator delete needs to know where the allocation started. To avoid
+  // use-after-destroy, we have to store the allocation start outside the User
+  // object memory. We always have at least one Use* before the User, so we can
+  // use that to store the allocation start.
+  ((void **)this)[-1] = AllocStart;
 }
+
+void User::operator delete(void *Usr) { ::operator delete(((void **)Usr)[-1]); }

>From 2b36b86cbdfabb14b8ae76cade9d2da980b376b1 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rkleckner at nvidia.com>
Date: Thu, 4 Dec 2025 00:09:35 +0000
Subject: [PATCH 2/2] Include missing User.h change

---
 llvm/include/llvm/IR/User.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/IR/User.h b/llvm/include/llvm/IR/User.h
index cbb4379b68c41..d980dd194b987 100644
--- a/llvm/include/llvm/IR/User.h
+++ b/llvm/include/llvm/IR/User.h
@@ -141,7 +141,8 @@ class User : public Value {
   LLVM_ABI void growHungoffUses(unsigned N, bool IsPhi = false);
 
 protected:
-  ~User() = default; // Use deleteValue() to delete a generic Instruction.
+  // Use deleteValue() to delete a generic User.
+  ~User();
 
 public:
   User(const User &) = delete;