[PATCH] ADT: Remove misaligned pointeres from DenseMapInfo

[Alexey Samsonov]

On Thu, Jan 8, 2015 at 1:22 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> > On 2015-Jan-08, at 12:05, Alexey Samsonov <vonosmas at gmail.com> wrote:
> >
> > Ping.
> >
> > On Fri, Jan 2, 2015 at 6:41 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
> > So, what do you think will be the correct resolution for this?
>
> I'm a little lost.  Your patch modifies behaviour for `DenseMap<T*>`, but
> you're talking about ubsan problems with `DenseMap<AssertingVH<T>>`.
>
> It sounds like there's undefined behaviour in
> `DenseMapInfo<AssertingVH<T>>`
> so the right fix is in there somewhere, but I'm not following what the
> actual cause of UB is.  (Sorry if you spelled it out and I missed it.)
>

Sorry for not being clear enough.

Here's the problem:
1) DenseMap<AssertingVH<T>> implementation calls
DenseMapInfo<AssertingVH<T>>::getEmptyKey()
2) DenseMapInfo<AssertingVH<T>>::getEmptyKey() calls
DenseMapInfo<T*>::getEmptyKey(), and uses this
  pointer (let's denote it as P) to construct AssertingVH object.
  AssertingVH constructor assumes that P is a valid pointer to some
subclass of Value, in particular it upcasts P to Value*.
  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.

So, we may observe the problem in AssertingVH<T>, but its root is invalid
fake pointer value generated in DenseMapInfo<T*>::getEmptyKey().
I don't see another easy way to work around the problem - when we
instantiate DenseMapInfo<T*>::getEmptyKey() we don't even know
alignment of T, and of course can't create the actual object of type T.



>
> > On Mon, Dec 29, 2014 at 5:45 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
> > On Mon, Dec 29, 2014 at 5:41 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
> >
> > On Mon, Dec 29, 2014 at 5:33 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
> > Casing (int*) to (float*) doesn't dereference a pointer. However,
> upcast/downcast (in general case) does dereference a pointer - you may need
> to read vtable and adjust the pointer value accordingly.
> >
> > I see, so the problem is we're up casting or down casting? I suspect
> such casts can be avoided? We should only be casting between integers and a
> single pointer type in the dense map stuff...
> >
> > If we have smth. like DenseMap<AssertingVH<T>, Foo> (or other
> ValueHandle kinds), we would cast empty/tombstone keys (i.e. fake pointers)
> for T* to Value*.
> >
> > --
> > Alexey Samsonov
> > vonosmas at gmail.com
> >
> >
> >
> > --
> > Alexey Samsonov
> > vonosmas at gmail.com
> >
> >
> >
> > --
> > Alexey Samsonov
> > vonosmas at gmail.com
> > _______________________________________________
> > 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150108/dda06c99/attachment.html>