[PATCH] D42731: [IR] - Make User construction exception safe

[Klaus Kretzschmar via Phabricator via llvm-commits]

kkretzsch updated this revision to Diff 134597.
kkretzsch marked 2 inline comments as done.
kkretzsch added a comment.

Thanks Reid, I've updated the diff. 
Concerning tests, yes we will add some tests which will test those exceptional OOM situations. We are thinking to introduce special out-of-memory malfunction tests in the llvm test-suite, which will be able to provoke bad_alloc exceptions when the number of allocations hits a given threshold. By increasing this threshold you will be able to test the unwinding at all places where allocations occur.

However since those tests can fail whenever someone adds code which is not bad_alloc-exception safe, we first have to discuss adding this new test-category with the llvm-dev community. That's why we didn't add tests in this change. But that will be our next step ...


Repository:
  rL LLVM

https://reviews.llvm.org/D42731

Files:
  include/llvm/IR/GlobalVariable.h
  include/llvm/IR/User.h


Index: include/llvm/IR/User.h
===================================================================
--- include/llvm/IR/User.h
+++ include/llvm/IR/User.h
@@ -99,13 +99,29 @@
 
   /// \brief Free memory allocated for User and Use objects.
   void operator delete(void *Usr);
-  /// \brief Placement delete - required by std, but never called.
-  void operator delete(void*, unsigned) {
+  /// \brief Placement delete - required by std, called if the ctor throws.
+  void operator delete(void *Usr, unsigned) {
+    // 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
   }
-  /// \brief Placement delete - required by std, but never called.
-  void operator delete(void*, unsigned, bool) {
+  /// \brief Placement delete - required by std, called if the ctor throws.
+  void operator delete(void *Usr, unsigned, bool) {
+    // 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
   }
 
 protected:
Index: include/llvm/IR/GlobalVariable.h
===================================================================
--- include/llvm/IR/GlobalVariable.h
+++ include/llvm/IR/GlobalVariable.h
@@ -68,9 +68,6 @@
 
   ~GlobalVariable() {
     dropAllReferences();
-
-    // FIXME: needed by operator delete
-    setGlobalVariableNumOperands(1);
   }
 
   // allocate space for exactly one operand
@@ -78,6 +75,16 @@
     return User::operator new(s, 1);
   }
 
+  // delete space for exactly one operand as created in the corresponding new operator
+  void operator delete(void *ptr){
+    assert(ptr != nullptr && "must not be nullptr");
+    User *Obj = static_cast<User *>(ptr);
+    // Number of operands can be set to 0 after construction and initialization. Make sure
+    // that number of operands is reset to 1, as this is needed in User::operator delete
+    Obj->setGlobalVariableNumOperands(1);
+    User::operator delete(Obj);
+  }
+
   /// Provide fast operand accessors
   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42731.134597.patch
Type: text/x-patch
Size: 2687 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180216/2725baf7/attachment.bin>