[llvm] r203273 - [ADT] Update PointerIntPair to handle pointer types with more than 31 bits free.

[David Blaikie]

On Fri, Mar 7, 2014 at 11:57 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> On Mar 7, 2014, at 11:32, David Blaikie <dblaikie at gmail.com> wrote:
>
> On Fri, Mar 7, 2014 at 11:19 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> Author: jrose
> Date: Fri Mar  7 13:19:56 2014
> New Revision: 203273
>
> URL: http://llvm.org/viewvc/llvm-project?rev=203273&view=rev
> Log:
> [ADT] Update PointerIntPair to handle pointer types with more than 31 bits
> free.
>
> Previously, the assertions in PointerIntPair would try to calculate the
> value
> (1 << NumLowBitsAvailable); the inferred type here is 'int', so if there
> were
> more than 31 bits available we'd get a shift overflow.
>
>
> I'm guessing this was found by the static analyzer? (how/why? the
> "bits free" would be an template parameter, not a runtime value - and
> I assume we never even approach 32 bits free... seems sort of like a
> false positive?)
>
> Or do we really have such a pointer somewhere that's aligned to 4 GB?
>
>
> I bet the analyzer could find it if such a path occurred, but the real
> answer is much more mundane: I've been working on an internal tool that
> actually has a Fixnum class like the one in the test case, which was
> originally just being used in a PointerUnion (alongside a real pointer). I
> tried using the same Fixnum<31> with a PointerIntPair, and Clang gave me a
> warning (-Wshift-op-overflow, I think).

Still a little confused - what kind of pointer were you going to fit
in the PointerIntPair? (& if you couldn't fit a pointer there, why not
just use the int directly and skip the pair)

>
> (Why 31? Because it represents a field in a serialized file that I wanted to
> keep under 32 bits, and because I didn't want the size of this particular
> type to change between 32-bit and 64-bit builds. The latter is probably just
> me being pedantic, though...I don't think anyone's going to build this
> particular tool for 32-bit.)
>
> So, not a terribly common use case (unless Fixnum gets pushed into
> llvm/ADT), but it didn't hurt to support it. Or rather, it won't hurt once
> everyone has rebuilt everything once...sorry about that.
>
> Jordan