[IR] Fix assertion error in User new/delete edge case

[marcauberer]

Fixes #129900

If operator delete was called after an unsuccessful constructor call after operator new, we ran into undefined behaviour.
This was discovered by our malfunction tests while preparing an upgrade to LLVM 20, that explicitly check for such kind of bugs.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Marc Auberer (marcauberer)

Changes

Fixes #129900

If operator delete was called after an unsuccessful call of operator new we ran into undefined behaviour.
This was discovered by our malfunction tests while preparing an upgrade to LLVM 20, that explicitly check for such kind of bugs.

---

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

1 Files Affected:

• (modified) llvm/lib/IR/User.cpp (+6)

diff --git a/llvm/lib/IR/User.cpp b/llvm/lib/IR/User.cpp
index b0aa785deb9af..ab44cb4b8a3f7 100644
--- a/llvm/lib/IR/User.cpp
+++ b/llvm/lib/IR/User.cpp
@@ -146,6 +146,9 @@ void *User::allocateFixedOperandUser(size_t Size, unsigned Us,
   Use *Start = reinterpret_cast<Use *>(Storage + DescBytesToAllocate);
   Use *End = Start + Us;
   User *Obj = reinterpret_cast<User *>(End);
+  Obj->NumUserOperands = Us;
+  Obj->HasHungOffUses = false;
+  Obj->HasDescriptor = DescBytes != 0;
   for (; Start != End; Start++)
     new (Start) Use(Obj);
 
@@ -172,6 +175,9 @@ void *User::operator new(size_t Size, HungOffOperandsAllocMarker) {
   void *Storage = ::operator new(Size + sizeof(Use *));
   Use **HungOffOperandList = static_cast<Use **>(Storage);
   User *Obj = reinterpret_cast<User *>(HungOffOperandList + 1);
+  Obj->NumUserOperands = 0;
+  Obj->HasHungOffUses = true;
+  Obj->HasDescriptor = false;
   *HungOffOperandList = nullptr;
   return Obj;
 }

[marcauberer]

@nikic Could you trigger a compile time tracker run for this change?

[nikic]

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=a614f2b489caa19001b2f44784514a6226f79cb7&to=e2454c7cb4245fceb05435cdcf775d4b1a47fc24&stat=instructions:u Impact is very small if there is any.

[marcauberer]

Cool, thanks!

[marcauberer]

/cherry-pick 8d38906

[llvmbot]

/pull-request #130580

[nikic]

Wouldn't the proper way to fix this be to change the operator delete overloads that take an alloc marker to actually use it?

llvm-project/llvm/include/llvm/IR/User.h

Lines 147 to 183 in 2e6402c

	/// Placement delete - required by std, called if the ctor throws.
	void operator delete(void *Usr, HungOffOperandsAllocMarker) {
	// Note: If a subclass manipulates the information which is required to
	// calculate the Usr memory pointer, e.g. NumUserOperands, the operator
	// delete of that subclass has to restore the changed information to the
	// original value, since the dtor of that class is not called if the ctor
	// fails.
	User::operator delete(Usr);
	#ifndef LLVM_ENABLE_EXCEPTIONS
	llvm_unreachable("Constructor throws?");
	#endif
	}
	/// Placement delete - required by std, called if the ctor throws.
	void operator delete(void *Usr, IntrusiveOperandsAllocMarker) {
	// Note: If a subclass manipulates the information which is required to calculate the
	// Usr memory pointer, e.g. NumUserOperands, the operator delete of that subclass has
	// to restore the changed information to the original value, since the dtor of that class
	// is not called if the ctor fails.
	User::operator delete(Usr);
	#ifndef LLVM_ENABLE_EXCEPTIONS
	llvm_unreachable("Constructor throws?");
	#endif
	}
	/// Placement delete - required by std, called if the ctor throws.
	void operator delete(void *Usr, IntrusiveOperandsAndDescriptorAllocMarker) {
	// Note: If a subclass manipulates the information which is required to calculate the
	// Usr memory pointer, e.g. NumUserOperands, the operator delete of that subclass has
	// to restore the changed information to the original value, since the dtor of that class
	// is not called if the ctor fails.
	User::operator delete(Usr);
	#ifndef LLVM_ENABLE_EXCEPTIONS
	llvm_unreachable("Constructor throws?");
	#endif
	}

[marcauberer]

@nikic Probably you are right, I am not particularly familiar in this corner ;)
Do you suggest to revert this and do it properly or live with this quick fix for now and apply the proper solution on top of it?

[nikic]

No need to revert, it's fine to apply it on top.