Fix coverity issues in OpenCL, cuda and hip adapters.

aarongreig · aarongreig · commit 771b743165df · 2023-12-14T15:16:56.000Z
diff --git a/source/adapters/cuda/command_buffer.cpp b/source/adapters/cuda/command_buffer.cpp
@@ -21,8 +21,8 @@
 
 ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
     ur_context_handle_t hContext, ur_device_handle_t hDevice)
-    : Context(hContext),
-      Device(hDevice), CudaGraph{nullptr}, CudaGraphExec{nullptr}, RefCount{1} {
+    : Context(hContext), Device(hDevice), CudaGraph{nullptr},
+      CudaGraphExec{nullptr}, RefCount{1}, NextSyncPoint{0} {
   urContextRetain(hContext);
   urDeviceRetain(hDevice);
 }
diff --git a/source/adapters/cuda/command_buffer.hpp b/source/adapters/cuda/command_buffer.hpp
@@ -184,7 +184,7 @@ struct ur_exp_command_buffer_handle_t_ {
 
   void RegisterSyncPoint(ur_exp_command_buffer_sync_point_t SyncPoint,
                          std::shared_ptr<CUgraphNode> CuNode) {
-    SyncPoints[SyncPoint] = CuNode;
+    SyncPoints[SyncPoint] = std::move(CuNode);
     NextSyncPoint++;
   }
 
@@ -193,12 +193,12 @@ struct ur_exp_command_buffer_handle_t_ {
   }
 
   // Helper to register next sync point
-  // @param CuNode Node to register as next sycn point
+  // @param CuNode Node to register as next sync point
   // @return Pointer to the sync that registers the Node
   ur_exp_command_buffer_sync_point_t
   AddSyncPoint(std::shared_ptr<CUgraphNode> CuNode) {
     ur_exp_command_buffer_sync_point_t SyncPoint = NextSyncPoint;
-    RegisterSyncPoint(SyncPoint, CuNode);
+    RegisterSyncPoint(SyncPoint, std::move(CuNode));
     return SyncPoint;
   }
 
diff --git a/source/adapters/cuda/device.cpp b/source/adapters/cuda/device.cpp
@@ -1143,17 +1143,18 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceCreateWithNativeHandle(
   if (Result != UR_RESULT_SUCCESS)
     return Result;
 
-  ur_platform_handle_t *Plat = static_cast<ur_platform_handle_t *>(
-      malloc(NumPlatforms * sizeof(ur_platform_handle_t)));
-  Result = urPlatformGet(&AdapterHandle, 1, NumPlatforms, Plat, nullptr);
+  std::vector<ur_platform_handle_t> Platforms(NumPlatforms);
+
+  Result = urPlatformGet(&AdapterHandle, 1, Platforms.size(), Platforms.data(),
+                         nullptr);
   if (Result != UR_RESULT_SUCCESS)
     return Result;
 
   // Iterate through platforms to find device that matches nativeHandle
-  for (uint32_t j = 0; j < NumPlatforms; ++j) {
-    auto SearchRes =
-        std::find_if(begin(Plat[j]->Devices), end(Plat[j]->Devices), IsDevice);
-    if (SearchRes != end(Plat[j]->Devices)) {
+  for (auto Platform : Platforms) {
+    auto SearchRes = std::find_if(begin(Platform->Devices),
+                                  end(Platform->Devices), IsDevice);
+    if (SearchRes != end(Platform->Devices)) {
       *phDevice = static_cast<ur_device_handle_t>((*SearchRes).get());
       return UR_RESULT_SUCCESS;
     }
diff --git a/source/adapters/cuda/event.cpp b/source/adapters/cuda/event.cpp
@@ -9,7 +9,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "event.hpp"
-#include "common.hpp"
 #include "context.hpp"
 #include "device.hpp"
 #include "queue.hpp"
@@ -19,35 +18,25 @@
 
 ur_event_handle_t_::ur_event_handle_t_(ur_command_t Type,
                                        ur_context_handle_t Context,
-                                       ur_queue_handle_t Queue, CUstream Stream,
+                                       ur_queue_handle_t Queue,
+                                       native_type EvEnd, native_type EvQueued,
+                                       native_type EvStart, CUstream Stream,
                                        uint32_t StreamToken)
     : CommandType{Type}, RefCount{1}, HasOwnership{true},
       HasBeenWaitedOn{false}, IsRecorded{false}, IsStarted{false},
-      StreamToken{StreamToken}, EvEnd{nullptr}, EvStart{nullptr},
-      EvQueued{nullptr}, Queue{Queue}, Stream{Stream}, Context{Context} {
-
-  bool ProfilingEnabled = Queue->URFlags & UR_QUEUE_FLAG_PROFILING_ENABLE;
-
-  UR_CHECK_ERROR(cuEventCreate(
-      &EvEnd, ProfilingEnabled ? CU_EVENT_DEFAULT : CU_EVENT_DISABLE_TIMING));
-
-  if (ProfilingEnabled) {
-    UR_CHECK_ERROR(cuEventCreate(&EvQueued, CU_EVENT_DEFAULT));
-    UR_CHECK_ERROR(cuEventCreate(&EvStart, CU_EVENT_DEFAULT));
-  }
-
-  if (Queue != nullptr) {
-    urQueueRetain(Queue);
-  }
+      StreamToken{StreamToken}, EventID{0}, EvEnd{EvEnd}, EvStart{EvStart},
+      EvQueued{EvQueued}, Queue{Queue}, Stream{Stream}, Context{Context} {
+  urQueueRetain(Queue);
   urContextRetain(Context);
 }
 
 ur_event_handle_t_::ur_event_handle_t_(ur_context_handle_t Context,
                                        CUevent EventNative)
     : CommandType{UR_COMMAND_EVENTS_WAIT}, RefCount{1}, HasOwnership{false},
       HasBeenWaitedOn{false}, IsRecorded{false}, IsStarted{false},
-      StreamToken{std::numeric_limits<uint32_t>::max()}, EvEnd{EventNative},
-      EvStart{nullptr}, EvQueued{nullptr}, Queue{nullptr}, Context{Context} {
+      StreamToken{std::numeric_limits<uint32_t>::max()}, EventID{0},
+      EvEnd{EventNative}, EvStart{nullptr}, EvQueued{nullptr}, Queue{nullptr},
+      Stream{nullptr}, Context{Context} {
   urContextRetain(Context);
 }
 
diff --git a/source/adapters/cuda/event.hpp b/source/adapters/cuda/event.hpp
@@ -12,6 +12,7 @@
 #include <cuda.h>
 #include <ur/ur.hpp>
 
+#include "common.hpp"
 #include "queue.hpp"
 
 /// UR Event mapping to CUevent
@@ -82,8 +83,17 @@ struct ur_event_handle_t_ {
   static ur_event_handle_t
   makeNative(ur_command_t Type, ur_queue_handle_t Queue, CUstream Stream,
              uint32_t StreamToken = std::numeric_limits<uint32_t>::max()) {
-    return new ur_event_handle_t_(Type, Queue->getContext(), Queue, Stream,
-                                  StreamToken);
+    bool ProfilingEnabled = Queue->URFlags & UR_QUEUE_FLAG_PROFILING_ENABLE;
+    native_type EvEnd = nullptr, EvQueued = nullptr, EvStart = nullptr;
+    UR_CHECK_ERROR(cuEventCreate(
+        &EvEnd, ProfilingEnabled ? CU_EVENT_DEFAULT : CU_EVENT_DISABLE_TIMING));
+
+    if (ProfilingEnabled) {
+      UR_CHECK_ERROR(cuEventCreate(&EvQueued, CU_EVENT_DEFAULT));
+      UR_CHECK_ERROR(cuEventCreate(&EvStart, CU_EVENT_DEFAULT));
+    }
+    return new ur_event_handle_t_(Type, Queue->getContext(), Queue, EvEnd,
+                                  EvQueued, EvStart, Stream, StreamToken);
   }
 
   static ur_event_handle_t makeWithNative(ur_context_handle_t context,
@@ -99,7 +109,8 @@ struct ur_event_handle_t_ {
   // This constructor is private to force programmers to use the makeNative /
   // make_user static members in order to create a pi_event for CUDA.
   ur_event_handle_t_(ur_command_t Type, ur_context_handle_t Context,
-                     ur_queue_handle_t Queue, CUstream Stream,
+                     ur_queue_handle_t Queue, native_type EvEnd,
+                     native_type EvQueued, native_type EvStart, CUstream Stream,
                      uint32_t StreamToken);
 
   // This constructor is private to force programmers to use the
diff --git a/source/adapters/cuda/image.cpp b/source/adapters/cuda/image.cpp
@@ -234,7 +234,7 @@ cudaToUrImageChannelFormat(CUarray_format cuda_format,
 
 ur_result_t urTextureCreate(ur_sampler_handle_t hSampler,
                             const ur_image_desc_t *pImageDesc,
-                            CUDA_RESOURCE_DESC ResourceDesc,
+                            const CUDA_RESOURCE_DESC &ResourceDesc,
                             ur_exp_image_handle_t *phRetImage) {
 
   try {
diff --git a/source/adapters/cuda/memory.hpp b/source/adapters/cuda/memory.hpp
@@ -190,15 +190,15 @@ struct ur_mem_handle_t_ {
   /// Constructs the UR allocation for an unsampled image object
   ur_mem_handle_t_(ur_context_handle_t Context, CUarray Array,
                    CUsurfObject Surf, ur_mem_type_t ImageType)
-      : Context{Context}, RefCount{1}, MemType{Type::Surface},
+      : Context{Context}, RefCount{1}, MemType{Type::Surface}, MemFlags{0},
         Mem{ImageMem{Array, (void *)Surf, ImageType, nullptr}} {
     urContextRetain(Context);
   }
 
   /// Constructs the UR allocation for a sampled image object
   ur_mem_handle_t_(ur_context_handle_t Context, CUarray Array, CUtexObject Tex,
                    ur_sampler_handle_t Sampler, ur_mem_type_t ImageType)
-      : Context{Context}, RefCount{1}, MemType{Type::Texture},
+      : Context{Context}, RefCount{1}, MemType{Type::Texture}, MemFlags{0},
         Mem{ImageMem{Array, (void *)Tex, ImageType, Sampler}} {
     urContextRetain(Context);
   }
diff --git a/source/adapters/cuda/program.cpp b/source/adapters/cuda/program.cpp
@@ -137,7 +137,7 @@ ur_result_t ur_program_handle_t_::buildProgram(const char *BuildOptions) {
 
   if (!this->BuildOptions.empty()) {
     unsigned int MaxRegs;
-    bool Valid = getMaxRegistersJitOptionValue(BuildOptions, MaxRegs);
+    bool Valid = getMaxRegistersJitOptionValue(this->BuildOptions, MaxRegs);
     if (Valid) {
       Options.push_back(CU_JIT_MAX_REGISTERS);
       OptionVals.push_back(reinterpret_cast<void *>(MaxRegs));
diff --git a/source/adapters/cuda/sampler.cpp b/source/adapters/cuda/sampler.cpp
@@ -17,7 +17,7 @@ urSamplerCreate(ur_context_handle_t hContext, const ur_sampler_desc_t *pDesc,
   std::unique_ptr<ur_sampler_handle_t_> Sampler{
       new ur_sampler_handle_t_(hContext)};
 
-  if (pDesc && pDesc->stype == UR_STRUCTURE_TYPE_SAMPLER_DESC) {
+  if (pDesc->stype == UR_STRUCTURE_TYPE_SAMPLER_DESC) {
     Sampler->Props |= pDesc->normalizedCoords;
     Sampler->Props |= pDesc->filterMode << 1;
     Sampler->Props |= pDesc->addressingMode << 2;
diff --git a/source/adapters/hip/device.hpp b/source/adapters/hip/device.hpp
@@ -33,7 +33,7 @@ struct ur_device_handle_t_ {
       : HIPDevice(HipDevice), RefCount{1}, Platform(Platform),
         HIPContext(Context), DeviceIndex(DeviceIndex) {}
 
-  ~ur_device_handle_t_() {
+  ~ur_device_handle_t_() noexcept(false) {
     UR_CHECK_ERROR(hipDevicePrimaryCtxRelease(HIPDevice));
   }
 
diff --git a/source/adapters/hip/enqueue.cpp b/source/adapters/hip/enqueue.cpp
@@ -987,8 +987,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageRead(
     const ur_event_handle_t *phEventWaitList, ur_event_handle_t *phEvent) {
   UR_ASSERT(hImage->isImage(), UR_RESULT_ERROR_INVALID_MEM_OBJECT);
 
-  ur_result_t Result = UR_RESULT_SUCCESS;
-
   ur_lock MemoryMigrationLock{hImage->MemoryMigrationMutex};
   auto Device = hQueue->getDevice();
   hipStream_t HIPStream = hQueue->getNextTransferStream();
@@ -1039,13 +1037,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageRead(
       UR_CHECK_ERROR(RetImplEvent->start());
     }
 
-    Result = commonEnqueueMemImageNDCopy(HIPStream, ImgType, AdjustedRegion,
-                                         Array, hipMemoryTypeArray, SrcOffset,
-                                         pDst, hipMemoryTypeHost, nullptr);
-
-    if (Result != UR_RESULT_SUCCESS) {
-      return Result;
-    }
+    UR_CHECK_ERROR(commonEnqueueMemImageNDCopy(
+        HIPStream, ImgType, AdjustedRegion, Array, hipMemoryTypeArray,
+        SrcOffset, pDst, hipMemoryTypeHost, nullptr));
 
     if (phEvent) {
       UR_CHECK_ERROR(RetImplEvent->record());
@@ -1061,7 +1055,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageRead(
     return UR_RESULT_ERROR_UNKNOWN;
   }
   return UR_RESULT_SUCCESS;
-  return Result;
 }
 
 UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageWrite(
@@ -1071,15 +1064,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageWrite(
     const ur_event_handle_t *phEventWaitList, ur_event_handle_t *phEvent) {
   UR_ASSERT(hImage->isImage(), UR_RESULT_ERROR_INVALID_MEM_OBJECT);
 
-  ur_result_t Result = UR_RESULT_SUCCESS;
-
   try {
     ScopedContext Active(hQueue->getDevice());
     hipStream_t HIPStream = hQueue->getNextTransferStream();
 
     if (phEventWaitList) {
-      Result = enqueueEventsWait(hQueue, HIPStream, numEventsInWaitList,
-                                 phEventWaitList);
+      UR_CHECK_ERROR(enqueueEventsWait(hQueue, HIPStream, numEventsInWaitList,
+                                       phEventWaitList));
     }
 
     hipArray *Array =
@@ -1107,13 +1098,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageWrite(
       UR_CHECK_ERROR(RetImplEvent->start());
     }
 
-    Result = commonEnqueueMemImageNDCopy(HIPStream, ImgType, AdjustedRegion,
-                                         pSrc, hipMemoryTypeHost, nullptr,
-                                         Array, hipMemoryTypeArray, DstOffset);
-
-    if (Result != UR_RESULT_SUCCESS) {
-      return Result;
-    }
+    UR_CHECK_ERROR(commonEnqueueMemImageNDCopy(
+        HIPStream, ImgType, AdjustedRegion, pSrc, hipMemoryTypeHost, nullptr,
+        Array, hipMemoryTypeArray, DstOffset));
 
     if (phEvent) {
       UR_CHECK_ERROR(RetImplEvent->record());
@@ -1126,8 +1113,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageWrite(
   }
 
   return UR_RESULT_SUCCESS;
-
-  return Result;
 }
 
 UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageCopy(
diff --git a/source/adapters/hip/event.cpp b/source/adapters/hip/event.cpp
@@ -19,7 +19,7 @@ ur_event_handle_t_::ur_event_handle_t_(ur_command_t Type,
                                        hipStream_t Stream, uint32_t StreamToken)
     : CommandType{Type}, RefCount{1}, HasOwnership{true},
       HasBeenWaitedOn{false}, IsRecorded{false}, IsStarted{false},
-      StreamToken{StreamToken}, EvEnd{nullptr}, EvStart{nullptr},
+      StreamToken{StreamToken}, EventId{0}, EvEnd{nullptr}, EvStart{nullptr},
       EvQueued{nullptr}, Queue{Queue}, Stream{Stream}, Context{Context} {
 
   bool ProfilingEnabled = Queue->URFlags & UR_QUEUE_FLAG_PROFILING_ENABLE;
@@ -32,18 +32,17 @@ ur_event_handle_t_::ur_event_handle_t_(ur_command_t Type,
     UR_CHECK_ERROR(hipEventCreateWithFlags(&EvStart, hipEventDefault));
   }
 
-  if (Queue != nullptr) {
-    urQueueRetain(Queue);
-  }
+  urQueueRetain(Queue);
   urContextRetain(Context);
 }
 
 ur_event_handle_t_::ur_event_handle_t_(ur_context_handle_t Context,
                                        hipEvent_t EventNative)
     : CommandType{UR_COMMAND_EVENTS_WAIT}, RefCount{1}, HasOwnership{false},
       HasBeenWaitedOn{false}, IsRecorded{false}, IsStarted{false},
-      StreamToken{std::numeric_limits<uint32_t>::max()}, EvEnd{EventNative},
-      EvStart{nullptr}, EvQueued{nullptr}, Queue{nullptr}, Context{Context} {
+      StreamToken{std::numeric_limits<uint32_t>::max()}, EventId{0},
+      EvEnd{EventNative}, EvStart{nullptr}, EvQueued{nullptr}, Queue{nullptr},
+      Stream{nullptr}, Context{Context} {
   urContextRetain(Context);
 }
 
@@ -72,7 +71,7 @@ ur_result_t ur_event_handle_t_::start() {
   return Result;
 }
 
-bool ur_event_handle_t_::isCompleted() const noexcept {
+bool ur_event_handle_t_::isCompleted() const {
   if (!IsRecorded) {
     return false;
   }
@@ -225,8 +224,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventGetInfo(ur_event_handle_t hEvent,
     return ReturnValue(hEvent->getCommandType());
   case UR_EVENT_INFO_REFERENCE_COUNT:
     return ReturnValue(hEvent->getReferenceCount());
-  case UR_EVENT_INFO_COMMAND_EXECUTION_STATUS:
-    return ReturnValue(hEvent->getExecutionStatus());
+  case UR_EVENT_INFO_COMMAND_EXECUTION_STATUS: {
+    try {
+      return ReturnValue(hEvent->getExecutionStatus());
+    } catch (ur_result_t Error) {
+      return Error;
+    }
+  }
   case UR_EVENT_INFO_CONTEXT:
     return ReturnValue(hEvent->getContext());
   default:
diff --git a/source/adapters/hip/event.hpp b/source/adapters/hip/event.hpp
@@ -42,10 +42,9 @@ struct ur_event_handle_t_ {
 
   bool isStarted() const noexcept { return IsStarted; }
 
-  bool isCompleted() const noexcept;
-
-  uint32_t getExecutionStatus() const noexcept {
+  bool isCompleted() const;
 
+  uint32_t getExecutionStatus() const {
     if (!isRecorded()) {
       return UR_EVENT_STATUS_SUBMITTED;
     }
diff --git a/source/adapters/hip/memory.hpp b/source/adapters/hip/memory.hpp
@@ -187,20 +187,16 @@ struct SurfaceMem {
              void *HostPtr)
       : Arrays(Context->Devices.size(), nullptr),
         SurfObjs(Context->Devices.size(), nullptr),
-        OuterMemStruct{OuterMemStruct},
-        ImageFormat{ImageFormat}, ImageDesc{ImageDesc}, HostPtr{HostPtr} {
+        OuterMemStruct{OuterMemStruct}, ImageFormat{ImageFormat},
+        ImageDesc{ImageDesc}, ArrayDesc{}, HostPtr{HostPtr} {
     // We have to use hipArray3DCreate, which has some caveats. The height and
     // depth parameters must be set to 0 produce 1D or 2D arrays. image_desc
     // gives a minimum value of 1, so we need to convert the answer.
     ArrayDesc.NumChannels = 4; // Only support 4 channel image
     ArrayDesc.Flags = 0;       // No flags required
     ArrayDesc.Width = ImageDesc.width;
-    if (ImageDesc.type == UR_MEM_TYPE_IMAGE1D) {
-      ArrayDesc.Height = 0;
-      ArrayDesc.Depth = 0;
-    } else if (ImageDesc.type == UR_MEM_TYPE_IMAGE2D) {
+    if (ImageDesc.type == UR_MEM_TYPE_IMAGE2D) {
       ArrayDesc.Height = ImageDesc.height;
-      ArrayDesc.Depth = 0;
     } else if (ImageDesc.type == UR_MEM_TYPE_IMAGE3D) {
       ArrayDesc.Height = ImageDesc.height;
       ArrayDesc.Depth = ImageDesc.depth;
@@ -456,7 +452,7 @@ struct ur_mem_handle_t_ {
     urContextRetain(Context);
   }
 
-  ~ur_mem_handle_t_() {
+  ~ur_mem_handle_t_() noexcept(false) {
     if (isBuffer() && isSubBuffer()) {
       urMemRelease(std::get<BufferMem>(Mem).Parent);
       return;
@@ -468,7 +464,7 @@ struct ur_mem_handle_t_ {
     return std::holds_alternative<BufferMem>(Mem);
   }
 
-  bool isSubBuffer() const noexcept {
+  bool isSubBuffer() const {
     return (isBuffer() && (std::get<BufferMem>(Mem).Parent != nullptr));
   }
 
diff --git a/source/adapters/hip/program.cpp b/source/adapters/hip/program.cpp
diff --git a/source/adapters/hip/sampler.cpp b/source/adapters/hip/sampler.cpp
diff --git a/source/adapters/opencl/program.cpp b/source/adapters/opencl/program.cpp

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ struct ur_exp_command_buffer_handle_t_ {`
`184`	`184`
`185`	`185`	`void RegisterSyncPoint(ur_exp_command_buffer_sync_point_t SyncPoint,`
`186`	`186`	`std::shared_ptr<CUgraphNode> CuNode) {`
`187`		`- SyncPoints[SyncPoint] = CuNode;`
	`187`	`+ SyncPoints[SyncPoint] = std::move(CuNode);`
`188`	`188`	`NextSyncPoint++;`
`189`	`189`	`}`
`190`	`190`
`@@ -193,12 +193,12 @@ struct ur_exp_command_buffer_handle_t_ {`
`193`	`193`	`}`
`194`	`194`
`195`	`195`	`// Helper to register next sync point`
`196`		`- // @param CuNode Node to register as next sycn point`
	`196`	`+ // @param CuNode Node to register as next sync point`
`197`	`197`	`// @return Pointer to the sync that registers the Node`
`198`	`198`	`ur_exp_command_buffer_sync_point_t`
`199`	`199`	`AddSyncPoint(std::shared_ptr<CUgraphNode> CuNode) {`
`200`	`200`	`ur_exp_command_buffer_sync_point_t SyncPoint = NextSyncPoint;`
`201`		`- RegisterSyncPoint(SyncPoint, CuNode);`
	`201`	`+ RegisterSyncPoint(SyncPoint, std::move(CuNode));`
`202`	`202`	`return SyncPoint;`
`203`	`203`	`}`
`204`	`204`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ struct ur_device_handle_t_ {`
`33`	`33`	`: HIPDevice(HipDevice), RefCount{1}, Platform(Platform),`
`34`	`34`	`HIPContext(Context), DeviceIndex(DeviceIndex) {}`
`35`	`35`
`36`		`- ~ur_device_handle_t_() {`
	`36`	`+ ~ur_device_handle_t_() noexcept(false) {`
`37`	`37`	`UR_CHECK_ERROR(hipDevicePrimaryCtxRelease(HIPDevice));`
`38`	`38`	`}`
`39`	`39`