search

microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22.99k stars 6.34k forks source link

[boost-thread] vcpkg causes __pRawDllMain regression by auto-linking library in static builds #20898

Closed jdx-john closed 5 months ago

jdx-john commented 2 years ago

I've been searching for ages to find out why MFC calls like AfxGetResourceHandle were suddenly failing in our DLLs, after we switched to vcpkg... the DLL uses no 3rd-party libs but with vpkg enabled the DLL size grew about 40Kb and these calls failed. We link to vcpkg ports statically, but to MFC as a shared DLL (Static-MD triplet.)

After some low-level digging, it turns out __pRawDllMain is being linked from boost-thread:

Searching C:\vcpkg\installed\x86-windows-vs2019-static-md\lib\boost_thread-vc140-mt.lib: Found __pRawDllMain Referenced in msvcrt.lib(dll_dllmain.obj) Loaded boost_thread-vc140-mt.lib(tss_pe.obj)

I tested by temporarily removing this .lib file and rebuilding, that the problem went away.

It's a known issue and there are two workarounds I have seen: https://stackoverflow.com/questions/3466944/upgrade-of-boost-1-35-to-1-43-causes-linker-error-with-prawdllmain-mfc-relate

  1. change the link-order so boost_thread-vc140-mt.lib is linked after mfcs140u.lib
  2. explicitly #include <boost/thread/win32/mfc_thread_init.hpp>

Since vcpkg uses auto-link, option 1. seems a very ugly kludge. And considering our project doesn't use boost, explicitly adding an #include seems pretty awful too. Since we only see regressions at run-time, we'd have to go adding this into many projects.

Is there any way to resolve this from a vcpkg angle? For instance, why are vcpkg port libs not linked after everything else... is there a way to do this?

Also see issue I raised on boost: https://github.com/boostorg/thread/issues/360

Neumann-A commented 2 years ago

Is there any way to resolve this from a vcpkg angle

Change the portfile.cmake to move boost_thread-vc140-mt.lib into the manual-link subfolder in lib (The VS integration is known to have such issues due to auto-linking)

jdx-john commented 2 years ago

I found these:

But not any instruction how I would modify a port to get the lib into the correct location... since it's a static-only issue I don't really want to hack it. Is this something that should be considered a port-bug or something I just do locally?

Also... is it not a reasonable request that vcpkg port libs are added after any project and automatically added system libs? That's all that is required in this case.

** it says GTest has to use manual-link but we use it and it just works via autolink. Is this doc out of date?

JackBoosY commented 2 years ago

As @Neumann-A said, all libraries containing function definitions in the system library should be placed in the manual-link folder.

PhoebeHui commented 2 years ago

I have built boost-thread:x64-windows-static and checked that _pRawDllMain is being linked from boost-thread as well, we should evaluate the effects firstly if we decide to move it to manual-link.

cc @ras0219 @ras0219-msft @vicroms @BillyONeal @yurybura @autoantwort @dg0yt

JackBoosY commented 2 years ago

Workaround: Modify ports/boost-thread/portfile.cmake, add the following code to line 21:

if (EXISTS "${CURRENT_PACKAGES_DIR}/lib/boost_thread-vc140-mt.lib")
    file(RENAME "${CURRENT_PACKAGES_DIR}/lib/boost_thread-vc140-mt.lib" "${CURRENT_PACKAGES_DIR}/lib/manual-link/boost_thread-vc140-mt.lib")
endif()
if (EXISTS "${CURRENT_PACKAGES_DIR}/debug/lib/boost_thread-vc140-mt-gd.lib")
    file(RENAME "${CURRENT_PACKAGES_DIR}/debug/lib/boost_thread-vc140-mt-gd.lib" "${CURRENT_PACKAGES_DIR}/debug/lib/manual-link/boost_thread-vc140-mt-gd.lib")
endif()
jdx-john commented 2 years ago

Thanks @JackBoosY for that and I'll keep an eye on this issue. A little ugly I suppose to have some boost libs autolink and others not, but considering how much digging it took to identify this I wouldn't wish the same bug on others!

Quick question, is the manual-link directory automatically added to the VS library search path in the same way headers are to the include path?

JackBoosY commented 2 years ago

@jdx-john Please note that vcpkg integration will only add _CURRENT_INSTALLEDDIR/include to the list of Additional Include Directories, and add _CURRENT_INSTALLEDDIR/lib or _CURRENT_INSTALLEDDIR/debug/lib to the list of Additional Library Directories.

jdx-john commented 2 years ago

