Skip to content

[AMDGPU] Update instrumentAddress method to support aligned size and unusual size accesses. #104804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 21, 2024
Merged

[AMDGPU] Update instrumentAddress method to support aligned size and unusual size accesses. #104804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions llvm/lib/Target/AMDGPU/AMDGPUAsanInstrumentation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,13 @@ static Value *memToShadow(Module &M, IRBuilder<> &IRB, Type *IntptrTy,
return IRB.CreateAdd(Shadow, ShadowBase);
}

void instrumentAddress(Module &M, IRBuilder<> &IRB, Instruction *OrigIns,
Instruction *InsertBefore, Value *Addr,
MaybeAlign Alignment, uint32_t TypeStoreSize,
bool IsWrite, Value *SizeArgument, bool UseCalls,
bool Recover, int AsanScale, int AsanOffset) {
static void instrumentAddressImpl(Module &M, IRBuilder<> &IRB,
Instruction *OrigIns,
Instruction *InsertBefore, Value *Addr,
Align Alignment, uint32_t TypeStoreSize,
bool IsWrite, Value *SizeArgument,
bool UseCalls, bool Recover, int AsanScale,
int AsanOffset) {
Type *AddrTy = Addr->getType();
Type *IntptrTy = M.getDataLayout().getIntPtrType(
M.getContext(), AddrTy->getPointerAddressSpace());
Expand All @@ -164,7 +166,7 @@ void instrumentAddress(Module &M, IRBuilder<> &IRB, Instruction *OrigIns,
Value *ShadowPtr =
memToShadow(M, IRB, IntptrTy, AddrLong, AsanScale, AsanOffset);
const uint64_t ShadowAlign =
std::max<uint64_t>(Alignment.valueOrOne().value() >> AsanScale, 1);
std::max<uint64_t>(Alignment.value() >> AsanScale, 1);
Value *ShadowValue = IRB.CreateAlignedLoad(
ShadowTy, IRB.CreateIntToPtr(ShadowPtr, ShadowPtrTy), Align(ShadowAlign));
Value *Cmp = IRB.CreateIsNotNull(ShadowValue);
Expand All @@ -179,6 +181,43 @@ void instrumentAddress(Module &M, IRBuilder<> &IRB, Instruction *OrigIns,
return;
}

void instrumentAddress(Module &M, IRBuilder<> &IRB, Instruction *OrigIns,
Instruction *InsertBefore, Value *Addr, Align Alignment,
TypeSize TypeStoreSize, bool IsWrite,
Value *SizeArgument, bool UseCalls, bool Recover,
int AsanScale, int AsanOffset) {
if (!TypeStoreSize.isScalable()) {
unsigned Granularity = 1 << AsanScale;
const auto FixedSize = TypeStoreSize.getFixedValue();
switch (FixedSize) {
case 8:
case 16:
case 32:
case 64:
case 128:
if (Alignment.value() >= Granularity ||
Alignment.value() >= FixedSize / 8)
return instrumentAddressImpl(
M, IRB, OrigIns, InsertBefore, Addr, Alignment, FixedSize, IsWrite,
SizeArgument, UseCalls, Recover, AsanScale, AsanOffset);
}
}
// Instrument unusual size or unusual alignment.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still do not understand why you need to special case this based on the size, comment this, or just send everything down the one path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't changed the logic much from instrumentUnusualSizeOrAlignment taken from asan pass. All these API and infact the file would not be required once we move the utility APIs from asan pass to a common header file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a lot of copying generic code.

We also shouldn't consider 12 byte access as "unusual"

IRB.SetInsertPoint(InsertBefore);
Type *AddrTy = Addr->getType();
Type *IntptrTy = M.getDataLayout().getIntPtrType(AddrTy);
Value *NumBits = IRB.CreateTypeSize(IntptrTy, TypeStoreSize);
Value *Size = IRB.CreateLShr(NumBits, ConstantInt::get(IntptrTy, 3));
Value *AddrLong = IRB.CreatePtrToInt(Addr, IntptrTy);
Value *SizeMinusOne = IRB.CreateAdd(Size, ConstantInt::get(IntptrTy, -1));
Value *LastByte =
IRB.CreateIntToPtr(IRB.CreateAdd(AddrLong, SizeMinusOne), AddrTy);
instrumentAddressImpl(M, IRB, OrigIns, InsertBefore, Addr, {}, 8, IsWrite,
SizeArgument, UseCalls, Recover, AsanScale, AsanOffset);
instrumentAddressImpl(M, IRB, OrigIns, InsertBefore, LastByte, {}, 8, IsWrite,
SizeArgument, UseCalls, Recover, AsanScale, AsanOffset);
Comment on lines +215 to +218
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not need to be recursive, it makes it more confusing than necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not recursive. instrumentAddress() calls instrumentAddressImpl()

}

void getInterestingMemoryOperands(
Module &M, Instruction *I,
SmallVectorImpl<InterestingMemoryOperand> &Interesting) {
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/Target/AMDGPU/AMDGPUAsanInstrumentation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ uint64_t getRedzoneSizeForGlobal(int Scale, uint64_t SizeInBytes);
/// Instrument the memory operand Addr.
/// Generates report blocks that catch the addressing errors.
void instrumentAddress(Module &M, IRBuilder<> &IRB, Instruction *OrigIns,
Instruction *InsertBefore, Value *Addr,
MaybeAlign Alignment, uint32_t TypeStoreSize,
bool IsWrite, Value *SizeArgument, bool UseCalls,
bool Recover, int Scale, int Offset);
Instruction *InsertBefore, Value *Addr, Align Alignment,
TypeSize TypeStoreSize, bool IsWrite,
Value *SizeArgument, bool UseCalls, bool Recover,
int Scale, int Offset);

/// Get all the memory operands from the instruction
/// that needs to be instrumented
Expand Down
Loading