[PATCH] ADT: Remove misaligned pointeres from DenseMapInfo

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Jan 8 15:39:39 PST 2015


> On 2015-Jan-08, at 15:15, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Thu, Jan 8, 2015 at 3:04 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> On Thu, Jan 8, 2015 at 2:59 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
>   This is where UBSan reports an error - P is not a pointer to an actual object, it's a fake value (0xfffff....), which is not even
>   properly aligned.
> 
> As I've said before, I believe UBSan is incorrect to report an error at this point. I don't believe the cast to a pointer is UB, I believe using that pointer for anything other than casting back to an integer is UB. We can't even compare it to another pointer (such a compare would also produce 'false', even for comparison with itself).
> 
> It looks like what's happening is that this invalid pointer value is being upcast, and that is what UBSan is complaining about. If so, I think UBSan is right, but the bug is in DenseMapInfo<AssertingVH<T>>: it shouldn't be assuming that DenseMapInfo<T*>'s getEmptyKey produces a valid pointer to a T; it obviously cannot do so.

Okay, this makes sense.  Thanks Alexey (and Richard) for clarifying.

Interestingly, there's only undefined behaviour in asserts builds,
since `AssertingVH` only inherits from `ValueHandleBase` (and upcasts)
in asserts builds.

This sort of thing should work:

    private:
    #ifndef NDEBUG
      AssertingVH(ValueTy *Raw, bool)
        : ValueHandleBase(Assert, reinterpret_cast<Value *>(Raw)) {}
    #endif

    public:
      static AssertingVH<T> getDenseMapKey(ValueTy *Raw) {
    #ifndef NDEBUG
        assert(Raw == DenseMapInfo<ValueTy *>::getEmptyKey() ||
               Raw == DenseMapInfo<ValueTy *>::getTombstoneKey());
        return AssertingVH(Raw, true);
    #else
        return AssertingVH(Raw);
    #endif
      }

Then we can have:

     template <class T> struct DenseMapInfo<AssertingVH<T>> {
       static AssertingVH<T> getEmptyKey() {
         return AssertingVH<T>::getDenseMapKey(DenseMapInfo<T *>::getEmptyKey());
       }

       static AssertingVH<T> getTombstoneKey() {
         return AssertingVH<T>::getDenseMapKey(DenseMapInfo<T *>::getTombstoneKey());
       }

       // ...
     };



More information about the llvm-commits mailing list