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

Parts of libcompiler_builtins are compiled as WX #183

Open
djc opened this issue Jul 24, 2017 · 6 comments
Open

Parts of libcompiler_builtins are compiled as WX #183

djc opened this issue Jul 24, 2017 · 6 comments

Comments

@djc
Copy link

djc commented Jul 24, 2017

Similar to rust-lang/rust#34482, I still see a number of warnings like this when compiling rust-1.19.0:

 * !WX --- --- usr/lib64/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-701380125126dfef.rlib:chkstk.o
 * !WX --- --- usr/lib64/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-701380125126dfef.rlib:chkstk2.o
 * !WX --- --- usr/lib64/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-701380125126dfef.rlib:floatundidf.o
 * !WX --- --- usr/lib64/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-701380125126dfef.rlib:floatundisf.o
 * !WX --- --- usr/lib64/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-701380125126dfef.rlib:floatundixf.o
@alexcrichton
Copy link
Member

Oh dear definitely seems like something to fix! @djc would you be up for submitting a patch for at least the easy piece, removing compilation of chkstk files on non-windows platforms?

@djc
Copy link
Author

djc commented Jul 25, 2017

Uh, I guess. Do you mean like this?

diff --git a/build.rs b/build.rs
index 25cc520..787906f 100644
--- a/build.rs
+++ b/build.rs
@@ -4172,8 +4172,6 @@ mod c {
                     &[
                         "i386/ashldi3.S",
                         "i386/ashrdi3.S",
-                        "i386/chkstk.S",
-                        "i386/chkstk2.S",
                         "i386/divdi3.S",
                         "i386/floatdidf.S",
                         "i386/floatdisf.S",
@@ -4188,6 +4186,14 @@ mod c {
                         "i386/umoddi3.S",
                     ],
                 );
+                if target_os == "windows" {
+                    sources.extend(
+                        &[
+                            "i386/chkstk.S",
+                            "i386/chkstk2.S",
+                        ],
+                    );
+                }
             }
         }

@alexcrichton
Copy link
Member

I think that should do it yeah!

If you'd like as well, it'd be awesome to add a test into this repo to ensure that we don't regress this.

@djc
Copy link
Author

djc commented Jul 25, 2017

I have no clue how I'd test that, but if you can guide me along maybe I can come up with something.

@alexcrichton
Copy link
Member

Right now the test script is just a shell script and has a few other examples, could that be added to?

bors added a commit that referenced this issue Sep 16, 2017
Implement x86 chkstk in "rust"

cc #183

Basically the same as the x86_64 ones, except `__alloca` doesn't need to fix the parameter register. I've manually verified that the disassembly is the same, and that these work in a compiled rust program.

The second commit disables compiling probestack functions for `feature = mangled-names`. They aren't needed during testing because they aren't comparison tested and the unmangled versions are the ones that actually get used.

r? @alexcrichton
@DimitryAndric
Copy link

Indeed, those files should only be built for Windows. See also llvm-mirror/compiler-rt@1955731

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

No branches or pull requests

3 participants