From 81a4efee6caca4826fa531176f3229fdb258f61a Mon Sep 17 00:00:00 2001 From: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com> Date: Mon, 3 Oct 2022 15:50:44 -0700 Subject: [PATCH] Prefast Fixes (#12952) **Description**: Fixes these TSA issues (no actual bugs fixed, but just changing code to make TSA happy) To fix 1944982 and 1944973 I changed DeleteOnUnloadPtr to not use 'new' and to just use placement new to go into a fixed buffer. This required changing the rocm usage of it also (probably a separate TSA bug on that one that I don't have) 1944982 Ryan Hill [prefast:Warning]: C26426 (in onnxruntime/core/providers/cuda/tensor/cast_op.cc) Global initializer calls a non-constexpr function 'operator new' (i.22). 1944973 Ryan Hill [prefast:Warning]: C26426 (in onnxruntime/core/providers/cuda/cuda_execution_provider_info.cc) Global initializer calls a non-constexpr function 'operator new' (i.22). 1944929 Ryan Hill [prefast:Warning]: C26436 (in onnxruntime/core/providers/cuda/cuda_provider_factory.cc) The type 'struct onnxruntime::ProviderInfo_CUDA_Impl' with a virtual function needs either public virtual or protected non-virtual destructor (c.35). --- .../cuda/cuda_execution_provider_info.cc | 18 ++++++++---------- .../providers/cuda/cuda_provider_factory.h | 2 ++ .../core/providers/cuda/tensor/cast_op.cc | 8 ++++---- .../rocm/rocm_execution_provider_info.cc | 6 +++--- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/onnxruntime/core/providers/cuda/cuda_execution_provider_info.cc b/onnxruntime/core/providers/cuda/cuda_execution_provider_info.cc index 257189318b..bce7bd21a8 100644 --- a/onnxruntime/core/providers/cuda/cuda_execution_provider_info.cc +++ b/onnxruntime/core/providers/cuda/cuda_execution_provider_info.cc @@ -27,18 +27,16 @@ constexpr const char* kCudnnConv1dPadToNc1d = "cudnn_conv1d_pad_to_nc1d"; } // namespace provider_option_names } // namespace cuda -namespace { -const DeleteOnUnloadPtr> ort_cudnn_conv_algo_search_mapping = new EnumNameMapping{ +const EnumNameMapping ort_cudnn_conv_algo_search_mapping{ {OrtCudnnConvAlgoSearchExhaustive, "EXHAUSTIVE"}, {OrtCudnnConvAlgoSearchHeuristic, "HEURISTIC"}, {OrtCudnnConvAlgoSearchDefault, "DEFAULT"}, }; -const DeleteOnUnloadPtr> arena_extend_strategy_mapping = new EnumNameMapping{ +const EnumNameMapping arena_extend_strategy_mapping{ {ArenaExtendStrategy::kNextPowerOfTwo, "kNextPowerOfTwo"}, {ArenaExtendStrategy::kSameAsRequested, "kSameAsRequested"}, }; -} // namespace CUDAExecutionProviderInfo CUDAExecutionProviderInfo::FromProviderOptions(const ProviderOptions& options) { CUDAExecutionProviderInfo info{}; @@ -86,10 +84,10 @@ CUDAExecutionProviderInfo CUDAExecutionProviderInfo::FromProviderOptions(const P .AddAssignmentToReference(cuda::provider_option_names::kMemLimit, info.gpu_mem_limit) .AddAssignmentToEnumReference( cuda::provider_option_names::kArenaExtendStrategy, - *arena_extend_strategy_mapping, info.arena_extend_strategy) + arena_extend_strategy_mapping, info.arena_extend_strategy) .AddAssignmentToEnumReference( cuda::provider_option_names::kCudnnConvAlgoSearch, - *ort_cudnn_conv_algo_search_mapping, info.cudnn_conv_algo_search) + ort_cudnn_conv_algo_search_mapping, info.cudnn_conv_algo_search) .AddAssignmentToReference(cuda::provider_option_names::kDoCopyInDefaultStream, info.do_copy_in_default_stream) .AddAssignmentToReference(cuda::provider_option_names::kCudnnConvUseMaxWorkspace, info.cudnn_conv_use_max_workspace) .AddAssignmentToReference(cuda::provider_option_names::kEnableCudaGraph, info.enable_cuda_graph) @@ -109,9 +107,9 @@ ProviderOptions CUDAExecutionProviderInfo::ToProviderOptions(const CUDAExecution {cuda::provider_option_names::kGpuExternalFree, MakeStringWithClassicLocale(reinterpret_cast(info.external_allocator_info.free))}, {cuda::provider_option_names::kGpuExternalEmptyCache, MakeStringWithClassicLocale(reinterpret_cast(info.external_allocator_info.empty_cache))}, {cuda::provider_option_names::kArenaExtendStrategy, - EnumToName(*arena_extend_strategy_mapping, info.arena_extend_strategy)}, + EnumToName(arena_extend_strategy_mapping, info.arena_extend_strategy)}, {cuda::provider_option_names::kCudnnConvAlgoSearch, - EnumToName(*ort_cudnn_conv_algo_search_mapping, info.cudnn_conv_algo_search)}, + EnumToName(ort_cudnn_conv_algo_search_mapping, info.cudnn_conv_algo_search)}, {cuda::provider_option_names::kDoCopyInDefaultStream, MakeStringWithClassicLocale(info.do_copy_in_default_stream)}, {cuda::provider_option_names::kCudnnConvUseMaxWorkspace, MakeStringWithClassicLocale(info.cudnn_conv_use_max_workspace)}, {cuda::provider_option_names::kEnableCudaGraph, MakeStringWithClassicLocale(info.enable_cuda_graph)}, @@ -125,8 +123,8 @@ ProviderOptions CUDAExecutionProviderInfo::ToProviderOptions(const OrtCUDAProvid const ProviderOptions options{ {cuda::provider_option_names::kDeviceId, MakeStringWithClassicLocale(info.device_id)}, {cuda::provider_option_names::kMemLimit, MakeStringWithClassicLocale(info.gpu_mem_limit)}, - {cuda::provider_option_names::kArenaExtendStrategy, EnumToName(*arena_extend_strategy_mapping, info.arena_extend_strategy)}, - {cuda::provider_option_names::kCudnnConvAlgoSearch, EnumToName(*ort_cudnn_conv_algo_search_mapping, info.cudnn_conv_algo_search)}, + {cuda::provider_option_names::kArenaExtendStrategy, EnumToName(arena_extend_strategy_mapping, info.arena_extend_strategy)}, + {cuda::provider_option_names::kCudnnConvAlgoSearch, EnumToName(ort_cudnn_conv_algo_search_mapping, info.cudnn_conv_algo_search)}, {cuda::provider_option_names::kDoCopyInDefaultStream, MakeStringWithClassicLocale(info.do_copy_in_default_stream)}, {cuda::provider_option_names::kCudnnConvUseMaxWorkspace, MakeStringWithClassicLocale(info.cudnn_conv_use_max_workspace)}, {cuda::provider_option_names::kCudnnConv1dPadToNc1d, MakeStringWithClassicLocale(info.cudnn_conv1d_pad_to_nc1d)} diff --git a/onnxruntime/core/providers/cuda/cuda_provider_factory.h b/onnxruntime/core/providers/cuda/cuda_provider_factory.h index ebafcdab20..093485b170 100644 --- a/onnxruntime/core/providers/cuda/cuda_provider_factory.h +++ b/onnxruntime/core/providers/cuda/cuda_provider_factory.h @@ -20,6 +20,8 @@ class NvtxRangeCreator; } struct ProviderInfo_CUDA { + virtual ~ProviderInfo_CUDA() {} // This is declared due to a TSA warning, the only instantiation of this class is a global variable of automatic storage. + virtual OrtStatus* SetCurrentGpuDeviceId(_In_ int device_id) = 0; virtual OrtStatus* GetCurrentGpuDeviceId(_In_ int* device_id) = 0; diff --git a/onnxruntime/core/providers/cuda/tensor/cast_op.cc b/onnxruntime/core/providers/cuda/tensor/cast_op.cc index 1f5d868186..98fc442423 100644 --- a/onnxruntime/core/providers/cuda/tensor/cast_op.cc +++ b/onnxruntime/core/providers/cuda/tensor/cast_op.cc @@ -9,7 +9,7 @@ using namespace onnxruntime::common; namespace onnxruntime { namespace cuda { -const DeleteOnUnloadPtr> castOpTypeConstraints = new std::vector { +const std::vector castOpTypeConstraints{ DataTypeImpl::GetTensorType(), DataTypeImpl::GetTensorType(), DataTypeImpl::GetTensorType(), @@ -34,7 +34,7 @@ const DeleteOnUnloadPtr> castOpTypeConstraints = new std kCudaExecutionProvider, \ (*KernelDefBuilder::Create()) \ .TypeConstraint("T1", DataTypeImpl::GetTensorType()) \ - .TypeConstraint("T2", *castOpTypeConstraints), \ + .TypeConstraint("T2", castOpTypeConstraints), \ Cast); \ ONNX_OPERATOR_VERSIONED_TYPED_KERNEL_EX( \ Cast, \ @@ -44,7 +44,7 @@ const DeleteOnUnloadPtr> castOpTypeConstraints = new std kCudaExecutionProvider, \ (*KernelDefBuilder::Create()) \ .TypeConstraint("T1", DataTypeImpl::GetTensorType()) \ - .TypeConstraint("T2", *castOpTypeConstraints), \ + .TypeConstraint("T2", castOpTypeConstraints), \ Cast); \ ONNX_OPERATOR_TYPED_KERNEL_EX( \ Cast, \ @@ -54,7 +54,7 @@ const DeleteOnUnloadPtr> castOpTypeConstraints = new std kCudaExecutionProvider, \ (*KernelDefBuilder::Create()) \ .TypeConstraint("T1", DataTypeImpl::GetTensorType()) \ - .TypeConstraint("T2", *castOpTypeConstraints), \ + .TypeConstraint("T2", castOpTypeConstraints), \ Cast); template diff --git a/onnxruntime/core/providers/rocm/rocm_execution_provider_info.cc b/onnxruntime/core/providers/rocm/rocm_execution_provider_info.cc index 5e17204bf7..6589aebe50 100644 --- a/onnxruntime/core/providers/rocm/rocm_execution_provider_info.cc +++ b/onnxruntime/core/providers/rocm/rocm_execution_provider_info.cc @@ -25,7 +25,7 @@ constexpr const char* kMiopenConvUseMaxWorkspace = "miopen_conv_use_max_workspac } // namespace rocm namespace { -const DeleteOnUnloadPtr> arena_extend_strategy_mapping = new EnumNameMapping{ +const EnumNameMapping arena_extend_strategy_mapping{ {ArenaExtendStrategy::kNextPowerOfTwo, "kNextPowerOfTwo"}, {ArenaExtendStrategy::kSameAsRequested, "kSameAsRequested"}, }; @@ -77,7 +77,7 @@ ROCMExecutionProviderInfo ROCMExecutionProviderInfo::FromProviderOptions(const P .AddAssignmentToReference(rocm::provider_option_names::kMemLimit, info.gpu_mem_limit) .AddAssignmentToEnumReference( rocm::provider_option_names::kArenaExtendStrategy, - *arena_extend_strategy_mapping, info.arena_extend_strategy) + arena_extend_strategy_mapping, info.arena_extend_strategy) .AddAssignmentToReference(rocm::provider_option_names::kMiopenConvExhaustiveSearch, info.miopen_conv_exhaustive_search) .AddAssignmentToReference(rocm::provider_option_names::kDoCopyInDefaultStream, info.do_copy_in_default_stream) .AddAssignmentToReference(rocm::provider_option_names::kMiopenConvUseMaxWorkspace, info.miopen_conv_use_max_workspace) @@ -96,7 +96,7 @@ ProviderOptions ROCMExecutionProviderInfo::ToProviderOptions(const ROCMExecution {rocm::provider_option_names::kGpuExternalFree, MakeStringWithClassicLocale(reinterpret_cast(info.external_allocator_info.free))}, {rocm::provider_option_names::kGpuExternalEmptyCache, MakeStringWithClassicLocale(reinterpret_cast(info.external_allocator_info.empty_cache))}, {rocm::provider_option_names::kArenaExtendStrategy, - EnumToName(*arena_extend_strategy_mapping, info.arena_extend_strategy)}, + EnumToName(arena_extend_strategy_mapping, info.arena_extend_strategy)}, {rocm::provider_option_names::kMiopenConvExhaustiveSearch, MakeStringWithClassicLocale(info.miopen_conv_exhaustive_search)}, {rocm::provider_option_names::kDoCopyInDefaultStream, MakeStringWithClassicLocale(info.do_copy_in_default_stream)}, {rocm::provider_option_names::kMiopenConvUseMaxWorkspace, MakeStringWithClassicLocale(info.miopen_conv_use_max_workspace)},