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

Fix overflow when converting ZST Vec to VecDeque #80003

Merged
merged 3 commits into from
Dec 18, 2020

Conversation

Stupremee
Copy link
Member

let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);

This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves #78532

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(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 Dec 13, 2020
@Stupremee Stupremee added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-collections Area: std::collections. A-zst Area: Zero-sized types (ZST). labels Dec 13, 2020
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Dec 13, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Dec 13, 2020

📌 Commit 09d528e has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 14, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
@bors
Copy link
Contributor

bors commented Dec 14, 2020

⌛ Testing commit 09d528e with merge 3ea4f68cdcd9b95f5c31fb7730e5e44d66f9e59e...

@bors
Copy link
Contributor

bors commented Dec 14, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 14, 2020
@Stupremee
Copy link
Member Author

I don't know why the build failed but I think it has nothing to do with my PR?

@dtolnay
Copy link
Member

dtolnay commented Dec 15, 2020

The i686-msvc-1 job failed with:

error: linking with `link.exe` failed: exit code: 1201
  |
  = note: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.28.29333\\bin\\HostX64\\x86\\link.exe" "/NOLOGO" "/NXCOMPAT" "/LARGEADDRESSAWARE" "/SAFESEH" "/LIBPATH:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.0.rcgu.o" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.1.rcgu.o" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.2.rcgu.o" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.3.rcgu.o" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.4.rcgu.o" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.5.rcgu.o" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.main.7rcbfp3g-cgu.6.rcgu.o" "/OUT:D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.exe" "D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags\\main.4s37gsrti678ik8u.rcgu.o" "/OPT:REF,NOICF" "/DEBUG" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\etc\\intrinsic.natvis" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\etc\\liballoc.natvis" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\etc\\libcore.natvis" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\etc\\libstd.natvis" "/LIBPATH:D:/a/rust/rust/build/i686-pc-windows-msvc/test/run-make-fulldeps/override-aliased-flags/override-aliased-flags" "/LIBPATH:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libstd-2deb73d3b5fa87cf.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libpanic_unwind-15c6a7fdf91ef35a.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\librustc_demangle-e074a35b22c4b5d2.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libhashbrown-6c3c9ebd17ae006e.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\librustc_std_workspace_alloc-13e844b011774ae1.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libunwind-a91a7398553297ff.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libcfg_if-35a0f6d337703982.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\liblibc-fe3cebfa1c67a4da.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\liballoc-db1c1d00bd877cb8.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\librustc_std_workspace_core-99335de001fe3f12.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libcore-c8fa98e615898128.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libcompiler_builtins-38e39f9411889523.rlib" "advapi32.lib" "ws2_32.lib" "userenv.lib" "msvcrt.lib"
  = note: LINK : fatal error LNK1201: error writing to program database 'D:\a\rust\rust\build\i686-pc-windows-msvc\test\run-make-fulldeps\override-aliased-flags\override-aliased-flags\main.pdb'; check for insufficient disk space, invalid path, or insufficient privilege

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 15, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 15, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 15, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 16, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 16, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 16, 2020
…on-panic, r=dtolnay

Fix overflow when converting ZST Vec to VecDeque

```rust
let v = vec![(); 100];
let queue = VecDeque::from(v);
println!("{:?}", queue);
```
This code will currently panic with a capacity overflow.
This PR resolves this issue and makes the code run fine.

Resolves rust-lang#78532
// because `usize::MAX` (the capacity returned by `capacity()` for ZST)
// is not a power of two and thus it'll always try
// to reserve more memory which will panic for ZST (rust-lang/rust#78532)
if (!buf.capacity().is_power_of_two() && mem::size_of::<T>() != 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maximum len of ZST Vec is larger than maximum len of ZST VecDeque (MAXIMUM_ZST_CAPACITY - 1), which would be another thing to check before constructing VecDeque.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if this should be changed in this PR, but it's definitely a check that should be added I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair. I mentioned it here, because it wasn't an issue so far, but will be as a result of changes in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can just open a new issue👍

Copy link
Contributor

Choose a reason for hiding this comment

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

We now create a VecDeque with a capacity which is not a power of two here, breaking one of its invariants.

This is either unsound or that invariant isn't actually required anywhere and we should remove it.

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#78164 (Prefer regions with an `external_name` in `approx_universal_upper_bound`)
 - rust-lang#80003 (Fix overflow when converting ZST Vec to VecDeque)
 - rust-lang#80023 (Enhance error message when misspelled label to value in break expression)
 - rust-lang#80046 (Add more documentation to `Diagnostic` and `DiagnosticBuilder`)
 - rust-lang#80109 (Remove redundant and unreliable coverage test results)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2e9ed6f into rust-lang:master Dec 18, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: std::collections. A-zst Area: Zero-sized types (ZST). S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

VecDeque from Vec fails with ZST
8 participants