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

Add excess capacity description for Vec::leak #79257

Closed

Conversation

chansuke
Copy link
Contributor

Fixes #79240

@rust-highfive
Copy link
Collaborator

r? @shepmaster

(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 Nov 21, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Nov 21, 2020

This is probably not the behaviour we want to commit to, see the discussion here: rust-lang/rfcs#2969 (comment)

library/alloc/src/vec.rs Outdated Show resolved Hide resolved
@chansuke chansuke force-pushed the add-excess-capacity-description branch from ec00700 to edd6110 Compare December 2, 2020 12:08
@chansuke
Copy link
Contributor Author

chansuke commented Dec 3, 2020

trying to fix free(): double free detected in tcache 2

@bors
Copy link
Contributor

bors commented Dec 31, 2020

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

@shepmaster
Copy link
Member

r? @m-ou-se

@rust-highfive rust-highfive assigned m-ou-se and unassigned shepmaster Jan 1, 2021
@chansuke chansuke force-pushed the add-excess-capacity-description branch from edd6110 to 9b7de1c Compare January 2, 2021 11:27
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.................................................................................................... 9000/11238
.................................................................................................... 9100/11238
.................................................................................................... 9200/11238
..................................i......i.......................................................... 9300/11238
.........................................................................iiiiii..iiiiii.i........... 9400/11238
.................................................................................................... 9600/11238
.................................................................................................... 9700/11238
.................................................................................................... 9800/11238
.................................................................................................... 9900/11238
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 27 tests
iiiiiiiiiiiiiiiiiiiiiiiiiii

 finished in 0.067 seconds
Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii....i.i....ii..........iiii.........i.....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.28s

 finished in 2.349 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
.................................................................................................... 500/541
................F........................
failures:

---- src/vec/mod.rs - vec::Vec<T, A>::leak (line 1757) stdout ----
Test executable failed (terminated by signal).
stderr:
stderr:
free(): double free detected in tcache 2


failures:
    src/vec/mod.rs - vec::Vec<T, A>::leak (line 1757)
    src/vec/mod.rs - vec::Vec<T, A>::leak (line 1757)

test result: FAILED. 539 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 11.83s

error: test failed, to rerun pass '--doc'


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "-p" "alloc" "--" "--quiet"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:20:45

@JohnCSimon
Copy link
Member

Ping from triage - @chansuke
can you please resolve the build failure? thank you.

@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot 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 Jan 19, 2021
@chansuke
Copy link
Contributor Author

chansuke commented Feb 3, 2021

@JohnCSimon I apologize for the late response. I will take a look at it this weekend.

@chansuke chansuke force-pushed the add-excess-capacity-description branch from 9b7de1c to b2d873f Compare February 7, 2021 01:52
@chansuke
Copy link
Contributor Author

chansuke commented Feb 7, 2021

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot 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 Feb 7, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2021
Comment on lines +1790 to +1798
/// let mut v = Vec::with_capacity(10);
/// v.extend([1, 2, 3].iter().cloned());
/// let slice = v.clone().into_boxed_slice();
///
/// unsafe {
/// let p = slice.as_ptr();
/// let rebuilt = std::slice::from_raw_parts(p, slice.len());
/// assert_eq!(rebuilt.len(), 3);
/// }
Copy link
Member

Choose a reason for hiding this comment

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

This example doesn't demonstrate any leaking. .into_boxed_slice() returns a Box, which the slice variable will own.

The issue this PR was originally trying to fix is that the documentation doesn't promise this function drops excess capacity. However, I don't think that's something we want to fix. Instead, I think it'd be better if leak explicitly does not drop the excess capacity. But for that, there's an RFC which has been standing still for a while: rust-lang/rfcs#2969

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m-ou-se Thank you for your support and help. I've been stucking and would like to close my PR.

@m-ou-se m-ou-se 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 Mar 3, 2021
@chansuke chansuke closed this Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for Vec::leak doesn't ackowledge dropping of excess capacity
9 participants