Skip to content

Commit

Permalink
Fix Objective-C static analysis warnings. (microsoft#20417)
Browse files Browse the repository at this point in the history
Replace most usages of [NSString stringWithUTF8String:] with checked helper function. The issue is that the former can return nil.
  • Loading branch information
edgchen1 authored Apr 24, 2024
1 parent dfd4bce commit 9cc5bad
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 16 deletions.
6 changes: 4 additions & 2 deletions cmake/onnxruntime_providers_coreml.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion objectivec/ort_checkpoint.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <variant>
#import "cxx_api.h"

#import "cxx_utils.h"
#import "error_utils.h"

NS_ASSUME_NONNULL_BEGIN
Expand Down Expand Up @@ -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<std::string>(&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);
}
Expand Down
4 changes: 2 additions & 2 deletions objectivec/ort_session.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <vector>

#import "cxx_api.h"
#import "cxx_utils.h"
#import "error_utils.h"
#import "ort_enums_internal.h"
#import "ort_env_internal.h"
Expand Down Expand Up @@ -198,8 +199,7 @@ - (BOOL)runWithInputs:(NSDictionary<NSString*, ORTValue*>*)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];
}

Expand Down
3 changes: 2 additions & 1 deletion onnxruntime/core/providers/coreml/model/host_utils.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Foundation/Foundation.h>

Expand Down Expand Up @@ -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
Expand Down
14 changes: 4 additions & 10 deletions onnxruntime/core/providers/coreml/model/model.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
Expand Down Expand Up @@ -165,7 +159,7 @@ Status CreateInputFeatureProvider(const std::unordered_map<std::string, OnnxTens
(error != nil) ? MakeString(", error: ", [[error localizedDescription] UTF8String]) : "");

MLFeatureValue* feature_value = [MLFeatureValue featureValueWithMultiArray:multi_array];
NSString* feature_name = Utf8StringToNSString(name.c_str());
NSString* feature_name = util::Utf8StringToNSString(name.c_str());
feature_dictionary[feature_name] = feature_value;
}

Expand Down Expand Up @@ -270,7 +264,7 @@ - (instancetype)initWithPath:(const std::string&)path
logger:(const logging::Logger&)logger
coreml_flags:(uint32_t)coreml_flags {
if (self = [super init]) {
coreml_model_path_ = [NSString stringWithUTF8String:path.c_str()];
coreml_model_path_ = util::Utf8StringToNSString(path.c_str());
logger_ = &logger;
coreml_flags_ = coreml_flags;
}
Expand Down Expand Up @@ -371,7 +365,7 @@ - (Status)predict:(const std::unordered_map<std::string, OnnxTensorData>&)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);
Expand Down
14 changes: 14 additions & 0 deletions onnxruntime/core/providers/coreml/model/objc_str_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

#pragma once

#import <Foundation/Foundation.h>

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
16 changes: 16 additions & 0 deletions onnxruntime/core/providers/coreml/model/objc_str_utils.mm
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 9cc5bad

Please sign in to comment.