[llvm] r203273 - [ADT] Update PointerIntPair to handle pointer types with more than 31 bits free.
David Blaikie
dblaikie at gmail.com
Fri Mar 7 11:32:45 PST 2014
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?
>
> Also, add a rudimentary unit test file for PointerIntPair.
>
> Added:
> llvm/trunk/unittests/ADT/PointerIntPairTest.cpp
> Modified:
> llvm/trunk/include/llvm/ADT/PointerIntPair.h
> llvm/trunk/unittests/ADT/CMakeLists.txt
>
> Modified: llvm/trunk/include/llvm/ADT/PointerIntPair.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/PointerIntPair.h?rev=203273&r1=203272&r2=203273&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/PointerIntPair.h (original)
> +++ llvm/trunk/include/llvm/ADT/PointerIntPair.h Fri Mar 7 13:19:56 2014
> @@ -17,6 +17,7 @@
> #include "llvm/Support/Compiler.h"
> #include "llvm/Support/PointerLikeTypeTraits.h"
> #include <cassert>
> +#include <limits>
>
> namespace llvm {
>
> @@ -41,6 +42,9 @@ template <typename PointerTy, unsigned I
> typename PtrTraits = PointerLikeTypeTraits<PointerTy> >
> class PointerIntPair {
> intptr_t Value;
> + static_assert(PtrTraits::NumLowBitsAvailable <
> + std::numeric_limits<uintptr_t>::digits,
> + "cannot use a pointer type that has all bits free");
> enum : uintptr_t {
> /// PointerBitMask - The bits that come from the pointer.
> PointerBitMask =
> @@ -79,7 +83,7 @@ public:
> void setPointer(PointerTy PtrVal) {
> intptr_t PtrWord
> = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(PtrVal));
> - assert((PtrWord & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&
> + assert((PtrWord & ~PointerBitMask) == 0 &&
> "Pointer is not sufficiently aligned");
> // Preserve all low bits, just update the pointer.
> Value = PtrWord | (Value & ~PointerBitMask);
> @@ -87,7 +91,7 @@ public:
>
> void setInt(IntType IntVal) {
> intptr_t IntWord = static_cast<intptr_t>(IntVal);
> - assert(IntWord < (1 << IntBits) && "Integer too large for field");
> + assert((IntWord & ~IntMask) == 0 && "Integer too large for field");
>
> // Preserve all bits other than the ones we are updating.
> Value &= ~ShiftedIntMask; // Remove integer field.
> @@ -97,7 +101,7 @@ public:
> void initWithPointer(PointerTy PtrVal) {
> intptr_t PtrWord
> = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(PtrVal));
> - assert((PtrWord & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&
> + assert((PtrWord & ~PointerBitMask) == 0 &&
> "Pointer is not sufficiently aligned");
> Value = PtrWord;
> }
> @@ -105,10 +109,10 @@ public:
> void setPointerAndInt(PointerTy PtrVal, IntType IntVal) {
> intptr_t PtrWord
> = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(PtrVal));
> - assert((PtrWord & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&
> + assert((PtrWord & ~PointerBitMask) == 0 &&
> "Pointer is not sufficiently aligned");
> intptr_t IntWord = static_cast<intptr_t>(IntVal);
> - assert(IntWord < (1 << IntBits) && "Integer too large for field");
> + assert((IntWord & ~IntMask) == 0 && "Integer too large for field");
>
> Value = PtrWord | (IntWord << IntShift);
> }
>
> Modified: llvm/trunk/unittests/ADT/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=203273&r1=203272&r2=203273&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/CMakeLists.txt (original)
> +++ llvm/trunk/unittests/ADT/CMakeLists.txt Fri Mar 7 13:19:56 2014
> @@ -24,6 +24,7 @@ set(ADTSources
> OptionalTest.cpp
> OwningPtrTest.cpp
> PackedVectorTest.cpp
> + PointerIntPairTest.cpp
> PointerUnionTest.cpp
> SCCIteratorTest.cpp
> SmallPtrSetTest.cpp
>
> Added: llvm/trunk/unittests/ADT/PointerIntPairTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/PointerIntPairTest.cpp?rev=203273&view=auto
> ==============================================================================
> --- llvm/trunk/unittests/ADT/PointerIntPairTest.cpp (added)
> +++ llvm/trunk/unittests/ADT/PointerIntPairTest.cpp Fri Mar 7 13:19:56 2014
> @@ -0,0 +1,74 @@
> +//===- llvm/unittest/ADT/PointerIntPairTest.cpp - Unit tests --------------===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "gtest/gtest.h"
> +#include "llvm/ADT/PointerIntPair.h"
> +#include <limits>
> +using namespace llvm;
> +
> +namespace {
> +
> +// Test fixture
> +class PointerIntPairTest : public testing::Test {
> +};
> +
> +TEST_F(PointerIntPairTest, GetSet) {
> + PointerIntPair<PointerIntPairTest *, 2> Pair{this, 1U};
> + EXPECT_EQ(this, Pair.getPointer());
> + EXPECT_EQ(1U, Pair.getInt());
> +
> + Pair.setInt(2);
> + EXPECT_EQ(this, Pair.getPointer());
> + EXPECT_EQ(2U, Pair.getInt());
> +
> + Pair.setPointer(nullptr);
> + EXPECT_EQ(nullptr, Pair.getPointer());
> + EXPECT_EQ(2U, Pair.getInt());
> +
> + Pair.setPointerAndInt(this, 3U);
> + EXPECT_EQ(this, Pair.getPointer());
> + EXPECT_EQ(3U, Pair.getInt());
> +}
> +
> +TEST_F(PointerIntPairTest, DefaultInitialize) {
> + PointerIntPair<PointerIntPairTest *, 2> Pair;
> + EXPECT_EQ(nullptr, Pair.getPointer());
> + EXPECT_EQ(0U, Pair.getInt());
> +}
> +
> +TEST_F(PointerIntPairTest, ManyUnusedBits) {
> + // In real code this would be a word-sized integer limited to 31 bits.
> + struct Fixnum31 {
> + uintptr_t Value;
> + };
> + class FixnumPointerTraits {
> + public:
> + static inline void *getAsVoidPointer(Fixnum31 Num) {
> + return reinterpret_cast<void *>(Num.Value << NumLowBitsAvailable);
> + }
> + static inline Fixnum31 getFromVoidPointer(void *P) {
> + // In real code this would assert that the value is in range.
> + return { reinterpret_cast<uintptr_t>(P) >> NumLowBitsAvailable };
> + }
> + enum { NumLowBitsAvailable = std::numeric_limits<uintptr_t>::digits - 31 };
> + };
> +
> + PointerIntPair<Fixnum31, 1, bool, FixnumPointerTraits> pair;
> + EXPECT_EQ((uintptr_t)0, pair.getPointer().Value);
> + EXPECT_EQ(false, pair.getInt());
> +
> + pair.setPointerAndInt({ 0x7FFFFFFF }, true );
> + EXPECT_EQ((uintptr_t)0x7FFFFFFF, pair.getPointer().Value);
> + EXPECT_EQ(true, pair.getInt());
> +
> + EXPECT_EQ(FixnumPointerTraits::NumLowBitsAvailable - 1,
> + PointerLikeTypeTraits<decltype(pair)>::NumLowBitsAvailable);
> +}
> +
> +} // end anonymous namespace
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list