Skip to content

Enable compiler-rt on Windows #33952

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

Closed
wants to merge 1 commit into from

Conversation

stevapple
Copy link
Contributor

Enable compiler-rt on Windows. It is required for swift-driver and other components to find clang_rt libs.

Resolves SR-NNNN.

@compnerd
Copy link
Member

Why is this required for swift driver? There's something else that must be going on as this shouldn't be needed.

@stevapple
Copy link
Contributor Author

clang_rt is required for “profile generate” and sanitizers.

@stevapple
Copy link
Contributor Author

I saw in the code that at least the latter one is supported (or is going to be supported) on Windows.

@compnerd
Copy link
Member

The profiling library doesn't work on Windows AFAIK (that is -pg isn't supported). The way to handle profiling is to use ETW.

For the sanitizers, I don't think that you can just enable compiler-rt to build the sanitizers. Not all the sanitizers work on Windows IIRC, and I think that you need to do a separate build for building the sanitizers as compiler-rt will build just the builtins.

@stevapple
Copy link
Contributor Author

The profiling library doesn't work on Windows AFAIK (that is -pg isn't supported).

By saying this, do you mean the flag is ignored on Windows, or will trigger an error?

The way to handle profiling is to use ETW.

Does ETW have a command-line interface? Is it going to be integrated(invoked) by Swift or not?

@stevapple
Copy link
Contributor Author

Not all the sanitizers work on Windows IIRC, and I think that you need to do a separate build for building the sanitizers as compiler-rt will build just the builtins.

At least ASan works. I don’t know where we can find sanitizers specifically optimized for Windows, but compiler-rt already provides a trustable one. Still, users should decide which sanitizer to use by themselves, the compatibility issue is okay to only be addressed in FAQ or somewhere.

@compnerd
Copy link
Member

The issue isn’t just that the profile runtime and some of the sanitizers doesn’t work, some of then don’t even build. Microsoft is already working to get ASAN working and part of MSVC, so it is actually best to leave that support out for the time being until that work settles.

ETW is an API, not a program. You need to adjust the code generation to work with it. There’s no command line interface to it AFAIK.

@stevapple
Copy link
Contributor Author

stevapple commented Sep 16, 2020

Based on your description, the most suitable plan for now is:

  • Disable profiling on Windows, and add ETW support to TODO;
  • Disable sanitizer support on Windows, until the LLVM sanitizers are stable.

If it’s okay, I’ll comment out the current codes and reject those flags. This PR and the one in compnerd/swift-build will be closed.

@compnerd
Copy link
Member

compnerd commented Sep 16, 2020

Sure, that sounds fine, though I think that the TODOs are probably better served as issues on JIRA. I don't think that rejecting the flags is necessary, and it unnecessarily increases the amount of work to even test things. I think it is better to just setup everything, but indicate that the features are known to not work via the readme (or emitting a note/warning that the functionality is not expected to work yet)

@stevapple
Copy link
Contributor Author

stevapple commented Sep 16, 2020

Created SR-13557

Also, see swiftlang/swift-driver#252 for the current solution

@shahmishal
Copy link
Member

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

  • How to change the base branch: (Link)
  • More detail about the branch update: (Link)

@shahmishal shahmishal closed this Oct 5, 2020
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