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

Force linking against pthreads statically #20

Merged
merged 1 commit into from
Nov 26, 2018

Conversation

mvojacek
Copy link
Contributor

when i went to compile a test project with bine on windows+msys2, the produced binary had dynamic imports for libwinpthread-1.dll. This library is required in the library path or pwd for the binary to run at all, and no visible error is produced. The binary runs fine in the msys environment, because this library is present, but on raw windows it doesnt.
This needs to be tested to verify its not just a peculiarity in my build environ, but these two flags, in this order, removed the import in the exe, and the binary is now truly standalone.

img

All the other libs are part of windows afaik.

@cretz
Copy link
Owner

cretz commented Nov 22, 2018

Even though you built tor-static in msys2, did you built the regular Go executable in regular Windows shell just putting gcc on the PATH? If not, try that, it's what I do. Otherwise, I'll investigate next week.

@mvojacek
Copy link
Contributor Author

mvojacek commented Nov 22, 2018 via email

@mvojacek
Copy link
Contributor Author

So, i built a few variations.
"original" is before my patch, "patched" after it.
"msys2" was built with msys2, otherwise windows native go+mingwgcc

img
Using my patch, both msys2 and native import the same dlls and functions, all part of windows.

img
Without the patch, both still import the same functions, but the extra winpthread dll is needed.

img
The weird part, winpthread is only used for clock_gettime.
Here is a full diff of imported functions, before and after my patch: https://pastebin.com/prC7jJH7

My guess is, somewhere clock_gettime from winpthread is required, and my patch pulls in the whole of the library, requiring (mostly unused?) imports for the windows threading api?

A quick grep shows that clock_gettime is used all over the place in tor,libevent,openssl,xz,etc., but they (probably all of them) have mechanisms to fall back to gettimeofday and others.
I think that by building tor-static in msys2 with the winpthread library available causes tor-static to be configured and built to use clock_gettime.

Could you check if the msys2 environment you use for building tor-static has libwinpthread?

@cretz
Copy link
Owner

cretz commented Nov 25, 2018

Thanks! This is basically the exact test I will run next week to confirm pthreads use (I'll also test 0.3.3). If I can replicate, we'll definitely merge this. Also, I have to test also whether the binary requires the DLL early on start and how much statically linking winpthreads affects binary size. I do have a binary I made recently with 0.3.5 statically compiled at https://github.com/cretz/rwtxt-crypt that I may also have to revisit.

@mvojacek
Copy link
Contributor Author

30960   gotortest.msys2original.exe
31000   gotortest.msys2patched.exe
30900   gotortest.original.exe
30936   gotortest.patched.exe

Sizes in kilobytes. It's about a net 30K for statically linking in winpthreads. There will be some extra startup overhead because the loader has to linkin the extra windows threading-related kernel calls, but that should be negligible, as all windows dll will already be loaded anyway.

@mvojacek
Copy link
Contributor Author

Oh, i just realised: The binaries are really big because i pulled in a lot of other dependencies for my tests. The difference between dynamic and static pthreads should be the same, though.

cretz added a commit that referenced this pull request Nov 26, 2018
@cretz cretz merged commit 465b965 into cretz:master Nov 26, 2018
@cretz
Copy link
Owner

cretz commented Nov 26, 2018

Ok, I have come to the same conclusions both on Tor 0.3.3 and Tor 0.3.5. Thanks for the fix!

cretz added a commit to cretz/tor-static that referenced this pull request Nov 26, 2018
cretz added a commit to cretz/tor-static that referenced this pull request Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants