From 9cc5badc498da1549f206ccff8bb57ecb8583031 Mon Sep 17 00:00:00 2001 From: Edward Chen <18449977+edgchen1@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:48:29 -0700 Subject: [PATCH] Fix Objective-C static analysis warnings. (#20417) Replace most usages of [NSString stringWithUTF8String:] with checked helper function. The issue is that the former can return nil. --- cmake/onnxruntime_providers_coreml.cmake | 6 ++++-- objectivec/ort_checkpoint.mm | 3 ++- objectivec/ort_session.mm | 4 ++-- .../core/providers/coreml/model/host_utils.mm | 3 ++- onnxruntime/core/providers/coreml/model/model.mm | 14 ++++---------- .../core/providers/coreml/model/objc_str_utils.h | 14 ++++++++++++++ .../providers/coreml/model/objc_str_utils.mm | 16 ++++++++++++++++ 7 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 onnxruntime/core/providers/coreml/model/objc_str_utils.h create mode 100644 onnxruntime/core/providers/coreml/model/objc_str_utils.mm diff --git a/cmake/onnxruntime_providers_coreml.cmake b/cmake/onnxruntime_providers_coreml.cmake index b8ebc4ca53239..0aa25a221bf27 100644 --- a/cmake/onnxruntime_providers_coreml.cmake +++ b/cmake/onnxruntime_providers_coreml.cmake @@ -126,10 +126,12 @@ endif() if (APPLE) file(GLOB onnxruntime_providers_coreml_objcc_srcs CONFIGURE_DEPENDS - "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/model.h" - "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/model.mm" "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/host_utils.h" "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/host_utils.mm" + "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/model.h" + "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/model.mm" + "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/objc_str_utils.h" + "${ONNXRUNTIME_ROOT}/core/providers/coreml/model/objc_str_utils.mm" ) else() # add the Model implementation that uses the protobuf types but excludes any actual CoreML dependencies diff --git a/objectivec/ort_checkpoint.mm b/objectivec/ort_checkpoint.mm index 12386457fadf1..2c7c9e417b52c 100644 --- a/objectivec/ort_checkpoint.mm +++ b/objectivec/ort_checkpoint.mm @@ -8,6 +8,7 @@ #include #import "cxx_api.h" +#import "cxx_utils.h" #import "error_utils.h" NS_ASSUME_NONNULL_BEGIN @@ -73,7 +74,7 @@ - (nullable NSString*)getStringPropertyWithName:(NSString*)name error:(NSError** try { Ort::Property value = [self CXXAPIOrtCheckpoint].GetProperty(name.UTF8String); if (std::string* str = std::get_if(&value)) { - return [NSString stringWithUTF8String:str->c_str()]; + return utils::toNSString(str->c_str()); } ORT_CXX_API_THROW("Property is not a string.", ORT_INVALID_ARGUMENT); } diff --git a/objectivec/ort_session.mm b/objectivec/ort_session.mm index 87288bd1e9dc7..3dcc88f1ebd5b 100644 --- a/objectivec/ort_session.mm +++ b/objectivec/ort_session.mm @@ -7,6 +7,7 @@ #include #import "cxx_api.h" +#import "cxx_utils.h" #import "error_utils.h" #import "ort_enums_internal.h" #import "ort_env_internal.h" @@ -198,8 +199,7 @@ - (BOOL)runWithInputs:(NSDictionary*)inputs for (size_t i = 0; i < nameCount; ++i) { auto name = getName(i, allocator); - NSString* nameNsstr = [NSString stringWithUTF8String:name.get()]; - NSAssert(nameNsstr != nil, @"nameNsstr must not be nil"); + NSString* nameNsstr = utils::toNSString(name.get()); [result addObject:nameNsstr]; } diff --git a/onnxruntime/core/providers/coreml/model/host_utils.mm b/onnxruntime/core/providers/coreml/model/host_utils.mm index 5487ea35388f5..70052f50ae1c2 100644 --- a/onnxruntime/core/providers/coreml/model/host_utils.mm +++ b/onnxruntime/core/providers/coreml/model/host_utils.mm @@ -3,6 +3,7 @@ #include "core/platform/env.h" #include "core/providers/coreml/model/host_utils.h" +#include "core/providers/coreml/model/objc_str_utils.h" #import @@ -36,7 +37,7 @@ int32_t CoreMLVersion() { #if !defined(NDEBUG) std::string path_override = Env::Default().GetEnvironmentVar(kOverrideModelOutputDirectoryEnvVar); if (!path_override.empty()) { - NSString* ns_path_override = [NSString stringWithUTF8String:path_override.c_str()]; + NSString* ns_path_override = Utf8StringToNSString(path_override.c_str()); temporary_directory_url = [NSURL fileURLWithPath:ns_path_override isDirectory:YES]; } #endif diff --git a/onnxruntime/core/providers/coreml/model/model.mm b/onnxruntime/core/providers/coreml/model/model.mm index 1434043e064f4..3edcdb3f95e46 100644 --- a/onnxruntime/core/providers/coreml/model/model.mm +++ b/onnxruntime/core/providers/coreml/model/model.mm @@ -23,6 +23,7 @@ #include "core/providers/coreml/builders/helper.h" #include "core/providers/coreml/coreml_provider_factory.h" #include "core/providers/coreml/model/host_utils.h" +#include "core/providers/coreml/model/objc_str_utils.h" #include "core/providers/coreml/shape_utils.h" // force the linker to create a dependency on the CoreML framework so that in MAUI usage we don't need @@ -33,13 +34,6 @@ using namespace onnxruntime::coreml; namespace { -// Converts a UTF8 const char* to an NSString. Throws on failure. -NSString* _Nonnull Utf8StringToNSString(const char* utf8_str) { - NSString* result = [NSString stringWithUTF8String:utf8_str]; - ORT_ENFORCE(result != nil, "NSString conversion failed."); - return result; -} - /** * Computes the static output shape used to allocate the output tensor. * `inferred_shape` is the inferred shape known at model compile time. It may contain dynamic dimensions (-1). @@ -165,7 +159,7 @@ Status CreateInputFeatureProvider(const std::unordered_map&)inputs for (const auto& [output_name, output_tensor_info] : outputs) { MLFeatureValue* output_value = - [output_features featureValueForName:Utf8StringToNSString(output_name.c_str())]; + [output_features featureValueForName:util::Utf8StringToNSString(output_name.c_str())]; if (output_value == nil) { return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "output_features has no value for ", output_name); diff --git a/onnxruntime/core/providers/coreml/model/objc_str_utils.h b/onnxruntime/core/providers/coreml/model/objc_str_utils.h new file mode 100644 index 0000000000000..38006c39bc588 --- /dev/null +++ b/onnxruntime/core/providers/coreml/model/objc_str_utils.h @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#pragma once + +#import + +namespace onnxruntime::coreml::util { + +// Converts a UTF8 const char* to an NSString. Throws on failure. +// Prefer this to directly calling [NSString stringWithUTF8String:] as that may return nil. +NSString* _Nonnull Utf8StringToNSString(const char* _Nonnull utf8_str); + +} // namespace onnxruntime::coreml::util diff --git a/onnxruntime/core/providers/coreml/model/objc_str_utils.mm b/onnxruntime/core/providers/coreml/model/objc_str_utils.mm new file mode 100644 index 0000000000000..0d280ce0b7f8f --- /dev/null +++ b/onnxruntime/core/providers/coreml/model/objc_str_utils.mm @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#include "core/providers/coreml/model/objc_str_utils.h" + +#include "core/common/common.h" + +namespace onnxruntime::coreml::util { + +NSString* _Nonnull Utf8StringToNSString(const char* _Nonnull utf8_str) { + NSString* result = [NSString stringWithUTF8String:utf8_str]; + ORT_ENFORCE(result != nil, "NSString conversion failed."); + return result; +} + +} // namespace onnxruntime::coreml::util