Rather than giving SmallPtrSetImpl a member field SmallArray which is magically

replaced by a bigger array in SmallPtrSet (by overridding it), instead just use a
pointer to the start of the storage, and have SmallPtrSet pass in the value to use.
This has the disadvantage that SmallPtrSet becomes bigger by one pointer.  It has
the advantage that it no longer uses tricky C++ rules, and is clearly correct while
I'm not sure the previous version was.  This was inspired by g++-4.6 pointing out
that SmallPtrSetImpl was writing off the end of SmallArray, which it was.  Since
SmallArray is replaced with a bigger array in SmallPtrSet, the write was still to
valid memory.  But it was writing off the end of the declared array type - sounds
kind of dubious to me, like it sounded dubious to g++-4.6.  Maybe g++-4.6 is wrong
and this construct is perfectly valid and correctly compiled by all compilers, but
I think it is better to avoid the whole can of worms by avoiding this construct.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@107285 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ADT/SmallPtrSet.h b/include/llvm/ADT/SmallPtrSet.h
index f1405fab9cb05d6e06028bb25d125c5e08663b9d..2af9c0260ddda1f0c54b5a240376dd8d36a4dbc7 100644 (file)
--- a/include/llvm/ADT/SmallPtrSet.h
+++ b/include/llvm/ADT/SmallPtrSet.h
@@ -46,8 +46,10 @@ class SmallPtrSetIteratorImpl;
 class SmallPtrSetImpl {
   friend class SmallPtrSetIteratorImpl;
 protected:
-  /// CurArray - This is the current set of buckets.  If it points to
-  /// SmallArray, then the set is in 'small mode'.
+  /// SmallArray - Points to a fixed size set of buckets, used in 'small mode'.
+  const void **SmallArray;
+  /// CurArray - This is the current set of buckets.  If equal to SmallArray,
+  /// then the set is in 'small mode'.
   const void **CurArray;
   /// CurArraySize - The allocated size of CurArray, always a power of two.
   /// Note that CurArray points to an array that has CurArraySize+1 elements in
@@ -57,15 +59,13 @@ protected:
   // If small, this is # elts allocated consequtively
   unsigned NumElements;
   unsigned NumTombstones;
-  const void *SmallArray[1];  // Must be last ivar.
 
   // Helper to copy construct a SmallPtrSet.
-  SmallPtrSetImpl(const SmallPtrSetImpl& that);
-  explicit SmallPtrSetImpl(unsigned SmallSize) {
+  SmallPtrSetImpl(const void **SmallStorage, const SmallPtrSetImpl& that);
+  explicit SmallPtrSetImpl(const void **SmallStorage, unsigned SmallSize) :
+    SmallArray(SmallStorage), CurArray(SmallStorage), CurArraySize(SmallSize) {
     assert(SmallSize && (SmallSize & (SmallSize-1)) == 0 &&
            "Initial size must be a power of two!");
-    CurArray = &SmallArray[0];
-    CurArraySize = SmallSize;
     // The end pointer, always valid, is set to a valid element to help the
     // iterator.
     CurArray[SmallSize] = 0;
@@ -123,7 +123,7 @@ protected:
   }
 
 private:
-  bool isSmall() const { return CurArray == &SmallArray[0]; }
+  bool isSmall() const { return CurArray == SmallArray; }
 
   unsigned Hash(const void *Ptr) const {
     return static_cast<unsigned>(((uintptr_t)Ptr >> 4) & (CurArraySize-1));
@@ -233,14 +233,16 @@ template<class PtrType, unsigned SmallSize>
 class SmallPtrSet : public SmallPtrSetImpl {
   // Make sure that SmallSize is a power of two, round up if not.
   enum { SmallSizePowTwo = NextPowerOfTwo<SmallSize>::Val };
-  void *SmallArray[SmallSizePowTwo+1];
+  /// SmallStorage - Fixed size storage used in 'small mode'.  The extra element
+  /// ensures that the end iterator actually points to valid memory.
+  const void *SmallStorage[SmallSizePowTwo+1];
   typedef PointerLikeTypeTraits<PtrType> PtrTraits;
 public:
-  SmallPtrSet() : SmallPtrSetImpl(SmallSizePowTwo) {}
-  SmallPtrSet(const SmallPtrSet &that) : SmallPtrSetImpl(that) {}
+  SmallPtrSet() : SmallPtrSetImpl(SmallStorage, SmallSizePowTwo) {}
+  SmallPtrSet(const SmallPtrSet &that) : SmallPtrSetImpl(SmallStorage, that) {}
 
   template<typename It>
-  SmallPtrSet(It I, It E) : SmallPtrSetImpl(SmallSizePowTwo) {
+  SmallPtrSet(It I, It E) : SmallPtrSetImpl(SmallStorage, SmallSizePowTwo) {
     insert(I, E);
   }
 

diff --git a/lib/Support/SmallPtrSet.cpp b/lib/Support/SmallPtrSet.cpp
index 68938fa5a5716849ce499d1b56c7ef0d4fc4da04..504e6497a3cbe5099b0cae5696abc14e2d899855 100644 (file)
--- a/lib/Support/SmallPtrSet.cpp
+++ b/lib/Support/SmallPtrSet.cpp
@@ -166,10 +166,13 @@ void SmallPtrSetImpl::Grow() {
   }
 }
 
-SmallPtrSetImpl::SmallPtrSetImpl(const SmallPtrSetImpl& that) {
+SmallPtrSetImpl::SmallPtrSetImpl(const void **SmallStorage,
+                                 const SmallPtrSetImpl& that) {
+  SmallArray = SmallStorage;
+
   // If we're becoming small, prepare to insert into our stack space
   if (that.isSmall()) {
-    CurArray = &SmallArray[0];
+    CurArray = SmallArray;
   // Otherwise, allocate new heap space (unless we were the same size)
   } else {
     CurArray = (const void**)malloc(sizeof(void*) * (that.CurArraySize+1));
@@ -197,7 +200,7 @@ void SmallPtrSetImpl::CopyFrom(const SmallPtrSetImpl &RHS) {
   if (RHS.isSmall()) {
     if (!isSmall())
       free(CurArray);
-    CurArray = &SmallArray[0];
+    CurArray = SmallArray;
   // Otherwise, allocate new heap space (unless we were the same size)
   } else if (CurArraySize != RHS.CurArraySize) {
     if (isSmall())