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
This commit is contained in:
Matt Morehouse 2018-01-10 23:53:11 +00:00
parent cbafa59527
commit 527352b6ac
1 changed files with 6 additions and 3 deletions

View File

@ -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);