Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't link with Jack when using MSVC to build application #792

Open
amurzeau opened this issue Aug 20, 2021 · 16 comments
Open

Can't link with Jack when using MSVC to build application #792

amurzeau opened this issue Aug 20, 2021 · 16 comments
Labels

Comments

@amurzeau
Copy link
Contributor

Describe the bug

When building an application (like qjackctl) using MSVC, there are link errors because of missing symbols in libjack64.lib:

qjackctlJackConnect.obj : error LNK2001: unresolved external symbol JACK_METADATA_PRETTY_NAME [C:\projects\qjackctl\build_win64\src\qjackctl.vcxproj]
qjackctlJackGraph.obj : error LNK2001: unresolved external symbol JACK_METADATA_PRETTY_NAME [C:\projects\qjackctl\build_win64\src\qjackctl.vcxproj]
qjackctlMainForm.obj : error LNK2001: unresolved external symbol JACK_METADATA_PRETTY_NAME [C:\projects\qjackctl\build_win64\src\qjackctl.vcxproj]
C:\projects\qjackctl\build_win64\src\RelWithDebInfo\qjackctl.exe : fatal error LNK1120: 1 unresolved externals [C:\projects\qjackctl\build_win64\src\qjackctl.vcxproj]

Exported data (like JACK_METADATA_PRETTY_NAME) must be imported using dllimport and cannot be imported with just a .lib file.

See here: https://developercommunity.visualstudio.com/t/unresolved-external-symbol-when-linking-with-def-f/1215422

The .def file is used to tell the linker which symbols to export in a DLL, but it doesn't cause them to be imported automatically when using the DLL from another component. You have to use __declspec(dllimport) in test2 to be able to access the exported data.

Exported symbols have the __imp prefix applied to their name. The __declspec(dllimport) directive instructs the linker to search for symbols that have this prefix. The reason why no __declspec(dllimport) still works for functions is because functions are exported using 2 symbols: one with an __imp prefix and one without.

Nonetheless, you should always use the __declspec(dllimport) directive when importing symbols from a library.

I hope this helps,

Kevin Cadieux
MSVC Team

Environment

Steps To Reproduce

See appveyor script here:
https://github.com/amurzeau/qjackctl/blob/a49d6921a7c7621a8641b453d918c054829d9dc4/appveyor.yml
and resulting build here:
https://ci.appveyor.com/project/amurzeau/qjackctl/builds/40460558/job/nmok18x7js587fye

Expected vs. actual behavior

If linking Jack using the MSVC compiler should be supported, it should be possible to link with it without errors.

@amurzeau amurzeau added the bug label Aug 20, 2021
@amurzeau
Copy link
Contributor Author

I've seen old issues/PR about using dllimport/dllexport, is there any reason to not use that (maybe technical difficulties to get it right / missing time ?)

@falkTX
Copy link
Member

falkTX commented Aug 20, 2021

I've seen old issues/PR about using dllimport/dllexport, is there any reason to not use that (maybe technical difficulties to get it right / missing time ?)

Not really, can you point me to those?
Maybe they are old by now. Feel free to open a new PR yourself, I very rarely use MSVC (first time was this year!) so I dont see the issue directly myself.
On Windows JACK is not installed by default (obviously) so applications tend to just not link to it directly but load it if available.

@amurzeau
Copy link
Contributor Author

It was #287.

Can you tell me how the library is built, how I can detect if the library is being built or used by external code ?
As I see, public headers (that applications will use to compile with Jack) are in common/jack ?
And the library is compiled here:

clientlib = bld(features = ['c', 'cxx', 'cxxshlib', 'cshlib'])
?

A define will be needed for clientlib and serverlib to tell the code to use dllexport, and when not defined (that is, when headers are being used by an application) use dllimport instead.
The macro would be named something like JACK_API_EXPORT.
Are "weak exports" supposed to do something particular on Windows ?

@falkTX
Copy link
Member

