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

Compiling against libcore with a soft-float target appears to require sin/cos/sqrt since recently #52301

Closed
alex opened this issue Jul 12, 2018 · 19 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries A-target-specs Area: Compile-target specifications C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alex
Copy link
Member

alex commented Jul 12, 2018

Since #51695 (I believe), the project I have for compiling linux kernel modules has started failing due to sin/cos/sqrt symbols not being available: fishinabarrel/linux-kernel-module-rust#61 (comment)

This uses a custom target file which is x86_64 with +soft-float.

I believe the issue is that stdsimd uses the llvm.sin.* intrinsics, and LLVM will emit a call to libm if the platform is soft-float (this is speculation, but well founded I think :-)).

Being able to depend on libcore without needing any additional runtime libraries is a hard requirement for compiling for targets like kernel modules. I'm not sure what the right direction for resolving this is.

@alex alex mentioned this issue Jul 12, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 12, 2018

I think we just have to fix this in stdsimd, by requiring those symbols only when used through std and not when used through core. @alexcrichton thoughts?

To test this upstream, maybe we could try to add some soft float targets that reproduce the issue, and then commit a fix. We will probably need a stdbuild feature in stdsimd (like the libc one) to detect when it is being built as part of std or core.


Or in other words, the real issue here is that floats are part of the language, but some operations on float require library support on some targets, and therefore those operations cannot be part of core.

@alexcrichton
Copy link
Member

I believe this isn't the first of its kind for libm intrinsics in libcore, but the general story around them is "if you use --gc-sections and don't use the code that uses the intrinsics it doesn't matter" . @alex do you know if you're compiling with --gc-sections and/or enabling that sort of flag in the linker?

@alex
Copy link
Member Author

alex commented Jul 12, 2018 via email

@alexcrichton
Copy link
Member

Ah ok, in general Rust code is designed to be compiled with --gc-sections, otherwise there's binary bloat issues as well. I believe that compiling with --gc-sections will remove the dependence on these symbols as you're not likely using the code anyway

@alex
Copy link
Member Author

alex commented Jul 12, 2018

