Skip to content
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

Jump to bottom

[DeviceAsan] Enlarge nullpointer redzone to better detect problem #2243

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[DeviceAsan] Enlarge nullpointer redzone to better detect problem #2243

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac4cb38
enlarge nullptr redzone
yingcong-wu Oct 25, 2024
6a79049
move to cpp, add comment
yingcong-wu Oct 25, 2024
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
5 changes: 3 additions & 2 deletions source/loader/layers/sanitizer/asan_report.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,10 @@ void ReportGenericError(const DeviceSanitizerReport &Report,
// Try to demangle the kernel name
KernelName = DemangleName(KernelName);

getContext()->logger.always("\n====ERROR: DeviceSanitizer: {} on {}",
getContext()->logger.always("\n====ERROR: DeviceSanitizer: {} on {} ({})",
ToString(Report.ErrorType),
ToString(Report.MemoryType));
ToString(Report.MemoryType),
(void *)Report.Address);
getContext()->logger.always(
"{} of size {} at kernel <{}> LID({}, {}, {}) GID({}, "
"{}, {})",
Expand Down
6 changes: 4 additions & 2 deletions source/loader/layers/sanitizer/asan_shadow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ ur_result_t ShadowMemoryCPU::Setup() {
ShadowEnd = ShadowBegin + ShadowSize;

// Set shadow memory for null pointer
auto URes = EnqueuePoisonShadow({}, 0, 1, kNullPointerRedzoneMagic);
auto URes = EnqueuePoisonShadow({}, 0, NULLPTR_REDZONE_SIZE,
kNullPointerRedzoneMagic);
if (URes != UR_RESULT_SUCCESS) {
getContext()->logger.error("EnqueuePoisonShadow(NullPointerRZ): {}",
URes);
Expand Down Expand Up @@ -113,7 +114,8 @@ ur_result_t ShadowMemoryGPU::Setup() {
// Set shadow memory for null pointer
ManagedQueue Queue(Context, Device);

Result = EnqueuePoisonShadow(Queue, 0, 1, kNullPointerRedzoneMagic);
Result = EnqueuePoisonShadow(Queue, 0, NULLPTR_REDZONE_SIZE,
kNullPointerRedzoneMagic);
if (Result != UR_RESULT_SUCCESS) {
getContext()->logger.error("EnqueuePoisonShadow(NullPointerRZ): {}",
Result);
Expand Down
2 changes: 2 additions & 0 deletions source/loader/layers/sanitizer/asan_shadow.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "common.hpp"
#include <unordered_set>

#define NULLPTR_REDZONE_SIZE (4096)
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment about why choose this size

Copy link
Contributor

@AllanZyne AllanZyne Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this macro needn't be in header file, and I suggest using constexpr to define this constant

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 choose this size becuase it is a common page value, and I don't have any other good reasons for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can query the size of page by GetVirtualMemGranularity.

Copy link
Contributor Author

@yingcong-wu yingcong-wu Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think that is necessary. Even when we have the page size, how many pages should we use? I think some simple value is good enough for now.
Also, this value don't have to align with any page size, that is not such requirement for it.


namespace ur_sanitizer_layer {

struct ShadowMemory {
Expand Down
Loading