[llvm] r271278 - Do not modify a std::vector while looping it.

[Saleem Abdulrasool via llvm-commits]

On Tue, May 31, 2016 at 6:45 AM, Yaron Keren via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: yrnkrn
> Date: Tue May 31 08:45:05 2016
> New Revision: 271278
>
> URL: http://llvm.org/viewvc/llvm-project?rev=271278&view=rev
> Log:
> Do not modify a std::vector while looping it.
>
> Introduced in r271244, this is probably undefined behaviour and asserts
> when
> compiled with Visual C++ debug mode.
>

Thanks.  I was trying to build under visual studio to get a stack trace.
Once I got that, it was obvious that I was invalidating the iterator.  This
change looks good to me.

Thanks for fixing this!


> On further note, the loop is quadratic with regard to the number of
> successors
> since removeSuccessor is linear and could probably be modified to linear
> time.
>
>
> Modified:
>     llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=271278&r1=271277&r2=271278&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue May 31 08:45:05 2016
> @@ -23841,8 +23841,12 @@ X86TargetLowering::EmitSjLjDispatchBlock
>        Subtarget.getRegisterInfo()->getCalleeSavedRegs(MF);
>    for (MachineBasicBlock *MBB : InvokeBBs) {
>      // Remove the landing pad successor from the invoke block and replace
> it
> -    // with the new dispatch block
> -    for (auto MBBS : make_range(MBB->succ_rbegin(), MBB->succ_rend())) {
> +    // with the new dispatch block.
> +    // Keep a copy of Successors since it's modified inside the loop.
> +    SmallVector<MachineBasicBlock *, 8> Successors(MBB->succ_rbegin(),
> +                                                   MBB->succ_rend());
> +    // FIXME: Avoid quadratic complexity.
> +    for (auto MBBS : Successors) {
>        if (MBBS->isEHPad()) {
>          MBB->removeSuccessor(MBBS);
>          MBBLPads.push_back(MBBS);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160531/3c8921d2/attachment.html>