From 527352b6ac01100f8a2d36ca895b663482202f00 Mon Sep 17 00:00:00 2001
From: Matt Morehouse <mascasa@google.com>
Date: Wed, 10 Jan 2018 23:53:11 +0000
Subject: [PATCH] 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

llvm-svn: 322241
---
 llvm/include/llvm/ADT/SmallVector.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index a9ac98d1ad4c..3d17e70bad6d 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -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 SmallVectorImpl<T> {
 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);