Skip to content

Commit

Permalink
[SYCL] Fix uninitialized fields Coverity hits (#15237)
Browse files Browse the repository at this point in the history
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!
  • Loading branch information
ianayl committed Sep 6, 2024
1 parent 0a2e5e3 commit 0546dc1
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 7 deletions.
2 changes: 1 addition & 1 deletion sycl/source/detail/event_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion sycl/source/detail/queue_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,9 @@ class queue_impl {
MIsInorder(has_property<property::queue::in_order>()),
MDiscardEvents(
has_property<ext::oneapi::property::queue::discard_events>()),
MIsProfilingEnabled(has_property<property::queue::enable_profiling>()) {
MIsProfilingEnabled(has_property<property::queue::enable_profiling>()),
MQueueID{
MNextAvailableQueueID.fetch_add(1, std::memory_order_relaxed)} {
queue_impl_interop(UrQueue);
}

Expand Down
4 changes: 2 additions & 2 deletions sycl/source/detail/scheduler/commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions sycl/source/detail/scheduler/commands.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<EnqueueResultT::ResultT> MEnqueueStatus;
Expand Down
2 changes: 1 addition & 1 deletion sycl/source/detail/sycl_mem_obj_t.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 0546dc1

Please sign in to comment.