Fix UBSan error reports in ValueMapCallbackVH and AssertingVH<T> empty/tombstone keys generation.

Summary:
One more attempt to fix UBSan reports: make sure DenseMapInfo::getEmptyKey()
and DenseMapInfo::getTombstoneKey() doesn't do any upcasts/downcasts to/from Value*.

Test Plan: check-llvm test suite with/without UBSan bootstrap

Reviewers: chandlerc, dexonsmith

Subscribers: llvm-commits, majnemer

Differential Revision: http://reviews.llvm.org/D6903

llvm-svn: 225558

2 files changed, 27 insertions, 32 deletions

diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index 9bd14cf2253..8fc7fdb0395 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -190,23 +190,20 @@ class AssertingVH
   friend struct DenseMapInfo<AssertingVH<ValueTy> >;
 
 #ifndef NDEBUG
-  ValueTy *getValPtr() const {
-    return static_cast<ValueTy*>(ValueHandleBase::getValPtr());
-  }
-  void setValPtr(ValueTy *P) {
-    ValueHandleBase::operator=(GetAsValue(P));
-  }
+  Value *getRawValPtr() const { return ValueHandleBase::getValPtr(); }
+  void setRawValPtr(Value *P) { ValueHandleBase::operator=(P); }
 #else
-  ValueTy *ThePtr;
-  ValueTy *getValPtr() const { return ThePtr; }
-  void setValPtr(ValueTy *P) { ThePtr = P; }
+  Value *ThePtr;
+  Value *getRawValPtr() const { return ThePtr; }
+  void setRawValPtr(Value *P) { ThePtr = P; }
 #endif
-
-  // Convert a ValueTy*, which may be const, to the type the base
-  // class expects.
+  // Convert a ValueTy*, which may be const, to the raw Value*.
   static Value *GetAsValue(Value *V) { return V; }
   static Value *GetAsValue(const Value *V) { return const_cast<Value*>(V); }
 
+  ValueTy *getValPtr() const { return static_cast<ValueTy *>(getRawValPtr()); }
+  void setValPtr(ValueTy *P) { setRawValPtr(GetAsValue(P)); }
+
 public:
 #ifndef NDEBUG
   AssertingVH() : ValueHandleBase(Assert) {}
@@ -214,7 +211,7 @@ public:
   AssertingVH(const AssertingVH &RHS) : ValueHandleBase(Assert, RHS) {}
 #else
   AssertingVH() : ThePtr(nullptr) {}
-  AssertingVH(ValueTy *P) : ThePtr(P) {}
+  AssertingVH(ValueTy *P) : ThePtr(GetAsValue(P)) {}
 #endif
 
   operator ValueTy*() const {
@@ -237,27 +234,23 @@ public:
 // Specialize DenseMapInfo to allow AssertingVH to participate in DenseMap.
 template<typename T>
 struct DenseMapInfo<AssertingVH<T> > {
-  typedef DenseMapInfo<T*> PointerInfo;
   static inline AssertingVH<T> getEmptyKey() {
-    return AssertingVH<T>(PointerInfo::getEmptyKey());
+    AssertingVH<T> Res;
+    Res.setRawValPtr(DenseMapInfo<Value *>::getEmptyKey());
+    return Res;
   }
-  static inline T* getTombstoneKey() {
-    return AssertingVH<T>(PointerInfo::getTombstoneKey());
+  static inline AssertingVH<T> getTombstoneKey() {
+    AssertingVH<T> Res;
+    Res.setRawValPtr(DenseMapInfo<Value *>::getTombstoneKey());
+    return Res;
   }
   static unsigned getHashValue(const AssertingVH<T> &Val) {
-    return PointerInfo::getHashValue(Val);
+    return DenseMapInfo<Value *>::getHashValue(Val.getRawValPtr());
   }
-#ifndef NDEBUG
   static bool isEqual(const AssertingVH<T> &LHS, const AssertingVH<T> &RHS) {
-    // Avoid downcasting AssertingVH<T> to T*, as empty/tombstone keys may not
-    // be properly aligned pointers to T*.
-    return LHS.ValueHandleBase::getValPtr() == RHS.ValueHandleBase::getValPtr();
+    return DenseMapInfo<Value *>::isEqual(LHS.getRawValPtr(),
+                                          RHS.getRawValPtr());
   }
-#else
-  static bool isEqual(const AssertingVH<T> &LHS, const AssertingVH<T> &RHS) {
-    return LHS == RHS;
-  }
-#endif
 };
 
 template <typename T>
diff --git a/llvm/include/llvm/IR/ValueMap.h b/llvm/include/llvm/IR/ValueMap.h
index 9999113e099..f2ea405f142 100644
--- a/llvm/include/llvm/IR/ValueMap.h
+++ b/llvm/include/llvm/IR/ValueMap.h
@@ -224,6 +224,9 @@ class ValueMapCallbackVH : public CallbackVH {
       : CallbackVH(const_cast<Value*>(static_cast<const Value*>(Key))),
         Map(Map) {}
 
+  // Private constructor used to create empty/tombstone DenseMap keys.
+  ValueMapCallbackVH(Value *V) : CallbackVH(V), Map(nullptr) {}
+
 public:
   KeyT Unwrap() const { return cast_or_null<KeySansPointerT>(getValPtr()); }
 
@@ -266,19 +269,18 @@ public:
 template<typename KeyT, typename ValueT, typename Config>
 struct DenseMapInfo<ValueMapCallbackVH<KeyT, ValueT, Config> > {
   typedef ValueMapCallbackVH<KeyT, ValueT, Config> VH;
-  typedef DenseMapInfo<KeyT> PointerInfo;
 
   static inline VH getEmptyKey() {
-    return VH(PointerInfo::getEmptyKey(), nullptr);
+    return VH(DenseMapInfo<Value *>::getEmptyKey());
   }
   static inline VH getTombstoneKey() {
-    return VH(PointerInfo::getTombstoneKey(), nullptr);
+    return VH(DenseMapInfo<Value *>::getTombstoneKey());
   }
   static unsigned getHashValue(const VH &Val) {
-    return PointerInfo::getHashValue(Val.Unwrap());
+    return DenseMapInfo<KeyT>::getHashValue(Val.Unwrap());
   }
   static unsigned getHashValue(const KeyT &Val) {
-    return PointerInfo::getHashValue(Val);
+    return DenseMapInfo<KeyT>::getHashValue(Val);
   }
   static bool isEqual(const VH &LHS, const VH &RHS) {
     return LHS == RHS;