[llvm] 7e0802a - [BasicAA] Fix order in which we pass MemoryLocations to alias()
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Tue May 10 12:09:48 PDT 2022
Author: Arthur Eubanks
Date: 2022-05-10T12:05:38-07:00
New Revision: 7e0802aeb5b9059c580479049e6074f6b97ba5d6
URL: https://github.com/llvm/llvm-project/commit/7e0802aeb5b9059c580479049e6074f6b97ba5d6
DIFF: https://github.com/llvm/llvm-project/commit/7e0802aeb5b9059c580479049e6074f6b97ba5d6.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/IR/IRBuilder.h
llvm/lib/Analysis/BasicAliasAnalysis.cpp
llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 5ac7890905720..adb0d72b0d575 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -540,6 +540,11 @@ class IRBuilderBase {
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);
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index a0eb823032251..cfb34686bd429 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -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;
diff --git a/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp b/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
index d8fd37e662d1b..6933b7c19223b 100644
--- a/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
+++ b/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
@@ -63,16 +63,17 @@ class BasicAATest : public testing::Test {
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);
+}
More information about the llvm-commits
mailing list