[llvm] r178166 - Avoid undefined behavior from passing a std::vector's own contents

[Jean-Luc Duprat]

I just ran into a similar issue with LLVM's SmallVector in case that was very similar to the one discussed in the standard library.
I light of Howard's comment, SmallVector should preserve the validity of both iterators/reference in this case:
 v.insert(v.begin(), v[3]);
if it is to behave similarly to std::vector.

I propose the attached fix to SmallVector.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: SmallVector.patch
Type: application/octet-stream
Size: 2290 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130401/d16b9956/attachment.obj>
-------------- next part --------------



JL



On Mar 27, 2013, at 13:47 , Howard Hinnant <hhinnant at apple.com> wrote:

> On Mar 27, 2013, at 4:34 PM, Dan Gohman <dan433584 at gmail.com> wrote:
> 
>> 
>> 
>> On Wed, Mar 27, 2013 at 12:16 PM, Howard Hinnant <hhinnant at apple.com> wrote:
>> On Mar 27, 2013, at 2:44 PM, Dan Gohman <dan433584 at gmail.com> wrote:
>> 
>>> Author: djg
>>> Date: Wed Mar 27 13:44:56 2013
>>> New Revision: 178166
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=178166&view=rev
>>> Log:
>>> Avoid undefined behavior from passing a std::vector's own contents
>>> in as an argument to push_back.
>> 
>> The original code is not undefined behavior.
>> 
>> My interpretation was that the push_back could cause reallocation which could invalidate the reference to the old element before it is read. Is there a guarantee that this won't happen?
> 
> There is not an allowance for the vendor to let it happen.  Such an allowance would look like the one for insert(p, i, j):
> 
>> pre: i and j are not iterators into a.
> 
> In practice, the new buffer has to be allocated and populated before the old buffer is deallocated, so there is no hardship on the vendor anyway.
> 
> The vendor does have to be careful with vector::insert(iterator, const T&), as self-referencing can be visible there.  But again in that case the standard puts the onus on the vendor, not on the client to just make things work.
> 
> Note that if you had moved from the source that would be another matter as 17.6.4.9 [res.on.arguments]/p1/b3 says:
> 
>> f a function argument binds to an rvalue reference parameter, the implementation may assume that this parameter is a unique reference to this argument.
> 
> So, for example:
> 
>   v.insert(v.begin(), std::move(v[3]));
> 
> would be asking for trouble, whereas 
> 
>   v.insert(v.begin(), v[3]);
> 
> is ok.
> 
> Howard
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits