Llint for casting between raw slice pointers with different element sizes #8445

asquared31415 · 2022-02-18T10:43:07Z

This lint disallows using as to convert from a raw pointer to a slice (e.g. *const [i32], *mut [Foo]) to any other raw pointer to a slice if the element types have different sizes. When a raw slice pointer is cast, the data pointer and count metadata are preserved. This means that when the size of the inner slice's element type changes, the total number of bytes pointed to by the count changes. For example a *const [i32] with length 4 (four i32 elements) is cast as *const [u8] the resulting pointer points to four u8 elements at the same address, losing most of the data. When the size increases the resulting pointer will point to more data, and accessing that data will be UB.

On its own, producing the pointer isn't actually a problem, but because any use of the pointer as a slice will either produce surprising behavior or cause UB I believe this is a correctness lint. If the pointer is not intended to be used as a slice, the user should instead use any of a number of methods to produce just a data pointer including an as cast to a thin pointer (e.g. p as *const i32) or if the pointer is being created from a slice, the as_ptr method on slices. Detecting the intended use of the pointer is outside the scope of this lint, but I believe this lint will also lead users to realize that a slice pointer is only for slices.

There is an exception to this lint when either of the slice element types are zero sized (e.g *mut [()]). The total number of bytes pointed to by the slice with a zero sized element is zero. In that case preserving the length metadata is likely intended as a workaround to get the length metadata of a slice pointer though a zero sized slice.

The lint does not forbid casting pointers to slices with the same element size as the cast was likely intended to reinterpret the data in the slice as some equivalently sized data and the resulting pointer will behave as intended.

changelog: Added [`cast_slice_different_sizes`], a lint that disallows using as-casts to convert between raw pointers to slices when the elements have different sizes.

rust-highfive · 2022-02-18T10:43:10Z

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

llogiq

This looks good, albeit a bit wordy. The help could be either more specific or removed wholesale, I don't think it's pulling it's weight.

clippy_lints/src/casts/cast_slice_different_sizes.rs

llogiq

This looks good in general. Just a small style nit on the docs.

clippy_lints/src/casts/mod.rs

llogiq · 2022-02-26T19:06:42Z

clippy_lints/src/casts/mod.rs

+    /// ```
+    #[clippy::version = "1.60.0"]
+    pub CAST_SLICE_DIFFERENT_SIZES,
+    correctness,


I wonder if there could be cases where the code is actually correct, even if surprising.

If that is the case, I worry that having a deny by default lint might lead to a downgrade to nursery. So in this case using the suspicious group might be a better choice.

The exceptions when one or both target slices have a zero sized inner type should cover what I believe are the only correct uses:

