[llvm] r183459 - CodeGenSchedule: smallvector.push_back(smallvector[0]) is dangerous

[Arnold Schwaighofer]

r183465 for the reserve workaround

On Jun 6, 2013, at 6:38 PM, Andrew Trick <atrick at apple.com> wrote:

> 
> On Jun 6, 2013, at 4:33 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
>> On Thu, Jun 6, 2013 at 4:23 PM, Arnold Schwaighofer
>> <aschwaighofer at apple.com> wrote:
>>> Author: arnolds
>>> Date: Thu Jun  6 18:23:14 2013
>>> New Revision: 183459
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=183459&view=rev
>>> Log:
>>> CodeGenSchedule: smallvector.push_back(smallvector[0]) is dangerous
>>> 
>>> The element passed to push_back is not copied before the vector reallocates.
>> 
>> A similar change to this has been previously reverted: r178319
>> 
>> We should probably fix SmallVector to allow this, consistent with
>> standard containers, to avoid this pitfall now & in the future.
> 
> Agreed.
> 
> Meanwhile I think the proper workaround is:
> 
> RWSequences.reserve(RWSequences.size()+1);
> RWSequences.push_back(RWSequences[OperIdx]);
> 
> ...rather than copying the vector. Anyone disagree?
> 
> -Andy
> 
>>> The client needs to copy the element first before passing it to push_back.
>>> 
>>> No test case, will be tested by follow-up swift scheduler model change (it
>>> segfaults without this change).
>>> 
>>> Modified:
>>>    llvm/trunk/utils/TableGen/CodeGenSchedule.cpp
>>> 
>>> Modified: llvm/trunk/utils/TableGen/CodeGenSchedule.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenSchedule.cpp?rev=183459&r1=183458&r2=183459&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/utils/TableGen/CodeGenSchedule.cpp (original)
>>> +++ llvm/trunk/utils/TableGen/CodeGenSchedule.cpp Thu Jun  6 18:23:14 2013
>>> @@ -1172,7 +1172,9 @@ pushVariant(const TransVariant &VInfo, b
>>>     unsigned OperIdx = RWSequences.size()-1;
>>>     // Make N-1 copies of this transition's last sequence.
>>>     for (unsigned i = 1, e = SelectedRWs.size(); i != e; ++i) {
>>> -      RWSequences.push_back(RWSequences[OperIdx]);
>>> +      // Create a temporary copy the vector could reallocate.
>>> +      SmallVector<unsigned, 4> Tmp = RWSequences[OperIdx];
>>> +      RWSequences.push_back(Tmp);
>>>     }
>>>     // Push each of the N elements of the SelectedRWs onto a copy of the last
>>>     // sequence (split the current operand into N operands).
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits