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

Windows: support more Mingw-w64 versions #2578

Closed
wants to merge 1 commit into from
Closed

Conversation

stefantalpalaru
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented May 19, 2021

Unit Test Results

     32 files     220 suites   7m 41s ⏱️
   523 tests    503 ✔️   20 💤 0 ❌
2 356 runs  2 240 ✔️ 116 💤 0 ❌

Results for commit 207b6f3.

♻️ This comment has been updated with latest results.

switch("passC", "-fno-omit-frame-pointer")
switch("passL", "-fno-omit-frame-pointer")
if not defined(windows):
# ...except on Windows where the Nim bug doesn't manifest and this option
Copy link
Contributor

@zah zah May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't the Nim bug manifest on Windows?

This is not a frequently appearing bug, but a theoretical risk in the way the conservative stack scanning is implemented that might result in a crash that would be extremely hard to track down.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No pointer mangling in the Mingw version of glibc. See this issue for the mechanism: nim-lang/Nim#10625

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the documentation regarding the lack of pointer mangling in mingw? Perhaps there is some compile-time assertions that can added to the code to verify that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A check for the definition of PTR_MANGLE would be a good additional precaution - it could be added at the bottom of the main nim module:

{.emit: """

#ifdef ASSUME_NO_PTR_MANGLE
#ifdef PTR_MANGLE
#error "This build assumes that pointer mangling is disabled. See config.nims for details"
#endif
#endif

""".}

@arnetheduck arnetheduck force-pushed the unstable branch 2 times, most recently from 657f9d5 to a4667d1 Compare January 6, 2022 16:14
Base automatically changed from unstable to stable February 15, 2022 20:57
@tersec tersec changed the base branch from stable to unstable March 21, 2022 06:12
@tersec
Copy link
Contributor

tersec commented May 13, 2022

There doesn't seem to be a compelling case for working around this gcc 8.x-specific bug, already fixed in gcc 9.1. If one reappears, this can be addressed at that point, but it's already been a year, and it's not proven necessary to fix. The more time passes, the less relevant some specific old MinGW or gcc version becomes.

clang is also an increasingly viable alternative for Windows.

@tersec tersec closed this May 13, 2022
@tersec tersec deleted the mingw branch June 14, 2022 22:05
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 this pull request may close these issues.

3 participants