[PATCH] Use properly aligned pointers for empty/tombstone DenseMap keys.

[David Blaikie]

On Tue, Aug 19, 2014 at 2:59 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
> PointerLikeTypeTraits::NumLowBitsAvailable have different values for
> different kinds of pointers (e.g. certain pointers can be cache-line
> aligned). Do you suggest to bump the default value of NumLowBitsAvailable
> for generic T* to return the natural alignment of T*?

I /think/ that'd be a reasonable thing to do. If anyone cares about
the number of low bits available, presumably they care even if those
bits are just provided by the natural alignment.

- David

>
>
> On Tue, Aug 19, 2014 at 2:09 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Tue, Aug 19, 2014 at 1:30 PM, Alexey Samsonov <vonosmas at gmail.com>
>> wrote:
>> > Hi rsmith, chandlerc,
>> >
>> > We should respect alignOf<T*>() when creating empty/tombstone keys
>> > for DenseMap<T*, foo>. Otherwise this can lead to casts on misaligned
>> > addresses, which is undefined behavior, as reported by UBSan.
>> >
>> > http://reviews.llvm.org/D4976
>> >
>> > Files:
>> >   include/llvm/ADT/DenseMapInfo.h
>> >
>> > Index: include/llvm/ADT/DenseMapInfo.h
>> > ===================================================================
>> > --- include/llvm/ADT/DenseMapInfo.h
>> > +++ include/llvm/ADT/DenseMapInfo.h
>> > @@ -14,6 +14,7 @@
>> >  #ifndef LLVM_ADT_DENSEMAPINFO_H
>> >  #define LLVM_ADT_DENSEMAPINFO_H
>> >
>> > +#include "llvm/Support/AlignOf.h"
>> >  #include "llvm/Support/PointerLikeTypeTraits.h"
>> >  #include "llvm/Support/type_traits.h"
>> >
>> > @@ -32,12 +33,20 @@
>> >  struct DenseMapInfo<T*> {
>> >    static inline T* getEmptyKey() {
>> >      uintptr_t Val = static_cast<uintptr_t>(-1);
>> > -    Val <<= PointerLikeTypeTraits<T*>::NumLowBitsAvailable;
>> > +    const unsigned Alignment =
>> > +        PointerLikeTypeTraits<T*>::NumLowBitsAvailable > alignOf<T*>()
>> > +            ? (unsigned)PointerLikeTypeTraits<T*>::NumLowBitsAvailable
>> > +            : alignOf<T*>();
>>
>> Should this be built into PointerLikeTypeTraits instead?
>>
>> > +    Val <<= Alignment;
>> >      return reinterpret_cast<T*>(Val);
>> >    }
>> >    static inline T* getTombstoneKey() {
>> >      uintptr_t Val = static_cast<uintptr_t>(-2);
>> > -    Val <<= PointerLikeTypeTraits<T*>::NumLowBitsAvailable;
>> > +    const unsigned Alignment =
>> > +        PointerLikeTypeTraits<T*>::NumLowBitsAvailable > alignOf<T*>()
>> > +            ? (unsigned)PointerLikeTypeTraits<T*>::NumLowBitsAvailable
>> > +            : alignOf<T*>();
>> > +    Val <<= Alignment;
>> >      return reinterpret_cast<T*>(Val);
>> >    }
>> >    static unsigned getHashValue(const T *PtrVal) {
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>
>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com