Fix custom date formats in i686 builds #3626
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Christoph Reiter reported on the Git for Windows issue tracker[1], that mingw_strftime() imports strftime() from ucrtbase.dll with the wrong calling convention. It should be __cdecl instead of WINAPI, which we always use in DECLARE_PROC_ADDR(). The MSYS2 project encountered cmake sefaults on x86 Windows caused by the same issue in the cmake source. [2] There are no known git crashes that where caused by this, yet, but we should try to prevent them. We import two other non-WINAPI functions via DECLARE_PROC_ADDR(), too. * NtSetSystemInformation() (NTAPI) * GetUserNameExW() (SEC_ENTRY) NTAPI, SEC_ENTRY and WINAPI are all ususally defined as __stdcall, but there are circumstances where they're defined differently. Teach DECLARE_PROC_ADDR() about calling conventions and be explicit about when we want to use which calling convention. Import winnt.h for the definition of NTAPI and sspi.h for SEC_ENTRY near their respective only users. [1] git-for-windows#3560 [2] msys2/MINGW-packages#10152 Reported-By: Christoph Reiter <[email protected]> Signed-off-by: Matthias Aßhauer <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
This is needed because the signature of `DECLARE_PROC_ADDR()` has changed, as per `ma/windows-dynload-fix`. Signed-off-by: Johannes Schindelin <[email protected]>
rimrul
approved these changes
Jan 12, 2022
| @@ -8,7 +8,7 @@ int win32_fsync_no_flush(int fd) | |||
|
|
|||
| #define FLUSH_FLAGS_FILE_DATA_ONLY 1 | |||
|
|
|||
| DECLARE_PROC_ADDR(ntdll.dll, NTSTATUS, NtFlushBuffersFileEx, | |||
| DECLARE_PROC_ADDR(ntdll.dll, NTSTATUS, NTAPI, NtFlushBuffersFileEx, | |||
There was a problem hiding this comment.
I'm guessing our NTAPI definition here comes from winternl.h here. I used winnt.h in t/helper/test-drop-caches.c we might want to make them use the same source at some point in the future, but this isn't urgent.
There was a problem hiding this comment.
I'm guessing our
NTAPIdefinition here comes fromwinternl.hhere. I usedwinnt.hint/helper/test-drop-caches.cwe might want to make them use the same source at some point in the future, but this isn't urgent.
winternl.h #includes windef.h, which in turn #includes winnt.h.
So there is nothing we need to do here.
Besides, we can allow ourselves to trust the mingw-w64-headers to not #define parts as central as NTAPI in incompatible ways depending which header file you #include.
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 12, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 12, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 12, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 12, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 12, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 13, 2022
Fix custom date formats in i686 builds
dscho
added a commit
that referenced
this pull request
Jan 13, 2022
Fix custom date formats in i686 builds
dscho
added a commit
that referenced
this pull request
Jan 13, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 14, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 15, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 15, 2022
Fix custom date formats in i686 builds
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jan 15, 2022
Fix custom date formats in i686 builds
dscho
added a commit
that referenced
this pull request
Jan 16, 2022
Fix custom date formats in i686 builds
dscho
added a commit
that referenced
this pull request
Jan 20, 2022
Fix custom date formats in i686 builds
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In i686 builds, this will crash with a segmentation fault:
(Note that if you try to reproduce this in a regular Git for Windows SDK, you will have to call
make NDEBUG=1to be able to reproduce the segmentation fault.)The reason is that the
strftime()function is imported with an incorrect calling convention. It is quite timely that Matthias Aßauer submitted a patch to address precisely this bug, and that contribution already made it into upstream'sseenbranch as thema/windows-dynload-fixtopic (and is slated to be fast-tracked into v2.35.0). This PR backports that topic, along with a fix-up for a patch that has not landed upstream yet so that the result still builds.This fixes #3624