From fad405cdfc39712fe2ebf2eb4dea54255ffa8347 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Tue, 3 Sep 2024 17:41:24 -0700 Subject: [PATCH] [SYCL] Fix uncaught exceptions and null dereference (#15173) Added proper handling of exceptions propagated to the outermost level. --------- Co-authored-by: Dmitry Vodopyanov --- sycl/source/detail/device_image_impl.hpp | 25 ++++++++++++++++-------- sycl/source/detail/global_handler.cpp | 13 +++++++++--- sycl/source/detail/graph_impl.cpp | 7 ++++--- sycl/source/detail/image_impl.hpp | 7 ++++++- sycl/source/event.cpp | 7 ++++++- 5 files changed, 43 insertions(+), 16 deletions(-) diff --git a/sycl/source/detail/device_image_impl.hpp b/sycl/source/detail/device_image_impl.hpp index e54fc0b2976c..0786457d3a7c 100644 --- a/sycl/source/detail/device_image_impl.hpp +++ b/sycl/source/detail/device_image_impl.hpp @@ -95,10 +95,14 @@ class device_image_impl { // Otherwise, if the device candidate is a sub-device it is also valid if // its parent is valid. - if (!getSyclObjImpl(DeviceCand)->isRootDevice()) - return has_kernel(KernelIDCand, - DeviceCand.get_info()); - + if (!getSyclObjImpl(DeviceCand)->isRootDevice()) { + try { + return has_kernel(KernelIDCand, + DeviceCand.get_info()); + } catch (std::exception &e) { + __SYCL_REPORT_EXCEPTION_TO_STREAM("exception in has_kernel", e); + } + } return false; } @@ -270,10 +274,15 @@ class device_image_impl { // TODO consider changing the lifetime of device_image_impl instead ur_buffer_properties_t Properties = {UR_STRUCTURE_TYPE_BUFFER_PROPERTIES, nullptr, MSpecConstsBlob.data()}; - memBufferCreateHelper( - Plugin, detail::getSyclObjImpl(MContext)->getHandleRef(), - UR_MEM_FLAG_READ_WRITE | UR_MEM_FLAG_ALLOC_COPY_HOST_POINTER, - MSpecConstsBlob.size(), &MSpecConstsBuffer, &Properties); + try { + memBufferCreateHelper( + Plugin, detail::getSyclObjImpl(MContext)->getHandleRef(), + UR_MEM_FLAG_READ_WRITE | UR_MEM_FLAG_ALLOC_COPY_HOST_POINTER, + MSpecConstsBlob.size(), &MSpecConstsBuffer, &Properties); + } catch (std::exception &e) { + __SYCL_REPORT_EXCEPTION_TO_STREAM( + "exception in get_spec_const_buffer_ref", e); + } } return MSpecConstsBuffer; } diff --git a/sycl/source/detail/global_handler.cpp b/sycl/source/detail/global_handler.cpp index 93979c33feb4..8376212984d7 100644 --- a/sycl/source/detail/global_handler.cpp +++ b/sycl/source/detail/global_handler.cpp @@ -355,8 +355,14 @@ void shutdown_late() { extern "C" __SYCL_EXPORT BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpReserved) { - bool PrintUrTrace = - sycl::detail::ur::trace(sycl::detail::ur::TraceLevel::TRACE_CALLS); + bool PrintUrTrace = false; + try { + PrintUrTrace = + sycl::detail::ur::trace(sycl::detail::ur::TraceLevel::TRACE_CALLS); + } catch (std::exception &e) { + __SYCL_REPORT_EXCEPTION_TO_STREAM("exception in DllMain", e); + return FALSE; + } // Perform actions based on the reason for calling. switch (fdwReason) { @@ -367,7 +373,8 @@ extern "C" __SYCL_EXPORT BOOL WINAPI DllMain(HINSTANCE hinstDLL, #ifdef XPTI_ENABLE_INSTRUMENTATION if (xptiTraceEnabled()) return TRUE; // When doing xpti tracing, we can't safely call shutdown. - // TODO: figure out what XPTI is doing that prevents release. + // TODO: figure out what XPTI is doing that prevents + // release. #endif shutdown_win(); diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index dc93f74cd7c4..9ee662d27715 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -1625,16 +1625,17 @@ void modifiable_command_graph::end_recording() { void modifiable_command_graph::end_recording(queue &RecordingQueue) { auto QueueImpl = sycl::detail::getSyclObjImpl(RecordingQueue); - if (QueueImpl && QueueImpl->getCommandGraph() == impl) { + if (!QueueImpl) + return; + if (QueueImpl->getCommandGraph() == impl) { QueueImpl->setCommandGraph(nullptr); graph_impl::WriteLock Lock(impl->MMutex); impl->removeQueue(QueueImpl); } - if (QueueImpl->getCommandGraph() != nullptr) { + if (QueueImpl->getCommandGraph() != nullptr) throw sycl::exception(sycl::make_error_code(errc::invalid), "end_recording called for a queue which is recording " "to a different graph."); - } } void modifiable_command_graph::end_recording( diff --git a/sycl/source/detail/image_impl.hpp b/sycl/source/detail/image_impl.hpp index 008d86df2799..5ee3b473aac8 100644 --- a/sycl/source/detail/image_impl.hpp +++ b/sycl/source/detail/image_impl.hpp @@ -238,7 +238,12 @@ class image_impl final : public SYCLMemObjT { // Returns the total number of elements in the image size_t get_count() const { return size(); } - size_t size() const noexcept { return MRange.size(); } + size_t size() const noexcept try { + return MRange.size(); + } catch (std::exception &e) { + __SYCL_REPORT_EXCEPTION_TO_STREAM("exception in size", e); + std::abort(); + } void *allocateMem(ContextImplPtr Context, bool InitFromUserData, void *HostPtr, ur_event_handle_t &OutEventToWait) override; diff --git a/sycl/source/event.cpp b/sycl/source/event.cpp index 3f9048284aba..169829801460 100644 --- a/sycl/source/event.cpp +++ b/sycl/source/event.cpp @@ -117,7 +117,12 @@ event::get_profiling_info() const { #undef __SYCL_PARAM_TRAITS_SPEC -backend event::get_backend() const noexcept { return getImplBackend(impl); } +backend event::get_backend() const noexcept try { + return getImplBackend(impl); +} catch (std::exception &e) { + __SYCL_REPORT_EXCEPTION_TO_STREAM("exception in get_backend", e); + std::abort(); +} ur_native_handle_t event::getNative() const { return impl->getNative(); }