Skip to content

Conversation

btj
Copy link
Contributor

@btj btj commented Aug 22, 2025

alloc_guard checks that its argument is at most isize::MAX, but it is called only with layout sizes, which are already guaranteed to be at most isize::MAX.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 22, 2025
@btj
Copy link
Contributor Author

btj commented Aug 22, 2025

I discovered this issue while writing a description of the proof I developed for AWS Rust Standard Library Verification Contest Challenge 19.

@btj btj marked this pull request as ready for review August 22, 2025 18:16
@rustbot
Copy link
Collaborator

rustbot commented Aug 22, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Aug 22, 2025
@samueltardieu
Copy link
Member

Isn't alloc_guard() the guarantee that the layout size doesn't exceed isize::MAX? Isn't there a call chain such as this one?

  • RawVec<T, Global>::with_capacity(u) where u is a usize
  • RawVecInner<Global>::with_capacity(u, T::LAYOUT)
  • RawVecInner::try_allocate_in(u, T::LAYOUT) which guards against overcapacity by calling alloc_guard()

@btj
Copy link
Contributor Author

btj commented Aug 27, 2025

No, this check already happens earlier in try_allocate_in, in the call of layout_array, which in turn calls Layout::repeat, which fails if the layout size would exceed isize::MAX. Because it is an invariant of Layout that the size is at most isize::MAX.

@samueltardieu
Copy link
Member

Shouldn't the SAFETY comments in grow_amortized() and grow_exact() be also updated? They say that "finish_grow would have resulted in a capacity overflow if we tried to allocate more than isize::MAX items" while in fact this is due to the call of layout_array() in both cases.

@btj
Copy link
Contributor Author

btj commented Aug 27, 2025

You're right. Updated.

@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2025

The limits should stay via Layout but just in case,

@bors2 try @rust-timer queue

Could you also squash please? r=me after that, assuming perf doesn't have anything weird.

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Sep 3, 2025
raw_vec.rs: Remove superfluous fn alloc_guard
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 3, 2025
@rust-bors
Copy link

rust-bors bot commented Sep 3, 2025

