Skip to content

Commit

Permalink
[SYCL] Initialize uninitialized handler_impl fields (#15323)
Browse files Browse the repository at this point in the history
In the same vein as #15237, this PR
fixes additional uninitialized values recently discovered by Coverity.
Similar to the resolution discussed in #15237, I have
default-initialized integer values that are defined later on to 0
instead of another more complex solution.

Additionally, since I had set `MExternalSempahore` in `handler_impl` to
`nullptr`, I added null checks where `MExternalSemaphore` is ultimately
returned to ensure `nullptr` doesn't actually get passed into the UR.
This is not necessarily necessary, but without this check Coverity would
probably generate another hit because of it.
  • Loading branch information
ianayl committed Sep 10, 2024
1 parent fbb1fb0 commit 178a42c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
6 changes: 6 additions & 0 deletions sycl/source/detail/cg.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,8 @@ class CGSemaphoreWait : public CG {
MExternalSemaphore(ExternalSemaphore), MWaitValue(WaitValue) {}

ur_exp_external_semaphore_handle_t getExternalSemaphore() const {
assert(MExternalSemaphore != nullptr &&
"MExternalSemaphore has not been defined yet.");
return MExternalSemaphore;
}
std::optional<uint64_t> getWaitValue() const { return MWaitValue; }
Expand All @@ -643,6 +645,10 @@ class CGSemaphoreSignal : public CG {
MExternalSemaphore(ExternalSemaphore), MSignalValue(SignalValue) {}

ur_exp_external_semaphore_handle_t getExternalSemaphore() const {
if (MExternalSemaphore == nullptr)
throw exception(make_error_code(errc::runtime),
"getExternalSemaphore(): MExternalSemaphore has not been "
"defined yet.");
return MExternalSemaphore;
}
std::optional<uint64_t> getSignalValue() const { return MSignalValue; }
Expand Down
12 changes: 6 additions & 6 deletions sycl/source/detail/handler_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ class handler_impl {

std::shared_ptr<detail::kernel_bundle_impl> MKernelBundle;

ur_usm_advice_flags_t MAdvice;
ur_usm_advice_flags_t MAdvice = 0;

// 2D memory operation information.
size_t MSrcPitch;
size_t MDstPitch;
size_t MWidth;
size_t MHeight;
size_t MSrcPitch = 0;
size_t MDstPitch = 0;
size_t MWidth = 0;
size_t MHeight = 0;

/// Offset into a device_global for copy operations.
size_t MOffset = 0;
Expand Down Expand Up @@ -134,7 +134,7 @@ class handler_impl {
ur_rect_region_t MCopyExtent = {};

// Extra information for semaphore interoperability
ur_exp_external_semaphore_handle_t MExternalSemaphore;
ur_exp_external_semaphore_handle_t MExternalSemaphore = nullptr;
std::optional<uint64_t> MWaitValue;
std::optional<uint64_t> MSignalValue;

Expand Down

0 comments on commit 178a42c

Please sign in to comment.