[llvm] 1544019 - [IR] Delete llvm::Constants using the correct type.

Eli Friedman via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 30 12:38:07 PDT 2020


Author: Eli Friedman
Date: 2020-06-30T12:37:53-07:00
New Revision: 15440191b57237eafb18600ac653b9a0023db391

URL: https://github.com/llvm/llvm-project/commit/15440191b57237eafb18600ac653b9a0023db391
DIFF: https://github.com/llvm/llvm-project/commit/15440191b57237eafb18600ac653b9a0023db391.diff

LOG: [IR] Delete llvm::Constants using the correct type.

In most cases, this doesn't have much impact: the destructors just call
the base class destructor anyway.  A few subclasses of ConstantExpr
actually store non-trivial data, though. Make sure we clean up
appropriately.

This is sort of ugly, but I don't see a good alternative given the
constraints.

Issue found by asan buildbots running the testcase for D80330.

Differential Revision: https://reviews.llvm.org/D82509

Added: 
    

Modified: 
    llvm/include/llvm/IR/Constant.h
    llvm/include/llvm/IR/Constants.h
    llvm/lib/Bitcode/Reader/ValueList.cpp
    llvm/lib/IR/Constants.cpp
    llvm/lib/IR/ConstantsContext.h
    llvm/lib/IR/Value.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Constant.h b/llvm/include/llvm/IR/Constant.h
index 174e7364c524..9a1d2b80c48e 100644
--- a/llvm/include/llvm/IR/Constant.h
+++ b/llvm/include/llvm/IR/Constant.h
@@ -43,6 +43,8 @@ class Constant : public User {
   Constant(Type *ty, ValueTy vty, Use *Ops, unsigned NumOps)
     : User(ty, vty, Ops, NumOps) {}
 
+  ~Constant() = default;
+
 public:
   void operator=(const Constant &) = delete;
   Constant(const Constant &) = delete;

diff  --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index c2c2cac649f1..486c718cc610 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -899,6 +899,8 @@ class ConstantExpr : public Constant {
     setValueSubclassData(Opcode);
   }
 
+  ~ConstantExpr() = default;
+
 public:
   // Static methods to construct a ConstantExpr of 
diff erent kinds.  Note that
   // these methods may return a object that is not an instance of the

diff  --git a/llvm/lib/Bitcode/Reader/ValueList.cpp b/llvm/lib/Bitcode/Reader/ValueList.cpp
index 431995fd40ac..63a206eeb022 100644
--- a/llvm/lib/Bitcode/Reader/ValueList.cpp
+++ b/llvm/lib/Bitcode/Reader/ValueList.cpp
@@ -220,6 +220,6 @@ void BitcodeReaderValueList::resolveConstantForwardRefs() {
 
     // Update all ValueHandles, they should be the only users at this point.
     Placeholder->replaceAllUsesWith(RealVal);
-    Placeholder->deleteValue();
+    delete cast<ConstantPlaceHolder>(Placeholder);
   }
 }

diff  --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 1afd73d95f56..d8e044ee4bdc 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -463,7 +463,74 @@ void Constant::destroyConstant() {
   }
 
   // Value has no outstanding references it is safe to delete it now...
