compiler-builtins icon indicating copy to clipboard operation
compiler-builtins copied to clipboard

Parts of libcompiler_builtins are compiled as WX

Open djc opened this issue 7 years ago • 6 comments

Similar to https://github.com/rust-lang/rust/issues/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

djc avatar Jul 24 '17 19:07 djc

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?

alexcrichton avatar Jul 24 '17 21:07 alexcrichton

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",
+                        ],
+                    );
+                }
             }
         }

djc avatar Jul 25 '17 07:07 djc

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.

alexcrichton avatar Jul 25 '17 14:07 alexcrichton

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

djc avatar Jul 25 '17 15:07 djc

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

alexcrichton avatar Jul 25 '17 21:07 alexcrichton

Indeed, those files should only be built for Windows. See also https://github.com/llvm-mirror/compiler-rt/commit/1955731016e1b27c652692c70fced5ff72b403ad

DimitryAndric avatar Dec 24 '17 21:12 DimitryAndric