# Make Vec::set\_len enforce the len \<= cap invariant

**URL:** https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927
**Category:** Uncategorized
**Created:** [November 29, 2018, 5:01pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927 "2018-11-29T17:01:22Z")
**Posts on this page:** 20
**Page:** 1

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 29, 2018, 5:01pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/1 "2018-11-29T17:01:22Z")

</div>

(Writing this down quickly, so I don’t have time to dig up any of the citations mentioned).

It seems slightly insane to me that `Vec::set_len` doesn’t panic if the length is set past the allocated capacity. I find it very, very hard to imagine a world where this is useful (and, if it is, is a failing of `Vec`'s other public APIs). I have vague memory of rather unpleasant bugs that have come up due to sloppy use of `set_len` in the standard library…

This API, as it stands, is so spectacularly unsafe (since it might cause `drop` to read unallocated memory!) that there isn’t even an equivalent for C++'s `std::vector`. I think it should either be removed (which would be unpleasant) or made so that pushing the length past reserved capacity is an unconditional panic. (Given that all array slicing is similarly instrumented, I don’t think this will be a performance regression, and will only break code that is breaking invariants in `unsafe`).

To elaborate on “spectacularly, hilariously dangerous”, being able to set set\_len past the end of the allocated space opens us up to buffer overflows, whose prevention is one of Rust’s selling points. You never ever ever ever ever ever ever ever want to be able to look at unallocated memory, unless you’re something exotic like an allocator.

---

<div class="post-metadata">

### Author: ![sfackler](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/sfackler/32/9_2.png) [@sfackler](https://internals.rust-lang.org/u/sfackler)
#### Post date: [November 29, 2018, 5:07pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/2 "2018-11-29T17:07:05Z")

</div>

> [@mcy](#):
>
> Given that all array slicing is similarly instrumented

`.get_unchecked(1..5)`

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 29, 2018, 6:20pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/3 "2018-11-29T18:20:57Z")

</div>

Sorry, _almost all_. I’m making the comparison with C++, where `std::vector::operator[]` is much more popular than `std::vector::at`, even though only `at` performs runtime bounds checks. I suppose it was an imperfect comparison, though hopefully I got my point across.

That said, I cannot imagine _any_ situation in which doing an unchecked set\_len on a `Vec` is at all acceptable. Marking it as `unsafe` and expecting non-`std` to not make mistakes (when, IIRC, even `std` makes mistakes) is not a solution.

---

<div class="post-metadata">

### Author: ![Ixrec](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ixrec/32/6754_2.png) [@Ixrec](https://internals.rust-lang.org/u/Ixrec)
#### Post date: [November 29, 2018, 6:21pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/4 "2018-11-29T18:21:32Z")

</div>

What’s the use case for set\_len in safe code where a bounds check would be acceptable?

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 29, 2018, 6:33pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/5 "2018-11-29T18:33:00Z")

</div>

There isn’t. `set_len` is already a hilariously unsafe function. In an ideal world we would remove it entirely, but I think that having a bikeshed about churn will delay this change. This change is intended to prevent the worst of potential bugs.

---

<div class="post-metadata">

### Author: ![mbrubeck](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mbrubeck/32/174_2.png) [@mbrubeck](https://internals.rust-lang.org/u/mbrubeck)
#### Post date: [November 29, 2018, 6:34pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/6 "2018-11-29T18:34:02Z")

</div>

I agree that `set_len` would be a better API if it enforced this invariant.

I was a bit worried that this could break existing code that calls `set_len` first, then immediately grows the capacity. However, I don’t think there’s any supported way to do this. For example, this code:

```rust
let mut vec = Vec::new();
unsafe {
    vec.set_len(n);
    vec.reserve(1); // grow the capacity to n + 1
}

```

…doesn’t work because the `reserve` call will try to copy out-of-bounds memory into the new allocation, assuming it doesn’t fail some internal check first.

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 29, 2018, 6:36pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/7 "2018-11-29T18:36:09Z")

</div>

