zstd icon indicating copy to clipboard operation
zstd copied to clipboard

Building zstd on WinXP with MinGW

Open jdx-gh opened this issue 4 years ago • 9 comments

I tried to build zstd on WinXP using MinGW-W64 7.3.0. After minor changes in CMakeLists everything seemed to be fine. But when I tried to run zstd.exe, it silently crashed. After some investigation I found out that kernel32.dll lacks InitializeConditionVariable() and three other functions related to condition variables. These functions are supported on Vista or higher. Because XP is obsolete, I do not expect a sophisticated solution, but a simple warning when zstd is built on XP would be nice.

BTW. In case of MinGW and XP thread safety is quite simple to achive – one should switch to pthreads by modyfying one line in threading.[ch]: #if defined(ZSTD_MULTITHREAD) && defined(_WIN32) to #if defined(ZSTD_MULTITHREAD) && defined(_WIN32) && !defined(__MINGW32__)

jdx-gh avatar Sep 27 '19 19:09 jdx-gh

Do all implementations of MinGW that define __MINGW32__ support pthreads? If so I will gladly take a patch to change this line.

terrelln avatar Sep 30 '19 17:09 terrelln

note: a simple work around is to not define ZSTD_MULTITHREAD while building zstd.exe, this choice should disable any invocation of MT symbol.

However, it does not solve the fact that the issue remain silent at link time.

Cyan4973 avatar Sep 30 '19 19:09 Cyan4973

One issue with the proposed solution is that it impacts all mingw builds, not just Windows XP ones. So one wants to be sure that it doesn't introduce negative side effects, such as linking errors, or performance losses.

Cyan4973 avatar Oct 21 '19 22:10 Cyan4973

By the way @jdxAtGitHub, which version of mingw do you install on your Windows XP system ?

I tried installing mingw64 via msys2, but installation fails on Windows XP : dwmapi.dll not found (it is apparently a Windows 7 library).

Cyan4973 avatar Oct 22 '19 00:10 Cyan4973

@terrelln I don't know, but both versions of MinGW-W64 (posix and win32 – see here for explanation), probably the most popular flavour, support pthreads.

@Cyan4973 The code is compiled and linked silently because of the following code in threading.h:

#ifdef WINVER
#  undef WINVER
#endif
#define WINVER       0x0600

#ifdef _WIN32_WINNT
#  undef _WIN32_WINNT
#endif
#define _WIN32_WINNT 0x0600

I've done a few tests and without this code building fails at compilation or linking stage, depending on gcc version. Anyway, I don't understand what's the purpose of the code. IMO it doesn't make sense.

IMO it's not worth to waste man-hours for a sophisticated patch for virtually dead OS like WinXP (however I may submit one in the future). As for now, a good compromise would be checking OS version at CMake stage and displaying a warning/error message in case of XP or lower Windows version. Also a few sentences in the documentation about the problem (i.e. that XP and lower don't support conditional variables) and possible solutions would be nice. BTW. I suspect that VS2010 on WinXP has the same problem.

Regarding MSYS2/MinGW – please note that recent versions of MSYS2 don't run on XP – see here. So just install old enough version of MSYS2 and then "manually" install one or more versions of MinGW-W64 from here (but don't install 8.1.0 – it has a bug related to bitfields). A tweak to PATH may be required.

jdx-gh avatar Oct 23 '19 00:10 jdx-gh

Your comment regarding the redefinition of WINVER seems valid.

Why are these macros redefined, I'm not sure. Better investigate. As an outcome, it's better to fail at compilation or link stage, rather than break at runtime.

edit : found this reference implementation on these macros : https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt?view=vs-2019 , which doesn't seem particularly applicable to our use case. But maybe it could explain why these macros were modified in the original code.

edit 2 : @terrelln , do you remember why this was added : https://github.com/facebook/zstd/commit/4204e03e77113ccc22ebf805a8a8da60066b83e8 ? It seems related to the introduction of conditionals.

Cyan4973 avatar Oct 23 '19 23:10 Cyan4973

Value 0x0600 means Windows Vista and condition variables are supported on Vista (or newer). The positive (side) effect of the code is that zstd can be successfully built e.g. on XP and resulting binaries can be run in appropriate Windows version (I successfully have run on Win7 zstd.exe and tests built on XP). The negative (side) effect has been described at the beginning of the thread. Anyway, I agree that the redefinition seems to be redundant and should be removed.

jdx-gh avatar Oct 24 '19 05:10 jdx-gh

We build for the latest sdk 10: <WindowsTargetPlatformVersion>10.0.18362.0</WindowsTargetPlatformVersion> #define _WIN32_WINNT 0x0A00 for <windows.h> rebrand the executable for XP: ioh->MajorOperatingSystemVersion = 4; ioh->MinorOperatingSystemVersion = 0; ioh->MajorSubsystemVersion = 4; ioh->MinorSubsystemVersion = 0; And LoadLibraryW() and GetProcAddress() non-XP dlls.

stati64 avatar Oct 24 '19 15:10 stati64

Exactly related to the original issue, when I use PeaZip 8.6.0 (32-bit) on Windows XP (which they support that as well as ReactOS), I get this error message pop-up:

zstd.exe - Entry Point Not Found

The procedure entry point InitializeConditionVariable could not be located in the dynamic link library KERNEL32.dll.

Despite the error, it continues to extract. I am actually able to extract 95% of a zstd compressed file before it hangs.

LukeShortCloud avatar Apr 28 '22 03:04 LukeShortCloud