microsoft / onnxruntime

ONNX Runtime: cross-platform, high performance ML inferencing and training accelerator
https://onnxruntime.ai
MIT License
14.37k stars 2.88k forks source link

[Mobile] onnxruntime-c and onnxruntime-extensions-c pod conflict with DocumentReader pod #15333

Open pksa opened 1 year ago

pksa commented 1 year ago

Describe the issue

My app already contains the DocumentReader pod from Regula(https://github.com/regulaforensics/DocumentReader-iOS) then add "pod 'onnxruntime-objc', '1.14.0'" and App runs fine.

When I have tried to create
auto m_envPtr = Ort::Env(); It gives EXC_BAD_ACCESS error.

For testing purposes I have removed the DocumentReader pod then It is working fine.

Please check the screenshot given. image

Please help me how to fix this issue. Thanks!

To reproduce

Download the project from this repo(https://github.com/microsoft/onnxruntime-inference-examples/tree/main/mobile/examples/super_resolution/ios/ORTSuperResolution) and just add the following pod in the Podfile.

pod 'DocumentReader', '~> 6.1'

Then just run the app on the device or simulator you will get the same error as mentioned above.

Urgency

No response

Platform

iOS

OS Version

16.3.1

ONNX Runtime Installation

Built from Source

Compiler Version (if 'Built from Source')

No response

Package Name (if 'Released Package')

None

ONNX Runtime Version or Commit ID

1.14.0

ONNX Runtime API

C++/C

Architecture

X64

Execution Provider

Default CPU

Execution Provider Library Version

No response

baijumeswani commented 1 year ago

@edgchen1 would you be able to provide some insights to @pksa so they can debug their application?

YUNQIUGUO commented 1 year ago

Hi, I just tried to add pod 'DocumentReader', '~> 6.1' to https://github.com/microsoft/onnxruntime-inference-examples/blob/c09567835f88f9698e36d0f675ecf79359b416a9/mobile/examples/super_resolution/ios/ORTSuperResolution/Podfile#L7-L11 the Podfile as you suggested (after the ort pods) and load the app again. I didn't see the BAD_ACCESS error at least locally on my end?

but I didn't have any code changes calling anything from the DocumentReader pods though.

Maybe is there a branch that you can share with us with your changes to the app? Thanks.

My app already contains the DocumentReader pod from Regula(https://github.com/regulaforensics/DocumentReader-iOS) then add "pod 'onnxruntime-objc', '1.14.0'" and App runs fine.

Also I noticed you added the onnxruntime-objc pods in additional to the onnxruntime-cpods? The app was originally intended to work with just the onnxruntime-c pod.

pksa commented 1 year ago

Hi, I just tried to add pod 'DocumentReader', '~> 6.1' to https://github.com/microsoft/onnxruntime-inference-examples/blob/c09567835f88f9698e36d0f675ecf79359b416a9/mobile/examples/super_resolution/ios/ORTSuperResolution/Podfile#L7-L11 the Podfile as you suggested (after the ort pods) and load the app again. I didn't see the BAD_ACCESS error at least locally on my end?

Hey! YUNQIUGUO, Apologies I forgot to mention one another pod of regular is pod 'DocumentReaderFullRFID', '~> 6.1'. Could you add this pod to the Podfile, install the pod and run the app? Then press Perform Super Resolution button.

My podfile look like

platform :ios, '11.0'

target 'ORTSuperResolution' do
  # Comment the next line if you don't want to use dynamic frameworks
  use_frameworks!

  # Pods for OrtSuperResolution
  pod 'onnxruntime-c'
  pod 'DocumentReader', '~> 6.1'
  pod 'DocumentReaderFullRFID', '~> 6.1'

  pod 'onnxruntime-extensions-c'

end
YUNQIUGUO commented 1 year ago

okay thanks. I think I am able to reproduce the same issue locally now. Not sure about the reason why adding an additional pod will cause ort environment creation failure yet. will take a look.

also at the meanwhile, have you tried other versions of this Regula framework? in case there's compatibility issue.

pksa commented 1 year ago

Thanks! Yes, I tried with different versions of the Regula framework but got the same error.

edgchen1 commented 1 year ago

I'm able to reproduce the issue with a build on main.

This line in the output window provides a clue: The given version [15] is not supported, only version 1 to 10 is supported in this build.

Upon closer inspection, I noticed a similar line appears in your screenshot too.

The message is kind of cryptic. It comes from here: https://github.com/microsoft/onnxruntime/blob/c57cf374b67f72575546d7b4c69a1af4972e2b54/onnxruntime/core/session/onnxruntime_c_api.cc#L2677-L2685

Used by the C++ API here: https://github.com/microsoft/onnxruntime/blob/c57cf374b67f72575546d7b4c69a1af4972e2b54/include/onnxruntime/core/session/onnxruntime_cxx_api.h#L114-L121

This message means there is a mismatch between the ORT_API_VERSION value defined in the header file and the one in the ORT library, with the former being too new. Moreover, the result of GetApi(ORT_API_VERSION) will be nullptr, which is probably why we see the BAD_ACCESS error. There's no check for GetApi(ORT_API_VERSION)'s result being null before GetApi() dereferences it and the Env constructor tries to use the result of GetApi(): https://github.com/microsoft/onnxruntime/blob/c57cf374b67f72575546d7b4c69a1af4972e2b54/include/onnxruntime/core/session/onnxruntime_cxx_inline.h#L410-L411

It seems that after adding the DocumentReaderFullRFID pod, the app starts using an older version of ORT. If you don't require ORT 1.14, you could try using an older version like 1.10 (where ORT_API_VERSION == 10) or earlier.

I think we can also make this message clearer.

pksa commented 1 year ago

A million thanks!

I have tried 2 ways.

1. Add version number as per mentioned 1.10 in the Podfile

I have tried adding version 1.10.0/1.10 in the Podfile but giving the following error:

pareshkanani@192 ORTSuperResolution % pod install
Analyzing dependencies
[!] CocoaPods could not find compatible versions for pod "onnxruntime-c":
  In Podfile:
    onnxruntime-c (= 1.10.0)

None of your spec sources contain a spec satisfying the dependency: `onnxruntime-c (= 1.10.0)`.

You have either:
 * out-of-date source repos which you can update with `pod repo update` or with `pod install --repo-update`.
 * mistyped the name or version.
 * not added the source repo that hosts the Podspec to your Podfile.

Podfile look like:

platform :ios, '11.0'

target 'ORTSuperResolution' do
  # Comment the next line if you don't want to use dynamic frameworks
  use_frameworks!

  # Pods for OrtSuperResolution
  pod 'onnxruntime-c', '1.10.0'
  pod 'DocumentReader', '~> 6.1'
  pod 'DocumentReaderFullRFID', '~> 6.1'

  pod 'onnxruntime-extensions-c'

end

post_install do |installer|
  installer.pods_project.targets.each do |target|
    target.build_configurations.each do |config|
      config.build_settings['CODE_SIGNING_ALLOWED'] = 'NO'
    end
  end
end

2. Create iOS build from onnxruntime with tag v1.10.0

I have followed all steps mentioned on the ONNXRuntime website(https://onnxruntime.ai/docs/build/ios.html)

Successful checkout git repo on my machine

After cloned, I run the following command

pareshkanani@192 onnxruntime % ./build.sh --config Release --use_xcode \ 
           --ios --ios_sysroot iphoneos --osx_arch arm64 --apple_deploy_target 11.0   

However, It also return an error

** BUILD FAILED **

The following build commands failed:
    CompileC /Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/build/iOS/host_protoc/build/libprotoc.build/Release/Objects-normal/x86_64/command_line_interface.o /Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/cmake/external/protobuf/src/google/protobuf/compiler/command_line_interface.cc normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (in target 'libprotoc' from project 'protobuf')
(1 failure)
Traceback (most recent call last):
  File "/Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/tools/ci_build/build.py", line 2362, in <module>
    sys.exit(main())
  File "/Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/tools/ci_build/build.py", line 2187, in main
    path_to_protoc_exe = build_protoc_for_host(
  File "/Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/tools/ci_build/build.py", line 1924, in build_protoc_for_host
    run_subprocess(cmd_args)
  File "/Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/tools/ci_build/build.py", line 639, in run_subprocess
    return run(*args, cwd=cwd, capture_stdout=capture_stdout, shell=shell, env=my_env)
  File "/Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/tools/python/util/run.py", line 42, in run
    completed_process = subprocess.run(
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/subprocess.py", line 526, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/Library/Frameworks/Python.framework/Versions/3.10/bin/cmake', '--build', '/Users/pareshkanani/Downloads/onnx1.10.0/onnxruntime/build/iOS/host_protoc', '--config', 'Release', '--target', 'protoc']' returned non-zero exit status 65.

I cannot understand what is going wrong. If you have iOS ONNXRuntime build with version number 1.10.0. Could you please share link or build here.

Thanks!

edgchen1 commented 1 year ago

In 1.10, 'onnxruntime-c' wasn't released yet. You can use 'onnxruntime-mobile-c' instead. The former is the full package with the full set of ops and ORT features, while the latter is a reduced-size mobile package with a subset of ops and features enabled. More info here: https://onnxruntime.ai/docs/install/#install-on-web-and-mobile

You can also build a full package from source. There are instructions for doing that here: https://onnxruntime.ai/docs/build/custom.html#ios

The script takes a config file with build settings. You could use something like this: https://github.com/microsoft/onnxruntime/blob/v1.10.0/tools/ci_build/github/apple/default_full_ios_framework_build_settings.json

Also, note that we haven't tested 'onnxruntime-extensions-c' with older versions of ORT like 1.10. I'm not sure how well it works.

skottmckay commented 1 year ago

pod 'onnxruntime-c'

Instead of attempting to go back to an old version, I'm wondering if the issue occurs if a minimum version of onnxruntime-c is required such as 1.14.0? i.e. if we force it to require a recent version of ORT does it prevent whatever is causing an old version of ORT to attempt to be used.

Not clear why/where that old version is coming from though. I don't see any dependency on that in the DocumentReader pods.

pksa commented 1 year ago

pod 'onnxruntime-c'

Instead of attempting to go back to an old version, I'm wondering if the issue occurs if a minimum version of onnxruntime-c is required such as 1.14.0? i.e. if we force it to require a recent version of ORT does it prevent whatever is causing an old version of ORT to attempt to be used.

Not clear why/where that old version is coming from though. I don't see any dependency on that in the DocumentReader pods.

Hi @skottmckay pods 'onnxruntime-objc', '1.14.0'/ onnxruntime-c , '1.14.0' is conflicting with DocumentReaderFullRFID pods Please check this screenshot and see at the bottom area of the screenshot where debug message print The given version [14] is not supported, only version 1 to 10 is supported in this build. and also causes an EXC_BAD_ACCESS error.

edgchen1 commented 1 year ago

Not clear why/where that old version is coming from though. I don't see any dependency on that in the DocumentReader pods.

Looks like there is a shared library, DocumentReaderCore:

% file Pods/DocumentReaderFullRFID/DocumentReaderCore.xcframework/ios-arm64_x86_64-simulator/DocumentReaderCore.framework/DocumentReaderCore
Pods/DocumentReaderFullRFID/DocumentReaderCore.xcframework/ios-arm64_x86_64-simulator/DocumentReaderCore.framework/DocumentReaderCore: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit dynamically linked shared library x86_64] [arm64]
Pods/DocumentReaderFullRFID/DocumentReaderCore.xcframework/ios-arm64_x86_64-simulator/DocumentReaderCore.framework/DocumentReaderCore (for architecture x86_64):        Mach-O 64-bit dynamically linked shared library x86_64
Pods/DocumentReaderFullRFID/DocumentReaderCore.xcframework/ios-arm64_x86_64-simulator/DocumentReaderCore.framework/DocumentReaderCore (for architecture arm64): Mach-O 64-bit dynamically linked shared library arm64

which exports symbol _OrtGetApiBase.

% nm -g Pods/DocumentReaderFullRFID/DocumentReaderCore.xcframework/ios-arm64_x86_64-simulator/DocumentReaderCore.framework/DocumentReaderCore | grep OrtGetApiBase
0000000000b0c550 T _OrtGetApiBase

The static library in onnxruntime-c also has a symbol with the same name.

skottmckay commented 1 year ago

If we change the link order (assuming that's possible) does that result in _OrtGetApiBase from the ORT pod being used?

YUNQIUGUO commented 1 year ago

hi @pksa, looks like the issue has been fixed in the new version (6.8) of DocumentReader-iOS: https://github.com/regulaforensics/DocumentReader-iOS/issues/64#issuecomment-1564417318

Can you help confirm if the issue has been resolved? thanks!

TheNetos commented 1 year ago

Greetings. Regula Dev Team here.

We have partially solved this problem in the 6.8 release.

As mentioned above - the problem is in the exported symbol OrtGetApiBase - in the source code it is marked as ORT_EXPORT, which provokes its export to the outside, thanks to this:

// To make symbols visible on macOS/iOS
#ifdef __APPLE__
#define ORT_EXPORT __attribute__((visibility("default")))
#else
#define ORT_EXPORT
#endif

Since DocumentReaderCore uses static onnxruntime assembly - this symbol ended up being exported outward through DocumentReaderCore.

In the next release (6.9 or 6.10) we will modify the operation of the ORT_EXPORT macro (via a patch in the onnxruntime build), prohibiting its export, and thus completely close this problem from our side.

Personally, I can share a suggestion to circumvent such situations - to allow those who build onnxruntime themselves to define the ORT_EXPORT macro. This would require an option in the cmake configuration (something like set(onnxruntime_ORT_EXPORT_MACRO "...")) and changes in the file https://github.com/microsoft/onnxruntime/blob/v1.15.1/include/onnxruntime/core/session/onnxruntime_c_api.h#L88.

#ifndef ORT_EXPORT
// ORT_EXPORT magic here
#endif // ORT_EXPORT

If necessary - ready to make a PR on this proposal.

Thanks.

YUNQIUGUO commented 1 year ago

@RyanUnderhill Any insights on this?

RyanUnderhill commented 1 year ago

Sounds like the problem is caused by DocumentReaderCore exporting the symbol and then it's ambiguous, but they have a future fix that should solve it. The only other thing I can think of to avoid it would be using explicit dynamic linking (loading the library dynamically then using a 'get proc address' style function to get the function pointer). Is that an option?

skottmckay commented 1 year ago

@RyanUnderhill we kinda force the export of the symbol by hardcoding ORT_EXPORT on Apple platforms to make the symbol visible no matter what.

https://github.com/microsoft/onnxruntime/blob/baeece44ba075009c6bfe95891a8c1b3d4571cb3/include/onnxruntime/core/session/onnxruntime_c_api.h#L88-L92

Could we add a generic cmake parameter for 'internal usage of ORT shared library' (in this case a product is using the ORT shared library from the iOS package to create a library that will be distributed, so usage is 'internal' to that) to set a #define that behaves similarly to what ORT_DLL_IMPORT does?