microsoft / onnxruntime

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

[Build] Broken binaries for Windows UWP: LNK1181 error missing delayimp.lib and dependency MSVCP140D.dll and VCRUNTIME140D.dll #19495

Open emaschino opened 8 months ago

emaschino commented 8 months ago

Describe the issue

Current onnxruntime.dll available in NuGet packages or built from sources fail to build/run on UWP apps. On a HoloLens 2 device for example, onnxruntime.dll incorrectly links to missing dependency MSVCP140[D].dll and VCRUNTIME140[D].dll (this problem was also reported in https://github.com/microsoft/onnxruntime/issues/4686). Furthermore, adding support for DirectML EP fails to link with missing delayimp.lib import library. I'm reporting these two issues here as I think they both share the same root cause: general Windows UWP support by ONNX Runtime.

Urgency

No response

Target platform

Windows UWP

Build script

build --builddir build{arm64|x86|x64}_uwp --config {Debug|Release} --parallel --compile_no_warning_as_error --skip_tests --build_shared_lib [--{arm64|x86}] --cmake_extra_defines CMAKE_SYSTEM_NAME=WindowsStore CMAKE_SYSTEM_VERSION=10.0 CMAKE_SYSTEM_PROCESSOR={aarch64|x86_64|i686} CMAKE_DEBUG_POSTFIX=_d [--use_dml] --path_to_protoc_exe "%ProgramFiles%/protoc-win64/bin/protoc.exe"

Error / output

dumpbin /DEPENDENTS output with missing dependency MSVCP140D.dll and VCRUNTIME140D.dll onnxruntime_onecore.txt

LNK1181 error with missing delayimp.lib import library onnxruntime_delayimp.txt

Visual Studio Version

Visual Studio 16 2019

GCC / Compiler Version

No response

emaschino commented 8 months ago

I've bisected the LNK1181 missing delayimp.lib error to commit https://github.com/microsoft/onnxruntime/pull/9065/commits/2cc1a769a239fb49edd7765e29d281b76474b437. Reverting the changes in L.1922-1929 in cmake/CMakeLists.txt produces a working UWP binary, linking to MSVCP140[D]_APP.dll and VCRUNTIME140[D]_APP.dll rather than MSVCP140[D].dll and VCRUNTIME140[D].dll. I'm not expert here, so don't know how to deal the Windows UWP case, as simply reverting the aforementioned changes is probably not the right fix. But from Microsoft documentation (https://learn.microsoft.com/en-us/windows-hardware/drivers/develop/building-for-onecore), OneCore.lib does NOT support UWP. OneCoreUAP.lib should be used instead. onnxruntime_non_onecore.txt

emaschino commented 8 months ago

While reverting the changes in cmake/CMakeLists.txt as detailed in the previous comment produces a working binary UWP binary, it's still lacking DirectML EP support. Adding the --use_dml compile flag yields to broken build with missing delayimp.lib. Here again, I've bisected the problem to commit https://github.com/microsoft/onnxruntime/pull/10929/commits/a300bf2cfec8205772bd612ee6c4c6dc52d590a4. Reverting the changes in cmake/onnxruntime_providers.cmake and cmake/target_delayload.cmake to keep from target_link_libraries(${target_name} PRIVATE delayimp.lib) to:

if (WINDOWS_STORE)
    target_link_libraries(${target_name} PRIVATE dloadhelper.lib)
else()
    target_link_libraries(${target_name} PRIVATE delayimp.lib)
endif()

fixes the problem. But since the whole commit was about to remove the WINDOWS_STORE specific code (https://github.com/microsoft/onnxruntime/pull/10929), I don't know how to deal with this. Other changes in this commit may also be reverted. From my tests, just reverting the aforementioned changes is sufficient.

emaschino commented 8 months ago

With the above changes, I'm able to get a working ONNX Runtime binary, with DirectML EP enabled for ARM64 UWP, for example. One question still remains: is DirectML really working on UWP? Indeed, I'm getting a considerable drop in performances when using DirectML EP rather than just the CPU on HoloLens 2 device, but that's probably the subject to another bug report, unless something in the UWP build presented here is still broken.

snnn commented 8 months ago

Thanks for pointing the problem out. I'm an ONNX Runtime's developer who manages the cmake files and build scripts. But, sorry I only have very limited knowledge on UWP and OneCore. Below is my understanding of the problem: OneCoreUAP is a superset of OneCore. So, if ONNX Runtime can work fine with OneCore.lib, supposedly we do not need to use OneCoreUAP? I know the DLLs that we published to nuget.org could not pass OneCore validation. As you said, they should not link to MSVCP140[D].dll and VCRUNTIME140[D].dll. But that's easy to solve: we can choose to statically link to VC Runtime. Then the problem will be gone. And that's the only problem that apivalidator found. Regarding to DirectML EP: I don't know if it could run on HoloLens devices. I am adding the DirectML's dev team into this conversation. Regarding to WINDOWS_STORE, I believe UWP apps are just normal Win32 applications. We do not need to do any special handling in the build system, unless a piece of our code used a Windows API that is prohibited by the store's policy. That's the main reason why https://github.com/microsoft/onnxruntime/commit/a300bf2cfec8205772bd612ee6c4c6dc52d590a4 was made.

emaschino commented 8 months ago

@snnn Thank you for your feedback.

I'm not that much familiar with UWP or OneCore either, just a random HoloLens 2 developer, so can't tell if OneCoreUAP is a superset of OneCore or not. However, reading Microsoft's documentation (https://learn.microsoft.com/en-us/windows-hardware/drivers/develop/building-for-onecore) again, it's clearly stated that to support UWP apps, you need to link to OneCoreUAP.lib rather than OneCore.lib:

Library     Scenario
OneCore.lib     All editions of Windows 7 and later, no UWP support
OneCoreUAP.lib  Windows 7 and later, UWP editions (Desktop, IoT, HoloLens, but not Nano Server) of Windows 10

Regarding the LNK1181 error with missing delayimp.lib, this other Microsoft's documentation (https://learn.microsoft.com/en-us/cpp/build/reference/linker-support-for-delay-loaded-dlls) also clearly states that You can specify which DLLs to delay load by using the /delayload:dllname linker option. If you don't plan to use your own version of a helper function, you must also link your program with delayimp.lib (for desktop applications) or dloadhelper.lib (for UWP apps). Hence the original if (WINDOWS_STORE) guard before commit https://github.com/microsoft/onnxruntime/commit/a300bf2cfec8205772bd612ee6c4c6dc52d590a4.

All this let me believe that UWP apps (or libraries for ONNX Runtime's concern) aren't normal Win32 apps (libraries) and should be dealt in a different manner than plain Win32. Now, how to do this properly at the whole ONNX Runtime codebase scale, I don't know. Maybe @tiagoshibata could give valuable advice? He/she authored several commits w.r.t. Windows/Windows Store in the ONNX Runtime codebase and is probably familiar with all these UWP and OneCore intrinsics.

About statically linking to VC runtime, I don't remember the details here, but if I'm not mistaken, dynamic linking was mandatory when enabling the DirectML EP.

emaschino commented 7 months ago

Confirmed: statically linking to VC runtime isn't supported. Adding --enable_msvc_static_runtime flag yields to the following error at configure stage:

CMake Error at C:/Program Files/CMake/share/cmake-3.24/Modules/CMakeTestCCompiler.cmake:69 (message):
  The C compiler

    "C:/Program Files (x86)/Microsoft Visual Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/bin/Hostx64/arm64/cl.exe"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: E:/dev/onnxruntime/build_arm64_uwp/Debug/CMakeFiles/CMakeTmp

    Run Build Command(s):C:/Program Files (x86)/Microsoft Visual Studio/2019/Professional/MSBuild/Current/Bin/MSBuild.exe cmTC_a00ae.vcxproj /p:Configuration=Debug /p:Platform=ARM64 /p:VisualStudioVersion=16.0 /v:m && Microsoft (R) Build Engine version 16.9.0+57a23d249 pour .NET Framework
    Copyright (C) Microsoft Corporation. All rights reserved.

    C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.CppBuild.targets(2024,5): error MSB8024: Using static version of the C++ runtime library is not supported. [E:\dev\onnxruntime\build_arm64_uwp\Debug\CMakeFiles\CMakeTmp\cmTC_a00ae.vcxproj]
github-actions[bot] commented 6 months ago

This issue has been automatically marked as stale due to inactivity and will be closed in 30 days if no further activity occurs. If further support is needed, please provide an update and/or more details.