SmallVector fix for use-after-dtor bug.

[nmusgrave]

Summary

Fix for use-after-dtor bug in SmallVector. Circular dependency in class hierarchy:

SmallVector contains a SmallVectorStorage member.
This member is never explicitly referenced, but removing it results in major
test failures.
SmallVectorStorage contains a SmallVectorTemplateCommon member, which SmallVector
is also a subclass of.

This fix resolves some of the issues, but there are still failures arising
in connection with SmallVector.

[dblaikie]

We try to keep as much code in the Impl rather than the SmallVector itself to reduce template code bloat.

Could you explain in more detail what the circular dependency/use-after-dtor issue is? Which entity's dtor is ~SmallVectorImpl using inappropriately?

[eugenis]

The problem is this:
SmallVectorTemplateCommon only has space for the first element of the vector; the rest of the space is provided by SmallVector and used by effectively overflowing the field SmallVectorTemplateCommon. Sanitizers are OK with it because the memory is, in fact, allocated.

Use-after-destroy detector complains about ~SmallVectorTemplateCommon accessing memory that belongs to SmallVector (the destructor for that has already ran at the point) when calling destructors for individual vector elements in destroy_range.

[dblaikie]

include/llvm/ADT/SmallVector.h

926

This is different from the original code, why?

I assume this would introduce a bug where in big-mode it doesn't call the dtors on the elements of the vector?

include/llvm/ADT/SmallVector.h

 
 protected:
   // Default ctor - Initialize to empty.
   explicit SmallVectorImpl(unsigned N)
     : SmallVectorTemplateBase<T, isPodLike<T>::value>(N*sizeof(T)) {
   }
 
 public:
-  ~SmallVectorImpl() {
-    // Destroy the constructed elements in the vector.
-    this->destroy_range(this->begin(), this->end());
-
-    // If this wasn't grown from the inline copy, deallocate the old space.
-    if (!this->isSmall())
-      free(this->begin());
-  }
-
-
   void clear() {
     this->destroy_range(this->begin(), this->end());
     this->EndX = this->BeginX;
   }
 
   void resize(size_type N) {
     if (N < this->size()) {
       this->destroy_range(this->begin()+N, this->end());
const SmallVector &operator=(SmallVectorImpl<T> &&RHS) {
     SmallVectorImpl<T>::operator=(::std::move(RHS));
     return *this;
   }
 
   const SmallVector &operator=(std::initializer_list<T> IL) {
     this->assign(IL);
     return *this;
   }
+
+  ~SmallVector() {
+    if (this->isSmall()) {
+      // Destroy the constructed elements in the vector.
+      this->destroy_range(this->begin(), this->end());
+    } else {
+      // If this wasn't grown from the inline copy, deallocate the old space.
+      free(this->begin());
+    }
+  }
 };
 
-template<typename T, unsigned N>
+template <typename T, unsigned N>
 static inline size_t capacity_in_bytes(const SmallVector<T, N> &X) {
   return X.capacity_in_bytes();
 }
 
 } // End llvm namespace
 
 namespace std {
   /// Implement std::swap in terms of SmallVector swap.