From 11c890ecc7d43d455e97ab5032eeb6870bbc8d29 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Fri, 12 Apr 2019 18:26:56 +0000 Subject: [PATCH] [InstCombine] Fix a nasty miscompile introduced w/masked.gather demanded elts This fixes a miscompile which was introduced in r356510 (https://reviews.llvm.org/D57372). The problem is that the original patch removed pointer operands where the load results we're demanded, but without considering the legality of the load itself. If the masked.gather had active, but undemanded, lanes, then we could end up creating a load which loaded from an undef address. The result could be a segfault, or, in theory, an arbitrary read from a random memory location into an used register. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358299 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp | 6 +++++- test/Transforms/InstCombine/masked_intrinsics.ll | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index 2dea7eea404..1d52938d1f8 100644 --- a/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp +++ b/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp @@ -1437,7 +1437,11 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts, switch (II->getIntrinsicID()) { case Intrinsic::masked_gather: // fallthrough case Intrinsic::masked_load: { - APInt DemandedPtrs(DemandedElts), DemandedPassThrough(DemandedElts); + // Subtlety: If we load from a pointer, the pointer must be valid + // regardless of whether the element is demanded. Doing otherwise risks + // segfaults which didn't exist in the original program. + APInt DemandedPtrs(APInt::getAllOnesValue(VWidth)), + DemandedPassThrough(DemandedElts); if (auto *CV = dyn_cast(II->getOperand(2))) for (unsigned i = 0; i < VWidth; i++) { Constant *CElt = CV->getAggregateElement(i); diff --git a/test/Transforms/InstCombine/masked_intrinsics.ll b/test/Transforms/InstCombine/masked_intrinsics.ll index 48c8f78f990..582fd8f5c82 100644 --- a/test/Transforms/InstCombine/masked_intrinsics.ll +++ b/test/Transforms/InstCombine/masked_intrinsics.ll @@ -56,10 +56,9 @@ define <2 x double> @load_lane0(<2 x double>* %ptr, double %pt) { ret <2 x double> %res } -; FIXME: the output here demonstrates a miscompile! define double @load_all(double* %base, double %pt) { ; CHECK-LABEL: @load_all( -; CHECK-NEXT: [[PTRS:%.*]] = getelementptr double, double* [[BASE:%.*]], <4 x i64> +; CHECK-NEXT: [[PTRS:%.*]] = getelementptr double, double* [[BASE:%.*]], <4 x i64> ; CHECK-NEXT: [[RES:%.*]] = call <4 x double> @llvm.masked.gather.v4f64.v4p0f64(<4 x double*> [[PTRS]], i32 4, <4 x i1> , <4 x double> undef) ; CHECK-NEXT: [[ELT:%.*]] = extractelement <4 x double> [[RES]], i64 2 ; CHECK-NEXT: ret double [[ELT]] -- 2.50.1