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

[Andrew Trick]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130606/d221bb19/attachment.html>