> [@mbrubeck](#):
>
> I was a bit worried that this could break existing code that calls `set_len` first, then immediately grows the capacity.

This should be explicitly banned. Grow first, then extend the length. If it breaks people, it's because they were already doing something very dangerous.

---

<div class="post-metadata">

### Author: ![cuviper](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cuviper/32/1897_2.png) [@cuviper](https://internals.rust-lang.org/u/cuviper)
#### Post date: [November 29, 2018, 6:41pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/8 "2018-11-29T18:41:06Z")

</div>

I'm in favor of enforcing the invariant, but note that the [documentation](https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.set_len) has an explicit example of this:

> [@](#):
>
> In this example, the vector gets expanded from zero to four items without any memory allocations occurring, resulting in vector values of unallocated memory:
> 
> ```rust
> let mut vec: Vec<char> = Vec::new();
> 
> unsafe {
> vec.set_len(4);
> }
> 
> ```

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 29, 2018, 6:52pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/9 "2018-11-29T18:52:05Z")

</div>

I’m not sure which bothers me more… that such a spectacularly dangerous snippet is in the docs or that no one batted an eye on the [commit that added that example](https://github.com/rust-lang/rust/commit/a005b2cd2ac679da7393e537aa05e2b7d32d36d5).

---

<div class="post-metadata">

### Author: ![cuviper](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cuviper/32/1897_2.png) [@cuviper](https://internals.rust-lang.org/u/cuviper)
#### Post date: [November 29, 2018, 7:02pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/10 "2018-11-29T19:02:54Z")

</div>

The danger wasn’t noted on the pull request either, just a formatting nitpick:

> <https://github.com/rust-lang/rust/pull/34911>

In case it’s determined to be a breaking change to assert the invariant, an alternative would be to deprecate and recreate it. I don’t know what to call a new method though – `checked_set_len`? That’s a mouthful, and ideally the safer choice would be more ergonomic.

---

<div class="post-metadata">

### Author: ![kornel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kornel/32/2711_2.png) [@kornel](https://internals.rust-lang.org/u/kornel)
#### Post date: [November 29, 2018, 11:31pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/11 "2018-11-29T23:31:50Z")

</div>

I use this function relatively often to get `Vec` with uninitialized memory (usually to give its pointer to C for writing).

I’d love if it was less hilariously dangerous by checking it’s merely uninitialized, not out of bounds. I don’t think there’s a legit use case for out of bounds.

Alternatively, I’d be happy if there was a nicer explicit API for cheaply growing the `Vec` by writing to uninitialized memory.

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 29, 2018, 11:40pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/12 "2018-11-29T23:40:20Z")

</div>

Agreed; I think something like this would be cool:

```rust
unsafe fn grow_and_initialize(&mut self, new_len: usize, initializer: impl FnOnce(&mut [T])) {
  self.reserve(new_len);
  f(self.raw_vec.as_mut());
  self.set_len(new_len);
}

```

I’d prefer if you could make this function safe, and pass an `unsafe fn(&mut [T])`, but the `Fn` traits aren’t that smart yet. =(

---

<div class="post-metadata">

### Author: ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)
#### Post date: [November 29, 2018, 11:53pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/13 "2018-11-29T23:53:21Z")

</div>

I suspect that this will never be checked in release, as the optimizer would probably not be able to remove it, and a big point of this method is that it doesn’t do things like that.

I think people would welcome a `debug_assert!` for it, even though that won’t fire in user code right now because of how we ship liballoc. Precedent:

> <https://github.com/rust-lang/rust/pull/52972>

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 30, 2018, 12:02am UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/14 "2018-11-30T00:02:00Z")

</div>

> [@scottmcm](#):
>
> a big point of this method is that it doesn’t do things like that.

...What? How this this in any way a reasonable selling point? I refuse to believe that a predict-false `blq` is such a regression that you don't want it. If you need it for `liballoc-internal` reasons, add a method that's public to the crate only.

---

<div class="post-metadata">

### Author: ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)
#### Post date: [November 30, 2018, 12:12am UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/15 "2018-11-30T00:12:32Z")

</div>

For panic-safety it’s critical to keep the length up-to-date, but that tends to be in a tight loop so you don’t want the branch and the landing pads and the LLVM-refuses-to-vectorize and similar.

This method is `unsafe`; it’s not the normal path. By the time you’re using it, you’re performance-optimizing something, so you care about small things. And the method isn’t any safer with the capacity check – it’s still trivial to get UB with it.

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 30, 2018, 12:18am UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/16 "2018-11-30T00:18:08Z")

</div>

> [@scottmcm](#):
>
> This method is `unsafe` ;

That has not stopped people from abusing this method. Something this close within reach should not be able to cause buffer overflows.

> [@scottmcm](#):
>
> By the time you’re using it, you’re performance-optimizing something, so you care about small things

If "you" is `liballoc`, then you can just set `self.len` directly. If "you" is a non-std crate, you should probably be wrapping `RawVec` instead of fudging `Vec`'s length.

> [@scottmcm](#):
>
> And the method isn’t any safer with the capacity check – it’s still trivial to get UB with it.

This is not about UB. With the capacity check, it is impossible to have a buffer overflow. That is infinitely safer in my book. The security track record of `set_len` does not inspire confidence.

Buffer overflows are really really really really bad, right up there with UAFs. Like, "haha your key material has been leaked get ready to get pwn'd" bad. Like, ok, I admit that you can't smash the stack with `Vec`, so it isn't as amazingly, spectacularly bad as poorly-implemented VLAs, but it's still pretty spectacularly bad.

---

<div class="post-metadata">

### Author: ![sfackler](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/sfackler/32/9_2.png) [@sfackler](https://internals.rust-lang.org/u/sfackler)
#### Post date: [November 30, 2018, 3:34am UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/17 "2018-11-30T03:34:12Z")

</div>

RawVec is not publicly exposed, so non-std crates are not going to be using it.

Could you be more specific about set\_len’s track record?

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 30, 2018, 2:26pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/18 "2018-11-30T14:26:14Z")

</div>

> [@sfackler](#):
>
> RawVec is not publicly exposed

Huh, I suppose I had a bad understanding of how the facade crates are put together. It seems odd that I can access unstable docs for `alloc::raw_vec::RawVec`.

In any case, there are surely crates that implement a length-less unsafe buffer like `RawVec`. I will stress again that, if you are doing work which benchmarks indicate needs to drop `Vec` invariants, there's a shiny new `std::alloc` API you can use. This whole thing falls into the "rethink your choices that lead to this situation" bucket.

> [@sfackler](#):
>
> Could you be more specific about set\_len’s track record?

I'm repeating what the security folks at work tell me from interacting with our Rust teams (I myself do not work on a Rust team). I'll try to get some citations out of them today, though I do recall something unpleasant about `[impl Copy]::repeat` and something about working around a miscompilation.

---

<div class="post-metadata">

### Author: ![sfackler](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/sfackler/32/9_2.png) [@sfackler](https://internals.rust-lang.org/u/sfackler)
#### Post date: [November 30, 2018, 3:35pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/19 "2018-11-30T15:35:47Z")

</div>

> [@mcy](#):
>
> there’s a shiny new `std::alloc` API you can use.

RawVec is not a shiny new std::alloc API. It's an internal implementation detail of Vec and HashMap that will never be stabilized.

---

<div class="post-metadata">

### Author: ![mcy](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mcy/32/6512_2.png) [@mcy](https://internals.rust-lang.org/u/mcy)
#### Post date: [November 30, 2018, 4:04pm UTC](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927/20 "2018-11-30T16:04:01Z")

</div>

When I said `std::alloc`, I was referring to allocating things yourself in nightly with `std::alloc::GlobalAlloc`, which is not the same as poking at `liballoc` internals.

I also think that it’s hardly fair to call `RawVec` an implementation detail, when the nomicon describes its internals in detail (mind, not as API, but as a long-form example of safe `unsafe` usage), and when we get to touch other internal state of a `Vec` directly (i.e., the eponymous `set_len`).

Making `alloc::raw_vec::RawVec` inaccessible is fine. What I’m saying is that it’s possible to implement your own `RawVec` without allowing manipulation of `Vec` in extraordinarily dangerous ways. It’s pretty clear that people who have valid uses for `set_len` as it exists today can just implement a growable unsafe buffer themselves, instead of leaving a massive security footgun within arms’ reach.

Relatedly, I believe Scott made a point about panic-safety, which I get, but which completely misses my point. I already insist in abort-on-panic (something I can turn on for my binaries), so I don’t care about panic-safety here; I care about libraries, `std` or otherwise, reaching for `set_len` without understanding that possibly touching unallocated memory by mistake isn’t just UB, it’s _asking_ to have your key material leaked and your service compromised.

(I _do_ apologize for my rather rambling posts on this topic. Trying to make a point about security sometimes causes me to be a bit harsher than I intend to.)

[Next page](https://internals.rust-lang.org/t/make-vec-set-len-enforce-the-len-cap-invariant/8927.md?page=2)
