Le lun. 15 janv. 2024 à 22:57, Martin Storsjö <mar...@martin.st> a écrit : > > On Thu, 11 Jan 2024, Antonin Décimo wrote: > > > Some WinAPI functions were introduced in recent versions of > > Windows. It's not possible to detect if they're available at compile > > time, so they're dynamically detected. > > The second sentence here kind of contradicts the next paragraph - it's > possible to detect it at compile time with _WIN32_WINNT, but we > intentionally don't want to use that. Perhaps if we'd change the second > sentence here into "We could detect whether to use the newer functions > based on the _WIN32_WINNT define. However, [...]" (continuing with the > second paragraph maybe) - what do you think about such a wording? > > > The issue is that _WIN32_WINNT defaults to Win10 these days, so a > > default build of a toolchain won't work for older targets, which is > > why it was agreed to do the change to try to load this function at > > runtime. > > > > https://sourceforge.net/p/mingw-w64/mailman/message/47371359/, > > https://sourceforge.net/p/mingw-w64/mailman/message/49958237/ and > > https://sourceforge.net/p/mingw-w64/mailman/message/48131379/. > > > > Multiple workarounds are needed for MSVC to make this detection > > portable. > > > > 1. Missing constructor attribute in MSVC > > > >> The `constructor` attribute causes the function to be called before > >> entering `main()`. > > > > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-constructor-function-attribute > > https://clang.llvm.org/docs/AttributeReference.html#constructor > > > > This feature is not supported by the MSVC compiler, and can be worked > > around by putting a function pointer to the constructor in the > > .CRT$XC? section of the executable. > > > > https://learn.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170#linker-features-for-initialization > > > > We chose the .CRT$XCT section, however the documentation warns: > > > >> The names .CRT$XCT and .CRT$XCV aren't used by either the compiler > >> or the CRT library right now, but there's no guarantee that they'll > >> remain unused in the future. And, your variables could still be > >> optimized away by the compiler. Consider the potential engineering, > >> maintenance, and portability issues before adopting this technique. > > > > 2. Missing typeof extension in MSVC > > > > As MSVC doesn't support the GCC __typeof__ extension, we explicit the > > type of the function pointer. > > > > Casting the result of GetProcAddress (a function pointer) to > > a (void *) (a data pointer) is allowed as an extension, mimics the > > POSIX dlsym function, and prevents compiler warnings. > > > > 3. Missing __atomic functions in MSVC > > > > MSVC doesn't support GCC __atomic functions. Finding whether > > GetSystemTimePreciseAsFileTime is present on the system can be done > > using a function tagged with the constructor attribute, avoiding the > > need of atomics to prevent races. > > > > 4. Gather the initialization bits into a single function > > > > Considering the amount of boilerplate code needed to instruct the > > linker on MSVC, gather the discovery of the WinAPI functions in a > > single winpthreads_init function. The WinAPI functions can be accessed > > through internal global function pointers. > > > > Signed-off-by: Antonin Décimo <anto...@tarides.com> > > --- > > Great write-up, thanks! > > > diff --git a/mingw-w64-libraries/winpthreads/src/misc.c > > b/mingw-w64-libraries/winpthreads/src/misc.c > > index 83caf262f..0aaeba913 100644 > > --- a/mingw-w64-libraries/winpthreads/src/misc.c > > +++ b/mingw-w64-libraries/winpthreads/src/misc.c > > @@ -24,15 +24,43 @@ > > #include "pthread.h" > > #include "misc.h" > > > > -static ULONGLONG (*GetTickCount64FuncPtr) (VOID); > > +void (WINAPI *GetSystemTimeBestAsFileTimeFuncPtr) (LPFILETIME) = NULL; > > +static ULONGLONG (WINAPI *GetTickCount64FuncPtr) (VOID); > > As the GetSystemTimeBestAsFileTimeFuncPtr symbol isn't static, it'll > reside in the same namespace as the user's symbols, when linking in > winpthreads as a static library. Therefore, I think it'd be good to add > some sort of prefix to avoid unintentional clashes - perhaps "_pthread_", > which I see used in some places. > > // Martin
All good points, which I'll take into account for the 3rd version of the patch series. Thanks a lot again for the nice reviews! -- Antonin _______________________________________________ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