falkTX commented Aug 20, 2021

That is a ticket, not a PR ;)

You can check github actions for how it is built, since we have automated builds there.
If you fork and do some changes, you should be able to test the generated builds too.

As I see, public headers (that applications will use to compile with Jack) are in common/jack ?

Yes

And the library is compiled here:

Yes, but there is more than 1 library. There is libjack, libjackserver and libjacknet, not sure if more now..

There is already a macro for the exported symbols.
https://github.com/jackaudio/jack2/blob/develop/common/jack/systemdeps.h#L133

@falkTX
Copy link
Member

falkTX commented Aug 20, 2021

Actually there is a separate macro for the exported symbols inside the jack server/client code, besides that one.
Depends on the target platform https://github.com/jackaudio/jack2/search?q=%22define+SERVER_EXPORT%22

@amurzeau
Copy link
Contributor Author

Yes it seems SERVER_EXPORT is what is needed for jack APIs, with already made macro to differentiate uses (dllimport) vs build (dllexport)

@falkTX
Copy link
Member

falkTX commented Aug 20, 2021

Since you are able to reproduce the issue, feel free to try some changes and then the generated builds.
ping me if I forget to allow to run the automated builds.

@amurzeau
Copy link
Contributor Author

Well in fact SERVER_EXPORT is not available from application, as it is defined in private headers.

I don't think JACK_LIB_EXPORT should be reused because it is used to export symbols for in-process "applications" (which are really DLLs in fact) ?
These in-process applications need to dllexport their symbols using JACK_LIB_EXPORT and dllimport Jack's symbols so a different macro is required.

I can just move all SERVER_EXPORT related code to common/jack/systemdeps.h instead (it is not used currently anyway).
I will rename it to JACK_API_EXPORT to avoid name-clashing with applications own macros (SERVER_EXPORT is too generic)

@amurzeau
Copy link
Contributor Author

Ah no SERVER_EXPORT is used, I though the search was just "SERVER_EXPORT" but it was "#define SERVER_EXPORT"

@amurzeau
Copy link
Contributor Author

Current API is exported using LIB_EXPORT, this is the one I will use (and rename to move it to headers)

@amurzeau
Copy link
Contributor Author

Is it supported to build statically the clientlib and serverlib ?

@falkTX
Copy link
Member

falkTX commented Aug 20, 2021

Is it supported to build statically the clientlib and serverlib ?

no, and it doesnt make any sense. if you do that, no external client would be able to connect.

@amurzeau
Copy link
Contributor Author

I've tried to do something but I fear I will just get a bunch a failure which would requires major code change to fix (I'm not sure to understand all impacts of these change to be honest across the different libraries ^^)
https://github.com/amurzeau/jack2/runs/3384628326?check_suite_focus=true

@falkTX
Copy link
Member

falkTX commented Aug 20, 2021

You cant include common/jack headers in main jack2 code, I cant tell you why because I didnt do it, but the header files are defined separately. basically the header stuff is defined twice.

@amurzeau
Copy link
Contributor Author

I've tried to only modify common/jack headers as they would only be used by applications that link to clientlib, but it seems to be more complex than that, I get errors about symbols being both exported and imported or just missing:

[123/226] Linking build/common/libjacknet.dll
                 from ../common/JackNetManager.cpp:19:
../common/jack/jack.h:1519:15: warning: ‘jack_error_callback’ already declared with dllexport attribute: dllimport ignored [-Wattributes]
 1519 | extern void (*jack_error_callback)(const char *msg) JACK_OPTIONAL_WEAK_EXPORT;
      |               ^~~~~~~~~~~~~~~~~~~
../common/jack/jack.h:1540:15: warning: ‘jack_info_callback’ already declared with dllexport attribute: dllimport ignored [-Wattributes]
 1540 | extern void (*jack_info_callback)(const char *msg) JACK_OPTIONAL_WEAK_EXPORT;
      |               ^~~~~~~~~~~~~~~~~~

