[llvm] r322241 - SmallVector: fix use-after-poison MSAN error in destructor

[Matt Morehouse via llvm-commits]

Author: morehouse
Date: Wed Jan 10 15:53:11 2018
New Revision: 322241

URL: http://llvm.org/viewvc/llvm-project?rev=322241&view=rev
Log:
SmallVector: fix use-after-poison MSAN error in destructor

Summary:
Addresses issue: https://bugs.llvm.org/show_bug.cgi?id=34595

The topmost class, `SmallVector`, has internal storage for some
elements; `N - 1` elements' bytes worth of space.  Meanwhile a base
class `SmallVectorTemplateCommon` has room for one element as well,
totaling `N` elements' worth of space.

The space for the N elements is contiguous and straddles
`SmallVectorTemplateCommon` and `SmallVector`.

A class "between" those two owning the storage, `SmallVectorImpl`, in
its destructor, calls the destructor for elements contained in the
vector, if any.  It uses `destroy_range(begin, end)` and deletes all
items in sequence, starting from the end.

By the time the destructor for `SmallVectorImpl` is running, though, the
memory for elements `[1, N)` is already poisoned, due to `SmallVector`'s
destructor having done its thing already.

So if the element type `T` has a nontrivial destructor that accesses any
members of the `T` instance being destroyed, we'll run into a
user-after-poison bug.

This patch moves the destruction loop into `SmallVector`'s destructor,
so any memory being accessed while dtors are running is not yet
poisoned.

Confirmed this broke before (and now works with this patch) with these
compiler flags:

  -fsanitize=memory
  -fsanitize-memory-use-after-dtor
  -fsanitize-memory-track-origins

and with the cmake flag
`-DLLVM_USE_SANITIZER='MemoryWithOrigins;Undefined'` as well as
`MSAN_OPTIONS=poison_in_dtor=1`.

Patch By: elsteveogrande

Reviewers: eugenis, morehouse, dblaikie

Reviewed By: eugenis, dblaikie

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D41916

Modified:
    llvm/trunk/include/llvm/ADT/SmallVector.h

Modified: llvm/trunk/include/llvm/ADT/SmallVector.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=322241&r1=322240&r2=322241&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/SmallVector.h (original)
+++ llvm/trunk/include/llvm/ADT/SmallVector.h Wed Jan 10 15:53:11 2018
@@ -339,9 +339,7 @@ public:
   SmallVectorImpl(const SmallVectorImpl &) = delete;
 
   ~SmallVectorImpl() {
-    // Destroy the constructed elements in the vector.
-    this->destroy_range(this->begin(), this->end());
-
+    // Subclass has already destructed this vector's elements.
     // If this wasn't grown from the inline copy, deallocate the old space.
     if (!this->isSmall())
       free(this->begin());
@@ -868,6 +866,11 @@ class SmallVector : public SmallVectorIm
 public:
   SmallVector() : SmallVectorImpl<T>(N) {}
 
+  ~SmallVector() {
+    // Destroy the constructed elements in the vector.
+    this->destroy_range(this->begin(), this->end());
+  }
+
   explicit SmallVector(size_t Size, const T &Value = T())
     : SmallVectorImpl<T>(N) {
     this->assign(Size, Value);