[PATCH] D87326: [ADT] Fix reference invalidation when self referencing a SmallVector

Nathan James via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 13 01:32:10 PDT 2020


njames93 added a comment.

In D87326#2269796 <https://reviews.llvm.org/D87326#2269796>, @MaskRay wrote:

> Yeah, seems that SmallVector::push_back should be fixed to allow `a.push_back(a[0])`. SmallVector::insert has been fixed in rL134554 <https://reviews.llvm.org/rL134554>

It hasn't, that handles the case where the element to insert is moved when shifting the elements down to make room, but not when the element to insert is moved because of container growth.

In D87326#2269802 <https://reviews.llvm.org/D87326#2269802>, @dblaikie wrote:

>> They did but that was only because the test cases don't grow multiple times from what I can see
>
> Hmm - I thought this was about self-referential inserts/push_back, etc? Was/is there also bugs related to multiple growth? Could you show a small test case that'd fail asan with the current in-tree code related to multiple growth?

When I say grow multiple times, I mean the container has already grown once, then when the call to insert etc is made the container grows a second time. In that case the element to insert would now be in freed memory which would cause asan to fail. However right now that test case doesn't seem to appear, it does happen where the container grows from small size to insert. but as the memory is stack based there, its fine, from asans POV, to reference it.
In D87237 <https://reviews.llvm.org/D87237>, I experimented by putting asan instrumentation inside SmallVector, which caused a test case to fail when ran under asan.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87326/new/

https://reviews.llvm.org/D87326



More information about the llvm-commits mailing list