Skip to content

fix link error on shared libs with -MT#20834

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
sthalik:msvc-shared-library-static-crt
Oct 13, 2021
Merged

fix link error on shared libs with -MT#20834
opencv-pushbot merged 1 commit intoopencv:3.4from
sthalik:msvc-shared-library-static-crt

Conversation

@sthalik
Copy link
Copy Markdown
Contributor

@sthalik sthalik commented Oct 7, 2021 •
edited
Loading

To reproduce the link failure, use

> vcvars32
> cmake -GNinja -DCMAKE_BUILD_TYPE=RELEASE -DBUILD_SHARED_LIBS=1 -DBUILD_WITH_STATIC_CRT=1 -DCMAKE_CXX_FLAGS_RELEASE=-MT -DCMAKE_C_FLAGS_RELEASE=-MT ..
> ninja opencv_imgcodecs

The linker then fails to include any CRT version.

@sthalik sthalik force-pushed the msvc-shared-library-static-crt branch from 1a6460e to 0bb6677 Compare October 7, 2021 16:50
@alalek
Copy link
Copy Markdown
Member

alalek commented Oct 8, 2021

-DBUILD_SHARED_LIBS=1 -DBUILD_WITH_STATIC_CRT=1

This is very unusual setup.
OpenCV produces several DLL for this build configuration. Each DLL has own instance of static CRT which includes own memory management.
This is buggy configuration in general, because we don't guarantee that memory allocated and freed from the same module/DLL. There are many objects are shared between DLLs.

Details:

alalek
alalek reviewed Oct 8, 2021
View reviewed changes
modules/highgui/CMakeLists.txt Outdated
@@ -164,7 +164,10 @@ if(APPLE)
endif()

if(MSVC)
Copy link
Copy Markdown
Member

@alalek alalek Oct 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use this condition instead (got from OpenCVCRTLinkage.cmake):

-if(MSVC)
+if(MSVC AND NOT BUILD_SHARED_LIBS AND BUILD_WITH_STATIC_CRT)

Copy link
Copy Markdown
Contributor Author

@sthalik sthalik Oct 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended that the condition is only executed when BUILD_WITH_STATIC_CRT is true? The intention seems to be opposite. Maybe it's a precedence error in OpenCVCRTLinkage.cmake?

The intention was to remove stray static CRT when modules are linked against dependencies linked with -MT. The condition now applies only to static CRT builds.

Copy link
Copy Markdown
Member

@alalek alalek Oct 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, "nodefaultlib" may be used for supporting switching between single-threaded / multi-threaded CRT on application level (outside of OpenCV build) using just single set of static OpenCV binaries.

This may work several years ago, but modern MSVS compilers use /FAILIFMISMATCH:RuntimeLibrary= check. So that approach is unlikely work.
Also commits history contains mentions about MinGW fixes.

I believe we should think how to drop "nodefault" magic, but probably not in this PR.
Lets fix the compilation problem of your case first (please apply the diff above).

Copy link
Copy Markdown
Contributor Author

@sthalik sthalik Oct 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@sthalik sthalik force-pushed the msvc-shared-library-static-crt branch from 0bb6677 to 3d93675 Compare October 12, 2021 15:50
alalek
alalek approved these changes Oct 13, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@opencv-pushbot opencv-pushbot merged commit 7c0b26e into opencv:3.4 Oct 13, 2021
This was referenced Oct 15, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek approved these changes

Assignees

@alalek alalek

Labels

bug category: build/install platform: win32

Projects

None yet

Milestone

3.4.17

Development

Successfully merging this pull request may close these issues.

3 participants

@sthalik @alalek @opencv-pushbot