-  delete this;
+  deleteConstant(this);
+}
+
+void llvm::deleteConstant(Constant *C) {
+  switch (C->getValueID()) {
+  case Constant::ConstantIntVal:
+    delete static_cast<ConstantInt *>(C);
+    break;
+  case Constant::ConstantFPVal:
+    delete static_cast<ConstantFP *>(C);
+    break;
+  case Constant::ConstantAggregateZeroVal:
+    delete static_cast<ConstantAggregateZero *>(C);
+    break;
+  case Constant::ConstantArrayVal:
+    delete static_cast<ConstantArray *>(C);
+    break;
+  case Constant::ConstantStructVal:
+    delete static_cast<ConstantStruct *>(C);
+    break;
+  case Constant::ConstantVectorVal:
+    delete static_cast<ConstantVector *>(C);
+    break;
+  case Constant::ConstantPointerNullVal:
+    delete static_cast<ConstantPointerNull *>(C);
+    break;
+  case Constant::ConstantDataArrayVal:
+    delete static_cast<ConstantDataArray *>(C);
+    break;
+  case Constant::ConstantDataVectorVal:
+    delete static_cast<ConstantDataVector *>(C);
+    break;
+  case Constant::ConstantTokenNoneVal:
+    delete static_cast<ConstantTokenNone *>(C);
+    break;
+  case Constant::BlockAddressVal:
+    delete static_cast<BlockAddress *>(C);
+    break;
+  case Constant::UndefValueVal:
+    delete static_cast<UndefValue *>(C);
+    break;
+  case Constant::ConstantExprVal:
+    if (isa<UnaryConstantExpr>(C))
+      delete static_cast<UnaryConstantExpr *>(C);
+    else if (isa<BinaryConstantExpr>(C))
+      delete static_cast<BinaryConstantExpr *>(C);
+    else if (isa<SelectConstantExpr>(C))
+      delete static_cast<SelectConstantExpr *>(C);
+    else if (isa<ExtractElementConstantExpr>(C))
+      delete static_cast<ExtractElementConstantExpr *>(C);
+    else if (isa<InsertElementConstantExpr>(C))
+      delete static_cast<InsertElementConstantExpr *>(C);
+    else if (isa<ShuffleVectorConstantExpr>(C))
+      delete static_cast<ShuffleVectorConstantExpr *>(C);
+    else if (isa<ExtractValueConstantExpr>(C))
+      delete static_cast<ExtractValueConstantExpr *>(C);
+    else if (isa<InsertValueConstantExpr>(C))
+      delete static_cast<InsertValueConstantExpr *>(C);
+    else if (isa<GetElementPtrConstantExpr>(C))
+      delete static_cast<GetElementPtrConstantExpr *>(C);
+    else if (isa<CompareConstantExpr>(C))
+      delete static_cast<CompareConstantExpr *>(C);
+    else
+      llvm_unreachable("Unexpected constant expr");
+    break;
+  default:
+    llvm_unreachable("Unexpected constant");
+  }
 }
 
 static bool canTrapImpl(const Constant *C,

diff  --git a/llvm/lib/IR/ConstantsContext.h b/llvm/lib/IR/ConstantsContext.h
index 4cd2621f625d..fadbc2169816 100644
--- a/llvm/lib/IR/ConstantsContext.h
+++ b/llvm/lib/IR/ConstantsContext.h
@@ -43,7 +43,7 @@ namespace llvm {
 
 /// UnaryConstantExpr - This class is private to Constants.cpp, and is used
 /// behind the scenes to implement unary constant exprs.
-class UnaryConstantExpr : public ConstantExpr {
+class UnaryConstantExpr final : public ConstantExpr {
 public:
   UnaryConstantExpr(unsigned Opcode, Constant *C, Type *Ty)
     : ConstantExpr(Ty, Opcode, &Op<0>(), 1) {
@@ -60,7 +60,7 @@ class UnaryConstantExpr : public ConstantExpr {
 
 /// BinaryConstantExpr - This class is private to Constants.cpp, and is used
 /// behind the scenes to implement binary constant exprs.
-class BinaryConstantExpr : public ConstantExpr {
+class BinaryConstantExpr final : public ConstantExpr {
 public:
   BinaryConstantExpr(unsigned Opcode, Constant *C1, Constant *C2,
                      unsigned Flags)
@@ -81,7 +81,7 @@ class BinaryConstantExpr : public ConstantExpr {
 
 /// SelectConstantExpr - This class is private to Constants.cpp, and is used
 /// behind the scenes to implement select constant exprs.
-class SelectConstantExpr : public ConstantExpr {
+class SelectConstantExpr final : public ConstantExpr {
 public:
   SelectConstantExpr(Constant *C1, Constant *C2, Constant *C3)
     : ConstantExpr(C2->getType(), Instruction::Select, &Op<0>(), 3) {
@@ -102,7 +102,7 @@ class SelectConstantExpr : public ConstantExpr {
 /// ExtractElementConstantExpr - This class is private to
 /// Constants.cpp, and is used behind the scenes to implement
 /// extractelement constant exprs.
-class ExtractElementConstantExpr : public ConstantExpr {
+class ExtractElementConstantExpr final : public ConstantExpr {
 public:
   ExtractElementConstantExpr(Constant *C1, Constant *C2)
     : ConstantExpr(cast<VectorType>(C1->getType())->getElementType(),
@@ -123,7 +123,7 @@ class ExtractElementConstantExpr : public ConstantExpr {
 /// InsertElementConstantExpr - This class is private to
 /// Constants.cpp, and is used behind the scenes to implement
 /// insertelement constant exprs.
-class InsertElementConstantExpr : public ConstantExpr {
+class InsertElementConstantExpr final : public ConstantExpr {
 public:
   InsertElementConstantExpr(Constant *C1, Constant *C2, Constant *C3)
     : ConstantExpr(C1->getType(), Instruction::InsertElement,
@@ -145,7 +145,7 @@ class InsertElementConstantExpr : public ConstantExpr {
 /// ShuffleVectorConstantExpr - This class is private to
 /// Constants.cpp, and is used behind the scenes to implement
 /// shufflevector constant exprs.
-class ShuffleVectorConstantExpr : public ConstantExpr {
+class ShuffleVectorConstantExpr final : public ConstantExpr {
 public:
   ShuffleVectorConstantExpr(Constant *C1, Constant *C2, ArrayRef<int> Mask)
       : ConstantExpr(VectorType::get(
@@ -173,7 +173,7 @@ class ShuffleVectorConstantExpr : public ConstantExpr {
 /// ExtractValueConstantExpr - This class is private to
 /// Constants.cpp, and is used behind the scenes to implement
 /// extractvalue constant exprs.
-class ExtractValueConstantExpr : public ConstantExpr {
+class ExtractValueConstantExpr final : public ConstantExpr {
 public:
   ExtractValueConstantExpr(Constant *Agg, ArrayRef<unsigned> IdxList,
                            Type *DestTy)
@@ -204,7 +204,7 @@ class ExtractValueConstantExpr : public ConstantExpr {
 /// InsertValueConstantExpr - This class is private to
 /// Constants.cpp, and is used behind the scenes to implement
 /// insertvalue constant exprs.
-class InsertValueConstantExpr : public ConstantExpr {
+class InsertValueConstantExpr final : public ConstantExpr {
 public:
   InsertValueConstantExpr(Constant *Agg, Constant *Val,
                           ArrayRef<unsigned> IdxList, Type *DestTy)
@@ -235,7 +235,7 @@ class InsertValueConstantExpr : public ConstantExpr {
 
 /// GetElementPtrConstantExpr - This class is private to Constants.cpp, and is
 /// used behind the scenes to implement getelementpr constant exprs.
-class GetElementPtrConstantExpr : public ConstantExpr {
+class GetElementPtrConstantExpr final : public ConstantExpr {
   Type *SrcElementTy;
   Type *ResElementTy;
 
@@ -269,7 +269,7 @@ class GetElementPtrConstantExpr : public ConstantExpr {
 // CompareConstantExpr - This class is private to Constants.cpp, and is used
 // behind the scenes to implement ICmp and FCmp constant expressions. This is
 // needed in order to store the predicate value for these instructions.
-class CompareConstantExpr : public ConstantExpr {
+class CompareConstantExpr final : public ConstantExpr {
 public:
   unsigned short predicate;
   CompareConstantExpr(Type *ty, Instruction::OtherOps opc,
@@ -597,6 +597,10 @@ struct ConstantExprKeyType {
   }
 };
 
+// Free memory for a given constant.  Assumes the constant has already been
+// removed from all relevant maps.
+void deleteConstant(Constant *C);
+
 template <class ConstantClass> class ConstantUniqueMap {
 public:
   using ValType = typename ConstantInfo<ConstantClass>::ValType;
@@ -660,7 +664,7 @@ template <class ConstantClass> class ConstantUniqueMap {
 
   void freeConstants() {
     for (auto &I : Map)
-      delete I; // Asserts that use_empty().
+      deleteConstant(I);
   }
 
 private:
@@ -733,6 +737,11 @@ template <class ConstantClass> class ConstantUniqueMap {
   }
 };
 
+template <> inline void ConstantUniqueMap<InlineAsm>::freeConstants() {
+  for (auto &I : Map)
+    delete I;
+}
+
 } // end namespace llvm
 
 #endif // LLVM_LIB_IR_CONSTANTSCONTEXT_H

diff  --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index cfb2bcd778dc..efb8d53e8964 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -111,6 +111,10 @@ void Value::deleteValue() {
     static_cast<DerivedUser *>(this)->DeleteValue(                             \
         static_cast<DerivedUser *>(this));                                     \
     break;
+#define HANDLE_CONSTANT(Name)                                                  \
+  case Value::Name##Val:                                                       \
+    llvm_unreachable("constants should be destroyed with destroyConstant");    \
+    break;
 #define HANDLE_INSTRUCTION(Name)  /* nothing */
 #include "llvm/IR/Value.def"
 


        


More information about the llvm-commits mailing list