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

Expose Linux syscall interface #63745

Closed
wants to merge 8 commits into from

Conversation

newpavlov
Copy link
Contributor

@newpavlov newpavlov commented Aug 20, 2019

Based on the syscall crate. Right now only x86, x86-64, arm and aarch64 architectures are supported.

This PR adds basic functions which after stabilization will allow usage of Linux syscalls on stable Rust. Syscall constants and a convenience macro imitating a variadic function can reside in third-party crates (e.g. in the aforementioned syscall). Since AFAIK only Linux guarantees stability of the syscall API, I've omitted code for FreeBSD and macOS. It was proposed to add those functions to core::os, but I think it's worth to start with a more conservative approach.

For an earlier discussion see: https://internals.rust-lang.org/t/10614

cc @kmcallister

@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 20, 2019
@rust-highfive

This comment has been minimized.

@jonas-schievink
Copy link
Contributor

Are all those #[inline(always)] needed, or would #[inline] be enough? inline(always) is respected even in debug mode, which is often unnecessary.

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed needs-fcp This change is insta-stable, so needs a completed FCP to proceed. labels Aug 20, 2019
@Centril
Copy link
Contributor

Centril commented Aug 20, 2019

Since this is unstable it doesn't need FCP, but the use of inline assembly will need T-lang approval.

@rust-highfive

This comment has been minimized.

@newpavlov
Copy link
Contributor Author

@jonas-schievink
I think you always want syscalls inlined since they are extremely tiny, so why leave this decision up to compiler?