To get the length metadata of a slice pointer on stable Rust by going through a &[()] (because there's zero bytes operated on).

To come from a pointer like *const [()] that has the correct inner data and size metadata (would be really weird but probably not wrong)

I asked on the Rust community discord if anyone had any other ideas about possible correct uses of a cast like this and nobody had any ideas.

Any time that data is actually accessed behind the pointer it either can be done with a thin pointer or with a slice pointer with a correct length. While I suppose it's technically the dereference that could cause behavior, I don't think there's ever a way to correctly use a slice pointer with the incorrect length, so I believe that it should be deny by default.

llogiq · 2022-03-04T05:59:36Z

Thank you!

@bors r+

bors · 2022-03-04T05:59:38Z

📌 Commit cb88dd5 has been approved by llogiq

bors · 2022-03-04T05:59:44Z

⌛ Testing commit cb88dd5 with merge 8ea6d97...

Llint for casting between raw slice pointers with different element sizes This lint disallows using `as` to convert from a raw pointer to a slice (e.g. `*const [i32]`, `*mut [Foo]`) to any other raw pointer to a slice if the element types have different sizes. When a raw slice pointer is cast, the data pointer and count metadata are preserved. This means that when the size of the inner slice's element type changes, the total number of bytes pointed to by the count changes. For example a `*const [i32]` with length 4 (four `i32` elements) is cast `as *const [u8]` the resulting pointer points to four `u8` elements at the same address, losing most of the data. When the size *increases* the resulting pointer will point to *more* data, and accessing that data will be UB. On its own, *producing* the pointer isn't actually a problem, but because any use of the pointer as a slice will either produce surprising behavior or cause UB I believe this is a correctness lint. If the pointer is not intended to be used as a slice, the user should instead use any of a number of methods to produce just a data pointer including an `as` cast to a thin pointer (e.g. `p as *const i32`) or if the pointer is being created from a slice, the `as_ptr` method on slices. Detecting the intended use of the pointer is outside the scope of this lint, but I believe this lint will also lead users to realize that a slice pointer is only for slices. There is an exception to this lint when either of the slice element types are zero sized (e.g `*mut [()]`). The total number of bytes pointed to by the slice with a zero sized element is zero. In that case preserving the length metadata is likely intended as a workaround to get the length metadata of a slice pointer though a zero sized slice. The lint does not forbid casting pointers to slices with the *same* element size as the cast was likely intended to reinterpret the data in the slice as some equivalently sized data and the resulting pointer will behave as intended. --- changelog: Added ``[`cast_slice_different_sizes`]``, a lint that disallows using `as`-casts to convert between raw pointers to slices when the elements have different sizes.

bors · 2022-03-04T06:00:11Z

💔 Test failed - checks-action_test

llogiq · 2022-03-04T19:47:22Z

@bors r+

bors · 2022-03-04T19:47:24Z

📌 Commit f932c30 has been approved by llogiq

bors · 2022-03-04T19:47:32Z

⌛ Testing commit f932c30 with merge bc8fee5...

Llint for casting between raw slice pointers with different element sizes This lint disallows using `as` to convert from a raw pointer to a slice (e.g. `*const [i32]`, `*mut [Foo]`) to any other raw pointer to a slice if the element types have different sizes. When a raw slice pointer is cast, the data pointer and count metadata are preserved. This means that when the size of the inner slice's element type changes, the total number of bytes pointed to by the count changes. For example a `*const [i32]` with length 4 (four `i32` elements) is cast `as *const [u8]` the resulting pointer points to four `u8` elements at the same address, losing most of the data. When the size *increases* the resulting pointer will point to *more* data, and accessing that data will be UB. On its own, *producing* the pointer isn't actually a problem, but because any use of the pointer as a slice will either produce surprising behavior or cause UB I believe this is a correctness lint. If the pointer is not intended to be used as a slice, the user should instead use any of a number of methods to produce just a data pointer including an `as` cast to a thin pointer (e.g. `p as *const i32`) or if the pointer is being created from a slice, the `as_ptr` method on slices. Detecting the intended use of the pointer is outside the scope of this lint, but I believe this lint will also lead users to realize that a slice pointer is only for slices. There is an exception to this lint when either of the slice element types are zero sized (e.g `*mut [()]`). The total number of bytes pointed to by the slice with a zero sized element is zero. In that case preserving the length metadata is likely intended as a workaround to get the length metadata of a slice pointer though a zero sized slice. The lint does not forbid casting pointers to slices with the *same* element size as the cast was likely intended to reinterpret the data in the slice as some equivalently sized data and the resulting pointer will behave as intended. --- changelog: Added ``[`cast_slice_different_sizes`]``, a lint that disallows using `as`-casts to convert between raw pointers to slices when the elements have different sizes.

bors · 2022-03-04T19:48:08Z

💔 Test failed - checks-action_test

asquared31415 · 2022-03-04T19:51:13Z

Edited the PR description, I think it didn't like the newline right after the changelog:

xFrednet · 2022-03-04T19:57:40Z

Sounds correct, thanks for the change 🙃

@bors retry

bors · 2022-03-04T19:57:46Z

⌛ Testing commit f932c30 with merge 147bbe2...

Llint for casting between raw slice pointers with different element sizes This lint disallows using `as` to convert from a raw pointer to a slice (e.g. `*const [i32]`, `*mut [Foo]`) to any other raw pointer to a slice if the element types have different sizes. When a raw slice pointer is cast, the data pointer and count metadata are preserved. This means that when the size of the inner slice's element type changes, the total number of bytes pointed to by the count changes. For example a `*const [i32]` with length 4 (four `i32` elements) is cast `as *const [u8]` the resulting pointer points to four `u8` elements at the same address, losing most of the data. When the size *increases* the resulting pointer will point to *more* data, and accessing that data will be UB. On its own, *producing* the pointer isn't actually a problem, but because any use of the pointer as a slice will either produce surprising behavior or cause UB I believe this is a correctness lint. If the pointer is not intended to be used as a slice, the user should instead use any of a number of methods to produce just a data pointer including an `as` cast to a thin pointer (e.g. `p as *const i32`) or if the pointer is being created from a slice, the `as_ptr` method on slices. Detecting the intended use of the pointer is outside the scope of this lint, but I believe this lint will also lead users to realize that a slice pointer is only for slices. There is an exception to this lint when either of the slice element types are zero sized (e.g `*mut [()]`). The total number of bytes pointed to by the slice with a zero sized element is zero. In that case preserving the length metadata is likely intended as a workaround to get the length metadata of a slice pointer though a zero sized slice. The lint does not forbid casting pointers to slices with the *same* element size as the cast was likely intended to reinterpret the data in the slice as some equivalently sized data and the resulting pointer will behave as intended. --- changelog: Added ``[`cast_slice_different_sizes`]``, a lint that disallows using `as`-casts to convert between raw pointers to slices when the elements have different sizes.

bors · 2022-03-04T20:13:15Z

💔 Test failed - checks-action_test

xFrednet · 2022-03-04T20:14:55Z

Downloading artifact 'target' to: '/home/runner/work/rust-clippy/rust-clippy/target'
Error: The HTTP request timed out after 00:01:40.
Error: Exit code 1 returned from process: file name '/home/runner/runners/2.288.1/bin/Runner.PluginHost', arguments 'action "GitHub.Runner.Plugins.Artifact.DownloadArtifact, Runner.Plugins"'.

The CI seams to be acting up. Here is one more try ^^

@bors retry

bors · 2022-03-04T20:15:02Z

⌛ Testing commit f932c30 with merge f508ddb...

Llint for casting between raw slice pointers with different element sizes This lint disallows using `as` to convert from a raw pointer to a slice (e.g. `*const [i32]`, `*mut [Foo]`) to any other raw pointer to a slice if the element types have different sizes. When a raw slice pointer is cast, the data pointer and count metadata are preserved. This means that when the size of the inner slice's element type changes, the total number of bytes pointed to by the count changes. For example a `*const [i32]` with length 4 (four `i32` elements) is cast `as *const [u8]` the resulting pointer points to four `u8` elements at the same address, losing most of the data. When the size *increases* the resulting pointer will point to *more* data, and accessing that data will be UB. On its own, *producing* the pointer isn't actually a problem, but because any use of the pointer as a slice will either produce surprising behavior or cause UB I believe this is a correctness lint. If the pointer is not intended to be used as a slice, the user should instead use any of a number of methods to produce just a data pointer including an `as` cast to a thin pointer (e.g. `p as *const i32`) or if the pointer is being created from a slice, the `as_ptr` method on slices. Detecting the intended use of the pointer is outside the scope of this lint, but I believe this lint will also lead users to realize that a slice pointer is only for slices. There is an exception to this lint when either of the slice element types are zero sized (e.g `*mut [()]`). The total number of bytes pointed to by the slice with a zero sized element is zero. In that case preserving the length metadata is likely intended as a workaround to get the length metadata of a slice pointer though a zero sized slice. The lint does not forbid casting pointers to slices with the *same* element size as the cast was likely intended to reinterpret the data in the slice as some equivalently sized data and the resulting pointer will behave as intended. --- changelog: Added ``[`cast_slice_different_sizes`]``, a lint that disallows using `as`-casts to convert between raw pointers to slices when the elements have different sizes.

bors · 2022-03-04T20:29:04Z

💔 Test failed - checks-action_test

llogiq · 2022-03-06T07:46:52Z

@bors retry

more CI flakyness...

bors · 2022-03-06T07:46:59Z

⌛ Testing commit f932c30 with merge 0c483f6...

bors · 2022-03-06T08:02:11Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 0c483f6 to master...

…ednet fix ICE in `cast_slice_different_sizes` fixes #8708 changelog: fixes an ICE introduced in #8445

rust-highfive assigned llogiq Feb 18, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 18, 2022

asquared31415 force-pushed the slice_ptr_cast branch from e246285 to 893d7ac Compare February 18, 2022 10:48

llogiq reviewed Feb 18, 2022

View reviewed changes

clippy_lints/src/casts/cast_slice_different_sizes.rs Outdated Show resolved Hide resolved

asquared31415 force-pushed the slice_ptr_cast branch 2 times, most recently from 40a9e7c to 387eff7 Compare February 24, 2022 15:45

llogiq reviewed Feb 26, 2022

View reviewed changes

asquared31415 force-pushed the slice_ptr_cast branch from 387eff7 to cb88dd5 Compare March 4, 2022 04:06

lint for casting raw pointers to slices with different element sizes

f932c30

asquared31415 force-pushed the slice_ptr_cast branch from cb88dd5 to f932c30 Compare March 4, 2022 18:23

bors merged commit 0c483f6 into rust-lang:master Mar 6, 2022

This was referenced Apr 16, 2022

Regression: raw pointer slice cast causes ICE #8708

Closed

fix ICE in cast_slice_different_sizes #8720

Merged

bors added a commit that referenced this pull request Apr 30, 2022

Auto merge of #8720 - asquared31415:ptr-cast-ice-fix, r=Alexendoo,xFr…

ec46992

…ednet fix ICE in `cast_slice_different_sizes` fixes #8708 changelog: fixes an ICE introduced in #8445

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Llint for casting between raw slice pointers with different element sizes #8445

Llint for casting between raw slice pointers with different element sizes #8445

asquared31415 commented Feb 18, 2022 •

edited

Loading

rust-highfive commented Feb 18, 2022

llogiq left a comment

llogiq left a comment

llogiq Feb 26, 2022 •

edited

Loading

asquared31415 Mar 4, 2022

llogiq commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

llogiq commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

asquared31415 commented Mar 4, 2022

xFrednet commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

xFrednet commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

llogiq commented Mar 6, 2022

bors commented Mar 6, 2022

bors commented Mar 6, 2022

Llint for casting between raw slice pointers with different element sizes #8445

Llint for casting between raw slice pointers with different element sizes #8445

Conversation

asquared31415 commented Feb 18, 2022 • edited Loading

rust-highfive commented Feb 18, 2022

llogiq left a comment

Choose a reason for hiding this comment

llogiq left a comment

Choose a reason for hiding this comment

llogiq Feb 26, 2022 • edited Loading

Choose a reason for hiding this comment

asquared31415 Mar 4, 2022

Choose a reason for hiding this comment

llogiq commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

llogiq commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

asquared31415 commented Mar 4, 2022

xFrednet commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

xFrednet commented Mar 4, 2022

bors commented Mar 4, 2022

bors commented Mar 4, 2022

llogiq commented Mar 6, 2022

bors commented Mar 6, 2022

bors commented Mar 6, 2022

asquared31415 commented Feb 18, 2022 •

edited

Loading

llogiq Feb 26, 2022 •

edited

Loading