[BasicAA] Fix order in which we pass MemoryLocations to alias()

D98718 caused the order of Values/MemoryLocations we pass to alias() to
be significant due to storing the offset in the PartialAlias case. But
some callers weren't audited and were still passing swapped arguments,
causing the returned PartialAlias offset to be negative in some
cases. For example, the newly added unittests would return -1
instead of 1.

Fixes #55343, a miscompile.

Reviewed By: asbirlea, nikic

Differential Revision: https://reviews.llvm.org/D125328
This commit is contained in:
Arthur Eubanks 2022-05-09 19:32:14 -07:00
parent 17a73992dd
commit 7e0802aeb5
3 changed files with 86 additions and 13 deletions

View File

@ -540,6 +540,11 @@ public:
return Type::getVoidTy(Context);
}
/// Fetch the type representing a pointer.
PointerType *getPtrTy(unsigned AddrSpace = 0) {
return PointerType::get(Context, AddrSpace);
}
/// Fetch the type representing a pointer to an 8-bit integer value.
PointerType *getInt8PtrTy(unsigned AddrSpace = 0) {
return Type::getInt8PtrTy(Context, AddrSpace);

View File

@ -1391,15 +1391,15 @@ BasicAAResult::aliasSelect(const SelectInst *SI, LocationSize SISize,
// If both arms of the Select node NoAlias or MustAlias V2, then returns
// NoAlias / MustAlias. Otherwise, returns MayAlias.
AliasResult Alias = getBestAAResults().alias(
MemoryLocation(V2, V2Size),
MemoryLocation(SI->getTrueValue(), SISize), AAQI);
AliasResult Alias =
getBestAAResults().alias(MemoryLocation(SI->getTrueValue(), SISize),
MemoryLocation(V2, V2Size), AAQI);
if (Alias == AliasResult::MayAlias)
return AliasResult::MayAlias;
AliasResult ThisAlias = getBestAAResults().alias(
MemoryLocation(V2, V2Size),
MemoryLocation(SI->getFalseValue(), SISize), AAQI);
AliasResult ThisAlias =
getBestAAResults().alias(MemoryLocation(SI->getFalseValue(), SISize),
MemoryLocation(V2, V2Size), AAQI);
return MergeAliasResults(ThisAlias, Alias);
}
@ -1521,8 +1521,7 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
AAQueryInfo *UseAAQI = BlockInserted ? &NewAAQI : &AAQI;
AliasResult Alias = getBestAAResults().alias(
MemoryLocation(V2, V2Size),
MemoryLocation(V1Srcs[0], PNSize), *UseAAQI);
MemoryLocation(V1Srcs[0], PNSize), MemoryLocation(V2, V2Size), *UseAAQI);
// Early exit if the check of the first PHI source against V2 is MayAlias.
// Other results are not possible.
@ -1539,7 +1538,7 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
Value *V = V1Srcs[i];
AliasResult ThisAlias = getBestAAResults().alias(
MemoryLocation(V2, V2Size), MemoryLocation(V, PNSize), *UseAAQI);
MemoryLocation(V, PNSize), MemoryLocation(V2, V2Size), *UseAAQI);
Alias = MergeAliasResults(ThisAlias, Alias);
if (Alias == AliasResult::MayAlias)
break;

View File

@ -63,16 +63,17 @@ protected:
public:
BasicAATest()
: M("BasicAATest", C), B(C), DL(DLString), TLI(TLII), F(nullptr) {}
: M("BasicAATest", C), B(C), DL(DLString), TLI(TLII), F(nullptr) {
C.setOpaquePointers(true);
}
};
// Check that a function arg can't trivially alias a global when we're accessing
// >sizeof(global) bytes through that arg, unless the access size is just an
// upper-bound.
TEST_F(BasicAATest, AliasInstWithObjectOfImpreciseSize) {
F = Function::Create(
FunctionType::get(B.getVoidTy(), {B.getInt32Ty()->getPointerTo()}, false),
GlobalValue::ExternalLinkage, "F", &M);
F = Function::Create(FunctionType::get(B.getVoidTy(), {B.getPtrTy()}, false),
GlobalValue::ExternalLinkage, "F", &M);
BasicBlock *Entry(BasicBlock::Create(C, "", F));
B.SetInsertPoint(Entry);
@ -131,3 +132,71 @@ TEST_F(BasicAATest, AliasInstWithFullObjectOfImpreciseSize) {
AAQI),
AliasResult::MayAlias);
}
TEST_F(BasicAATest, PartialAliasOffsetPhi) {
F = Function::Create(
FunctionType::get(B.getVoidTy(), {B.getPtrTy(), B.getInt1Ty()}, false),
GlobalValue::ExternalLinkage, "F", &M);
Value *Ptr = F->arg_begin();
Value *I = F->arg_begin() + 1;
BasicBlock *Entry(BasicBlock::Create(C, "", F));
BasicBlock *B1(BasicBlock::Create(C, "", F));
BasicBlock *B2(BasicBlock::Create(C, "", F));
BasicBlock *End(BasicBlock::Create(C, "", F));
B.SetInsertPoint(Entry);
B.CreateCondBr(I, B1, B2);
B.SetInsertPoint(B1);
auto *Ptr1 =
cast<GetElementPtrInst>(B.CreateGEP(B.getInt8Ty(), Ptr, B.getInt32(1)));
B.CreateBr(End);
B.SetInsertPoint(B2);
auto *Ptr2 =
cast<GetElementPtrInst>(B.CreateGEP(B.getInt8Ty(), Ptr, B.getInt32(1)));
B.CreateBr(End);
B.SetInsertPoint(End);
auto *Phi = B.CreatePHI(B.getPtrTy(), 2);
Phi->addIncoming(Ptr1, B1);
Phi->addIncoming(Ptr2, B2);
B.CreateRetVoid();
auto &AllAnalyses = setupAnalyses();
BasicAAResult &BasicAA = AllAnalyses.BAA;
AAQueryInfo &AAQI = AllAnalyses.AAQI;
AliasResult AR =
BasicAA.alias(MemoryLocation(Ptr, LocationSize::precise(2)),
MemoryLocation(Phi, LocationSize::precise(1)), AAQI);
ASSERT_EQ(AR.getOffset(), 1);
}
TEST_F(BasicAATest, PartialAliasOffsetSelect) {
F = Function::Create(
FunctionType::get(B.getVoidTy(), {B.getPtrTy(), B.getInt1Ty()}, false),
GlobalValue::ExternalLinkage, "F", &M);
Value *Ptr = F->arg_begin();
Value *I = F->arg_begin() + 1;
BasicBlock *Entry(BasicBlock::Create(C, "", F));
B.SetInsertPoint(Entry);
auto *Ptr1 =
cast<GetElementPtrInst>(B.CreateGEP(B.getInt8Ty(), Ptr, B.getInt32(1)));
auto *Ptr2 =
cast<GetElementPtrInst>(B.CreateGEP(B.getInt8Ty(), Ptr, B.getInt32(1)));
auto *Select = B.CreateSelect(I, Ptr1, Ptr2);
B.CreateRetVoid();
auto &AllAnalyses = setupAnalyses();
BasicAAResult &BasicAA = AllAnalyses.BAA;
AAQueryInfo &AAQI = AllAnalyses.AAQI;
AliasResult AR =
BasicAA.alias(MemoryLocation(Ptr, LocationSize::precise(2)),
MemoryLocation(Select, LocationSize::precise(1)), AAQI);
ASSERT_EQ(AR.getOffset(), 1);
}