-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ABI breakages on ucrt64 #9363
Comments
What do you mean by broken imports? Does the program fail to execute normal operations or not? |
the screenshots are from peview a handy gui that shows imported and exported symbols from windows binaries equivalent cli:
python-3.9.6-3
|
that's pretty odd. There's only 2 commits difference to mingw-w64 between there, mingw-w64/mingw-w64@8548ab7 and mingw-w64/mingw-w64@1db0f47. |
If clang64 is fine then it's unlikely to be mingw-w64 issue (it uses UCRT after all) given the changes between mingw-w64 versions. |
maybe the binutils used to build mingw-w64-crt? (just brainstorming, no actual idea) |
"Works in my machine". The |
Why do you think they are broken? And what tool are you using to show them? |
Maybe its a bit more fiddly for me because im running windows 7?
peview.exe from https://github.com/processhacker/processhacker/releases/download/v2.39/processhacker-2.39-bin.zip
i just tried compiling that example with mingw64 passing |
That could be, no one tests for win7 anymore.
Ah, yes I see that too now. |
@lhmouse do you know what could cause this? (#9363 (comment)) |
I have no idea. Maybe @mstorsjo knows more. I don't have any Windows 7 machine at hand. I may have a look next week. |
Simply rebuilding |
Can anyone reproduce this issue in Windows 10? |
I can confirm that #9386 fixes |
But why? Maybe comparing the import libraries would reveal something? |
I was putting together a tool to try to detect this (specifically, finding an image trying to import |
https://gist.github.com/jeremyd2019/d3cf9ae792958b9f470ff9a57d3c5f30 - this uses the img* code from https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-tools/genpeimg/src/ for dealing with the PE headers. No error checking or anything... just a quick hack |
Oops, just noticed that my quick tool was looking at argv[0] not argv[1]... (I was running under ucrt64 and hadn't updated |
It would be nice to know what went wrong, to hopefully avoid it happening again, and maybe adding a test for it to mingw-w64-crt-git |
I'm not well versed in how this all works, so I hope someone else can. I compared the output of Here are both versions: |
Yeah I tried looking at objdump output, but I didn't see anything. |
I'll try to add a check for CI so we notice when it comes back. |
which would also explain why we don't see it with clang |
Without knowing what went wrong, I don't know if just looking for Clang/lld uses a different import library format than binutils too. |
I understand what's wrong. GNU ld.bfd sorts object files within archives alphabetically, in some cases. It's essential for how DLL import tables are constructed. Let's have a look at libucrt.a in the -2 version of the archive, the good one that works. If you have an undefined reference to The prefix, Normally, I think this lexical sorting only is applied within object files of each library, and in normal cases (when one import library corresponds to one DLL), each import library would contain only one random prefix. But in the case of In the good version of the import library, libapi_ms_win_crt_heap_l1_1_0_a used a prefix of Now if you look at the -1 version of the libraries, the broken one, both the heap and math libraries had gotten the same random prefix (As a related side note; you might have noticed the bug where MS link.exe can seemlingly manage to link against an import library created by GNU ld.bfd, but if you pass it two such libraries in the same invocation, they end up intermixed. But if you instead use an import library created by GNU dlltool instead of straight from the linker, MS link.exe doesn't run into this issue. The reason is that ld.bfd doesn't use a random prefix, iirc, but dlltool does.) Therefore: It's not enough to just check if heap+math are mixed, it can happen to any pair of libraries in libucrt.a. A better check would be to make sure that all the libraries in libucrt.a use unique prefixes, or simpler, just check that all members in libucrt.a use different member file names. (Import libraries created by MS tools, and by lld and llvm-dlltool, use the short import library format, and there the member names are quite different and you'll have lots of members with the same names.) One could think that a random string of 4 chars should be quite enough random space to avoid collisions if you only pick around a dozen random strings. But maybe the random string selection is seeded with something as coarse as the current realtime clock in whole seconds. And then it's suddenly very very plausible that all of them end up with the same prefix. So improving the random seed here could reduce the risk of the issue occurring. Looking at some other import libraries, in the |
So something like |
could |
Oh, that's probably it indeed. That would probably avoid the whole issue with potential random clashing of the member names. If one would make GNU ld use the output name as member object prefix for its generated import libraries, some projects could also avoid needing to use dlltool separately for making MSVC compatible import libraries :-) |
Checking if all imports happen only once should work, no? |
that doesn't seem to help, the prefix is still different between two consecutive builds. |
From what I understand it uses |
Hmm, is there any code that would correspond to what I'm seeing in the ubuntu import library then:
|
Oh, this explains it: https://salsa.debian.org/mingw-w64-team/mingw-w64/-/blob/master/debian/patches/dlltool-temp-prefix.patch Unfortunately, llvm-dlltool doesn't know that option yet. (It's not of much use there so it'd need to just ignore it.) |
configure could check if the option is supported and only use it if it is... (yay autotools) |
If you are building an 'image'. I think mingw-w64-crt-git only builds archives though, I was trying to find something that could be checked against mingw-w64-crt-git directly. I suppose another important test of mingw-w64-crt-git is that it can be used to successfully link executables 😉 |
Even in the -2 version, there are a lot of duplicated object files in libmincore.a and libwindowsapp.a, and for f in ucrt64/x86_64-w64-mingw32/lib/*.a; do
ar t "${f}" | sort | uniq -d | grep -E '\.o\s*$' && echo $f
done |
…tool also helps reproducibility See msys2#9363 (comment)
I posted a patch on mingw-w64-public where I implement detection for support for the flag.
The former seems to be an actual duplicate, the latter is a case of the library containing both |
The libmincore.a and libwindowsapp.a duplicates are gone from -3, leaving just the strtof and crt0_w ones |
But some api-ms-win*.def files contain duplicate names. We could follow the list from here https://docs.microsoft.com/en-us/uwp/win32-and-com/win32-apis. But in many cases that list do not match with actual library file in WinSDK. Also fear of breaking existing things. |
That’s a totally different thing from the discussed here, please don’t derail the discussion. Whether a library in WinSDK contains a function or not is the only truth. Out of the api-ms-* DLLs, many of them contain the same function. But when you link against an import library in WinSDK, that import library will only contain that function once, for one such DLL that provides it. |
When GNU dlltool generates import libraries, it picks a semi-random prefix string for its file names based on the pid of the process. Normally, the prefix doesn't matter much, but when we merge multiple import libraries into one, like for libucrt.a, the prefixes need to be unique (otherwise their import tables get entangled). In practice it has been noticed that these aren't always unique (see msys2/MINGW-packages#9363). Instead pass an option to give it use a unique prefix for each library (based on the target dll name). LLVM dlltool uses a different format of import library, where there's no corresponding semi random prefix. LLVM dlltool doesn't support the --temp-prefix option either (yet). Therefore, try to detect whether the option is supported. Based on a Debian patch by Stephen Kitt. Signed-off-by: Martin Storsjö <martin@martin.st>
FYI I pushed upstream commits now that add the same |
…tool also helps reproducibility See msys2#9363 (comment)
It seems binaries built after august have broken ucrt imports
Example (libtiff, however it can be seen in python, libdeflate)
4.3.0-3
4.3.0-4
im guessing binutils 2.37 broke something, but its just a guess
those broken imports are not present in either mingw64 or clang64 so it seems to be gcc on ucrt specific
The text was updated successfully, but these errors were encountered: