From d5972724450e37c578c91f40427b63e2468cdff3 Mon Sep 17 00:00:00 2001 From: Martin Storsjo Date: Thu, 30 May 2019 20:53:21 +0000 Subject: [PATCH] [InstCombine] Avoid use after free in DenseMap, when built with GCC Previously, this used a statement like this: Map[A] = Map[B]; This is equivalent to the following: const auto &Src = Map[B]; auto &Dest = Map[A]; Dest = Src; The second statement, "auto &Dest = Map[A];" can insert a new element into the DenseMap, which can potentially grow and reallocate the DenseMap's internal storage, which will invalidate the existing reference to the source. When doing the actual assignment, the Src reference is dereferenced, accessing memory that was freed when the DenseMap grew. This issue hasn't shown up when LLVM was built with Clang, because the right hand side ended up dereferenced before evaulating the left hand side. (If the value type is a larger data type, Clang doesn't do this but behaves like GCC.) With GCC, a cast to Value* isn't enough to make it dereference the right hand side reference before invoking operator[] (while that is enough to make Clang/LLVM do the right thing for larger types), but storing it in an intermediate variable in a separate statement works. This fixes PR42065. Differential Revision: https://reviews.llvm.org/D62624 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362150 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/InstCombine/InstCombineCompares.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp index b3eb75ea8a8..a04e56916f8 100644 --- a/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -703,7 +703,10 @@ static Value *rewriteGEPAsOffset(Value *Start, Value *Base, continue; if (auto *CI = dyn_cast(Val)) { - NewInsts[CI] = NewInsts[CI->getOperand(0)]; + // Don't get rid of the intermediate variable here; the store can grow + // the map which will invalidate the reference to the input value. + Value *V = NewInsts[CI->getOperand(0)]; + NewInsts[CI] = V; continue; } if (auto *GEP = dyn_cast(Val)) { -- 2.11.4.GIT