@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-08-20T15:19:49.9873241Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-08-20T15:19:50.0076607Z ##[command]git config gc.auto 0
2019-08-20T15:19:50.0116659Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-08-20T15:19:50.0181868Z ##[command]git config --get-all http.proxy
2019-08-20T15:19:50.0330688Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63745/merge:refs/remotes/pull/63745/merge
---
2019-08-20T15:20:25.1346214Z do so (now or later) by using -b with the checkout command again. Example:
2019-08-20T15:20:25.1346242Z 
2019-08-20T15:20:25.1346451Z   git checkout -b <new-branch-name>
2019-08-20T15:20:25.1346479Z 
2019-08-20T15:20:25.1346523Z HEAD is now at d7dd4db7e Merge 5aa44a689b0c0bfb1790d70542fec3e98b26d44f into 51879c3abaedb926739095d19a2af638ee6a07d8
2019-08-20T15:20:25.1529356Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-08-20T15:20:25.1532255Z ==============================================================================
2019-08-20T15:20:25.1532309Z Task         : Bash
2019-08-20T15:20:25.1532355Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-08-20T15:25:57.9661785Z     Checking hashbrown v0.5.0
2019-08-20T15:25:58.9906429Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:58.9906973Z   --> src/libstd/os/linux/syscall/mod.rs:15:1
2019-08-20T15:25:58.9907213Z    |
2019-08-20T15:25:58.9907529Z 14 | /// Execute syscall with 0 arguments.
2019-08-20T15:25:58.9907892Z    | ------------------------------------- previous doc comment
2019-08-20T15:25:58.9908205Z 15 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:58.9908659Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:58.9908908Z    |
2019-08-20T15:25:58.9909538Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:58.9922893Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:58.9923255Z   --> src/libstd/os/linux/syscall/mod.rs:22:1
2019-08-20T15:25:58.9923574Z    |
2019-08-20T15:25:58.9923932Z 21 | /// Execute syscall with 1 argument.
2019-08-20T15:25:58.9923932Z 21 | /// Execute syscall with 1 argument.
2019-08-20T15:25:58.9924277Z    | ------------------------------------ previous doc comment
2019-08-20T15:25:58.9924775Z 22 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:58.9925217Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:58.9925503Z    |
2019-08-20T15:25:58.9926152Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:58.9941656Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:58.9941996Z   --> src/libstd/os/linux/syscall/mod.rs:29:1
2019-08-20T15:25:58.9942847Z    |
2019-08-20T15:25:58.9943172Z 28 | /// Execute syscall with 2 arguments.
2019-08-20T15:25:58.9943172Z 28 | /// Execute syscall with 2 arguments.
2019-08-20T15:25:58.9943479Z    | ------------------------------------- previous doc comment
2019-08-20T15:25:58.9944191Z 29 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:58.9945047Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:58.9945303Z    |
2019-08-20T15:25:58.9945805Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:58.9946165Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:58.9946488Z   --> src/libstd/os/linux/syscall/mod.rs:36:1
2019-08-20T15:25:58.9946718Z    |
2019-08-20T15:25:58.9947008Z 35 | /// Execute syscall with 3 arguments.
2019-08-20T15:25:58.9947008Z 35 | /// Execute syscall with 3 arguments.
2019-08-20T15:25:58.9947394Z    | ------------------------------------- previous doc comment
2019-08-20T15:25:58.9947718Z 36 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:58.9948294Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:58.9948537Z    |
2019-08-20T15:25:58.9948959Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:58.9959031Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:58.9959393Z   --> src/libstd/os/linux/syscall/mod.rs:43:1
2019-08-20T15:25:58.9959645Z    |
2019-08-20T15:25:58.9960092Z 42 | /// Execute syscall with 4 arguments.
2019-08-20T15:25:58.9960092Z 42 | /// Execute syscall with 4 arguments.
2019-08-20T15:25:58.9960582Z    | ------------------------------------- previous doc comment
2019-08-20T15:25:58.9990111Z 43 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:58.9990526Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:58.9990778Z    |
2019-08-20T15:25:58.9991389Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:59.0032239Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:59.0032570Z   --> src/libstd/os/linux/syscall/mod.rs:52:1
2019-08-20T15:25:59.0032804Z    |
2019-08-20T15:25:59.0033517Z 51 | /// Execute syscall with 5 arguments.
2019-08-20T15:25:59.0033517Z 51 | /// Execute syscall with 5 arguments.
2019-08-20T15:25:59.0033858Z    | ------------------------------------- previous doc comment
2019-08-20T15:25:59.0034182Z 52 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:59.0034932Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:59.0035348Z    |
2019-08-20T15:25:59.0035802Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:59.0036156Z error: an inner attribute is not permitted following an outer doc comment
2019-08-20T15:25:59.0036446Z   --> src/libstd/os/linux/syscall/mod.rs:61:1
2019-08-20T15:25:59.0036673Z    |
2019-08-20T15:25:59.0036960Z 60 | /// Execute syscall with 6 arguments.
2019-08-20T15:25:59.0036960Z 60 | /// Execute syscall with 6 arguments.
2019-08-20T15:25:59.0037306Z    | ------------------------------------- previous doc comment
2019-08-20T15:25:59.0037626Z 61 | #![unstable(feature = "linux_syscall", issue = "0")]
2019-08-20T15:25:59.0038007Z    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not permitted following an outer attibute
2019-08-20T15:25:59.0038410Z    |
2019-08-20T15:25:59.0038980Z    = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.
2019-08-20T15:25:59.0039041Z 
2019-08-20T15:26:01.4472243Z error[E0061]: this function takes 6 parameters but 7 parameters were supplied
2019-08-20T15:26:01.4473297Z   --> src/libstd/os/linux/syscall/mod.rs:66:5
2019-08-20T15:26:01.4473848Z    |
2019-08-20T15:26:01.4475138Z 66 |       platform::syscall5(n, a1, a2, a3, a4, a5, a6)
2019-08-20T15:26:01.4476541Z    | 
2019-08-20T15:26:01.4476541Z    | 
2019-08-20T15:26:01.4477155Z   ::: src/libstd/os/linux/syscall/x86_64.rs:54:1
2019-08-20T15:26:01.4477681Z    |
2019-08-20T15:26:01.4478524Z 54 | / pub unsafe fn syscall5(n: usize, a1: usize, a2: usize, a3: usize,
2019-08-20T15:26:01.4479129Z 55 | |                                 a4: usize, a5: usize) -> usize {
2019-08-20T15:26:01.4479903Z 56 | |     let ret : usize;
2019-08-20T15:26:01.4480558Z 57 | |     asm!("syscall" : "={rax}"(ret)
2019-08-20T15:26:01.4482126Z 62 | |     ret
2019-08-20T15:26:01.4482693Z 63 | | }
2019-08-20T15:26:01.4483273Z    | |_- defined here
2019-08-20T15:26:01.4483536Z 
2019-08-20T15:26:01.4483536Z 
2019-08-20T15:26:01.7932348Z error: aborting due to 8 previous errors
2019-08-20T15:26:01.7932610Z 
2019-08-20T15:26:01.7933103Z For more information about this error, try `rustc --explain E0061`.
2019-08-20T15:26:01.8301099Z error: Could not compile `std`.
2019-08-20T15:26:01.8301207Z 
2019-08-20T15:26:01.8301482Z To learn more, run the command again with --verbose.
2019-08-20T15:26:01.8333695Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json-render-diagnostics"
2019-08-20T15:26:01.8347038Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2019-08-20T15:26:01.8347127Z Build completed unsuccessfully in 0:02:45
2019-08-20T15:26:01.8400698Z == clock drift check ==
2019-08-20T15:26:01.8825655Z   local time: Tue Aug 20 15:26:01 UTC 2019
2019-08-20T15:26:01.8825655Z   local time: Tue Aug 20 15:26:01 UTC 2019
2019-08-20T15:26:02.0871058Z   network time: Tue, 20 Aug 2019 15:26:02 GMT
2019-08-20T15:26:02.0872024Z == end clock drift check ==
2019-08-20T15:26:09.1046132Z ##[error]Bash exited with code '1'.
2019-08-20T15:26:09.1107950Z ##[section]Starting: Checkout
2019-08-20T15:26:09.1109871Z ==============================================================================
2019-08-20T15:26:09.1109938Z Task         : Get sources
2019-08-20T15:26:09.1109984Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@newpavlov
Copy link
Contributor Author

Should I create a tracking issue now or wait for T-lang approval?

@Centril
Copy link
Contributor

Centril commented Aug 20, 2019

Now would be preferable. The language team will need to approve stabilization itself later.

@newpavlov newpavlov mentioned this pull request Aug 20, 2019
2 tasks
Co-Authored-By: Peter Atashian <retep998@gmail.com>
let ret : usize;

//
// this fails when building without optimizations:
Copy link
Member

@joshtriplett joshtriplett Aug 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would something like the optimize attribute work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could, but since optimize attribute is ignored in the presence of -C opt-level=n, I am not sure if we should use it. Although it's worth to check if this bug is still present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@newpavlov I'm really surprised that optimize is ignored based on command-line options; this is a good use case for it always working.

I'd love to avoid this extra overhead if possible, but this doesn't need to be a blocker.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be concerned if optimize(...) would impact whether the build is successful / not. optimize(...) is a hint and has zero guarantees.

#[inline(always)]
pub unsafe fn syscall0(n: usize) -> usize {
let ret : usize;
asm!("int $$0x80" : "={eax}"(ret)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be calling via the VDSO instead?

@joshtriplett
Copy link
Member

joshtriplett commented Aug 24, 2019 via email

@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-08-29T10:45:13.5274711Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-08-29T10:45:13.5462575Z ##[command]git config gc.auto 0
2019-08-29T10:45:13.5543678Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-08-29T10:45:13.5602429Z ##[command]git config --get-all http.proxy
2019-08-29T10:45:13.5745804Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63745/merge:refs/remotes/pull/63745/merge
---
2019-08-29T10:51:16.3680616Z     Checking hashbrown v0.5.0
2019-08-29T10:51:18.0645483Z error[E0544]: multiple stability levels
2019-08-29T10:51:18.0646545Z  --> src/libstd/os/linux/syscall/mod.rs:2:1
2019-08-29T10:51:18.0646935Z   |
2019-08-29T10:51:18.0647366Z 2 | #![unstable(feature = "linux_syscall", issue = "63748")]
2019-08-29T10:51:18.0647972Z 
2019-08-29T10:51:19.8038560Z error: aborting due to previous error
2019-08-29T10:51:19.8039257Z 
2019-08-29T10:51:19.8374687Z error: Could not compile `std`.
---
2019-08-29T10:51:19.8452489Z == clock drift check ==
2019-08-29T10:51:19.8477600Z   local time: Thu Aug 29 10:51:19 UTC 2019
2019-08-29T10:51:19.9307915Z   network time: Thu, 29 Aug 2019 10:51:19 GMT
2019-08-29T10:51:19.9311940Z == end clock drift check ==
2019-08-29T10:51:24.2782997Z ##[error]Bash exited with code '1'.
2019-08-29T10:51:24.2810690Z ##[section]Starting: Checkout
2019-08-29T10:51:24.2812318Z ==============================================================================
2019-08-29T10:51:24.2812373Z Task         : Get sources
2019-08-29T10:51:24.2812438Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Sep 9, 2019
@joshtriplett
Copy link
Member

As a data point: C libraries like glibc provide functions like syscall, and it seems useful for us to do the same.

To look at this a slightly different way: if asm! were stable today, I'd still suggest that there's value in having syscall functions in the standard library.

@alexcrichton
Copy link
Member

The purpose of the FCP isn't really to say whether this is a good feature or not, it's purely about it's inclusion in the standard library. To that end Rust has its own style and preference for what goes into the standard library, and while we take inspiration from other languages we don't necessarily copy ideas even if they were successful. For example Rust's standard library is way smaller than Python's, it's also far more portable than C's. I agree it's useful to have Linux syscall utilities in a Rust library but I personally believe it's not fitting in the style of the Rust standard library to have it there when a crates.io crate would suffice.

@newpavlov
Copy link
Contributor Author

newpavlov commented Sep 10, 2019

@alexcrichton

I personally believe it's not fitting in the style of the Rust standard library to have it there when a crates.io crate would suffice.

You can say absolutely the same about all OS-dependent functionality. IIUC std::fs and std::net can live in a crates.io crate without issues as well, they are even more suitable for that than syscalls since they do not have to rely on nightly-only features for a pure-Rust solution (ofc in a sense that we are allowed to use libc/winapi/etc.). Portability argument is also weak in my opinion, we already have non-portable OS-specific functionality and adding syscalls will not change much in this regard.

Syscalls is the most fundamental way of talking to a Linux system, and all other functionality for Linux targets is built on top of it (granted, right now we offload all of the work to libc, but it may change). So I believe having syscalls in std is a very fitting for a systems programming language. Plus third-party crates may become unmaintained (as has happened with the syscall crate), so having it in std will allow authors to depend on raw syscalls more confidently.

@withoutboats
Copy link
Contributor

A few separate comments on the discussion so far:

  • I'm not convinced the problems with the libc interface are enough to justify adding this to std when the libc interface is available on stable using just the libc crate. I realize there's a connection to a libc-free std, but this proposal alone doesn't get us much closer to that because std will still depend on libc just as much as it does today.

  • I think it's really unclear how we will ever move forward with adding any significant additional OS-specific functionality to std. This is connected to a larger sense of stagnancy within std, where I'm unclear if the way we've done things historically has worked out well or not, and it feels like there's very little inertia behind solving the various organizational problems with how std currently handles portability issues.

  • I do not believe this should ever have been tagged T-lang. I find the pattern of overreach in what gets tagged T-lang rather exhausting.

@newpavlov
Copy link
Contributor Author

newpavlov commented Sep 10, 2019

I realize there's a connection to a libc-free std, but this proposal alone doesn't get us much closer to that because std will still depend on libc just as much as it does today.

I like to think about it as a first (small) step in this direction. Also note that there is a proposal to incrementally port std to syscalls where we don't need libc for interoperability with C/C++. (Fully libc-free std will need a separate target as was proposed by @japaric in the linked RFC issue)

Plus it's not only about libc-free std, but also about writing libraries and applications which use syscalls directly instead of linking to shared libraries (e.g. liburing), which may not be present on a user system. Yes, we can go via libc's syscall or use cc to link externals .S files, but it looks like a needless indirection to me.

@withoutboats
Copy link
Contributor

I understand, but right now we haven't made any commitment to follow that plan through. If we had decided it was an important goal for the project to have a libc-free std, I'd be much more in favor of this API addition.

@joshtriplett
Copy link
Member

joshtriplett commented Sep 11, 2019 via email

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 11, 2019

@joshtriplett

if asm! were stable today, I'd still suggest that there's value in having syscall functions in the standard library.

What would that value be?

@alexcrichton
Copy link
Member

I am by no means the only voice of the library team, I'm just writing down my own personal opinions. Others may feel quite differently!

I agree with others that the idea of a libc-less target should not at all be anywhere near the motivation for this PR, such a target existing and whether or not we include the syscalls in libstd is both orthogonal in terms of API design and also orthogonal in terms of technical implementation. This proposal must be worthy in its own right regardless of whether a libc-less target is considered.

@ojeda
Copy link
Contributor

ojeda commented Sep 12, 2019

In a way, these functions constitute the "FFI to the kernel", because they allow to call and pass data to it. And that fits pretty well in std::ffi's description: "This module provides utilities to handle data across non-Rust interfaces, like (...) the underlying operating system".

For example Rust's standard library (...) it's also far more portable than C's.

What do you mean? C's standard library is probably the most widely ported library.

@retep998
Copy link
Member

What do you mean? C's standard library is probably the most widely ported library.

Code using the C standard library is not that portable though. The C standard library on Windows is a trash fire where getting unicode support in filesystem paths requires using non-portable wide versions of everything, and there's so many platform and compiler specific extensions to the C standard library that many software relies on. The Rust standard library is much more consistent and sane across the set of platforms it supports than the C standard library.

@ojeda
Copy link
Contributor

ojeda commented Sep 13, 2019

Code using the C standard library is not that portable though. The C standard library on Windows is a trash fire where getting unicode support in filesystem paths requires using non-portable wide versions of everything

That's on a given implementation and their decisions, not on C. It does not make the code non-portable either. For instance, you may be fine using fopen if you know before-hand the set of filenames to be open. Similarly, some platforms do not even have files, but we do not call all C code non-portable just because of them.

there's so many platform and compiler specific extensions to the C standard library that many software relies on.

Those extensions are not the C standard library. If software relies on them, that's a dependency. The same way we would have a dependency if we had to use the syscall crate here. We could also make unworldly claims like "Rust's standard library is fundamentally non-portable given std::os", but I think we agree that wouldn't be useful either.

This is why I think syscall being in std does not really subtract from Rust's portability (and, in fact, makes std more complete, considering std::os/std::ffi are already there).

The Rust standard library is much more consistent and sane across the set of platforms it supports than the C standard library.

Now that is true. The C spec is dated and, if it were to be written nowadays, it would probably be way better (e.g. w.r.t. enforcing Unicode support). However, we should note that the set of C platforms is way bigger: it is way easier to be consistent and sane across every single platform if you have less platforms to begin with (you are perfectly so if your set's size is 1 ;-), so let's give credit where credit is due.

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 13, 2019

It was claimed that these APIs add value even if we were to stabilize asm! and that comment has 6 thumbs up as of today, so I take that as a hint that a lot of users agree with that and I'm missing something.

I can't imagine What would that value be? because if we were to stabilize asm!, cargo add syscalls would make these APIs available to all Rust code without any downsides AFAICT.

From the amount of discussion it is at least clear to me that this is RFC material. I don't really expect an RFC for this to be able to answer that question (*), but I'm still intrigued about what am I missing.

(*) I think saying that this adds no value if asm! were to be stabilized would be ok because asm! is not stable today, and it is unclear when that would be. global_asm! looks better though, and using the cc crate works on stable Rust today, at the inconvenience of requiring a C compiler, but if you are using Linux syscalls, such a compiler is always available AFAICT.

@newpavlov
Copy link
Contributor Author

newpavlov commented Sep 13, 2019

@gnzlbg
As I've stated here, syscalls is the most fundamental way of talking to Linux. Considering that we already expose a fair amount of OS-dependent higher-level functionality in std, I don't see why syscalls should not be exposed as well. And it's a really low-level piece of code which easy to implement wrong. This is why having it in std is important in my opinion from user confidence PoV (as I've noted earlier syscalls crate is not maintained anymore). And I don't think it will be a big maintenance burden, since after syscalls API is "done", it's highly unlikely to require any changes in future (well, apart from introducing changes to the asm! macro). global_asm! and cc are workarounds, not proper solutions in my opinion, e.g. you want syscalls to be inlined if possible (LTO may help here, but I am not sure).

I don't see the point of writing an RFC after seeing the feedback from the team members. What will be changed by doing it? It will be just a waste of time to write an RFC which will be closed shortly afterwards...

I guess there is an idea of introducing a separate no_std crate for syscalls which will be distributed together with Rust releases (like test, alloc, etc.). It would allow to side-step the issue of adding syscalls to core and it will allow us to use asm! for syscalls. But it will be even a bigger change, than the one proposed in this PR, so I doubt it will be accepted.

@bjorn3
Copy link
Member

bjorn3 commented Sep 13, 2019

A disadventage of introducing this api, is that it makes asm support more important for a codegen backend. For cg_clif I currently already had to patch away a few inline asm sections. In most of these cases, I could just make them panic, as they should only be called when certain target features are enabled. Disabling those would just fix it. For syscalls, that would obviously not work.

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 13, 2019

@newpavlov

I don't see the point of writing an RFC after seeing the feedback from the team members. What will be changed by doing it? It will be just a waste of time to write an RFC which will be closed shortly afterwards...

There is a real problem here that is affecting a couple of users and for which the current workarounds aren't great. I think that a good solution for solving this problem has high chances of going successfully through the RFC process.

The feedback of the team members isn't about such an RFC, but about this PR in particular, which proposes a particular solution to that problem. This might very well be the best solution to the problem that there is, but from the information provided, nobody can really tell.

@ojeda
Copy link
Contributor

ojeda commented Sep 13, 2019

If using asm! is an issue, we could always link an assembly file. The key is whether std should provide (or not) the functions.

@bill-myers
Copy link
Contributor

bill-myers commented Sep 13, 2019

It seems a good idea to add this to libstd, since in addition to making it available to users on stable, it can then be used to incrementally change libstd code to directly make Linux syscalls instead of calling libc, which is slightly faster and could eventually allow to remove the libc dependency completely, at least for some programs, allowing to have smaller statically linked Rust binaries.

Regarding the objection of making it private, the Linux syscall interface is immutable and this is the canonical way of exposing it, so once the code is in libstd we might as well expose it.

@Ixrec
Copy link
Contributor

Ixrec commented Sep 14, 2019

To try and proactively prevent any splitting of this discussion: the bulk of the activity was shifted back to the internals thread with this post yesterday.

@rfcbot
Copy link

rfcbot commented Sep 18, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 18, 2019
@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Sep 28, 2019
@rfcbot
Copy link

rfcbot commented Sep 28, 2019

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Sep 28, 2019
@alexcrichton
Copy link
Member

Ok, I'm going to close this due to the FCP conclusion

@newpavlov newpavlov deleted the linux_syscall branch September 30, 2019 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.