[UR] Make the device sanitizer layer a shared library - #22950
Conversation
The loader can be statically linked into more than one library of a process - an OpenMP offload application using SYCL interop loads both libomptarget and libsycl, each with its own copy of libur_loader.a. The layer state is hidden, so the linker cannot merge those copies, and the sanitizers ended up with two independent states over one device. Both reserved device shadow memory, and the second reservation failed with UR_RESULT_ERROR_OUT_OF_DEVICE_MEMORY. Move the sanitizer layer into libur_sanitizer_layer.so, loaded on demand from the same locations an adapter is loaded from, so all loader instances of a process share one instance of it. Every loader still initializes the layer with its own ddi table; the layer reference counts those and hooks up the extra tables via context_t::interceptDdiTable instead of setting up a second state. The library exports nothing but urLoaderLayerGetInterface, which also makes the ld -r/objcopy pre-link that hid the LLVM symbolizer symbols inside the static loader unnecessary - they are now private to this library by construction. Enabling a layer whose library cannot be loaded now fails urLoaderInit rather than silently running unsanitized. The generic half of this lives in ur_loader::SharedLayer so that other layers can be moved out of the loader the same way.
|
@intel/unified-runtime-reviewers please take a look |
| // An enabled layer that fails to come up (e.g. a missing shared library) fails | ||
| // the whole loader; coming up silently without it would be worse. | ||
| ur_result_t context_t::initLayers() { | ||
| for (auto &[layer, _] : layers) { | ||
| layer->init(&urDdiTable, enabledLayerNames, codelocData); | ||
| ur_result_t result = | ||
| layer->init(&urDdiTable, enabledLayerNames, codelocData); | ||
| if (result != UR_RESULT_SUCCESS) { | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Is it OK that the initialization of the tracing layer is silently skipped when the sanitizer shared library fails to load?
The order of layers is following: validation → sanitizer → tracing, so if the new ur_sanitizer_layer.so is missing/not installed while both sanitizer and tracing are enabled, the loop returns immediately and tracing's init() never runs.
There is still the comment:
// Initialize tracing layer after sanitizer layer to make sure tracing
// layer will properly print all API calls.https://github.com/intel/llvm/blob/sycl/unified-runtime/source/loader/ur_lib.hpp#L76-L77
There was a problem hiding this comment.
Yes, it's OK. If someone enables the sanitizer layer, and it fails because .so is missing, this code will make everything fail, and loader won't be initialized. So it's not important that later layers aren't initialized.
There was a problem hiding this comment.
OK, but from the SYCL's point of view initLayers() is called by:
https://github.com/intel/llvm/blob/sycl/sycl/source/detail/ur.cpp#L209
CHECK_UR_SUCCESS(loaderInit(device_flags, LoaderConfig));CHECK_UR_SUCCESS() only prints to stderr and falls through - it doesn't abort or propagate the error, so when sanitizer's .so is missing:
- context_t::Init() returns an error code and SYCL logs it and keeps going - it doesn't stop the process or refuse to enumerate adapters,
- the DDI table is left in a partially wrapped state: validation is active, sanitizer isn't, but tracing also never got initialized as a side effect of loop order, not because tracing itself was unavailable.
- so the observable result is: the application keeps running, devices/queues work, but tracing/XPTI instrumentation is silently absent, with only an easily-missed stderr line as a clue.
There was a problem hiding this comment.
Ok, I think you are right. I we can make a separate PR that fixes this in SYCL. In realistic scenarios, this can't happen, since the sanitizer is always built by default with the loader. And this path is only exercised if sanitizer is explicitly enabled.
4 participants
The loader can be statically linked into more than one library of a process. Its state is hidden, so the linker cannot merge those copies, and the sanitizers ended up with two independent states over one device. Both reserved device shadow memory, and the second reservation failed with UR_RESULT_ERROR_OUT_OF_DEVICE_MEMORY.
Move the sanitizer layer into libur_sanitizer_layer.so, loaded on demand from the same locations an adapter is loaded from, so all loader instances of a process share one instance of it. Every loader still initializes the layer with its own ddi table; the layer reference counts those and hooks up the extra tables via context_t::interceptDdiTable instead of setting up a second state.
Assisted-by: Claude