☀️ Try build successful (CI)
Build commit: dacfb46 (dacfb46383aa2dc3ddd1589aa5c170ef9de84542, parent: 51ff895062ba60a7cba53f57af928c3fb7b0f2f4)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (dacfb46): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -4.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.4% [-4.4%, -4.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.4% [-4.4%, -4.4%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 465.874s -> 463.341s (-0.54%)
Artifact size: 388.34 MiB -> 388.26 MiB (-0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 3, 2025
It checks that its argument is at most isize::MAX, but it is called
only with layout sizes, which are already guaranteed to be at most
isize::MAX.
@btj btj force-pushed the drop-alloc-guard branch from d185b06 to d9dc20c Compare September 3, 2025 05:35
@btj
Copy link
Contributor Author

btj commented Sep 3, 2025

r? tgross35

@rustbot rustbot assigned tgross35 and unassigned thomcc Sep 3, 2025
@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2025

Thank you!
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 3, 2025

📌 Commit d9dc20c has been approved by tgross35

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 3, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 3, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 3, 2025
raw_vec.rs: Remove superfluous fn alloc_guard

`alloc_guard` checks that its argument is at most `isize::MAX`, but it is called only with layout sizes, which are already guaranteed to be at most `isize::MAX`.
bors added a commit that referenced this pull request Sep 3, 2025
Rollup of 9 pull requests

Successful merges:

 - #143725 (core: add Peekable::next_if_map)
 - #145209 (Stabilize `path_add_extension`)
 - #145750 (raw_vec.rs: Remove superfluous fn alloc_guard)
 - #145962 (Ensure we emit an allocator shim when only some crate types need one)
 - #145963 (Add LSX accelerated implementation for source file analysis)
 - #146054 (add `#[must_use]` to `array::repeat`)
 - #146090 (Derive `PartialEq` for `InvisibleOrigin`)
 - #146120 (Correct typo in `rustc_errors` comment)
 - #146131 (rustdoc-search: add test case for indexing every item type)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Sep 3, 2025
raw_vec.rs: Remove superfluous fn alloc_guard

`alloc_guard` checks that its argument is at most `isize::MAX`, but it is called only with layout sizes, which are already guaranteed to be at most `isize::MAX`.
bors added a commit that referenced this pull request Sep 3, 2025
Rollup of 15 pull requests

Successful merges:

 - #143725 (core: add Peekable::next_if_map)
 - #145209 (Stabilize `path_add_extension`)
 - #145750 (raw_vec.rs: Remove superfluous fn alloc_guard)
 - #145962 (Ensure we emit an allocator shim when only some crate types need one)
 - #145963 (Add LSX accelerated implementation for source file analysis)
 - #146054 (add `#[must_use]` to `array::repeat`)
 - #146090 (Derive `PartialEq` for `InvisibleOrigin`)
 - #146120 (Correct typo in `rustc_errors` comment)
 - #146127 (Rename `ToolRustc` to `ToolRustcPrivate`)
 - #146131 (rustdoc-search: add test case for indexing every item type)
 - #146133 (Revert "Make `lto` and `linker-plugin-lto` work the same for `compiler_builtins`)
 - #146134 (llvm: nvptx: Layout update to match LLVM)
 - #146136 (docs(std): add missing closing code block fences in doc comments)
 - #146137 (Disallow frontmatter in `--cfg` and `--check-cfg` arguments)
 - #146140 (compiletest: cygwin follows windows in using PATH for dynamic libraries)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 3, 2025
raw_vec.rs: Remove superfluous fn alloc_guard

`alloc_guard` checks that its argument is at most `isize::MAX`, but it is called only with layout sizes, which are already guaranteed to be at most `isize::MAX`.
bors added a commit that referenced this pull request Sep 3, 2025
Rollup of 16 pull requests

Successful merges:

 - #143725 (core: add Peekable::next_if_map)
 - #145209 (Stabilize `path_add_extension`)
 - #145342 (fix drop scope for `super let` bindings within `if let`)
 - #145750 (raw_vec.rs: Remove superfluous fn alloc_guard)
 - #145962 (Ensure we emit an allocator shim when only some crate types need one)
 - #145963 (Add LSX accelerated implementation for source file analysis)
 - #146054 (add `#[must_use]` to `array::repeat`)
 - #146090 (Derive `PartialEq` for `InvisibleOrigin`)
 - #146120 (Correct typo in `rustc_errors` comment)
 - #146127 (Rename `ToolRustc` to `ToolRustcPrivate`)
 - #146133 (Revert "Make `lto` and `linker-plugin-lto` work the same for `compiler_builtins`)
 - #146134 (llvm: nvptx: Layout update to match LLVM)
 - #146136 (docs(std): add missing closing code block fences in doc comments)
 - #146137 (Disallow frontmatter in `--cfg` and `--check-cfg` arguments)
 - #146140 (compiletest: cygwin follows windows in using PATH for dynamic libraries)
 - #146156 (miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 4, 2025
raw_vec.rs: Remove superfluous fn alloc_guard

`alloc_guard` checks that its argument is at most `isize::MAX`, but it is called only with layout sizes, which are already guaranteed to be at most `isize::MAX`.
bors added a commit that referenced this pull request Sep 4, 2025
Rollup of 24 pull requests

Successful merges:

 - #140459 (Add `read_buf` equivalents for positioned reads)
 - #143725 (core: add Peekable::next_if_map)
 - #145209 (Stabilize `path_add_extension`)
 - #145342 (fix drop scope for `super let` bindings within `if let`)
 - #145750 (raw_vec.rs: Remove superfluous fn alloc_guard)
 - #145827 (On unused binding or binding not present in all patterns, suggest potential typo of unit struct/variant or const)
 - #145932 (Allow `inline(always)` with a target feature behind a unstable feature `target_feature_inline_always`.)
 - #145962 (Ensure we emit an allocator shim when only some crate types need one)
 - #145963 (Add LSX accelerated implementation for source file analysis)
 - #146054 (add `#[must_use]` to `array::repeat`)
 - #146090 (Derive `PartialEq` for `InvisibleOrigin`)
 - #146112 (don't uppercase error messages)
 - #146120 (Correct typo in `rustc_errors` comment)
 - #146124 (Test `rustc-dev` in `distcheck`)
 - #146127 (Rename `ToolRustc` to `ToolRustcPrivate`)
 - #146131 (rustdoc-search: add test case for indexing every item type)
 - #146134 (llvm: nvptx: Layout update to match LLVM)
 - #146136 (docs(std): add missing closing code block fences in doc comments)
 - #146137 (Disallow frontmatter in `--cfg` and `--check-cfg` arguments)
 - #146140 (compiletest: cygwin follows windows in using PATH for dynamic libraries)
 - #146150 (fix(rustdoc): match rustc `--emit` precedence )
 - #146155 (Make bootstrap self test parallel)
 - #146161 ([rustdoc] Uncomment code to add scraped rustdoc examples in loaded paths)
 - #146172 (triagebot: configure some pings when certain attributes are used)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d71a9b6 into rust-lang:master Sep 4, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Sep 4, 2025
rust-timer added a commit that referenced this pull request Sep 4, 2025
Rollup merge of #145750 - btj:drop-alloc-guard, r=tgross35

raw_vec.rs: Remove superfluous fn alloc_guard

`alloc_guard` checks that its argument is at most `isize::MAX`, but it is called only with layout sizes, which are already guaranteed to be at most `isize::MAX`.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 5, 2025
Rollup of 24 pull requests

Successful merges:

 - rust-lang/rust#140459 (Add `read_buf` equivalents for positioned reads)
 - rust-lang/rust#143725 (core: add Peekable::next_if_map)
 - rust-lang/rust#145209 (Stabilize `path_add_extension`)
 - rust-lang/rust#145342 (fix drop scope for `super let` bindings within `if let`)
 - rust-lang/rust#145750 (raw_vec.rs: Remove superfluous fn alloc_guard)
 - rust-lang/rust#145827 (On unused binding or binding not present in all patterns, suggest potential typo of unit struct/variant or const)
 - rust-lang/rust#145932 (Allow `inline(always)` with a target feature behind a unstable feature `target_feature_inline_always`.)
 - rust-lang/rust#145962 (Ensure we emit an allocator shim when only some crate types need one)
 - rust-lang/rust#145963 (Add LSX accelerated implementation for source file analysis)
 - rust-lang/rust#146054 (add `#[must_use]` to `array::repeat`)
 - rust-lang/rust#146090 (Derive `PartialEq` for `InvisibleOrigin`)
 - rust-lang/rust#146112 (don't uppercase error messages)
 - rust-lang/rust#146120 (Correct typo in `rustc_errors` comment)
 - rust-lang/rust#146124 (Test `rustc-dev` in `distcheck`)
 - rust-lang/rust#146127 (Rename `ToolRustc` to `ToolRustcPrivate`)
 - rust-lang/rust#146131 (rustdoc-search: add test case for indexing every item type)
 - rust-lang/rust#146134 (llvm: nvptx: Layout update to match LLVM)
 - rust-lang/rust#146136 (docs(std): add missing closing code block fences in doc comments)
 - rust-lang/rust#146137 (Disallow frontmatter in `--cfg` and `--check-cfg` arguments)
 - rust-lang/rust#146140 (compiletest: cygwin follows windows in using PATH for dynamic libraries)
 - rust-lang/rust#146150 (fix(rustdoc): match rustc `--emit` precedence )
 - rust-lang/rust#146155 (Make bootstrap self test parallel)
 - rust-lang/rust#146161 ([rustdoc] Uncomment code to add scraped rustdoc examples in loaded paths)
 - rust-lang/rust#146172 (triagebot: configure some pings when certain attributes are used)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants