Skip to content

Commit f8ed45f

Browse files
npmillerkbenzie
npmiller
authored and
kbenzie
committed
Fix error handling in bindless images code (#18246)
`UR_CHECK_ERROR` throws an exception, so it must only be used in `try/catch` blocks. This patch fixes the uses of `UR_CHECK_ERROR` in the image code with a mix of adding missing `try/catch` blocks and replacing it with `UR_CALL`. Also skips checks in a catch block trying to cleanup resources before returning an error, we shouldn't re-throw from that and returning the initial error is the more helpful thing to do.
1 parent 3d66cc8 commit f8ed45f

2 files changed

Lines changed: 98 additions & 70 deletions

File tree

‎source/adapters/cuda/image.cpp‎

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ urToCudaImageChannelFormat(ur_image_channel_type_t image_channel_type,
7878
size_t pixel_size_bytes = 0;
7979
unsigned int num_channels = 0;
8080
unsigned int normalized_dtype_flag = 0;
81-
UR_CHECK_ERROR(urCalculateNumChannels(image_channel_order, &num_channels));
81+
UR_CALL(urCalculateNumChannels(image_channel_order, &num_channels));
8282

8383
switch (image_channel_type) {
8484
#define CASE(FROM, TO, SIZE, NORM) \
@@ -300,8 +300,14 @@ urBindlessImagesUnsampledImageHandleDestroyExp(
300300
hContext->getDevices().end(),
301301
hDevice) != hContext->getDevices().end(),
302302
UR_RESULT_ERROR_INVALID_CONTEXT);
303+
try {
304+
UR_CHECK_ERROR(cuSurfObjectDestroy((CUsurfObject)hImage));
305+
} catch (ur_result_t error) {
306+
return error;
307+
} catch (...) {
308+
return UR_RESULT_ERROR_UNKNOWN;
309+
}
303310

304-
UR_CHECK_ERROR(cuSurfObjectDestroy((CUsurfObject)hImage));
305311
return UR_RESULT_SUCCESS;
306312
}
307313

@@ -313,8 +319,14 @@ urBindlessImagesSampledImageHandleDestroyExp(
313319
hContext->getDevices().end(),
314320
hDevice) != hContext->getDevices().end(),
315321
UR_RESULT_ERROR_INVALID_CONTEXT);
322+
try {
323+
UR_CHECK_ERROR(cuTexObjectDestroy((CUtexObject)hImage));
324+
} catch (ur_result_t error) {
325+
return error;
326+
} catch (...) {
327+
return UR_RESULT_ERROR_UNKNOWN;
328+
}
316329

317-
UR_CHECK_ERROR(cuTexObjectDestroy((CUtexObject)hImage));
318330
return UR_RESULT_SUCCESS;
319331
}
320332

@@ -330,12 +342,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageAllocateExp(
330342
// Populate descriptor
331343
CUDA_ARRAY3D_DESCRIPTOR array_desc = {};
332344

333-
UR_CHECK_ERROR(urCalculateNumChannels(pImageFormat->channelOrder,
334-
&array_desc.NumChannels));
345+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder,
346+
&array_desc.NumChannels));
335347

336-
UR_CHECK_ERROR(urToCudaImageChannelFormat(
337-
pImageFormat->channelType, pImageFormat->channelOrder, &array_desc.Format,
338-
nullptr, nullptr));
348+
UR_CALL(urToCudaImageChannelFormat(pImageFormat->channelType,
349+
pImageFormat->channelOrder,
350+
&array_desc.Format, nullptr, nullptr));
339351

