[DeviceSanitizer] Support detecting out-of-bounds error on CPU Device & Static Local Memory #1210
Conversation
This commit adds the adapter implementations of the virtual memory extension functionality to be used in intel/llvm#8954. Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
This reverts commit 84a3afa.
This reverts commit 336ce89.
This reverts commit 9f90e43.
This reverts commit 9ae8a99.
This reverts commit 6ba94ac.
This reverts commit c36bd72.
…ase" This reverts commit 1096b0f.
This reverts commit 67fcd5e.
|
Hi @oneapi-src/unified-runtime-maintain, we had already internal reviewed this PR, can UR team help to review it? |
|
Looks like this fails to compile on macOS. I think the easiest solution would be to disable the sanitizer layer in CMake when macOS is detected. |
Yep, thanks for your suggestion. |
| return Result; | ||
| // FIXME: Currently, Level-Zero doesn't create independent VAs for each contexts, | ||
| // which will cause out-of-resource error when users use multiple contexts | ||
| static uptr ShadowOffset, ShadowOffsetEnd; |
There was a problem hiding this comment.
As mentioned on comment, Level-Zero uses one VA for multiple contexts, so if user creates multiple contexts, we have to reserve multiple huge shadow memories, which will cause out-of-resource error. So I added "static" here to let one shadow memory share with all contexts.
There was a problem hiding this comment.
can call_once be used instead? it might be more intuitive
There was a problem hiding this comment.
This piece of code will also suffer multi thread issue (CPU as well). I'm going to redesign DeviceInfo in next patch, and let each ur_device_handle_t correspond to only one unique instance of it, so that there will be only one shadow memory offset in each DeviceInfo.
For now, let's just leave it as this.
There was a problem hiding this comment.
do you mean this 'static' code will be removed after your redesign so that the multi-thread issue will be resolved?
There was a problem hiding this comment.
Yep, because DeviceInfo will be initialized only once.
There was a problem hiding this comment.
So this is a race now, right? Once all this is resolved, might be worth adding multi-threaded tests that exercise this code path.
There was a problem hiding this comment.
So this is a race now, right? Once all this is resolved, might be worth adding multi-threaded tests that exercise this code path.
Yes, I will.
| return Result; | ||
| // FIXME: Currently, Level-Zero doesn't create independent VAs for each contexts, | ||
| // which will cause out-of-resource error when users use multiple contexts | ||
| static uptr ShadowOffset, ShadowOffsetEnd; |
There was a problem hiding this comment.
do you mean this 'static' code will be removed after your redesign so that the multi-thread issue will be resolved?
|
Hi, do I need to get more reviewers? Or can this PR be merged? |
| // Poison shadow memory outside of asan runtime is not allowed, so we | ||
| // need to avoid memset's call from being intercepted. | ||
| static void *memset_ptr = []() { | ||
| void *handle = dlopen("libc.so.6", RTLD_LAZY); |
There was a problem hiding this comment.
should we hardcode the library version number?
There was a problem hiding this comment.
Makes sense. I changed it to LIBC_SO macro.
UR: oneapi-src/unified-runtime#1210 --------- Co-authored-by: Maosu Zhao <maosu.zhao@intel.com>
intel/llvm part: intel/llvm#12248