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

Stabilize ptr::slice_from_raw_parts[_mut] #68234

Open
wants to merge 2 commits into
base: master
from

Conversation

@CAD97
Copy link
Contributor

CAD97 commented Jan 14, 2020

Closes #36925, the tracking issue.
Initial impl: #60667

r? @rust-lang/libs

In addition to stabilizing, I've adjusted the example of ptr::slice_from_raw_parts to use slice_from_raw_parts instead of slice_from_raw_parts_mut, which was unnecessary for the example as written.

@CAD97 CAD97 requested a review from oli-obk Jan 14, 2020
@CAD97

This comment has been minimized.

Copy link
Contributor Author

CAD97 commented Jan 14, 2020

Hm right let's inform the bots about the review assignee properly

I confused it by asking for a group 😅

So I just picked who GitHub recommended

r? @oli-obk

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 14, 2020

The job mingw-check of your PR failed (pretty log, 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.
2020-01-14T21:48:23.5765249Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-14T21:48:23.5862894Z ##[command]git config gc.auto 0
2020-01-14T21:48:23.6124815Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-14T21:48:23.6198665Z ##[command]git config --get-all http.proxy
2020-01-14T21:48:23.6353326Z ##[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/68234/merge:refs/remotes/pull/68234/merge
---
2020-01-14T21:53:12.4201121Z   local time: Tue Jan 14 21:53:12 UTC 2020
2020-01-14T21:53:12.6971069Z   network time: Tue, 14 Jan 2020 21:53:12 GMT
2020-01-14T21:53:12.6975506Z == end clock drift check ==
2020-01-14T21:53:13.6312567Z 
2020-01-14T21:53:13.6419152Z ##[error]Bash exited with code '1'.
2020-01-14T21:53:13.6450036Z ##[section]Starting: Checkout
2020-01-14T21:53:13.6452827Z ==============================================================================
2020-01-14T21:53:13.6452883Z Task         : Get sources
2020-01-14T21:53:13.6452928Z 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)

@CAD97

This comment has been minimized.

Copy link
Contributor Author

CAD97 commented Jan 14, 2020

Actual error:

Click to expand the log.
2020-01-14T21:53:12.3332167Z error: the feature `slice_from_raw_parts` has been stable since 1.42.0 and no longer requires an attribute to enable
2020-01-14T21:53:12.3333402Z --> src/liballoc/lib.rs:107:12
2020-01-14T21:53:12.3333973Z |
2020-01-14T21:53:12.3334715Z 107 | #![feature(slice_from_raw_parts)]
2020-01-14T21:53:12.3335426Z | ^^^^^^^^^^^^^^^^^^^^
2020-01-14T21:53:12.3335944Z |
2020-01-14T21:53:12.3336914Z = note: `-D stable-features` implied by `-D warnings`
2020-01-14T21:53:12.3337186Z
2020-01-14T21:53:12.3957867Z error: aborting due to previous error
2020-01-14T21:53:12.3958825Z
2020-01-14T21:53:12.4101399Z error: could not compile `alloc`.
2020-01-14T21:53:12.4102638Z
2020-01-14T21:53:12.4103349Z To learn more, run the command again with --verbose.
2020-01-14T21:53:12.4176865Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2020-01-14T21:53:12.4177172Z Build completed unsuccessfully in 0:02:23
2020-01-14T21:53:12.4178280Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "-Zconfig-profile" "--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/libtest/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-01-14T21:53:12.4178532Z expected success, got: exit code: 101
2020-01-14T21:53:12.4192562Z == clock drift check ==
2020-01-14T21:53:12.4201121Z local time: Tue Jan 14 21:53:12 UTC 2020
2020-01-14T21:53:12.6971069Z network time: Tue, 14 Jan 2020 21:53:12 GMT
2020-01-14T21:53:12.6975506Z == end clock drift check == 

cc @TimNN: error fragments did not include error message

(rebased)

@CAD97 CAD97 force-pushed the CAD97:slice-from-raw-parts branch from 946656d to 5685650 Jan 14, 2020
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 14, 2020

The job mingw-check of your PR failed (pretty log, 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.
2020-01-14T22:56:47.5748686Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-14T22:56:47.5844440Z ##[command]git config gc.auto 0
2020-01-14T22:56:47.5915144Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-14T22:56:47.5969615Z ##[command]git config --get-all http.proxy
2020-01-14T22:56:47.6097637Z ##[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/68234/merge:refs/remotes/pull/68234/merge
---
2020-01-14T23:01:21.7860515Z   local time: Tue Jan 14 23:01:21 UTC 2020
2020-01-14T23:01:22.0794347Z   network time: Tue, 14 Jan 2020 23:01:22 GMT
2020-01-14T23:01:22.0795050Z == end clock drift check ==
2020-01-14T23:01:23.1927466Z 
2020-01-14T23:01:23.2020318Z ##[error]Bash exited with code '1'.
2020-01-14T23:01:23.2045319Z ##[section]Starting: Checkout
2020-01-14T23:01:23.2047604Z ==============================================================================
2020-01-14T23:01:23.2047660Z Task         : Get sources
2020-01-14T23:01:23.2047709Z 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)

@CAD97 CAD97 force-pushed the CAD97:slice-from-raw-parts branch 2 times, most recently from 5a36e2f to b8cd7ed Jan 14, 2020
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 15, 2020

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, 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.
2020-01-14T23:14:41.2390729Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-14T23:14:41.2401215Z ##[command]git config gc.auto 0
2020-01-14T23:14:41.2403543Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-14T23:14:41.2405483Z ##[command]git config --get-all http.proxy
2020-01-14T23:14:41.2407968Z ##[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/68234/merge:refs/remotes/pull/68234/merge
---
2020-01-15T00:09:17.4627714Z .................................................................................................... 1600/9519
2020-01-15T00:09:22.5857444Z .................................................................................................... 1700/9519
2020-01-15T00:09:31.3588013Z .................................................................................................... 1800/9519
2020-01-15T00:09:40.1214965Z ........i........................................................................................... 1900/9519
2020-01-15T00:09:46.8111555Z ..................................................................................................ii 2000/9519
2020-01-15T00:10:02.6149225Z iii................................................................................................. 2100/9519
2020-01-15T00:10:10.6691647Z .................................................................................................... 2300/9519
2020-01-15T00:10:13.0803614Z .................................................................................................... 2400/9519
2020-01-15T00:10:18.7404644Z .................................................................................................... 2500/9519
2020-01-15T00:10:38.6507327Z .................................................................................................... 2600/9519
---
2020-01-15T00:13:14.0048351Z .........................................i...............i.......................................... 4900/9519
2020-01-15T00:13:22.9962692Z .................................................................................................... 5000/9519
2020-01-15T00:13:29.3522958Z ....................................................................................i............... 5100/9519
2020-01-15T00:13:34.6776172Z .................................................................................................... 5200/9519
2020-01-15T00:13:44.6971247Z .......................................................ii.ii...........i............................ 5300/9519
2020-01-15T00:13:53.6581837Z .................................................................................................... 5500/9519
2020-01-15T00:14:03.5524039Z .................................................................................................... 5600/9519
2020-01-15T00:14:09.9470126Z ........................................i........................................................... 5700/9519
2020-01-15T00:14:16.4947168Z .................................................................................................... 5800/9519
2020-01-15T00:14:16.4947168Z .................................................................................................... 5800/9519
2020-01-15T00:14:27.0726464Z .................................................................................................... 5900/9519
2020-01-15T00:14:36.5906778Z ................................ii..i..ii...........i............................................... 6000/9519
2020-01-15T00:14:54.9351648Z .................................................................................................... 6200/9519
2020-01-15T00:15:02.9353782Z .................................................................................................... 6300/9519
2020-01-15T00:15:02.9353782Z .................................................................................................... 6300/9519
2020-01-15T00:15:14.1717110Z ...........................................................i..ii.................................... 6400/9519
2020-01-15T00:15:41.0651223Z .................................................................................................... 6600/9519
2020-01-15T00:15:43.1751666Z ...................................i................................................................ 6700/9519
2020-01-15T00:15:45.3288775Z .................................................................................................... 6800/9519
2020-01-15T00:15:48.6248598Z ...................................i................................................................ 6900/9519
---
2020-01-15T00:17:23.0992097Z .................................................................................................... 7500/9519
2020-01-15T00:17:27.4297811Z .................................................................................................... 7600/9519
2020-01-15T00:17:33.4029415Z .................................................................................................... 7700/9519
2020-01-15T00:17:40.4537269Z .................................................................................................... 7800/9519
2020-01-15T00:17:50.2435714Z ....................................................................................iiii............ 7900/9519
2020-01-15T00:18:06.6677648Z ..................i......i.......................................................................... 8100/9519
2020-01-15T00:18:11.7633433Z .................................................................................................... 8200/9519
2020-01-15T00:18:25.0072157Z .................................................................................................... 8300/9519
2020-01-15T00:18:34.9342721Z .................................................................................................... 8400/9519
---
2020-01-15T00:21:01.7045138Z  finished in 7.025
2020-01-15T00:21:01.7045460Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-15T00:21:01.7045501Z 
2020-01-15T00:21:01.7045559Z running 166 tests
2020-01-15T00:21:04.3612604Z iiii......i........ii..iiii...i....i...........i............i..i..................i....i............ 100/166
2020-01-15T00:21:06.5778734Z i.i.i...iii..iiiiiii.......................iii............ii......
2020-01-15T00:21:06.5781955Z 
2020-01-15T00:21:06.5784874Z  finished in 5.475
2020-01-15T00:21:06.5975864Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-15T00:21:06.7658156Z 
---
2020-01-15T00:21:08.6888350Z  finished in 2.091
2020-01-15T00:21:08.7071229Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-15T00:21:09.7079345Z 
2020-01-15T00:21:09.7079474Z running 9 tests
2020-01-15T00:21:09.7080414Z iiiiiiiii
2020-01-15T00:21:09.7080836Z 
2020-01-15T00:21:09.7080880Z  finished in 0.157
2020-01-15T00:21:09.7081171Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-15T00:21:09.7081340Z 
---
2020-01-15T00:21:28.5132330Z  finished in 19.631
2020-01-15T00:21:28.5359768Z Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-15T00:21:29.2145533Z 
2020-01-15T00:21:29.2145668Z running 116 tests
2020-01-15T00:21:53.2259173Z .iiiii..i.....i..i...i..i.i.i..i..i..ii....i.i....ii..........iiii..........i.....i..i.......ii.i.ii 100/116
2020-01-15T00:21:56.5994707Z .....iiii.....ii
2020-01-15T00:21:56.5996524Z 
2020-01-15T00:21:56.5997096Z  finished in 28.063
2020-01-15T00:21:56.6003264Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-15T00:21:56.6003931Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2020-01-15T00:34:16.1412730Z   local time: Wed Jan 15 00:34:16 UTC 2020
2020-01-15T00:34:16.2071362Z   network time: Wed, 15 Jan 2020 00:34:16 GMT
2020-01-15T00:34:16.2073678Z == end clock drift check ==
2020-01-15T00:34:16.5678924Z 
2020-01-15T00:34:16.5746707Z ##[error]Bash exited with code '1'.
2020-01-15T00:34:16.5783876Z ##[section]Starting: Checkout
2020-01-15T00:34:16.5785906Z ==============================================================================
2020-01-15T00:34:16.5785976Z Task         : Get sources
2020-01-15T00:34:16.5786024Z 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)

@CAD97 CAD97 force-pushed the CAD97:slice-from-raw-parts branch from b8cd7ed to f76177c Jan 15, 2020
@Centril Centril added the T-libs label Jan 15, 2020
@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jan 15, 2020

This is two free functions in the ptr module:

pub const fn slice_from_raw_parts<T>(data: *const T, len: usize) -> *const [T] {…}
pub const fn slice_from_raw_parts_mut<T>(data: *mut T, len: usize) -> *mut [T] {…}

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Jan 15, 2020

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@Amanieu

This comment has been minimized.

Copy link
Contributor

Amanieu commented Jan 15, 2020

@rfcbot concern &raw

I feel that the usefulness of this feature is tied with that of &raw: if in the end we decide to allow references to uninitialized data (as long as you don't deref them) then this function becomes much less useful since you can do the same thing with the existing slice::from_raw_parts(_mut) methods.

cc rust-lang/unsafe-code-guidelines#77

@CAD97

This comment has been minimized.

Copy link
Contributor Author

CAD97 commented Jan 15, 2020

@Amanieu the reason I want these methods actually has nothing to do with &raw; in fact, I know the allocation to be valid and initialized already and can use the reference functions freely, in theory. (I'm working with type erasing.)

The problem I'm dealing with is pointer provinence. I don't know (in my current generic API setup) whether the raw pointer I'm working with is from a &T or a &mut T.

If I use slice::from_raw_parts, then I'm manifesting a shared &_ reference. Any pointer derived from this reference also has shared provenance, and cannot be used to write. This is obviously incorrect if the actual pointer I'm reconstructing should be able to write. You are never ever allowed to go &_ -> &mut _. This one is definitely wrong no matter the decision around &raw and references to trash.

If I use slice::from_raw_parts_mut, then I'm manifesting a unique &mut _ reference. This one you might be able to argue is potentially fine under a potential memory model, but that would require saying &mut isn't unique until it's dereferenced at a minimum.

And this is ignoring the case of potentially misaligned or null pointers, which are instant UB to use with the reference functions.

Ultimately, I think there are enough reasons to have these functions, even if they aren't strictly required by the memory model (for aligned non-null pointers), because they're a simple helper API when you're working with pointers to begin with that sidestep all of the thorny questions around references in pointer workloads that will remain no matter the specific memory model.

TL;DR: these functions are mainly useful when you don't know provenance (dependent on memory model), alignment, or null (definitely UB today).

@Amanieu

This comment has been minimized.

Copy link
Contributor

Amanieu commented Jan 15, 2020

I'm convinced!

@rfcbot resolve &raw

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Jan 15, 2020

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

@CAD97

This comment has been minimized.

Copy link
Contributor Author

CAD97 commented Jan 15, 2020

I'm just going to go ahead and cc @rust-lang/wg-unsafe-code-guidelines just in case. This is an FCP-merge with some implication on unsafe guidelines.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 15, 2020

For the record I think this stabilization makes sense. It doesn't seem to have many deep implications to me -- it is simply saying that one can construct a slice given a *const T and a usize. It's hard to imagine that not being true through various means (e.g., transmute etc), even if using them would otherwise be relying on unspecified implementation details. So yeah 👍 from me.

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Jan 15, 2020

I feel that the usefulness of this feature is tied with that of &raw: if in the end we decide to allow references to uninitialized data (as long as you don't deref them) then this function becomes much less useful since you can do the same thing with the existing slice::from_raw_parts(_mut) methods.

It still has some uses though -- you still need the raw methods for potentially dangling or out-of-bounds or NULL or unaligned pointers. That seems like a good enough motivation to me.

src/libcore/ptr/mod.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.