340352
array_desc.Flags = 0; // No flags required
341353
array_desc.Width = pImageDesc->width;
@@ -387,12 +399,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageAllocateExp(
387399
*phImageMem = (ur_exp_image_mem_native_handle_t)ImageArray;
388400
} catch (ur_result_t Err) {
389401
if (ImageArray != CUarray{}) {
390-
UR_CHECK_ERROR(cuArrayDestroy(ImageArray));
402+
(void)cuArrayDestroy(ImageArray);
391403
}
392404
return Err;
393405
} catch (...) {
394406
if (ImageArray != CUarray{}) {
395-
UR_CHECK_ERROR(cuArrayDestroy(ImageArray));
407+
(void)cuArrayDestroy(ImageArray);
396408
}
397409
return UR_RESULT_ERROR_UNKNOWN;
398410
}
@@ -407,12 +419,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageAllocateExp(
407419
*phImageMem = (ur_exp_image_mem_native_handle_t)mip_array;
408420
} catch (ur_result_t Err) {
409421
if (mip_array) {
410-
UR_CHECK_ERROR(cuMipmappedArrayDestroy(mip_array));
422+
(void)cuMipmappedArrayDestroy(mip_array);
411423
}
412424
return Err;
413425
} catch (...) {
414426
if (mip_array) {
415-
UR_CHECK_ERROR(cuMipmappedArrayDestroy(mip_array));
427+
(void)cuMipmappedArrayDestroy(mip_array);
416428
}
417429
return UR_RESULT_ERROR_UNKNOWN;
418430
}
@@ -457,14 +469,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesUnsampledImageCreateExp(
457469
UR_RESULT_ERROR_INVALID_CONTEXT);
458470

