From 86ca6f20cae6e7a0afec38cbdcb7a0f63b166f4f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 29 Jul 2026 21:04:26 +0000 Subject: [PATCH 1/4] types: don't unshare type DAG during finalization I have a 300kb program (produced by a clanker) that has an exponentially-sized type DAG. This is fine until we call `Type::finalize`, which has the amusing comment // Now that we know our types have finite size, we can safely use a // post-order iterator to finalize them. Well, this -would- be safe if we weren't using the `NoSharing` tracker in our post-order iterator. AFAICT I did this just to be lazy. Stop being lazy and implement a sharing tracker that tracks BoundRefs, thereby only iterating through as many nodes an we actually allocated. I don't have a good unit test; for one thing, my test vector is 300kb so I don't want to put it in this repo (I'll throw it in qa-assets so it's at least available, though I need to think what kind of harness we should write for it). But also, the failure mode is that this code just iterates forever, allocating 600Gb+ of RAM, which Rust makes a bit hard to detect. We should investigate using an alternate allocator that can limit memory, or something, in the fuzzer. But the existing unit and fuzz tests should confirm that this doesn't break anything. In fact the fuzzer should run much faster now. --- src/types/context.rs | 30 +++++++++++++++++++++++++++--- src/types/incomplete.rs | 3 ++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/types/context.rs b/src/types/context.rs index 7992d2ae..0497bc6e 100644 --- a/src/types/context.rs +++ b/src/types/context.rs @@ -15,13 +15,14 @@ //! use std::any::TypeId; +use std::collections::HashMap; use std::fmt; use std::marker::PhantomData; use std::sync::{Arc, Mutex, MutexGuard}; use ghost_cell::GhostToken; -use crate::dag::{Dag, DagLike}; +use crate::dag::{Dag, DagLike, SharingTracker}; use crate::jet::Jet; use super::{ @@ -254,7 +255,7 @@ impl<'brand> Context<'brand> { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct BoundRef<'brand> { phantom: InvariantLifetime<'brand>, index: usize, @@ -285,7 +286,9 @@ impl super::PointerLike for BoundRef<'_> { } } -impl<'brand> DagLike for (&'_ Context<'brand>, BoundRef<'brand>) { +pub type CtxAndBoundRef<'ctx, 'brand> = (&'ctx Context<'brand>, BoundRef<'brand>); + +impl<'brand> DagLike for CtxAndBoundRef<'_, 'brand> { type Node = BoundRef<'brand>; fn data(&self) -> &BoundRef<'brand> { &self.1 @@ -303,6 +306,27 @@ impl<'brand> DagLike for (&'_ Context<'brand>, BoundRef<'brand>) { } } +#[derive(Clone, Debug, Default)] +pub struct BoundRefSharing<'brand> { + map: HashMap, usize>, +} + +impl<'brand> SharingTracker> for BoundRefSharing<'brand> { + fn record(&mut self, d: &CtxAndBoundRef<'_, 'brand>, index: usize) -> Option { + use std::collections::hash_map::Entry; + match self.map.entry(d.1.clone()) { + Entry::Occupied(occ) => Some(*occ.get()), + Entry::Vacant(vac) => { + vac.insert(index); + None + } + } + } + fn seen_before(&self, d: &CtxAndBoundRef<'_, 'brand>) -> Option { + self.map.get(&d.1).copied() + } +} + #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] pub struct OccursCheckId<'brand> { phantom: InvariantLifetime<'brand>, diff --git a/src/types/incomplete.rs b/src/types/incomplete.rs index d66473b4..96151706 100644 --- a/src/types/incomplete.rs +++ b/src/types/incomplete.rs @@ -11,6 +11,7 @@ use crate::dag::{Dag, DagLike, NoSharing}; use crate::types::union_bound::PointerLike; +use super::context::BoundRefSharing; use super::{Bound, BoundRef, Context}; use std::fmt; @@ -168,7 +169,7 @@ impl Incomplete { // Now that we know our bound has finite size, we can safely use a // post-order iterator on it. let mut finalized = vec![]; - for data in (ctx, bound_ref).post_order_iter::() { + for data in (ctx, bound_ref).post_order_iter::>() { let bound_get = data.node.0.get(&data.node.1); let final_data = match bound_get { Bound::Free(s) => Incomplete::Free(s), From 01eafba98268870fbd65de79ca465011b71ddc1d Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 6 Aug 2026 15:36:24 +0000 Subject: [PATCH 2/4] node: implement non-recursive Drop for Node For `Node` this is actually not too bad, because the type is mutally recursive with `Inner`, so we can move out of the Inner type without getting stupid "cannot move out of type that implements Drop" errors. High-level structure was written by ChatGPT 5.6 Sol, but I rewrote it to factor out the `into_dag` method and make the other code more terse. --- src/node/inner.rs | 20 ++++++++++++++++++++ src/node/mod.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/node/inner.rs b/src/node/inner.rs index d897c6fc..c7960d4b 100644 --- a/src/node/inner.rs +++ b/src/node/inner.rs @@ -298,6 +298,26 @@ impl Inner { } impl, W> Inner, X, W> { + /// Collapse the node information to a `Dag` + pub fn into_dag(self) -> Dag> { + match self { + Inner::Iden + | Inner::Unit + | Inner::Witness(_) + | Inner::Fail(_) + | Inner::Jet(_) + | Inner::Word(_) => Dag::Nullary, + Inner::InjL(c) + | Inner::InjR(c) + | Inner::Take(c) + | Inner::Drop(c) + | Inner::AssertL(c, _) + | Inner::AssertR(_, c) => Dag::Unary(c), + Inner::Comp(cl, cr) | Inner::Case(cl, cr) | Inner::Pair(cl, cr) => Dag::Binary(cl, cr), + Inner::Disconnect(cl, cr) => cr.disconnect_dag_arc(cl), + } + } + /// Collapse the node information to a `Dag` pub fn as_dag(&self) -> Dag<&C> { match self { diff --git a/src/node/mod.rs b/src/node/mod.rs index f0da02f8..d6d1007d 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -802,6 +802,33 @@ impl> Node { } } +impl Drop for Node { + fn drop(&mut self) { + fn push_children( + stack: &mut Vec>>, + inner: Inner>, N::Disconnect, N::Witness>, + ) { + use crate::dag::Dag; + match inner.into_dag() { + Dag::Nullary => {} + Dag::Unary(child) => stack.push(child), + Dag::Binary(left, right) => { + stack.push(left); + stack.push(right); + } + } + } + + let mut stack = Vec::new(); + push_children(&mut stack, std::mem::replace(&mut self.inner, Inner::Unit)); + while let Some(child) = stack.pop() { + if let Some(mut child) = Arc::into_inner(child) { + push_children(&mut stack, std::mem::replace(&mut child.inner, Inner::Unit)); + } + } + } +} + #[cfg(test)] #[cfg(all(feature = "test-utils", feature = "elements"))] mod tests { From b61e65ab727852668ba3c295c2c9dd72705f728e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 6 Aug 2026 15:41:45 +0000 Subject: [PATCH 3/4] types: limit display length/depth for Incomplete Just copy exactly the same logic we added for Type in src/types/mod.rs. When we panic on type-inference errors we often try to debug-dump an entire Incomplete, which may be exponential in size. Possibly we want BoundRef-sharing here too? At least for Debug output? I dunno. Certainly what we -don't- want is to run forever outputting nothing, which is the existing behavior that this commit fixes. --- src/types/incomplete.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/types/incomplete.rs b/src/types/incomplete.rs index 96151706..4bc1f5e9 100644 --- a/src/types/incomplete.rs +++ b/src/types/incomplete.rs @@ -13,6 +13,7 @@ use crate::types::union_bound::PointerLike; use super::context::BoundRefSharing; use super::{Bound, BoundRef, Context}; +use super::{MAX_DISPLAY_DEPTH, MAX_DISPLAY_LENGTH}; use std::fmt; use std::sync::Arc; @@ -56,7 +57,18 @@ impl fmt::Debug for Incomplete { impl fmt::Display for Incomplete { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut skip_next = false; - for data in self.verbose_pre_order_iter::(None) { + for data in self.verbose_pre_order_iter::(Some(MAX_DISPLAY_DEPTH)) { + if data.index > MAX_DISPLAY_LENGTH { + write!(f, "... [truncated type after {} nodes]", MAX_DISPLAY_LENGTH)?; + return Ok(()); + } + if data.depth == MAX_DISPLAY_DEPTH { + if data.n_children_yielded == 0 { + f.write_str("...")?; + } + continue; + } + if skip_next { skip_next = false; continue; From 0400e4198d16931b02694efb4ca865b4052cf3c2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 6 Aug 2026 15:46:02 +0000 Subject: [PATCH 4/4] impl non-recursive Drop for types::Incomplete Unlike the case for `Node`, `types::Incomplete` is a directly recursive type (or rather, it holds an `Arc`, but we can't implement anything on `Arc` so for our purposes it may as well just be `Incomplete`) which means that we hit a Rust bug preventing us moving stuff out of it. We need unsafe code to move out of the Rust bug. Hopefully this code, which is mostly comments and directly analogous to the code we just added to Node, is easy enough to follow. You can run the `root_unit_to_unit` in Miri which exercises this path, and if you introduce UB (say, by removing the call to `mem::forget`) it'll detect it. --- src/node/mod.rs | 1 + src/types/incomplete.rs | 78 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/node/mod.rs b/src/node/mod.rs index d6d1007d..3f3c91bc 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -804,6 +804,7 @@ impl> Node { impl Drop for Node { fn drop(&mut self) { + // Note: this is basically identical to the drop impl for types::Incomplete. fn push_children( stack: &mut Vec>>, inner: Inner>, N::Disconnect, N::Witness>, diff --git a/src/types/incomplete.rs b/src/types/incomplete.rs index 4bc1f5e9..48f16579 100644 --- a/src/types/incomplete.rs +++ b/src/types/incomplete.rs @@ -33,6 +33,59 @@ pub enum Incomplete { Final(Arc), } +impl Incomplete { + /// Private helper function for `Incomplete::drop`. See `node::Inner::into_dag` which is + /// similar but doesn't require unsafe code. + fn into_dag(mut self) -> Dag> { + use core::{mem, ptr}; + let ret = match &mut self { + Incomplete::Sum(ref mut left, ref mut right) + | Incomplete::Product(ref mut left, ref mut right) => { + // Because Rust is stupid, we cannot just move 'left' and 'right' out of 'self'. + // We get the error "cannot move out of type that implements Drop". This message + // dates to before Rust 1.0, before mem::forget was marked safe, and has no + // justification that stands up to any scrutiny. There have been mulitple RFCs to + // remove it but for some reason they have never gone anywhere. See for example + // + // https://internals.rust-lang.org/t/destructuring-droppable-structs/20993/41 + // + // Anyway, instead we have to use unsafe code here to do this obviously-safe + // operation in an overcomplicated and hard-to-review way. + unsafe { + // SAFETY we are calling `ptr::read` on valid pointers (they come directly + // from references, which are always valid), and we will mem::forget their old + // locations before any early returns or panics. + + let left = ptr::read(left); + let right = ptr::read(right); + Dag::Binary(left, right) + } + } + Incomplete::Cycle => Dag::Nullary, + Incomplete::Free(s) => { + // SAFETY: see above. We are manually dropping `s` here, which we have to do + // by reading it out of &mut self, because Rust is stupid. + unsafe { + ptr::read(s); + } + Dag::Nullary + } + Incomplete::Final(fin) => { + // SAFETY: see above + unsafe { + ptr::read(fin); + } + Dag::Nullary + } + }; + // Because `Incomplete::drop` calls this method, we cannot allow `self` to be dropped + // under any circumstances, or else we will infinitely recurse and stack-overflow. (This + // is why we had to do ptr::read in every branch above). + mem::forget(self); + ret + } +} + impl DagLike for &'_ Incomplete { type Node = Incomplete; fn data(&self) -> &Incomplete { @@ -105,6 +158,31 @@ impl fmt::Display for Incomplete { } } +impl Drop for Incomplete { + fn drop(&mut self) { + // Note: this is basically identical to the drop impl for node::Node. + fn push_children(stack: &mut Vec>, inner: Incomplete) { + use crate::dag::Dag; + match inner.into_dag() { + Dag::Nullary => {} + Dag::Unary(child) => stack.push(child), + Dag::Binary(left, right) => { + stack.push(left); + stack.push(right); + } + } + } + + let mut stack = Vec::new(); + push_children(&mut stack, std::mem::replace(self, Incomplete::Cycle)); + while let Some(child) = stack.pop() { + if let Some(mut child) = Arc::into_inner(child) { + push_children(&mut stack, std::mem::replace(&mut child, Incomplete::Cycle)); + } + } + } +} + impl Incomplete { /// Whether this "incomplete bound" is the unit type. pub fn is_unit(&self) -> bool {