So if you use manual-link, you have to explicitly add the triplet's manual directory to project settings? This seems quite error-prone if you ever change triplet name. Removing all the library directories from all our VC++ projects was very pleasing when we adopted vcpkg! I suppose one can use a shared .props file but is it a reasonable change request that manual-link is added, or is there a good reason why this would break things?

@JackBoosY does vcpkg allow any way to control the order libraries are added? As per https://stackoverflow.com/a/48260595/197229 this also should reduce problems at least in some cases.

jdx-john commented 2 years ago

To reiterate, @JackBoosY are we saying I have to add an additional library path like $(VcpkgRoot)\installed\$(VcpkgTriplet)\lib\manual-link? This works but seems horrible, is there a macro or something to simplify? When $(VcpkgRoot)\installed\$(VcpkgTriplet)\lib is on the search path, adding the manual-link sub-dir seems like it should be easy?

BillyONeal commented 2 years ago

So if you use manual-link, you have to explicitly add the triplet's manual directory to project settings?

We put the mnanual-link directory in libpath, but we can't automatically detect that you wanted that particular lib so you need to add the library to the link line.

The way the detection works is that we add literally every lib in the non-manual-link directory to the link line, then inspect the file the linker produces, and copy over the dependencies that were actually referenced.

jdx-john commented 2 years ago

We put the mnanual-link directory in libpath, but we can't automatically detect that you wanted that particular lib so you need to add the library to the link line.

That's not working for me in VS2019, though it is exactly what I hoped! I tested by copying an offending lib from lib -> lib/manual-link and the only way I could get it to build was to explicitly put that directory path as an additional library directory. Jack's post above seems to contradict you that manual-link is not added?

The way the detection works is that we add literally every lib in the non-manual-link directory to the link line, then inspect the file the linker produces, and copy over the dependencies that were actually referenced.

Yeah I turned on verbose linker output and you could see it searching every single lib for symbols.

BillyONeal commented 2 years ago

That's not working for me in VS2019, though it is exactly what I hoped!

You can see where we set it here:

https://github.com/microsoft/vcpkg/blob/b430ff7ec36375cd554811eafd62c8f70304595f/scripts/buildsystems/msbuild/vcpkg.targets#L87-L91

There was a bug fixed somewhat recently where we were giving the path to the linker but not the librarian ( https://github.com/microsoft/vcpkg/pull/20054 ); I don't know what specific SHA you're using...

jdx-john commented 2 years ago

Workaround: Modify ports/boost-thread/portfile.cmake, add the following code to line 21:

if (EXISTS "${CURRENT_PACKAGES_DIR}/lib/boost_thread-vc140-mt.lib")
    file(RENAME "${CURRENT_PACKAGES_DIR}/lib/boost_thread-vc140-mt.lib" "${CURRENT_PACKAGES_DIR}/lib/manual-link/boost_thread-vc140-mt.lib")
endif()
if (EXISTS "${CURRENT_PACKAGES_DIR}/debug/lib/boost_thread-vc140-mt-gd.lib")
    file(RENAME "${CURRENT_PACKAGES_DIR}/debug/lib/boost_thread-vc140-mt-gd.lib" "${CURRENT_PACKAGES_DIR}/debug/lib/manual-link/boost_thread-vc140-mt-gd.lib")
endif()

Slight fix needed to create the dir first:

if (EXISTS "${CURRENT_PACKAGES_DIR}/lib/boost_thread-vc140-mt.lib")
    file(MAKE_DIRECTORY ${CURRENT_PACKAGES_DIR}/lib/manual-link)
    file(RENAME "${CURRENT_PACKAGES_DIR}/lib/boost_thread-vc140-mt.lib" "${CURRENT_PACKAGES_DIR}/lib/manual-link/boost_thread-vc140-mt.lib")
endif()
if (EXISTS "${CURRENT_PACKAGES_DIR}/debug/lib/boost_thread-vc140-mt-gd.lib")
    file(MAKE_DIRECTORY ${CURRENT_PACKAGES_DIR}/debug/lib/manual-link)
    file(RENAME "${CURRENT_PACKAGES_DIR}/debug/lib/boost_thread-vc140-mt-gd.lib" "${CURRENT_PACKAGES_DIR}/debug/lib/manual-link/boost_thread-vc140-mt-gd.lib")
endif()

However this doesn't work because boost-coroutine expects to find it in lib. I am not sure why a static-lib build needs it but clearly this isn't quite as simple a fix. I don't really want to find myself modifying loads of dependants as each affects the next.

Can I simply (in the interim) move the library after building in the install directory - maybe add a step in my own PWSH script that I use to build vcpkg?

MonicaLiu0311 commented 5 months ago

Will close issue because #32309 and #38533.