[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