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

[Alexey Samsonov]

================
Comment at: include/llvm/IR/ValueHandle.h:192-203
@@ -191,14 +191,14 @@
 
 #ifndef NDEBUG
   ValueTy *getValPtr() const {
     return static_cast<ValueTy*>(ValueHandleBase::getValPtr());
   }
   void setValPtr(ValueTy *P) {
     ValueHandleBase::operator=(GetAsValue(P));
   }
 #else
-  ValueTy *ThePtr;
-  ValueTy *getValPtr() const { return ThePtr; }
-  void setValPtr(ValueTy *P) { ThePtr = P; }
+  Value *ThePtr;
+  ValueTy *getValPtr() const { return static_cast<ValueTy *>(ThePtr); }
+  void setValPtr(ValueTy *P) { ThePtr = GetAsValue(P); }
 #endif
 
----------------
chandlerc wrote:
> I would define helpers here for getRawValPtr and setRawValPtr which do the indirection between NDEBUG and debug builds. Then use them below (see comments).
Done

================
Comment at: include/llvm/IR/ValueHandle.h:239-272
@@ -239,23 +238,36 @@
 struct DenseMapInfo<AssertingVH<T> > {
-  typedef DenseMapInfo<T*> PointerInfo;
   static inline AssertingVH<T> getEmptyKey() {
-    return AssertingVH<T>(PointerInfo::getEmptyKey());
+    AssertingVH<T> Res;
+    Value *V = DenseMapInfo<Value *>::getEmptyKey();
+#ifndef NDEBUG
+    Res.ValueHandleBase::operator=(V);
+#else
+    Res.ThePtr = V;
+#endif
+    return Res;
   }
-  static inline T* getTombstoneKey() {
-    return AssertingVH<T>(PointerInfo::getTombstoneKey());
+  static inline AssertingVH<T> getTombstoneKey() {
+    AssertingVH<T> Res;
+    Value *V = DenseMapInfo<Value *>::getTombstoneKey();
+#ifndef NDEBUG
+    Res.ValueHandleBase::operator=(V);
+#else
+    Res.ThePtr = V;
+#endif
+    return Res;
   }
   static unsigned getHashValue(const AssertingVH<T> &Val) {
-    return PointerInfo::getHashValue(Val);
+    return DenseMapInfo<T *>::getHashValue(Val);
   }
 #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();
   }
 #else
   static bool isEqual(const AssertingVH<T> &LHS, const AssertingVH<T> &RHS) {
     return LHS == RHS;
   }
 #endif
 };
----------------
chandlerc wrote:
> All of these NDEBUG vs. not can go away by using helpers defined once in the class above to get the raw Value* and set the raw Value*.
> 
> I also think you want to have both isEqual and getHashValue use the raw Value* and delegate to DenseMapInfo<Value *>.
Done. However, what if some subclass MyValue of Value provides its own DenseMapInfo<MyValue*>::isEqual or DenseMapInfo<MyValue*>::getHashValue ?

http://reviews.llvm.org/D6903

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/