Skip to content

Commit

Permalink
Revert "Merge pull request #1843 from AllanZyne/review/yang/invalid_a…
Browse files Browse the repository at this point in the history
…rguments"

This reverts commit 61a67b3.
  • Loading branch information
callumfare committed Aug 29, 2024
1 parent d040a8f commit 97650ef
Show file tree
Hide file tree
Showing 11 changed files with 28 additions and 292 deletions.
2 changes: 0 additions & 2 deletions source/loader/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,6 @@ if(UR_ENABLE_SANITIZER)
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/asan_report.hpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/asan_shadow_setup.cpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/asan_shadow_setup.hpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/asan_validator.cpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/asan_validator.hpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/common.hpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/stacktrace.cpp
${CMAKE_CURRENT_SOURCE_DIR}/layers/sanitizer/stacktrace.hpp
Expand Down
23 changes: 1 addition & 22 deletions source/loader/layers/sanitizer/asan_interceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "asan_quarantine.hpp"
#include "asan_report.hpp"
#include "asan_shadow_setup.hpp"
#include "asan_validator.hpp"
#include "stacktrace.hpp"
#include "ur_sanitizer_utils.hpp"

Expand Down Expand Up @@ -628,9 +627,6 @@ SanitizerInterceptor::insertDevice(ur_device_handle_t Device,

DI = std::make_shared<ur_sanitizer_layer::DeviceInfo>(Device);

DI->IsSupportSharedSystemUSM = GetDeviceUSMCapability(
Device, UR_DEVICE_INFO_USM_SYSTEM_SHARED_SUPPORT);

// Query alignment
UR_CALL(getContext()->urDdiTable.Device.pfnGetInfo(
Device, UR_DEVICE_INFO_MEM_BASE_ADDR_ALIGN, sizeof(DI->Alignment),
Expand Down Expand Up @@ -699,25 +695,8 @@ ur_result_t SanitizerInterceptor::prepareLaunch(
auto Program = GetProgram(Kernel);

do {
auto KernelInfo = getKernelInfo(Kernel);

// Validate pointer arguments
if (Options(logger).DetectKernelArguments) {
for (const auto &[ArgIndex, PtrPair] : KernelInfo->PointerArgs) {
auto Ptr = PtrPair.first;
if (Ptr == nullptr) {
continue;
}
if (auto ValidateResult = ValidateUSMPointer(
Context, DeviceInfo->Handle, (uptr)Ptr)) {
ReportInvalidKernelArgument(Kernel, ArgIndex, (uptr)Ptr,
ValidateResult, PtrPair.second);
exit(1);
}
}
}

// Set membuffer arguments
auto KernelInfo = getKernelInfo(Kernel);
for (const auto &[ArgIndex, MemBuffer] : KernelInfo->BufferArgs) {
char *ArgPointer = nullptr;
UR_CALL(MemBuffer->getHandle(DeviceInfo->Handle, ArgPointer));
Expand Down
5 changes: 0 additions & 5 deletions source/loader/layers/sanitizer/asan_interceptor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ struct DeviceInfo {
uptr ShadowOffset = 0;
uptr ShadowOffsetEnd = 0;

// Device features
bool IsSupportSharedSystemUSM = false;

ur_mutex Mutex;
std::queue<std::shared_ptr<AllocInfo>> Quarantine;
size_t QuarantineSize = 0;
Expand Down Expand Up @@ -81,8 +78,6 @@ struct KernelInfo {
ur_shared_mutex Mutex;
std::atomic<int32_t> RefCount = 1;
std::unordered_map<uint32_t, std::shared_ptr<MemBuffer>> BufferArgs;
std::unordered_map<uint32_t, std::pair<const void *, StackTrace>>
PointerArgs;

// Need preserve the order of local arguments
std::map<uint32_t, LocalArgsInfo> LocalArgs;
Expand Down
2 changes: 0 additions & 2 deletions source/loader/layers/sanitizer/asan_options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ struct AsanOptions {
uint32_t MaxQuarantineSizeMB = 0;
bool DetectLocals = true;
bool DetectPrivates = true;
bool DetectKernelArguments = true;

private:
AsanOptions(logger::Logger &logger) {
Expand Down Expand Up @@ -94,7 +93,6 @@ struct AsanOptions {
SetBoolOption("debug", Debug);
SetBoolOption("detect_locals", DetectLocals);
SetBoolOption("detect_privates", DetectPrivates);
SetBoolOption("detect_kernel_arguments", DetectKernelArguments);

auto KV = OptionsEnvMap->find("quarantine_size_mb");
if (KV != OptionsEnvMap->end()) {
Expand Down
92 changes: 26 additions & 66 deletions source/loader/layers/sanitizer/asan_report.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,16 @@
*/

#include "asan_report.hpp"
#include "asan_options.hpp"

#include "asan_allocator.hpp"
#include "asan_interceptor.hpp"
#include "asan_libdevice.hpp"
#include "asan_options.hpp"
#include "asan_validator.hpp"
#include "ur_sanitizer_layer.hpp"
#include "ur_sanitizer_utils.hpp"

namespace ur_sanitizer_layer {

namespace {

void PrintAllocateInfo(uptr Addr, const AllocInfo *AI) {
getContext()->logger.always("{} is located inside of {} region [{}, {})",
(void *)Addr, ToString(AI->Type),
(void *)AI->UserBegin, (void *)AI->UserEnd);
getContext()->logger.always("allocated here:");
AI->AllocStack.print();
if (AI->IsReleased) {
getContext()->logger.always("freed here:");
AI->ReleaseStack.print();
}
}

} // namespace

void ReportBadFree(uptr Addr, const StackTrace &stack,
const std::shared_ptr<AllocInfo> &AI) {
getContext()->logger.always(
Expand All @@ -50,7 +34,11 @@ void ReportBadFree(uptr Addr, const StackTrace &stack,

assert(AI && !AI->IsReleased && "Chunk must be not released");

PrintAllocateInfo(Addr, AI.get());
getContext()->logger.always("{} is located inside of {} region [{}, {})",
(void *)Addr, ToString(AI->Type),
(void *)AI->UserBegin, (void *)AI->UserEnd);
getContext()->logger.always("allocated here:");
AI->AllocStack.print();
}

void ReportBadContext(uptr Addr, const StackTrace &stack,
Expand All @@ -60,7 +48,16 @@ void ReportBadContext(uptr Addr, const StackTrace &stack,
(void *)Addr);
stack.print();

PrintAllocateInfo(Addr, AI.get());
getContext()->logger.always("{} is located inside of {} region [{}, {})",
(void *)Addr, ToString(AI->Type),
(void *)AI->UserBegin, (void *)AI->UserEnd);
getContext()->logger.always("allocated here:");
AI->AllocStack.print();

if (AI->IsReleased) {
getContext()->logger.always("freed here:");
AI->ReleaseStack.print();
}
}

void ReportDoubleFree(uptr Addr, const StackTrace &Stack,
Expand Down Expand Up @@ -142,10 +139,16 @@ void ReportUseAfterFree(const DeviceSanitizerReport &Report,
"Failed to find which chunck {} is allocated",
(void *)Report.Address);
}
assert(AllocInfo->IsReleased &&
"It must be released since it's use-after-free");
assert(AllocInfo->IsReleased);

PrintAllocateInfo(Report.Address, AllocInfo.get());
getContext()->logger.always(
"{} is located inside of {} region [{}, {})",
(void *)Report.Address, ToString(AllocInfo->Type),
(void *)AllocInfo->UserBegin, (void *)AllocInfo->UserEnd);
getContext()->logger.always("allocated here:");
AllocInfo->AllocStack.print();
getContext()->logger.always("released here:");
AllocInfo->ReleaseStack.print();
}
} else {
getContext()->logger.always(
Expand All @@ -154,47 +157,4 @@ void ReportUseAfterFree(const DeviceSanitizerReport &Report,
}
}

void ReportInvalidKernelArgument(ur_kernel_handle_t Kernel, uint32_t ArgIndex,
uptr Addr, const ValidateUSMResult &VR,
StackTrace Stack) {
getContext()->logger.always("\n====ERROR: DeviceSanitizer: "
"invalid-argument on kernel <{}>",
DemangleName(GetKernelName(Kernel)));
Stack.print();
auto &AI = VR.AI;
switch (VR.Type) {
case ValidateUSMResult::MAYBE_HOST_POINTER:
getContext()->logger.always("The {}th argument {} is not a USM pointer",
ArgIndex + 1, (void *)Addr);
break;
case ValidateUSMResult::RELEASED_POINTER:
getContext()->logger.always(
"The {}th argument {} is a released USM pointer", ArgIndex,
(void *)Addr);
PrintAllocateInfo(Addr, AI.get());
break;
case ValidateUSMResult::BAD_CONTEXT:
getContext()->logger.always(
"The {}th argument {} is allocated in other context", ArgIndex,
(void *)Addr);
PrintAllocateInfo(Addr, AI.get());
break;
case ValidateUSMResult::BAD_DEVICE:
getContext()->logger.always(
"The {}th argument {} is allocated in other device", ArgIndex,
(void *)Addr);
PrintAllocateInfo(Addr, AI.get());
break;
case ValidateUSMResult::OUT_OF_BOUNDS:
getContext()->logger.always(
"The {}th argument {} is located outside of its region [{}, {})",
ArgIndex, (void *)Addr, (void *)AI->UserBegin, (void *)AI->UserEnd);
getContext()->logger.always("allocated here:");
AI->AllocStack.print();
break;
default:
break;
}
}

} // namespace ur_sanitizer_layer
5 changes: 0 additions & 5 deletions source/loader/layers/sanitizer/asan_report.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ namespace ur_sanitizer_layer {
struct DeviceSanitizerReport;
struct AllocInfo;
struct StackTrace;
struct ValidateUSMResult;

void ReportBadFree(uptr Addr, const StackTrace &stack,
const std::shared_ptr<AllocInfo> &AllocInfo);
Expand All @@ -41,8 +40,4 @@ void ReportGenericError(const DeviceSanitizerReport &Report,
void ReportUseAfterFree(const DeviceSanitizerReport &Report,
ur_kernel_handle_t Kernel, ur_context_handle_t Context);

void ReportInvalidKernelArgument(ur_kernel_handle_t Kernel, uint32_t ArgIndex,
uptr Addr, const ValidateUSMResult &VR,
StackTrace Stack);

} // namespace ur_sanitizer_layer
77 changes: 0 additions & 77 deletions source/loader/layers/sanitizer/asan_validator.cpp

This file was deleted.

50 changes: 0 additions & 50 deletions source/loader/layers/sanitizer/asan_validator.hpp

This file was deleted.

Loading

0 comments on commit 97650ef

Please sign in to comment.