459471
unsigned int NumChannels = 0;
460-
UR_CHECK_ERROR(
461-
urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
472+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
462473

463474
CUarray_format format;
464475
size_t PixelSizeBytes;
465-
UR_CHECK_ERROR(urToCudaImageChannelFormat(pImageFormat->channelType,
466-
pImageFormat->channelOrder, &format,
467-
&PixelSizeBytes, nullptr));
476+
UR_CALL(urToCudaImageChannelFormat(pImageFormat->channelType,
477+
pImageFormat->channelOrder, &format,
478+
&PixelSizeBytes, nullptr));
468479

469480
try {
470481

@@ -504,15 +515,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesSampledImageCreateExp(
504515
ScopedContext Active(hDevice);
505516

506517
unsigned int NumChannels = 0;
507-
UR_CHECK_ERROR(
508-
urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
518+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
509519

510520
CUarray_format format;
511521
size_t PixelSizeBytes;
512522
unsigned int normalized_dtype_flag;
513-
UR_CHECK_ERROR(urToCudaImageChannelFormat(
514-
pImageFormat->channelType, pImageFormat->channelOrder, &format,
515-
&PixelSizeBytes, &normalized_dtype_flag));
523+
UR_CALL(urToCudaImageChannelFormat(pImageFormat->channelType,
524+
pImageFormat->channelOrder, &format,
525+
&PixelSizeBytes, &normalized_dtype_flag));
516526

517527
try {
518528
CUDA_RESOURCE_DESC image_res_desc = {};
@@ -598,14 +608,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageCopyExp(
598608
unsigned int NumChannels = 0;
599609
size_t PixelSizeBytes = 0;
600610

601-
UR_CHECK_ERROR(
602-
urCalculateNumChannels(pSrcImageFormat->channelOrder, &NumChannels));
611+
UR_CALL(urCalculateNumChannels(pSrcImageFormat->channelOrder, &NumChannels));
603612

604613
// We need to get this now in bytes for calculating the total image size
605614
// later.
606-
UR_CHECK_ERROR(urToCudaImageChannelFormat(pSrcImageFormat->channelType,
607-
pSrcImageFormat->channelOrder,
608-
nullptr, &PixelSizeBytes, nullptr));
615+
UR_CALL(urToCudaImageChannelFormat(pSrcImageFormat->channelType,
616+
pSrcImageFormat->channelOrder, nullptr,
617+
&PixelSizeBytes, nullptr));
609618

610619
try {
611620
ScopedContext Active(hQueue->getDevice());
@@ -945,7 +954,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageGetInfoExp(
945954
}
946955

947956
CUDA_ARRAY3D_DESCRIPTOR ArrayDesc;
948-
UR_CHECK_ERROR(cuArray3DGetDescriptor(&ArrayDesc, hCUarray));
957+
Err = cuArray3DGetDescriptor(&ArrayDesc, hCUarray);
958+
if (Err != CUDA_SUCCESS) {
959+
return mapErrorUR(Err);
960+
}
961+
949962
switch (propName) {
950963
case UR_IMAGE_INFO_WIDTH:
951964
if (pPropValue) {
@@ -974,7 +987,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageGetInfoExp(
974987
case UR_IMAGE_INFO_FORMAT:
975988
ur_image_channel_type_t ChannelType;
976989
ur_image_channel_order_t ChannelOrder;
977-
UR_CHECK_ERROR(cudaToUrImageChannelFormat(ArrayDesc.Format, &ChannelType));
990+
UR_CALL(cudaToUrImageChannelFormat(ArrayDesc.Format, &ChannelType));
978991
// CUDA does not have a notion of channel "order" in the same way that
979992
// SYCL 1.2.1 does.
980993
switch (ArrayDesc.NumChannels) {
@@ -1118,13 +1131,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesMapExternalArrayExp(
11181131
UR_RESULT_ERROR_INVALID_CONTEXT);
11191132

11201133
unsigned int NumChannels = 0;
1121-
UR_CHECK_ERROR(
1122-
urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
1134+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
11231135

11241136
CUarray_format format;
1125-
UR_CHECK_ERROR(urToCudaImageChannelFormat(pImageFormat->channelType,
1126-
pImageFormat->channelOrder, &format,
1127-
nullptr, nullptr));
1137+
UR_CALL(urToCudaImageChannelFormat(pImageFormat->channelType,
1138+
pImageFormat->channelOrder, &format,
1139+
nullptr, nullptr));
11281140

11291141
try {
11301142
ScopedContext Active(hDevice);

‎source/adapters/hip/image.cpp‎

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ urToHipImageChannelFormat(ur_image_channel_type_t image_channel_type,
7575
size_t pixel_size_bytes = 0;
7676
unsigned int num_channels = 0;
7777
unsigned int normalized_dtype_flag = 0;
78-
UR_CHECK_ERROR(urCalculateNumChannels(image_channel_order, &num_channels));
78+
UR_CALL(urCalculateNumChannels(image_channel_order, &num_channels));
7979

8080
switch (image_channel_type) {
8181
#define CASE(FROM, TO, SIZE, NORM) \
@@ -293,8 +293,15 @@ urBindlessImagesUnsampledImageHandleDestroyExp(
293293
hDevice) != hContext->getDevices().end(),
294294
UR_RESULT_ERROR_INVALID_CONTEXT);
295295

296-
UR_CHECK_ERROR(
297-
hipDestroySurfaceObject(reinterpret_cast<hipSurfaceObject_t>(hImage)));
296+
try {
297+
UR_CHECK_ERROR(
298+
hipDestroySurfaceObject(reinterpret_cast<hipSurfaceObject_t>(hImage)));
299+
} catch (ur_result_t error) {
300+
return error;
301+
} catch (...) {
302+
return UR_RESULT_ERROR_UNKNOWN;
303+
}
304+
298305
return UR_RESULT_SUCCESS;
299306
}
300307

@@ -306,9 +313,15 @@ urBindlessImagesSampledImageHandleDestroyExp(
306313
hContext->getDevices().end(),
307314
hDevice) != hContext->getDevices().end(),
308315
UR_RESULT_ERROR_INVALID_CONTEXT);
316+
try {
317+
UR_CHECK_ERROR(
318+
hipTexObjectDestroy(reinterpret_cast<hipTextureObject_t>(hImage)));
319+
} catch (ur_result_t error) {
320+
return error;
321+
} catch (...) {
322+
return UR_RESULT_ERROR_UNKNOWN;
323+
}
309324

310-
UR_CHECK_ERROR(
311-
hipTexObjectDestroy(reinterpret_cast<hipTextureObject_t>(hImage)));
312325
return UR_RESULT_SUCCESS;
313326
}
314327

@@ -324,12 +337,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageAllocateExp(
324337
// Populate descriptor
325338
HIP_ARRAY3D_DESCRIPTOR array_desc = {};
326339

327-
UR_CHECK_ERROR(urCalculateNumChannels(pImageFormat->channelOrder,
328-
&array_desc.NumChannels));
340+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder,
341+
&array_desc.NumChannels));
329342

330-
UR_CHECK_ERROR(urToHipImageChannelFormat(
331-
pImageFormat->channelType, pImageFormat->channelOrder, &array_desc.Format,
332-
nullptr, nullptr));
343+
UR_CALL(urToHipImageChannelFormat(pImageFormat->channelType,
344+
pImageFormat->channelOrder,
345+
&array_desc.Format, nullptr, nullptr));
333346

334347
array_desc.Flags = 0; // No flags required
335348
array_desc.Width = pImageDesc->width;
@@ -377,12 +390,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageAllocateExp(
377390
reinterpret_cast<ur_exp_image_mem_native_handle_t>(ImageArray);
378391
} catch (ur_result_t Err) {
379392
if (ImageArray) {
380-
UR_CHECK_ERROR(hipArrayDestroy(ImageArray));
393+
(void)hipArrayDestroy(ImageArray);
381394
}
382395
return Err;
383396
} catch (...) {
384397
if (ImageArray) {
385-
UR_CHECK_ERROR(hipArrayDestroy(ImageArray));
398+
(void)hipArrayDestroy(ImageArray);
386399
}
387400
return UR_RESULT_ERROR_UNKNOWN;
388401
}
@@ -398,12 +411,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageAllocateExp(
398411
reinterpret_cast<ur_exp_image_mem_native_handle_t>(mip_array);
399412
} catch (ur_result_t Err) {
400413
if (mip_array) {
401-
UR_CHECK_ERROR(hipMipmappedArrayDestroy(mip_array));
414+
(void)hipMipmappedArrayDestroy(mip_array);
402415
}
403416
return Err;
404417
} catch (...) {
405418
if (mip_array) {
406-
UR_CHECK_ERROR(hipMipmappedArrayDestroy(mip_array));
419+
(void)hipMipmappedArrayDestroy(mip_array);
407420
}
408421
return UR_RESULT_ERROR_UNKNOWN;
409422
}
@@ -444,14 +457,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesUnsampledImageCreateExp(
444457
UR_RESULT_ERROR_INVALID_CONTEXT);
445458

446459
unsigned int NumChannels = 0;
447-
UR_CHECK_ERROR(
448-
urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
460+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
449461

450462
hipArray_Format format;
451463
size_t PixelSizeBytes;
452-
UR_CHECK_ERROR(urToHipImageChannelFormat(pImageFormat->channelType,
453-
pImageFormat->channelOrder, &format,
454-
&PixelSizeBytes, nullptr));
464+
UR_CALL(urToHipImageChannelFormat(pImageFormat->channelType,
465+
pImageFormat->channelOrder, &format,
466+
&PixelSizeBytes, nullptr));
455467

456468
try {
457469

@@ -491,15 +503,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesSampledImageCreateExp(
491503
ScopedDevice Active(hDevice);
492504

493505
unsigned int NumChannels = 0;
494-
UR_CHECK_ERROR(
495-
urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
506+
UR_CALL(urCalculateNumChannels(pImageFormat->channelOrder, &NumChannels));
496507

497508
hipArray_Format format;
498509
size_t PixelSizeBytes;
499510
unsigned int normalized_dtype_flag;
500-
UR_CHECK_ERROR(urToHipImageChannelFormat(
501-
pImageFormat->channelType, pImageFormat->channelOrder, &format,
502-
&PixelSizeBytes, &normalized_dtype_flag));
511+
UR_CALL(urToHipImageChannelFormat(pImageFormat->channelType,
512+
pImageFormat->channelOrder, &format,
513+
&PixelSizeBytes, &normalized_dtype_flag));
503514

504515
try {
505516
HIP_RESOURCE_DESC image_res_desc = {};
@@ -583,14 +594,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageCopyExp(
583594
unsigned int NumChannels = 0;
584595
size_t PixelSizeBytes = 0;
585596

586-
UR_CHECK_ERROR(
587-
urCalculateNumChannels(pSrcImageFormat->channelOrder, &NumChannels));
597+
UR_CALL(urCalculateNumChannels(pSrcImageFormat->channelOrder, &NumChannels));
588598

589599
// We need to get this now in bytes for calculating the total image size
590600
// later.
591-
UR_CHECK_ERROR(urToHipImageChannelFormat(pSrcImageFormat->channelType,
592-
pSrcImageFormat->channelOrder,
593-
nullptr, &PixelSizeBytes, nullptr));
601+
UR_CALL(urToHipImageChannelFormat(pSrcImageFormat->channelType,
602+
pSrcImageFormat->channelOrder, nullptr,
603+
&PixelSizeBytes, nullptr));
594604

595605
try {
596606
ScopedDevice Active(hQueue->getDevice());
@@ -927,21 +937,27 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageGetInfoExp(
927937
// ROCm 5.6.0, so we can't query image array information for older versions.
928938
#if HIP_VERSION >= 50600000
929939
unsigned int memType{};
930-
UR_CHECK_ERROR(
940+
hipError_t Err =
931941
hipPointerGetAttribute(&memType, HIP_POINTER_ATTRIBUTE_MEMORY_TYPE,
932-
reinterpret_cast<hipDeviceptr_t>(hImageMem)));
942+
reinterpret_cast<hipDeviceptr_t>(hImageMem));
943+
if (Err != hipSuccess) {
944+
return mapErrorUR(Err);
945+
}
933946
UR_ASSERT(memType == hipMemoryTypeArray, UR_RESULT_ERROR_INVALID_VALUE);
934947

935948
hipArray_t ImageArray;
936949
// If hipMipmappedArrayGetLevel failed, hImageMem is already hipArray_t.
937-
if (hipError_t Err = hipMipmappedArrayGetLevel(
938-
&ImageArray, reinterpret_cast<hipMipmappedArray_t>(hImageMem), 0);
939-
Err != hipSuccess) {
950+
Err = hipMipmappedArrayGetLevel(
951+
&ImageArray, reinterpret_cast<hipMipmappedArray_t>(hImageMem), 0);
952+
if (Err != hipSuccess) {
940953
ImageArray = reinterpret_cast<hipArray_t>(hImageMem);
941954
}
942955

943956
HIP_ARRAY3D_DESCRIPTOR ArrayDesc;
944-
UR_CHECK_ERROR(hipArray3DGetDescriptor(&ArrayDesc, ImageArray));
957+
Err = hipArray3DGetDescriptor(&ArrayDesc, ImageArray);
958+
if (Err != hipSuccess) {
959+
return mapErrorUR(Err);
960+
}
945961

946962
switch (propName) {
947963
case UR_IMAGE_INFO_WIDTH:
@@ -971,7 +987,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageGetInfoExp(
971987
case UR_IMAGE_INFO_FORMAT: {
972988
ur_image_channel_type_t ChannelType{};
973989
ur_image_channel_order_t ChannelOrder{};
974-
UR_CHECK_ERROR(hipToUrImageChannelFormat(ArrayDesc.Format, &ChannelType));
990+
UR_CALL(hipToUrImageChannelFormat(ArrayDesc.Format, &ChannelType));
975991
// HIP does not have a notion of channel "order" in the same way that
976992
// SYCL 1.2.1 does.
977993
switch (ArrayDesc.NumChannels) {

0 commit comments

Comments
 (0)