[llvm] r269908 - [VectorUtils] Fix nasty use-after-free

[Wei Mi via llvm-commits]

Hi James,

Thanks for the fix. It should fix the use-after-free error. However, I
am still worrying about the garbage left in the WidenMap. If people
use the map after truncateToMinimalBitwidths, they may see this nasty
bug again. We may need some mechanism to update the WidenMap after a
vectorized instruction is removed.

Thanks,
Wei.



On Wed, May 18, 2016 at 4:57 AM, James Molloy via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: jamesm
> Date: Wed May 18 06:57:58 2016
> New Revision: 269908
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269908&view=rev
> Log:
> [VectorUtils] Fix nasty use-after-free
>
> In truncateToMinimalBitwidths() we were RAUW'ing an instruction then erasing it. However, that intruction could be cached in the map we're iterating over. The first check is "I->use_empty()" which in most cases would return true, as the (deleted) object was RAUW'd first so would have zero use count. However in some cases the object could have been polluted or written over and this wouldn't be the case. Also it makes valgrind, asan and traditionalists who don't like their compiler to crash sad.
>
> No testcase as there are no externally visible symptoms apart from a crash if the stars align.
>
> Fixes PR26509.
>
> Modified:
>     llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp
>
> Modified: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=269908&r1=269907&r2=269908&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp (original)
> +++ llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp Wed May 18 06:57:58 2016
> @@ -3385,10 +3385,11 @@ void InnerLoopVectorizer::truncateToMini
>    // truncated version of `I` and reextend its result. InstCombine runs
>    // later and will remove any ext/trunc pairs.
>    //
> +  SmallPtrSet<Value *, 4> Erased;
>    for (auto &KV : MinBWs) {
>      VectorParts &Parts = WidenMap.get(KV.first);
>      for (Value *&I : Parts) {
> -      if (I->use_empty())
> +      if (Erased.count(I) || I->use_empty())
>          continue;
>        Type *OriginalTy = I->getType();
>        Type *ScalarTruncatedTy =
> @@ -3474,6 +3475,7 @@ void InnerLoopVectorizer::truncateToMini
>        Value *Res = B.CreateZExtOrTrunc(NewI, OriginalTy);
>        I->replaceAllUsesWith(Res);
>        cast<Instruction>(I)->eraseFromParent();
> +      Erased.insert(I);
>        I = Res;
>      }
>    }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits