Skip to content

Unsound: at -C opt-level >= 1 GVN rewrites the Box deref temp from deref_copy to copy, so unelaborate_derefs misses it and every write through a nested Box lands on a copy — panicking programs verify as safe #244

Description

@coord-e

Summary

unelaborate_derefs/extract_elaborated_deref (src/analyze/local_def.rs) recognizes an elaborated Box deref chain in two steps: a deref temp _t = deref_copy P (Rvalue::CopyForDeref), followed by the ElaborateBoxDerefs shape _p = (_t.0.0 as *const T) Transmute. Both temps are NOP'd and replaced by P, so *_p is analyzed as a deref of the original place.

rustc's GVN pass rewrites _t = deref_copy P into _t = copy P — a plain Rvalue::Use(Operand::Copy). GVN runs from mir_opt_level >= 2, i.e. from -C opt-level=1 upward. extract_elaborated_deref only matches Rvalue::CopyForDeref, so _t is no longer replaced. The Transmute pattern that follows still matches, but its rest_place is now the surviving temp _t instead of P.

Thrust models Box<T> by value (PointerType::own), so _t = copy P binds _t to an independent copy of the box's contents. The mutable borrow and the write therefore land on that copy, while P keeps its pre-write value. Every later read of P sees the stale value.

Both failure directions follow, and both are silent:

  • a program whose assertion is false at run time verifies as safeunsound;
  • a program whose assertion is true is rejected with Unsat — over-rejection.

Thrust's verdict therefore depends on the optimization level, and in the direction that matters: the same source is correctly rejected in a debug build and verifies as safe in a release build.

A single Box local (let mut x = Box::new(1i64); *x += 1;) is unaffected: there is no preceding deref temp, so the Transmute pattern matches on the box local itself. The bug needs the box to sit behind one more projection or deref — which is every way a Box actually appears in a program: a struct field, a tuple element, an enum payload, a Vec element, another Box, or a &mut Box<T> parameter.

Minimal reproducer

No annotations needed — a Box in a tuple:

fn main() {
    let mut t = (Box::new(1_i64), 0_i64);
    *t.0 += 1;
    assert!(*t.0 == 1);   // false: *t.0 is 2
}
$ cargo run -- -Adead_code -C debug-assertions=false min.rs && echo safe
error: verification error: Unsat

error: aborting due to 1 previous error

$ cargo run -- -Adead_code -C debug-assertions=false -C opt-level=2 min.rs && echo safe
safe

The program panics when run:

$ rustc -O -o min min.rs && ./min
thread 'main' panicked at min.rs:4:5:
assertion failed: *t.0 == 1

The same holds for a struct field:

struct S { b: Box<i64> }

fn main() {
    let mut s = S { b: Box::new(1_i64) };
    *s.b += 1;
    assert!(*s.b == 1);   // false: *s.b is 2 — `safe` at -C opt-level >= 1
}

and for a nested Box — which is tests/ui/pass/box_nested.rs with the assertion negated:

fn main() {
    let mut x = Box::new(Box::new(1_i64));
    **x += 1;
    assert!(**x == 1);    // false: **x is 2 — `safe` at -C opt-level >= 1
}

tests/ui/pass/box_nested.rs itself (the honest assert!(**x == 2)) is the over-rejection side: it is safe at the default optimization level and Unsat from -C opt-level=1 upward.

Behavior matrix (all with -Adead_code -C debug-assertions=false)

program 0 (default) 1 2 3 s z
tuple, assert!(*t.0 == 1)false Unsat ✔ safe safe safe safe safe
tuple, assert!(*t.0 == 2) — true safe ✔ Unsat Unsat Unsat Unsat Unsat
struct field, assert!(*s.b == 1)false Unsat ✔ safe safe safe safe safe
struct field, assert!(*s.b == 2) — true safe ✔ Unsat Unsat Unsat Unsat Unsat
nested box, assert!(**x == 1)false Unsat ✔ safe safe safe safe safe
tests/ui/pass/box_nested.rs safe ✔ Unsat Unsat Unsat Unsat Unsat
let mut x = Box::new(1i64); *x += 1; (single box) safe ✔ safe ✔ safe ✔ safe ✔ safe ✔ safe ✔

Every other shape that puts a Box behind one more projection reproduces the unsoundness at -C opt-level=1 too — an enum payload (enum E { A(Box<i64>) }, mutated through match &mut e), a Vec<Box<i64>> element, a Box field mutated through a &mut self method, a &mut Box<i64> parameter, and a write performed by a callee (fn incr(x: &mut i64) applied to &mut **x). Some of these additionally hit an unrelated Option::unwrap() panic in src/refine/env.rs:399 at -C opt-level=2/3 (const-propagated aggregate constants), which only masks the wrong answer at those two levels; at 1, s and z they all report safe.

Root cause

ElaborateBoxDerefs lowers *boxed into a Unique/NonNull/Transmute chain, and Derefer hoists the base of that chain into a deref temp. For *t.0 the runtime MIR at -C opt-level=0 is:

_6 = deref_copy (_1.0: std::boxed::Box<i64>);                     // Rvalue::CopyForDeref
_8 = copy ((_6.0: Unique<i64>).0: NonNull<i64>) as *const i64 (Transmute);
_3 = &mut (*_8);
(*_3) = Add(copy (*_3), const 1_i64);

unelaborate_derefs NOPs the first statement and replaces _6 with (_1.0); the second then matches the Box pattern with rest_place = (_1.0) and is NOP'd too, replacing _8 with (_1.0). The write is analyzed as a mutable borrow of (_1.0) — correct.

GVN rewrites exactly those two statements (3-2-013.GVN diff for the struct-field variant):

-        _6 = deref_copy (_1.0: std::boxed::Box<i64>);
+        _6 = copy (_1.0: std::boxed::Box<i64>);
         _8 = copy ((_6.0: Unique<i64>).0: NonNull<i64>) as *const i64 (Transmute);
         (*_8) = Add(copy (*_8), const 1_i64);
         StorageLive(_3);
         StorageLive(_4);
-        _7 = deref_copy (_1.0: std::boxed::Box<i64>);
+        _7 = copy (_1.0: std::boxed::Box<i64>);
         _9 = copy ((_7.0: Unique<i64>).0: NonNull<i64>) as *const i64 (Transmute);
         _4 = copy (*_9);

extract_elaborated_deref returns None for _6 = copy (_1.0):

if let mir::Rvalue::CopyForDeref(place) = &rvalue {
    return Some((lhs_local, *place));
}

so _6 survives as an ordinary local, and the Transmute statement is unelaborated against rest_place = _6. The analyzed write becomes a borrow of *_6, where _6 was bound by analyze_assignment to a fresh copy of (_1.0)'s valueBox<i64> is Own(Int) in rty, so copying it copies the pointee rather than aliasing it. (_1.0) is never borrowed, so its value is never replaced by a prophecy, and the read-back through _7 (another independent copy) returns the original value.

CHC evidence

For the tuple reproducer, THRUST_OUTPUT_DIR at -C opt-level=1 gives (c1, the assertion-holds clause; v10 is t):

(= v10 (tuple<Int-Int> v11 0))         ; t = (v11, 0)  with v11 = 1
(= v4 (tuple_proj<Int-Int>.0 v10)) (= v5 v4) (= v14 v5)   ; copy #2 of the box  -> read back
(= v6 (tuple_proj<Int-Int>.0 v10)) (= v7 v6) (= v15 v7)   ; copy #3 of the box  -> written
(= (mut<Int> v18 v17) (mut<Int> v15 v16))                 ; &mut of copy #3
(= (mut_final<Int> (mut<Int> v18 v17)) (+ v9 1))          ; the write: v17 = 2
(= v1 v14) (= v12 v1) (= v12 1)                           ; the assert reads copy #2

v10 is never rebuilt from a prophecy, so v12 = v14 = tuple_proj.0(v10) = v11 = 1. The panic clause c2 carries (not (= v12 1)) in its body with head false, so it is discharged vacuously and the system is sat — reported as safe.

The same clause at -C opt-level=0 shows the correct threading: the borrow is taken on the tuple's own field and the tuple is rebuilt around the prophecy,

(= v1 (tuple<Int-Int> v11 v10))                           ; t rebuilt with the prophecy v11
(= (mut<Int> v13 v12) (mut<Int> v9 v11))                  ; current = v9, final = v11
(= (mut_final<Int> (mut<Int> v13 v12)) (+ v6 1))          ; v11 = 2
(= v8 (tuple_proj<Int-Int>.0 v1)) (= v8 1)                ; v8 = v11 = 2, contradicts 1

so the false assertion is correctly rejected there.

Confirmed by experiment

Teaching extract_elaborated_deref the post-GVN shape — a copy of a Box-typed place, which in MIR is only ever the pointer copy of such a deref temp, since Box is not Copy:

         if let mir::Rvalue::CopyForDeref(place) = &rvalue {
             return Some((lhs_local, *place));
         }
+
+        if let mir::Rvalue::Use(mir::Operand::Copy(place)) = &rvalue {
+            if place.ty(&self.body.local_decls, self.tcx).ty.is_box() {
+                return Some((lhs_local, *place));
+            }
+        }

makes every reproducer above behave identically at -C opt-level=0..3, s and z: the false assertions are rejected at every level, the true ones (including tests/ui/pass/box_nested.rs) verify at every level, and the whole tests/ui/pass suite still passes at the default optimization level. This is offered as evidence for the diagnosis, not as a proposed patch — matching the rewritten shape in extract_elaborated_deref may not be the right place, and the guard wants more thought than "the place is a box".

Notes

Environment

  • branch main @ cd6b330
  • solver: Z3 5.0.0 (HORN / Spacer)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions