Document unsafety in `src/libcore/slice/mod.rs` #73555

LeSeulArtichaut · 2020-06-20T17:44:38Z

rust-highfive · 2020-06-20T19:40:59Z

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.

##[section]Starting: Linux mingw-check
##[section]Starting: Initialize job
Agent name: 'Azure Pipelines 3'
Agent machine name: 'fv-az578'
Current agent version: '2.170.1'
##[group]Operating System
16.04.6
LTS
LTS
##[endgroup]
##[group]Virtual Environment
Environment: ubuntu-16.04
Version: 20200614.1
Included Software: https://github.com/actions/virtual-environments/blob/ubuntu16/20200614.1/images/linux/Ubuntu1604-README.md
##[endgroup]
Agent running as: 'vsts'
Prepare build directory.
Set build variables.
Download all required tasks.
Download all required tasks.
Downloading task: Bash (3.163.3)
Checking job knob settings.
   Knob: AgentToolsDirectory = /opt/hostedtoolcache Source: ${AGENT_TOOLSDIRECTORY} 
   Knob: AgentPerflog = /home/vsts/perflog Source: ${VSTS_AGENT_PERFLOG} 
Start tracking orphan processes.
##[section]Finishing: Initialize job
##[section]Starting: Configure Job Name
==============================================================================
---
========================== Starting Command Output ===========================
[command]/bin/bash --noprofile --norc /home/vsts/work/_temp/cfc49bc2-2384-49a7-9c6d-422d9fe9e6e0.sh

##[section]Finishing: Disable git automatic line ending conversion
##[section]Starting: Checkout rust-lang/rust@refs/pull/73555/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
---
##[command]git remote add origin https://github.com/rust-lang/rust
##[command]git config gc.auto 0
##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
##[command]git config --get-all http.proxy
##[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/73555/merge:refs/remotes/pull/73555/merge
---
 ---> a9ec21d337b3
Step 6/7 : ENV RUN_CHECK_WITH_PARALLEL_QUERIES 1
 ---> Using cache
 ---> 5ff2c13d8dba
Step 7/7 : ENV SCRIPT python3 ../x.py test src/tools/expand-yaml-anchors &&            python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu &&            python3 ../x.py build --stage 0 src/tools/build-manifest &&            python3 ../x.py test --stage 0 src/tools/compiletest &&            python3 ../x.py test src/tools/tidy &&            python3 ../x.py doc --stage 0 src/libstd &&            /scripts/validate-toolstate.sh
 ---> 6b931e755c7e
Successfully built 6b931e755c7e
Successfully tagged rust-ci:latest
Built container sha256:6b931e755c7ea1f69816640eb9df74fafd40a545d0e5aa8341d35009dabb0f3c
---
    Checking chalk-rust-ir v0.10.0
    Checking rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
    Checking rustc_hir v0.0.0 (/checkout/src/librustc_hir)
    Checking rustc_session v0.0.0 (/checkout/src/librustc_session)
    Checking rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
    Checking rustc_parse v0.0.0 (/checkout/src/librustc_parse)
    Checking rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
    Checking rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
    Checking rustc_ast_passes v0.0.0 (/checkout/src/librustc_ast_passes)
---
configure: rust.codegen-units-std := 1
configure: rust.verify-llvm-ir  := True
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
Hugepagesize:       2048 kB
DirectMap4k:      145344 kB
DirectMap2M:     3000320 kB
DirectMap1G:     6291456 kB
+ python3 ../x.py test src/tools/expand-yaml-anchors
Ensuring the YAML anchors in the GitHub Actions config were expanded
Ensuring the YAML anchors in the GitHub Actions config were expanded
Building stage0 tool expand-yaml-anchors (x86_64-unknown-linux-gnu)
---
   Compiling linked-hash-map v0.5.2
   Compiling lazy_static v1.4.0
   Compiling yaml-rust v0.4.3
   Compiling quote v1.0.2
   Compiling thiserror-impl v1.0.5
   Compiling thiserror v1.0.5
   Compiling yaml-merge-keys v0.4.0
   Compiling expand-yaml-anchors v0.1.0 (/checkout/src/tools/expand-yaml-anchors)
Build completed successfully in 0:00:30
+ python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu
    Finished dev [unoptimized] target(s) in 0.19s
Checking rustdoc artifacts (x86_64-unknown-linux-gnu -> i686-pc-windows-gnu)
---
    Checking rustc_span v0.0.0 (/checkout/src/librustc_span)
    Checking rustc_errors v0.0.0 (/checkout/src/librustc_errors)
    Checking rustc_feature v0.0.0 (/checkout/src/librustc_feature)
    Checking rustc_parse_format v0.0.0 (/checkout/src/librustc_parse_format)
    Checking chalk-rust-ir v0.10.0
    Checking rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
    Checking rustc_session v0.0.0 (/checkout/src/librustc_session)
    Checking rustc_hir v0.0.0 (/checkout/src/librustc_hir)
    Checking chalk-solve v0.10.0
    Checking rustc_parse v0.0.0 (/checkout/src/librustc_parse)
---
skip untracked path cpu-usage.csv during rustfmt invocations
skip untracked path src/doc/book/ during rustfmt invocations
skip untracked path src/doc/rust-by-example/ during rustfmt invocations
skip untracked path src/llvm-project/ during rustfmt invocations
Diff in /checkout/src/libcore/slice/mod.rs at line 6156:
         // SAFETY: `left` and `right` are references and are thus guaranteed to be valid.
         // We use the minimum of both lengths which guarantees that both regions are
         // valid for reads in that interval.
-        let order = unsafe {
-            memcmp(left.as_ptr(), right.as_ptr(), cmp::min(left.len(), right.len()))
+        let order =
+        let order =
+            unsafe { memcmp(left.as_ptr(), right.as_ptr(), cmp::min(left.len(), right.len())) };
         if order == 0 {
             left.len().cmp(&right.len())
         } else if order < 0 {
Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustfmt" "--config-path" "/checkout" "--edition" "2018" "--unstable-features" "--skip-children" "--check" "/checkout/src/libcore/slice/mod.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.
Build completed unsuccessfully in 0:00:45
== clock drift check ==
  local time: Sat Jun 20 18:07:21 UTC 2020
  network time: Sat, 20 Jun 2020 18:07:21 GMT
  network time: Sat, 20 Jun 2020 18:07:21 GMT
== end clock drift check ==

##[error]Bash exited with code '1'.
##[section]Finishing: Run build
##[section]Starting: Checkout rust-lang/rust@refs/pull/73555/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
Author       : Microsoft
Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
==============================================================================
Cleaning any cached credential from repository: rust-lang/rust (GitHub)
##[section]Finishing: Checkout rust-lang/rust@refs/pull/73555/merge to s
Cleaning up task key
Start cleaning up orphan processes.
Terminate orphan process: pid (3592) (python)
##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

bors · 2020-06-23T11:51:57Z

☔ The latest upstream changes (presumably #73643) made this pull request unmergeable. Please resolve the merge conflicts.

LeSeulArtichaut · 2020-06-23T17:12:08Z

Rebased to resolve the conflicts.

Mark-Simulacrum · 2020-06-26T21:48:11Z

src/libcore/slice/mod.rs

+                // SAFETY: the condition of the `while` guarantees that
+                // `i` and `ln - i - chunk` are inside the slice.
+                // The resulting pointers `pa` and `pb` are therefore valid,
+                // and can be read from and written to.


These comments also need to touch on the larger-pieced bytes being still in-bounds based on the indexing. Personally I would like to see some short proof-y wording, maybe something like this.

Personally I feel like just saying "well the condition guarantees things" doesn't help much since it doesn't allow for modifying or understanding why this is the case.

An unaligned u32 can be read from i if i + 1 < ln (and obviously i < ln), because each element is 2 bytes and we're reading 4. i + chunk - 1 < ln / 2 # while condition i + 2 - 1 < ln / 2 i + 1 < ln / 2 Since it's less than the length divided by 2, then it must be in bounds.

Mark-Simulacrum · 2020-06-26T21:49:05Z

src/libcore/slice/mod.rs

@@ -1548,14 +1571,14 @@ impl<T> [T] {
        while size > 1 {
            let half = size / 2;
            let mid = base + half;
-            // mid is always in [0, size), that means mid is >= 0 and < size.
+            // SAFETY: mid is always in [0, size), that means mid is >= 0 and < size.


Suggested change

// SAFETY: mid is always in [0, size), that means mid is >= 0 and < size.

// SAFETY:

Obviously pre-existing but this is not really saying anything and the next two lines are precisely what we need

Mark-Simulacrum · 2020-06-26T21:52:26Z

src/libcore/slice/mod.rs

@@ -2013,6 +2036,13 @@ impl<T> [T] {
        let mut next_read: usize = 1;
        let mut next_write: usize = 1;

+        // SAFETY: the `while` condition guarantees `next_read` and `next_write`
+        // are less than `len`, thus are inside `self`. `prev_ptr_write` points to


We should make a note that next_write is incremented at most once per loop somewhere in here.

Mark-Simulacrum · 2020-06-26T21:53:46Z

src/libcore/slice/mod.rs

+        // `prev_ptr_write` is never less than 0 and is inside the slice.
+        // This fulfils the requirements for dereferencing `ptr_read`, `prev_ptr_write`
+        // and `ptr_write`, and for using `ptr.add(next_read)`, `ptr.add(next_write - 1)`
+        // and `prev_ptr_write.offset(1)`.


FWIW some of these constructions seem pretty strange, I wouldn't want to edit the code in this PR but ptr.add(next_write - 1) seems like it should be next_write.sub(1).

Mark-Simulacrum · 2020-06-26T22:08:17Z

src/libcore/slice/mod.rs

@@ -2300,6 +2334,9 @@ impl<T> [T] {
        T: Copy,
    {
        assert_eq!(self.len(), src.len(), "destination and source slices have different lengths");
+        // SAFETY: `self` is valid for `self.len()` bytes by definition, and `src` was


Suggested change

// SAFETY: `self` is valid for `self.len()` bytes by definition, and `src` was

// SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was

Mark-Simulacrum · 2020-06-26T22:08:46Z

src/libcore/slice/mod.rs

@@ -2300,6 +2334,9 @@ impl<T> [T] {
        T: Copy,
    {
        assert_eq!(self.len(), src.len(), "destination and source slices have different lengths");
+        // SAFETY: `self` is valid for `self.len()` bytes by definition, and `src` was
+        // checked to have the same length. Both slices cannot be overlapping because


Suggested change

// checked to have the same length. Both slices cannot be overlapping because

// checked to have the same length. The slices cannot overlap because

Mark-Simulacrum · 2020-06-26T22:08:55Z

src/libcore/slice/mod.rs

@@ -2300,6 +2334,9 @@ impl<T> [T] {
        T: Copy,
    {
        assert_eq!(self.len(), src.len(), "destination and source slices have different lengths");
+        // SAFETY: `self` is valid for `self.len()` bytes by definition, and `src` was
+        // checked to have the same length. Both slices cannot be overlapping because
+        // Rust's mutable references are exclusive.


Suggested change

// Rust's mutable references are exclusive.

// mutable references are exclusive.

bors · 2020-07-14T20:28:20Z

☔ The latest upstream changes (presumably #74330) made this pull request unmergeable. Please resolve the merge conflicts.

Dylan-DPC-zz · 2020-08-01T12:26:48Z

Closing this after a discussion with the author since they don't have time to commit to this and it is small enough that can be redone later or someone could pick it up if needed. Thanks

…e-slice, r=LukasKalbertodt Document unsafety in library/core/src/slice/mod.rs Restart where rust-lang#73555 left off, helping with rust-lang#66219.

rust-highfive assigned Mark-Simulacrum Jun 20, 2020

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

LeSeulArtichaut mentioned this pull request Jun 20, 2020

Document all unsafety used in libcore and libstd #66219

Closed

LeSeulArtichaut force-pushed the document-unsafety branch from 1930b7a to bcd9688 Compare June 20, 2020 20:38

Document unsafety in src/libcore/slice/mod.rs

a4095ca

LeSeulArtichaut force-pushed the document-unsafety branch from bcd9688 to a4095ca Compare June 23, 2020 17:10

Mark-Simulacrum reviewed Jun 28, 2020

View reviewed changes

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 28, 2020

Muirrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2020

Dylan-DPC-zz closed this Aug 1, 2020

Dylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 1, 2020

poliorcetics mentioned this pull request Aug 2, 2020

Document unsafety in library/core/src/slice/mod.rs #75066

Merged

LeSeulArtichaut deleted the document-unsafety branch February 18, 2021 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document unsafety in `src/libcore/slice/mod.rs` #73555

Document unsafety in `src/libcore/slice/mod.rs` #73555

LeSeulArtichaut commented Jun 20, 2020

rust-highfive commented Jun 20, 2020

bors commented Jun 23, 2020

LeSeulArtichaut commented Jun 23, 2020

Mark-Simulacrum Jun 26, 2020

Mark-Simulacrum Jun 26, 2020

Mark-Simulacrum Jun 26, 2020

Mark-Simulacrum Jun 26, 2020

Mark-Simulacrum Jun 26, 2020

Mark-Simulacrum Jun 26, 2020

Mark-Simulacrum Jun 26, 2020

bors commented Jul 14, 2020

Dylan-DPC-zz commented Aug 1, 2020

	// SAFETY: mid is always in [0, size), that means mid is >= 0 and < size.
	// SAFETY:

	// SAFETY: `self` is valid for `self.len()` bytes by definition, and `src` was
	// SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was

	// checked to have the same length. Both slices cannot be overlapping because
	// checked to have the same length. The slices cannot overlap because

	// Rust's mutable references are exclusive.
	// mutable references are exclusive.

Document unsafety in src/libcore/slice/mod.rs #73555

Document unsafety in src/libcore/slice/mod.rs #73555

Conversation

LeSeulArtichaut commented Jun 20, 2020

rust-highfive commented Jun 20, 2020

bors commented Jun 23, 2020

LeSeulArtichaut commented Jun 23, 2020

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Jun 26, 2020

Choose a reason for hiding this comment

bors commented Jul 14, 2020

Dylan-DPC-zz commented Aug 1, 2020

Document unsafety in `src/libcore/slice/mod.rs` #73555

Document unsafety in `src/libcore/slice/mod.rs` #73555