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: Add sanitizers for llvm-mingw, increase stack for ASan #94845

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

alvinhochun
Copy link
Contributor

ASan and UBSan are supported by llvm-mingw. They can be enabled by passing use_asan=yes use_ubsan=yes to scons.

@alvinhochun alvinhochun requested a review from a team as a code owner July 27, 2024 20:29
ASan and UBSan are supported by llvm-mingw. They can be enabled by
passing `use_asan=yes use_ubsan=yes` to scons.
@alvinhochun alvinhochun force-pushed the llvm-mingw-sanitizers branch from 71e83fa to 0cda0b9 Compare July 27, 2024 20:32
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Not sure if README is up-to-date, but seems like sanitizers have some limitations:

  • Address Sanitizer doesn't produce working backtraces for i686.
  • The sanitizers are only supported on x86.

So we probably should have env["arch"] checks when enabling it.

@alvinhochun
Copy link
Contributor Author

Backtraces do work for i686, I've used it before. The readme is a bit outdated on this.

The sanitizers are indeed not yet working on arm32/aarch64 according to the maintainer (UBSan might work but it's not enabled for aarch64 in the build) so I suppose I can add a check for it. If they ever gets fixed we can remove the check in the future.

@alvinhochun alvinhochun requested a review from bruvzg July 28, 2024 09:22
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Aug 27, 2024
@akien-mga akien-mga merged commit 73acb2a into godotengine:master Aug 27, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@TokageItLab
Copy link
Member

Due to this PR, the build command no longer works. Is something wrong with sanitization?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants