diff options
author | Hans Wennborg <hans@chromium.org> | 2019-11-13 11:59:22 +0100 |
---|---|---|
committer | Hans Wennborg <hans@chromium.org> | 2019-11-13 12:19:02 +0100 |
commit | 6ea47759008526dc11f5064b266de95c61915581 (patch) | |
tree | c722d72d7942805728803ac106eeb7ba3c362b89 /llvm/lib/Analysis/ValueTracking.cpp | |
parent | fed17867cd42c5fa8a7a561637d539fbde6f511f (diff) | |
download | bcm5719-llvm-6ea47759008526dc11f5064b266de95c61915581.tar.gz bcm5719-llvm-6ea47759008526dc11f5064b266de95c61915581.zip |
Revert 57dd4b0 "[ValueTracking] Allow context-sensitive nullness check for non-pointers"
This caused miscompiles of Chromium (https://crbug.com/1023818). The reduced
repro is small enough to fit here:
$ cat /tmp/a.c
unsigned char f(unsigned char *p) {
unsigned char result = 0;
for (int shift = 0; shift < 1; ++shift)
result |= p[0] << (shift * 8);
return result;
}
$ bin/clang -O2 -S -o - /tmp/a.c | grep -A4 f:
f: # @f
.cfi_startproc
# %bb.0: # %entry
xorl %eax, %eax
retq
That's nicely optimized, but I don't think it's the right result :-)
> Same as D60846 but with a fix for the problem encountered there which
> was a missing context adjustment in the handling of PHI nodes.
>
> The test that caused D60846 to be reverted was added in e15ab8f277c7.
>
> Reviewers: nikic, nlopes, mkazantsev,spatel, dlrobertson, uabelho, hakzsam
>
> Subscribers: hiraditya, bollu, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D69571
This reverts commit 57dd4b03e4806bbb4760ab6150940150d884df20.
Diffstat (limited to 'llvm/lib/Analysis/ValueTracking.cpp')
-rw-r--r-- | llvm/lib/Analysis/ValueTracking.cpp | 9 |
1 files changed, 5 insertions, 4 deletions
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index b26c6294dce..f03a4a6eee4 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -1902,8 +1902,8 @@ static bool isGEPKnownNonNull(const GEPOperator *GEP, unsigned Depth, static bool isKnownNonNullFromDominatingCondition(const Value *V, const Instruction *CtxI, const DominatorTree *DT) { - if (isa<Constant>(V)) - return false; + assert(V->getType()->isPointerTy() && "V must be pointer type"); + assert(!isa<ConstantData>(V) && "Did not expect ConstantPointerNull"); if (!CtxI || !DT) return false; @@ -2078,11 +2078,12 @@ bool isKnownNonZero(const Value *V, unsigned Depth, const Query &Q) { } } - if (isKnownNonNullFromDominatingCondition(V, Q.CxtI, Q.DT)) - return true; // Check for recursive pointer simplifications. if (V->getType()->isPointerTy()) { + if (isKnownNonNullFromDominatingCondition(V, Q.CxtI, Q.DT)) + return true; + // Look through bitcast operations, GEPs, and int2ptr instructions as they // do not alter the value, or at least not the nullness property of the // value, e.g., int2ptr is allowed to zero/sign extend the value. |