/usr/bin/i686-w64-mingw32-ld: common/JackResampler.cpp.5.o:JackResampler.cpp:(.text$_ZN4Jack14JackRingBufferD2Ev+0x3d): undefined reference to `_imp__jack_ringbuffer_free'
/usr/bin/i686-w64-mingw32-ld: common/JackResampler.cpp.5.o:JackResampler.cpp:(.text$_ZN4Jack13JackResamplerD1Ev[__ZN4Jack13JackResamplerD1Ev]+0x3d): undefined reference to `_imp__jack_ringbuffer_free'
/usr/bin/i686-w64-mingw32-ld: common/JackResampler.cpp.5.o:JackResampler.cpp:(.text$_ZN4Jack14JackRingBufferD0Ev+0x3d): undefined reference to `_imp__jack_ringbuffer_free'
/usr/bin/i686-w64-mingw32-ld: common/JackResampler.cpp.5.o:JackResampler.cpp:(.text$_ZN4Jack13JackResamplerD0Ev[__ZN4Jack13JackResamplerD0Ev]+0x3d): undefined reference to `_imp__jack_ringbuffer_free'
/usr/bin/i686-w64-mingw32-ld: common/JackResampler.cpp.5.o:JackResampler.cpp:(.text$_ZN4Jack14JackRingBuffer5ResetEj+0x19): undefined reference to `_imp__jack_ringbuffer_reset'
/usr/bin/i686-w64-mingw32-ld: common/JackResampler.cpp.5.o:JackResampler.cpp:(.text$_ZN4Jack14JackRingBuffer5ResetEj+0x31): undefined reference to `_imp__jack_ringbuffer_reset_size'

https://github.com/amurzeau/jack2/runs/3386254201?check_suite_focus=true

It seems there are files compiled multiple time for different targets, but I can't wrap my head around that and see what's being used for which target
For these above error, the issue is that:

  • ringbuffer functions are exported as public API of clientlib
  • libjacknet.dll use headers from common/jack/ringbuffer.h, thus these API are declared as dllimport
  • libjacknet.dll builds ringbuffer.c too (but that's not really an issue in itself)
  • libjacknet.dll doesn't depend on clientlib (not serverlib) as it is supposed to include ringbuffer function implementations, but as it used the public API header common/jack/ringbuffer.h, the linker tries to find ringbuffer functions exported from another library (which is not the case) and fail

I guess the build could work if a whole different set of headers would be installed as public headers instead of common/jack/
This means this should work:

  • Copy the whole common/jack directory to something like public_api/jack
  • Add dllimport via macro only in public_api/jack headers (keep common/jack headers unmodified)
  • Change the install step to install public_api/jack headers instead of common/jack

I think this would work but it will be probably a pain to maintain ....

@amurzeau
Copy link
Contributor Author

Just confirmed, I've copied the modified common/jack headers from my commit to an existing installation of jack, and linking qjackctl works with MSVC now

amurzeau pushed a commit to amurzeau/jack2 that referenced this issue Aug 20, 2021
When building jack, symbols are declared using dllexport.
When headers are used by an application, symbols will be declared using
dllimport.

Fixes: jackaudio#792
amurzeau pushed a commit to amurzeau/jack2 that referenced this issue Aug 21, 2021
When building jack, symbols are declared using dllexport.
When headers are used by an application, symbols will be declared using
dllimport.

Fixes: jackaudio#792
amurzeau added a commit to amurzeau/jack2 that referenced this issue Aug 21, 2021
When building jack, symbols are declared using dllexport.
When headers are used by an application, symbols will be declared using
dllimport.

Fixes: jackaudio#792
theartful added a commit to theartful/synfig that referenced this issue Sep 10, 2022
jackaudio does not play well with msvc, since almost all of the symbols
are not exported
see: jackaudio/jack2#792
     jackaudio/jack2#793
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants