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

[ARM64] Git 2.47.0.2 installer will crash on pre-release Windows due to cygwin bug #5239

Closed
ArminG-MSFT opened this issue Nov 2, 2024 · 2 comments · Fixed by git-for-windows/msys2-runtime#76
Milestone

Comments

@ArminG-MSFT
Copy link

I posted this issue also here (#4808) but didn't realize that issue was already closed.

We as Microsoft discovered internally that Git installer will start crashing soon as a result of changes in the OS that will go to pre-release flighting. This is caused by an known issue in cygwin.
History in the bug listed above.

In summary: Cygwin compiled for AMD64 is crashing when running on ARM64 when emulated. This is because cygwin performs an 'optimisation' where it tries to find some assembly code in ntdll.dll. That is highly unsupported and should not be done at all! On AMD64 they sort of get away with it (often doesn't work, but doesn't crash) but looking for x64 assembly code in an ARM64EC ntdll.dll is not only going to fail, but can lead to crashes.

Accidental shifts in ntdll.dll earlier this year made the issue appear and other shifts a bit later made it disappear. New optimizations and code changes in ntdll.dll that will appear publicly soon, will cause the crash to come back.

Fortunately cygwin already made a fix by not doing this scanning when running on ARM64 emulated. After all, you'll never find the correct x64 assembly sequence anyway when running on ARM64. However, it seems Git has not pulled over this fix yet. If so, it is essential Git will do this ASAP to prevent crashes in the future.

Fix here: cygwin/cygwin@4e77fa9

Latest version 2.47.0.2 installer is affected and will start crashing soon on ARM64 pre-release flights if not fixed. We may be able to delay the code changes from going out for a while, but we really need Git to help pull this fix in.

(And if I'm mistaken and the fix is already pulled in, I'd like to debug with you why this is happening.)

@rimrul
Copy link
Member

rimrul commented Nov 2, 2024

Fortunately cygwin already made a fix by not doing this scanning when running on ARM64 emulated.

Only in the code for upcoming version 3.6. This change is not part of current Cygwin 3.5.4.

After all, you'll never find the correct x64 assembly sequence anyway when running on ARM64. However, it seems Git has not pulled over this fix yet.

Mainly because it's not part of any officially released Cygwin version yet.

If so, it is essential Git will do this ASAP to prevent crashes in the future.

Yes, it might make sense to backport this soon-ish.

@dscho
Copy link
Member

dscho commented Nov 3, 2024

This is because cygwin performs an 'optimisation' where it tries to find some assembly code in ntdll.dll. That is highly unsupported and should not be done at all!

I heard this a lot, but invariably I heard not a peep anymore as soon as I asked how Cygwin should achieve the same effect instead. Cygwin needs to be able to emulate Linux' cwd behavior, and as long as there is no proper support to do so, I would prefer not to criticize Cygwin if they are forced to use nasty work-arounds.

If so, it is essential Git will do this ASAP to prevent crashes in the future.

Yes, it might make sense to backport this soon-ish.

@ArminG-MSFT would you mind opening that PR at git-for-windows/msys2-runtime, please?

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 a pull request may close this issue.

3 participants