From 0546dc14f57ecd5bdc35a87458d146a993625809 Mon Sep 17 00:00:00 2001 From: Ian Li Date: Fri, 6 Sep 2024 11:42:12 -0400 Subject: [PATCH] [SYCL] Fix uninitialized fields Coverity hits (#15237) This PR fixes Coverity hits regarding uninitialized class fields in the runtime. I'd like to bring attention to `sycl_mem_obj_t.hpp` however: There, I have initialized `MSizeInBytes` of the `SYCLMemObjT` class to 0: This should not cause any problems (at least not more), as currently all subclasses of `SYCLMemObjT` that actually use the `MSizeInBytes` have it defined (`buffer_impl`, `image_impl`) when their respective constructors are called. However, this does mean programmers must remember to initialize `MSizeInBytes` when using `image_impl`. To avoid this, I could rewrite some of the constructors in e.g. `image_impl` and `SYCLMemObjT`, but I'd like to not overcomplicate the problem here. So I was hoping for some other opinions: Is initializing as 0 sufficient, or should I go ahead and make the changes to the constructors anyway to be safe? Thanks in advance! --- sycl/source/detail/event_impl.hpp | 2 +- sycl/source/detail/queue_impl.hpp | 4 +++- sycl/source/detail/scheduler/commands.cpp | 4 ++-- sycl/source/detail/scheduler/commands.hpp | 4 ++-- sycl/source/detail/sycl_mem_obj_t.hpp | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sycl/source/detail/event_impl.hpp b/sycl/source/detail/event_impl.hpp index 07c33acb2b054..b560d721728a6 100644 --- a/sycl/source/detail/event_impl.hpp +++ b/sycl/source/detail/event_impl.hpp @@ -390,7 +390,7 @@ class event_impl { // If this event represents a submission to a // ur_exp_command_buffer_sync_point_t the sync point for that submission is // stored here. - ur_exp_command_buffer_sync_point_t MSyncPoint; + ur_exp_command_buffer_sync_point_t MSyncPoint = 0; // If this event represents a submission to a // ur_exp_command_buffer_command_handle_t the command-buffer command diff --git a/sycl/source/detail/queue_impl.hpp b/sycl/source/detail/queue_impl.hpp index f1cf8dbd9a32f..e15e9bc69503e 100644 --- a/sycl/source/detail/queue_impl.hpp +++ b/sycl/source/detail/queue_impl.hpp @@ -239,7 +239,9 @@ class queue_impl { MIsInorder(has_property()), MDiscardEvents( has_property()), - MIsProfilingEnabled(has_property()) { + MIsProfilingEnabled(has_property()), + MQueueID{ + MNextAvailableQueueID.fetch_add(1, std::memory_order_relaxed)} { queue_impl_interop(UrQueue); } diff --git a/sycl/source/detail/scheduler/commands.cpp b/sycl/source/detail/scheduler/commands.cpp index c9edad0cbb2fe..953ad2bee0444 100644 --- a/sycl/source/detail/scheduler/commands.cpp +++ b/sycl/source/detail/scheduler/commands.cpp @@ -994,9 +994,9 @@ const char *Command::getBlockReason() const { return "A Buffer is locked by the host accessor"; case BlockReason::HostTask: return "Blocked by host task"; + default: + return "Unknown block reason"; } - - return "Unknown block reason"; } void Command::copySubmissionCodeLocation() { diff --git a/sycl/source/detail/scheduler/commands.hpp b/sycl/source/detail/scheduler/commands.hpp index 40d52f1059c15..17b285068dc4d 100644 --- a/sycl/source/detail/scheduler/commands.hpp +++ b/sycl/source/detail/scheduler/commands.hpp @@ -321,10 +321,10 @@ class Command { /// Used for marking the node during graph traversal. Marks MMarks; - enum class BlockReason : int { HostAccessor = 0, HostTask }; + enum class BlockReason : int { Unset = -1, HostAccessor = 0, HostTask }; // Only have reasonable value while MIsBlockable is true - BlockReason MBlockReason; + BlockReason MBlockReason = BlockReason::Unset; /// Describes the status of the command. std::atomic MEnqueueStatus; diff --git a/sycl/source/detail/sycl_mem_obj_t.hpp b/sycl/source/detail/sycl_mem_obj_t.hpp index de400e85267b0..358d7dcc7d214 100644 --- a/sycl/source/detail/sycl_mem_obj_t.hpp +++ b/sycl/source/detail/sycl_mem_obj_t.hpp @@ -353,7 +353,7 @@ class SYCLMemObjT : public SYCLMemObjI { // Indicates if memory object should write memory to the host on destruction. bool MNeedWriteBack; // Size of memory. - size_t MSizeInBytes; + size_t MSizeInBytes = 0; // User's pointer passed to constructor. void *MUserPtr; // Copy of memory passed by user to constructor.