Skip to content

Commit

Permalink
[Objective-C] Enable static analysis, second try (microsoft#8875)
Browse files Browse the repository at this point in the history
The previous attempt to enable static analysis (microsoft#8842) didn't actually run the static analysis checks.

- Run clang-tidy directly.
- Address static analysis warnings.
  • Loading branch information
edgchen1 authored Aug 30, 2021
1 parent 84f9271 commit b75c108
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 48 deletions.
4 changes: 3 additions & 1 deletion objectivec/src/ort_session.mm
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ - (BOOL)runWithInputs:(NSDictionary<NSString*, ORTValue*>*)inputs

for (size_t i = 0; i < nameCount; ++i) {
auto name = std::unique_ptr<char[], decltype(deleter)>{getName(i, allocator), deleter};
[result addObject:[NSString stringWithUTF8String:name.get()]];
NSString* nameNsstr = [NSString stringWithUTF8String:name.get()];
NSAssert(nameNsstr != nil, @"nameNsstr must not be nil");
[result addObject:nameNsstr];
}

return result;
Expand Down
21 changes: 13 additions & 8 deletions onnxruntime/core/providers/coreml/model/model.mm
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,10 @@ - (instancetype)initWithInputs:(const std::unordered_map<std::string, onnxruntim
if (featureNames_ == nil) {
NSMutableArray* names = [[NSMutableArray alloc] init];
for (const auto& input : *inputs_) {
[names addObject:[NSString stringWithCString:input.first.c_str()
encoding:[NSString defaultCStringEncoding]]];
NSString* inputName = [NSString stringWithCString:input.first.c_str()
encoding:[NSString defaultCStringEncoding]];
NSAssert(inputName != nil, @"inputName must not be nil");
[names addObject:inputName];
}

featureNames_ = [NSSet setWithArray:names];
Expand Down Expand Up @@ -121,6 +123,7 @@ - (nullable MLFeatureValue*)featureValueForName:(nonnull NSString*)featureName {
return nil;
}

NSAssert(mlArray != nil, @"mlArray must not be nil");
auto* mlFeatureValue = [MLFeatureValue featureValueWithMultiArray:mlArray];
return mlFeatureValue;
}
Expand Down Expand Up @@ -172,6 +175,7 @@ - (void)dealloc {
- (onnxruntime::common::Status)loadModel {
NSError* error = nil;
NSURL* modelUrl = [NSURL URLWithString:coreml_model_path_];
NSAssert(modelUrl != nil, @"modelUrl must not be nil");
NSURL* compileUrl = [MLModel compileModelAtURL:modelUrl error:&error];

if (error != nil) {
Expand Down Expand Up @@ -222,6 +226,7 @@ - (void)dealloc {
for (auto& output : outputs) {
NSString* output_name = [NSString stringWithCString:output.first.c_str()
encoding:[NSString defaultCStringEncoding]];
NSAssert(output_name != nil, @"output_name must not be nil");
MLFeatureValue* output_value =
[output_feature featureValueForName:output_name];

Expand All @@ -246,22 +251,22 @@ - (void)dealloc {
1,
std::multiplies<int64_t>());

size_t output_data_byte_size = 0;
const auto type = output_tensor.tensor_info.data_type;
switch (type) {
case ONNX_NAMESPACE::TensorProto_DataType_FLOAT:
output_data_byte_size = num_elements * sizeof(float);
case ONNX_NAMESPACE::TensorProto_DataType_FLOAT: {
const auto output_data_byte_size = num_elements * sizeof(float);
memcpy(output_tensor.buffer, model_output_data, output_data_byte_size);
break;
case ONNX_NAMESPACE::TensorProto_DataType_INT32:
output_data_byte_size = num_elements * sizeof(int32_t);
}
case ONNX_NAMESPACE::TensorProto_DataType_INT32: {
const auto output_data_byte_size = num_elements * sizeof(int32_t);
memcpy(output_tensor.buffer, model_output_data, output_data_byte_size);
break;
}
// For this case, since Coreml Spec only uses int32 for model output while onnx provides
// int64 for model output data type. We are doing a type casting (int32 -> int64) here
// when copying the model to ORT
case ONNX_NAMESPACE::TensorProto_DataType_INT64:
output_data_byte_size = num_elements * sizeof(int64_t);
if (model_output_type == MLMultiArrayDataTypeInt32) {
int32_t* model_output_data_prime = static_cast<int32_t*>(model_output_data);
int64_t* output_tensor_buffer_prime = static_cast<int64_t*>(output_tensor.buffer);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
codechecker==6.16.0
ninja==1.10.2
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ jobs:
pool:
vmImage: 'macOS-10.15'

timeoutInMinutes: 10
timeoutInMinutes: 60

steps:
- task: UsePythonVersion@0
Expand All @@ -24,43 +24,16 @@ jobs:
--config Debug \
--build_shared_lib --use_coreml --build_objc \
--cmake_extra_defines CMAKE_EXPORT_COMPILE_COMMANDS=ON \
--update
--update \
--build --parallel
displayName: Generate compile_commands.json
- script: |
set -e
LLVM_DIR="$(brew --prefix llvm)"
CODECHECKER_CONFIG_FILE="$(dirname $(which codechecker))/../share/codechecker/config/package_layout.json"
sed -i "" \
-e 's#"clangsa": "[^"]*"#"clangsa": "'"${LLVM_DIR}"'/bin/clang"#' \
-e 's#"clang-tidy": "[^"]*"#"clang-tidy": "'"${LLVM_DIR}"'/bin/clang-tidy"#' \
"${CODECHECKER_CONFIG_FILE}"
cat "${CODECHECKER_CONFIG_FILE}"
displayName: Update CodeChecker configuration
- script: |
codechecker analyze \
--file "$(Build.SourcesDirectory)/objectivec/*" \
"$(Build.SourcesDirectory)/onnxruntime/core/platform/apple/logging/apple_log_sink.mm" \
"$(Build.SourcesDirectory)/onnxruntime/core/providers/coreml/model/*.mm" \
--output "$(Build.BinariesDirectory)/codechecker.analyze.out" \
"$(Build.BinariesDirectory)/Debug/compile_commands.json"
CODECHECKER_ANALYZE_EXIT_CODE=$?
echo "codechecker analyze exited with code: ${CODECHECKER_ANALYZE_EXIT_CODE}"
displayName: Run CodeChecker analysis
- script: |
# skip results from external dependencies
echo "-$(Build.SourcesDirectory)/cmake/external/*" > "$(Build.BinariesDirectory)/codechecker.parse.skipfile"
codechecker parse \
--ignore "$(Build.BinariesDirectory)/codechecker.parse.skipfile" \
"$(Build.BinariesDirectory)/codechecker.analyze.out"
CODECHECKER_PARSE_EXIT_CODE=$?
echo "codechecker parse exited with code: ${CODECHECKER_PARSE_EXIT_CODE}"
displayName: Show CodeChecker analysis results
"$(brew --prefix llvm)/bin/clang-tidy" \
-p="$(Build.BinariesDirectory)/Debug" \
--checks="-*,clang-analyzer-*" \
--header-filter="objectivec/include|objectivec/src|onnxruntime/core" \
./objectivec/src/*.mm \
./onnxruntime/core/platform/apple/logging/apple_log_sink.mm \
./onnxruntime/core/providers/coreml/model/*.mm
displayName: Analyze Objective-C/C++ source code

0 comments on commit b75c108

Please sign in to comment.