@alexcrichton I haven't dug in any depth, but adding --gc-sections doesn't appear to have resolved it (fishinabarrel/linux-kernel-module-rust#61 latest commit + travis run).

@alexcrichton
Copy link
Member

@alex hm is it possible to get the full linker command line printed out by the makefiles? If these symbols are still being referenced when --gc-sections is passed that's perhaps a bug! Or perhaps a facet of another flag being passed to the linker.

@geofft
Copy link
Contributor

geofft commented Jul 13, 2018

Or in other words, the real issue here is that floats are part of the language, but some operations on float require library support on some targets, and therefore those operations cannot be part of core.

Yes, this seems right - while digging into this I noticed that f32::sqrt() and f64::sqrt() are in libstd and not libcore specifically for this reason (#27823).

Rethinking that and saying "use --gc-sections" seems reasonable from first principles but I think it's not what libcore/libstd have done historically (or in other words, if you want to do this it seems like it would make sense to reintroduce these functions to libcore :) ).

Also --gc-sections has caused weird bugs for us (see fishinabarrel/linux-kernel-module-rust@7cd1003) so I'm not a huge fan but it's potentially the right thing long-term....

@geofft
Copy link
Contributor

geofft commented Jul 13, 2018

@alexcrichton For what it's worth, --gc-sections does in fact fix this on Ubuntu 18.04 (kernel 4.15.0-1007-aws, binutils 2.30) but not on Travis's Ubuntu 14.04 image (kernel 4.4.0-101-generic, binutils 2.24 I think). On 18.04 the relevant linker command, from running the kernel buildsystem with make V=1, is

  ld -m elf_x86_64 -z max-page-size=0x200000   --entry=init_module --gc-sections -r -o /home/ubuntu/t2/example-sysctl/examplesysctl.o /home/ubuntu/t2/example-sysctl/target/x86_64-linux-kernel-module/debug/libexample_sysctl.a

On Travis, the corresponding command is the same except without -z max-page-size (i.e., it does also include --gc-sections). See https://travis-ci.org/alex/linux-kernel-module-rust/builds/403377295#L731

I wonder if libcore isn't being compiled with -ffunction-sections (or the Rust equivalent of it?) on 14.04 and therefore it can't be GC'd. Note that we are compiling libcore ourselves via cargo-xbuild.

@alexcrichton
Copy link
Member

@geofft FWIW it's always been the case that --gc-sections is recommended with Rust, otherwise rust binaries have a lot of bloat they otherwise don't need. The core/std libraires are compiled to just a handful of object files (used to be just two!) and you don't need 90% of the code most of the time, and we rely on -ffunction-sections and -fdata-sections (turned on by default for Rust) plus --gc-sections to remove all the unneeded code.

While items like sqrt may have been moved from core to std items like a % b which are lowered to fmod by LLVM were not moved. We're somewhat inconsistent about this!

That linker command looks like it's doing a relocatable link (producing another object file with -r) and I think that ignores --gc-sections? Did the build process change between those kernel versions?

@reynoldsbd
Copy link

I think I'm also running up against this issue, although my circumstances are slightly different:

  • Target does not have +soft-float, but the cos/cosf/sin/sinf symbols are still failing to resolve
  • Target uses the lld-link flavor, so --gc-sections is not available (the LINK analog /OPT:REF,NOICF is specified)

@alexcrichton
Copy link
Member

@reynoldsbd I wonder if that's maybe a bug in LLD? Or maybe a version of LLD which didn't have it implemented?

@reynoldsbd
Copy link

@alexcrichton I've tried a few different versions of LLD, including version 7, but the issue does not go away. Could definitely be a bug, but I'm also wondering if PE/COFF linking just has different semantics when it comes to GC'ing. According to the docs linked in my previous comment, /OPT:REF only applies to COMDAT sections. Perhaps the sections referencing these symbols are not COMDATs?

That being said, @gnzlbg's solution of just removing the references when building via core seems more sustainable to me.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 24, 2018

Can you try with the latest nightly again? The modules causing this issue have been removed, so this should have been fixed.

@reynoldsbd
Copy link

@gnzlbg that did it! My issue has disappeared on the latest nightly. Thank you!

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 26, 2018

@reynoldsbd it would be nice if you could point us to an example repo reproducing the issue, so that we can prevent it from reappearing in the future. I tried with the arm soft float targets, but did not manage to reproduce it (they compiled and run the code just fine).

@reynoldsbd
Copy link

reynoldsbd commented Jul 27, 2018

@gnzlbg not exactly a minimal repro, but I have this crate which I have confirmed is impacted by this issue. Just clone, install dependencies listed in the Readme, and make test.

I tested with nightly-2018-07-09 (build failed) and yesterday's nightly (build succeeded).

@jonas-schievink jonas-schievink added A-linkage Area: linking into static, shared libraries and binaries A-target-specs Area: Compile-target specifications C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 6, 2019
@steveklabnik
Copy link
Member

Triage: it seems to me that maybe there's not actually a bug here, or if there is, it's much much larger than the original report. That is, designing rust codgen to not rely on --gc-sections is the "fix" in this case, I guess.

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 10, 2019

@steveklabnik IIUC the bug was fixed more or less around here: #52301 (comment)

Maybe we could add some testing to make sure that libcore compiles with +soft-float, but for that it should be enough to build targets that exercise that. @reynoldsbd provided one here: #52301 (comment)

@alex
Copy link
Member Author

alex commented Nov 3, 2019

This issue has been resolved for the purposes of building linux kernel modules for a while, so I'm fine with closing.

@alex alex closed this as completed Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries A-target-specs Area: Compile-target specifications C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants