Update compiler-rt for Windows fixes

[angelsl]

r? @alexcrichton

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[angelsl]

Pull request on compiler-rt: rust-lang/compiler-rt#14

[angelsl]

Don't merge yet, doesn't fix Windows MSVC

[alexcrichton]

Thanks for the quick update! I'd like to make sure we add regression tests if possible, however, to ensure that this doesn't happen again.

For the MinGW case I discovered that it's related to linking in C++ code, so could you also add a run-make? I don't think that our C++ compiler detection is the best, so it's fine for the test just to be enabled on MinGW for now and we can enable it elsewhere later. It just needs to be something like:

// foo.cpp
extern "C" void foo() {
    int *a = new int(3);
    delete a;
}

extern { fn foo(); }
fn main() {
    unsafe { foo(); }
}

$ g++ foo.cpp -c
$ ar crus libfoo.a foo.o
$ rustc foo.rs -L. -lfoo -lstdc++
$ ./foo.exe

[alexcrichton]

OK, after further investigation about libcmt/msvcrt, it looks like this was just working by luck beforehand. A few of the objects compiled in that test were not compiled with /MD (which I believe msvcrt.lib corresponds to), and then when compiler-rt.lib was also imposing some restrictions on the CRT in use it ended up causing problems.

This brings up the interesting question, however, of how these files should be compiled. It looks like the MSVC build of compiler-rt may not be quite ready just yet? I would expect it to compile to a library that actually has 0 dependencies, but it looks like it even depends on symbols like VirtualProtect and other CRT functions like fflush perhaps.

I'm not entirely sure what compiler-rt is doing, but it seems like we're pulling in more than we may want.

[bors]

☔️ The latest upstream changes (presumably #29477) made this pull request unmergeable. Please resolve the merge conflicts.

[angelsl]

Updated. @alexcrichton

[alexcrichton]

@bors: r+ 9bb2da3

[bors]

⌛️ Testing commit 9bb2da3 with merge 33424a8...

[bors]

💔 Test failed - auto-win-msvc-64-opt

[angelsl]

Fixed for proper MinGW-only detection. @alexcrichton

[alexcrichton]

@bors: r+ 9fe4e96

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

[vadimcn]

Does anybody know, what happened to @vhbit's SjLj fix introduced in rust-lang/compiler-rt@df1e1ca? I don't see any of that stuff in the current compiler-rt branch. Was this change deemed unnecessary?

[angelsl]

@vadimcn It seems like LLVM upstream incorporated something like @vhbit's changes, so I didn't port that change over.

Not sure if it works though, I have no way to test.

[vhbit]

@vadimcn thanks for bringing this up!
@angelsl

LLVM upstream incorporated something like

nope. As I can see from link you've provided LLVM has just different names of function depending on conditional, while my code has also different handling which is crucial. I'd be happy if those changes are kept otherwise iOS support will be broken.

[angelsl]

@vhbit OK, I cherry-picked your changes. Can you help me see if it looks correct?

Are there any other fixes I should port? ping: @vadimcn as well

[vadimcn]

@angelsl: Can you please rebase on top of the current master? I wanted to pick up the recently added _alloca functions.

@vhbit: If these changes are not Rust-specific, we should upstream them to prevent this kind of thing in the future.
Would be best if you did it yourself, so there'd be no questions about attribution.
But in any case, could you please add some comments? E.g., what's different between this and the DWARF personality routine? Is this code SjLj-specific or ARM-specific?
Thanks!

[vhbit]

@angelsl yep, looks correct

@vadimcn this code is specific to SjLj. I've actually tried to upstream it a while ago but got no response. I'll try to do it again soon or delegate it to someone if will not be able to allocate time.