Standalone MIRI - Refactor Value to allow one relocation inline

4 files changed, 274 insertions, 244 deletions

diff --git a/tools/standalone_miri/miri.cpp b/tools/standalone_miri/miri.cpp
index 11c620ec..1a235880 100644
--- a/tools/standalone_miri/miri.cpp
+++ b/tools/standalone_miri/miri.cpp
@@ -562,7 +562,8 @@ struct MirHelpers
             }
             if( const auto* s = this->thread.m_modtree.get_static_opt(*ce) ) {
                 ty = s->ty.wrapped(TypeWrapper::Ty::Borrow, 0);
-                return Value::new_pointer(ty, Allocation::PTR_BASE + 0, RelocationPtr::new_alloc(s->val.allocation));
+                LOG_ASSERT(s->val.m_inner.is_alloc, "Statics should already have an allocation assigned");
+                return Value::new_pointer(ty, Allocation::PTR_BASE + 0, RelocationPtr::new_alloc(s->val.m_inner.alloc.alloc));
             }
             LOG_ERROR("Constant::ItemAddr - " << *ce << " - not found");
             } break;
@@ -683,11 +684,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
                 if( !alloc && src_base_value.m_value )
                 {
                     LOG_DEBUG("Borrow - Creating allocation for " << src_base_value);
-                    if( !src_base_value.m_value->allocation )
-                    {
-                        src_base_value.m_value->create_allocation();
-                    }
-                    alloc = RelocationPtr::new_alloc( src_base_value.m_value->allocation );
+                    alloc = RelocationPtr::new_alloc( src_base_value.m_value->borrow("Borrow") );
                 }
                 if( alloc.is_alloc() )
                     LOG_DEBUG("Borrow - alloc=" << alloc << " (" << alloc.alloc() << ")");
@@ -1275,8 +1272,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
                     if( auto r = v_l.get_relocation(0) )
                     {
                         LOG_DEBUG("- Restore relocation " << r);
-                        new_val.create_allocation();
-                        new_val.allocation->set_reloc(0, ::std::min(POINTER_SIZE, new_val.size()), r);
+                        new_val.set_reloc(0, ::std::min(POINTER_SIZE, new_val.size()), r);
                     }
 
                     break;
@@ -1321,8 +1317,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
                     val_l.get().write_to_value(new_val, 0);
                     if( new_val_reloc )
                     {
-                        new_val.create_allocation();
-                        new_val.allocation->set_reloc(0, ::std::min(POINTER_SIZE, new_val.size()), ::std::move(new_val_reloc));
+                        new_val.set_reloc(0, ::std::min(POINTER_SIZE, new_val.size()), ::std::move(new_val_reloc));
                     }
                     break;
                 }
@@ -1518,15 +1513,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
                 auto v = state.get_value_and_type(se.slot, ty);
 
                 // - Take a pointer to the inner
-                auto alloc = v.m_alloc;
-                if( !alloc )
-                {
-                    if( !v.m_value->allocation )
-                    {
-                        v.m_value->create_allocation();
-                    }
-                    alloc = RelocationPtr::new_alloc( v.m_value->allocation );
-                }
+                auto alloc = (v.m_value ? RelocationPtr::new_alloc(v.m_value->borrow("drop")) : v.m_alloc);
                 size_t ofs = v.m_offset;
                 //LOG_ASSERT(ty.get_meta_type() == RawType::Unreachable, "Dropping an unsized type with Statement::Drop - " << ty);
 
@@ -1587,6 +1574,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
             auto v = state.get_value_and_type(te.val, ty);
             LOG_ASSERT(ty.get_wrapper() == nullptr, "Matching on wrapped value - " << ty);
             LOG_ASSERT(ty.inner_type == RawType::Composite, "Matching on non-coposite - " << ty);
+            LOG_DEBUG("Switch v = " << v);
 
             // TODO: Convert the variant list into something that makes it easier to switch on.
             size_t found_target = SIZE_MAX;
@@ -1615,6 +1603,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
                         continue ;
                     if( ::std::memcmp(tmp.data(), var.tag_data.data(), tmp.size()) == 0 )
                     {
+                        LOG_DEBUG("Explicit match " << i);
                         found_target = i;
                         break ;
                     }
@@ -1623,6 +1612,7 @@ bool InterpreterThread::step_one(Value& out_thread_result)
 
             if( found_target == SIZE_MAX )
             {
+                LOG_DEBUG("Default match " << default_target);
                 found_target = default_target;
             }
             if( found_target == SIZE_MAX )
@@ -1825,7 +1815,8 @@ bool InterpreterThread::pop_stack(Value& out_thread_result)
             assert( blk.terminator.is_Call() );
             const auto& te = blk.terminator.as_Call();
 
-            LOG_DEBUG(te.ret_val << " = " << res_v << " (resume " << cur_frame.fcn->my_path << ")");
+            LOG_DEBUG("Resume " << cur_frame.fcn->my_path);
+            LOG_DEBUG(te.ret_val << " = " << res_v);
 
             cur_frame.stmt_idx = 0;
             // If a panic is in progress (in thread state), take the panic block instead
@@ -1967,6 +1958,8 @@ bool InterpreterThread::call_extern(Value& rv, const ::std::string& link_name, c
         auto size = args.at(0).read_usize(0);
         auto align = args.at(1).read_usize(0);
         LOG_DEBUG(link_name << "(size=" << size << ", align=" << align << "): name=" << alloc_name);
+
+        // TODO: Use the alignment when making an allocation?
         auto alloc = Allocation::new_alloc(size, ::std::move(alloc_name));
         LOG_TRACE("- alloc=" << alloc << " (" << alloc->size() << " bytes)");
         auto rty = ::HIR::TypeRef(RawType::Unit).wrap( TypeWrapper::Ty::Pointer, 0 );
@@ -1976,12 +1969,10 @@ bool InterpreterThread::call_extern(Value& rv, const ::std::string& link_name, c
             alloc->mark_bytes_valid(0, size);
         }
 
-        // TODO: Use the alignment when making an allocation?
         rv = Value::new_pointer(rty, Allocation::PTR_BASE, RelocationPtr::new_alloc(::std::move(alloc)));
     }
     else if( link_name == "__rust_reallocate" || link_name == "__rust_realloc" )
     {
-        LOG_ASSERT(args.at(0).allocation, "__rust_reallocate first argument doesn't have an allocation");
         auto alloc_ptr = args.at(0).get_relocation(0);
         auto ptr_ofs = args.at(0).read_usize(0);
         auto oldsize = args.at(1).read_usize(0);
@@ -2001,7 +1992,6 @@ bool InterpreterThread::call_extern(Value& rv, const ::std::string& link_name, c
     }
     else if( link_name == "__rust_deallocate" || link_name == "__rust_dealloc" )
     {
-        LOG_ASSERT(args.at(0).allocation, "__rust_deallocate first argument doesn't have an allocation");
         auto alloc_ptr = args.at(0).get_relocation(0);
         auto ptr_ofs = args.at(0).read_usize(0);
         LOG_ASSERT(ptr_ofs == Allocation::PTR_BASE, "__rust_deallocate with offset pointer");
@@ -2350,10 +2340,9 @@ bool InterpreterThread::call_extern(Value& rv, const ::std::string& link_name, c
         {
             const auto& e = m_thread.tls_values[key];
             rv = Value::new_usize(e.first);
-            rv.create_allocation();
             if( e.second )
             {
-                rv.allocation->set_reloc(0, POINTER_SIZE, e.second);
+                rv.set_reloc(0, POINTER_SIZE, e.second);
             }
         }
         else
@@ -2490,11 +2479,10 @@ bool InterpreterThread::call_extern(Value& rv, const ::std::string& link_name, c
         const void* ret = memchr(ptr, c, n);
 
         rv = Value(::HIR::TypeRef(RawType::USize));
-        rv.create_allocation();
         if( ret )
         {
-            rv.write_usize(0, args.at(0).read_usize(0) + ( static_cast<const uint8_t*>(ret) - static_cast<const uint8_t*>(ptr) ));
-            rv.allocation->relocations.push_back({ 0, ptr_alloc });
+            auto rv_ofs = args.at(0).read_usize(0) + ( static_cast<const uint8_t*>(ret) - static_cast<const uint8_t*>(ptr) );
+            rv.write_ptr(0, rv_ofs, ptr_alloc);
         }
         else
         {
@@ -2511,11 +2499,10 @@ bool InterpreterThread::call_extern(Value& rv, const ::std::string& link_name, c
         const void* ret = memrchr(ptr, c, n);
 
         rv = Value(::HIR::TypeRef(RawType::USize));
-        rv.create_allocation();
         if( ret )
         {
-            rv.write_usize(0, args.at(0).read_usize(0) + ( static_cast<const uint8_t*>(ret) - static_cast<const uint8_t*>(ptr) ));
-            rv.allocation->relocations.push_back({ 0, ptr_alloc });
+            auto rv_ofs = args.at(0).read_usize(0) + ( static_cast<const uint8_t*>(ret) - static_cast<const uint8_t*>(ptr) );
+            rv.write_ptr(0, rv_ofs, ptr_alloc);
         }
         else
         {
diff --git a/tools/standalone_miri/module_tree.cpp b/tools/standalone_miri/module_tree.cpp
index 71a545fa..2d1eb838 100644
--- a/tools/standalone_miri/module_tree.cpp
+++ b/tools/standalone_miri/module_tree.cpp
@@ -152,8 +152,7 @@ bool Parser::parse_one()
         Static s;
         s.val = Value(ty);
         // - Statics need to always have an allocation (for references)
-        if( !s.val.allocation )
-            s.val.create_allocation();
+        s.val.ensure_allocation();
         s.val.write_bytes(0, data.data(), data.size());
         s.ty = ty;
 
@@ -173,14 +172,13 @@ bool Parser::parse_one()
                     auto reloc_str = ::std::move(lex.consume().strval);
 
                     auto a = Allocation::new_alloc( reloc_str.size(), FMT_STRING("static " << p) );
-                    //a.alloc().set_tag();
                     a->write_bytes(0, reloc_str.data(), reloc_str.size());
-                    s.val.allocation->relocations.push_back({ ofs, /*size,*/ RelocationPtr::new_alloc(::std::move(a)) });
+                    s.val.set_reloc( ofs, size, RelocationPtr::new_alloc(::std::move(a)) );
                 }
                 else if( lex.next() == "::" || lex.next() == "<" )
                 {
                     auto reloc_path = parse_path();
-                    s.val.allocation->relocations.push_back({ ofs, /*size,*/ RelocationPtr::new_fcn(reloc_path) });
+                    s.val.set_reloc( ofs, size, RelocationPtr::new_fcn(reloc_path) );
                 }
                 else
                 {
diff --git a/tools/standalone_miri/value.cpp b/tools/standalone_miri/value.cpp
index 8860c8d9..a497f0bd 100644
--- a/tools/standalone_miri/value.cpp
+++ b/tools/standalone_miri/value.cpp
@@ -21,6 +21,37 @@ namespace {
             return false;
         return ofs + size <= max_size;
     }
+
+    void set_bit(uint8_t* p, size_t i, bool v)
+    {
+        if(v) {
+            p[i/8] |= 1 << (i%8);
+        }
+        else {
+            p[i/8] &= ~(1 << (i%8));
+        }
+    }
+    bool get_bit(const uint8_t* p, size_t i) {
+        return (p[i/8] & (1 << (i%8))) != 0;
+    }
+    void copy_bits(uint8_t* dst, size_t dst_ofs, const uint8_t* src, size_t src_ofs,  size_t len)
+    {
+        // Even number of bytes, fast copy
+        if( dst_ofs % 8 == 0 && src_ofs % 8 == 0 && len % 8 == 0 )
+        {
+            for(size_t i = 0; i < len/8; i ++)
+            {
+                dst[dst_ofs/8 + i] = src[src_ofs/8 + i];
+            }
+        }
+        else
+        {
+            for(size_t i = 0; i < len; i ++)
+            {
+                set_bit( dst, dst_ofs+i, get_bit(src, src_ofs+i) );
+            }
+        }
+    }
 };
 
 ::std::ostream& operator<<(::std::ostream& os, const Allocation* x)
@@ -383,57 +414,25 @@ Value Allocation::read_value(size_t ofs, size_t size) const
     {
         if( ofs <= r.slot_ofs && r.slot_ofs < ofs + size )
         {
+            // NOTE: A relocation at offset zero is allowed
+            if( r.slot_ofs == ofs )
+                continue ;
             has_reloc = true;
         }
     }
-    if( has_reloc || size > sizeof(rv.direct_data.data) )
-    {
-        rv.allocation = Allocation::new_alloc(size, FMT_STRING("Allocation::read_value(" << ofs << "," << size << ")"));
-
-        rv.write_bytes(0, this->data_ptr() + ofs, size);
+    rv = Value::with_size(size, has_reloc);
+    rv.write_bytes(0, this->data_ptr() + ofs, size);
 
-        for(const auto& r : this->relocations)
-        {
-            if( ofs <= r.slot_ofs && r.slot_ofs < ofs + size )
-            {
-                rv.allocation->relocations.push_back({ r.slot_ofs - ofs, r.backing_alloc });
-            }
-        }
-
-        // Copy the mask bits
-        for(size_t i = 0; i < size; i ++)
-        {
-            size_t j = ofs + i;
-            const uint8_t test_mask = (1 << (j%8));
-            const uint8_t set_mask = (1 << (i%8));
-            bool v = (this->m_mask[j/8] & test_mask) != 0;
-            if( v )
-            {
-                rv.allocation->m_mask[i/8] |= set_mask;
-            }
-        }
-    }
-    else
+    for(const auto& r : this->relocations)
     {
-        rv.direct_data.size = static_cast<uint8_t>(size);
-
-        rv.write_bytes(0, this->data_ptr() + ofs, size);
-        rv.direct_data.mask[0] = 0;
-        rv.direct_data.mask[1] = 0;
-
-        // Copy the mask bits
-        for(size_t i = 0; i < size; i ++)
+        if( ofs <= r.slot_ofs && r.slot_ofs < ofs + size )
         {
-            size_t j = ofs + i;
-            const uint8_t tst_mask = 1 << (j%8);
-            const uint8_t set_mask = 1 << (i%8);
-            bool v = (this->m_mask[j/8] & tst_mask) != 0;
-            if( v )
-            {
-                rv.direct_data.mask[i/8] |= set_mask;
-            }
+            rv.set_reloc(r.slot_ofs - ofs, /*r.size*/POINTER_SIZE, r.backing_alloc);
         }
     }
+    // Copy the mask bits
+    copy_bits(rv.get_mask_mut(), 0, m_mask.data(), ofs, size);
+
     return rv;
 }
 void Allocation::read_bytes(size_t ofs, void* dst, size_t count) const
@@ -461,14 +460,15 @@ void Allocation::write_value(size_t ofs, Value v)
         LOG_ERROR("Use of freed memory " << this);
     //if( this->is_read_only )
     //    LOG_ERROR("Writing to read-only allocation " << this);
-    if( v.allocation )
+    if( v.m_inner.is_alloc )
     {
-        size_t  v_size = v.allocation->size();
-        const auto& src_alloc = *v.allocation;
+        const auto& src_alloc = *v.m_inner.alloc.alloc;
+        size_t  v_size = src_alloc.size();
+        assert(&src_alloc != this); // Shouldn't happen?
+
         // Take a copy of the source mask
         auto s_mask = src_alloc.m_mask;
-
-        // Save relocations first, because `Foo = Foo` is valid.
+        // Save relocations first, because `Foo = Foo` is valid?
         ::std::vector<Relocation>   new_relocs = src_alloc.relocations;
         // - write_bytes removes any relocations in this region.
         write_bytes(ofs, src_alloc.data_ptr(), v_size);
@@ -487,39 +487,16 @@ void Allocation::write_value(size_t ofs, Value v)
         }
 
         // Set mask in destination
-        if( ofs % 8 != 0 || v_size % 8 != 0 )
-        {
-            // Lazy way, sets/clears individual bits
-            for(size_t i = 0; i < v_size; i ++)
-            {
-                uint8_t dbit = 1 << ((ofs+i) % 8);
-                if( s_mask[i/8] & (1 << (i %8)) )
-                    this->m_mask[ (ofs+i) / 8 ] |= dbit;
-                else
-                    this->m_mask[ (ofs+i) / 8 ] &= ~dbit;
-            }
-        }
-        else
-        {
-            // Copy the mask bytes directly
-            for(size_t i = 0; i < v_size / 8; i ++)
-            {
-                this->m_mask[ofs/8+i] = s_mask[i];
-            }
-        }
+        copy_bits(m_mask.data(), ofs,  s_mask.data(), 0,  v_size);
     }
     else
     {
-        this->write_bytes(ofs, v.direct_data.data, v.direct_data.size);
-
-        // Lazy way, sets/clears individual bits
-        for(size_t i = 0; i < v.direct_data.size; i ++)
+        this->write_bytes(ofs, v.data_ptr(), v.size());
+        copy_bits(m_mask.data(), ofs,  v.get_mask(), 0,  v.size());
+        // TODO: Copy relocation
+        if( v.m_inner.direct.reloc_0 )
         {
-            uint8_t dbit = 1 << ((ofs+i) % 8);
-            if( v.direct_data.mask[i/8] & (1 << (i %8)) )
-                this->m_mask[ (ofs+i) / 8 ] |= dbit;
-            else
-                this->m_mask[ (ofs+i) / 8 ] &= ~dbit;
+            this->set_reloc(ofs, POINTER_SIZE,  ::std::move(v.m_inner.direct.reloc_0));
         }
     }
 }
@@ -618,58 +595,55 @@ void Allocation::set_reloc(size_t ofs, size_t len, RelocationPtr reloc)
 
 Value::Value()
 {
-    this->direct_data.size = 0;
-    memset(this->direct_data.mask, 0, sizeof(this->direct_data.mask));
+    memset(&m_inner, 0, sizeof(m_inner));
 }
 Value::Value(::HIR::TypeRef ty)
 {
     size_t size = ty.get_size();
 
     // Support inline data if the data will fit within the inline region (which is the size of the metadata)
-    if( ty.get_size() <= sizeof(this->direct_data.data) )
+    if( size <= sizeof(m_inner.direct.data) )
     {
         // AND the type doesn't contain a pointer (of any kind)
-        if( ! ty.has_pointer() )
+        // TODO: Pointers _are_ allowed now (but only one)
+        if( true || ! ty.has_pointer() )
         {
             // Will fit in a inline allocation, nice.
             //LOG_TRACE("No pointers in " << ty << ", storing inline");
-            this->direct_data.size = static_cast<uint8_t>(size);
-            this->direct_data.mask[0] = 0;
-            this->direct_data.mask[1] = 0;
+            new(&m_inner.direct) Inner::Direct(size);
             return ;
         }
     }
 
     // Fallback: Make a new allocation
     //LOG_TRACE(" Creating allocation for " << ty);
-    this->allocation = Allocation::new_alloc(size, FMT_STRING(ty));
+    new(&m_inner.alloc) Inner::Alloc( Allocation::new_alloc(size, FMT_STRING(ty)) );
+    assert(m_inner.is_alloc);
 }
 Value Value::with_size(size_t size, bool have_allocation)
 {
     Value   rv;
-    if(have_allocation || size > sizeof(rv.direct_data.data))
+    if(have_allocation || size > sizeof(m_inner.direct.data))
     {
-        rv.allocation = Allocation::new_alloc(size, FMT_STRING("with_size(" << size << ")"));
+        new(&rv.m_inner.alloc) Inner::Alloc( Allocation::new_alloc(size, FMT_STRING("with_size(" << size << ")")) );
     }
     else
     {
-        rv.direct_data.size = static_cast<uint8_t>(size);
-        memset(rv.direct_data.mask, 0, sizeof(rv.direct_data.mask));
+        new(&rv.m_inner.direct) Inner::Direct(size);
     }
     return rv;
 }
 Value Value::new_fnptr(const ::HIR::Path& fn_path)
 {
     Value   rv( ::HIR::TypeRef(::HIR::CoreType { RawType::Function }) );
-    assert(rv.allocation);
-    rv.allocation->write_ptr(0, Allocation::PTR_BASE, RelocationPtr::new_fcn(fn_path));
+    rv.write_ptr(0, Allocation::PTR_BASE, RelocationPtr::new_fcn(fn_path));
     return rv;
 }
 Value Value::new_ffiptr(FFIPointer ffi)
 {
     Value   rv( ::HIR::TypeRef(::HIR::CoreType { RawType::USize }) );
-    rv.create_allocation();
-    rv.allocation->write_ptr(0, Allocation::PTR_BASE, RelocationPtr::new_ffi(ffi));
+    assert( !rv.m_inner.is_alloc );
+    rv.write_ptr(0, Allocation::PTR_BASE, RelocationPtr::new_ffi(ffi));
     return rv;
 }
 Value Value::new_pointer(::HIR::TypeRef ty, uint64_t v, RelocationPtr r) {
@@ -707,52 +681,49 @@ Value Value::new_i64(int64_t v) {
 
 void Value::create_allocation()
 {
-    assert(!this->allocation);
-    this->allocation = Allocation::new_alloc(this->direct_data.size, "create_allocation");
-    if( this->direct_data.size > 0 )
-        this->allocation->m_mask[0] = this->direct_data.mask[0];
-    if( this->direct_data.size > 8 )
-        this->allocation->m_mask[1] = this->direct_data.mask[1];
-    ::std::memcpy(this->allocation->data_ptr(), this->direct_data.data, this->direct_data.size);
+    assert(!m_inner.is_alloc);
+    auto new_alloc = Allocation::new_alloc(m_inner.direct.size, "create_allocation");   // TODO: Provide a better name?
+    auto& direct = m_inner.direct;
+    if( direct.size > 0 )
+        new_alloc->m_mask[0] = direct.mask[0];
+    if( direct.size > 8 )
+        new_alloc->m_mask[1] = direct.mask[1];
+    ::std::memcpy(new_alloc->data_ptr(), direct.data, direct.size);
+    if( direct.reloc_0 )
+    {
+        new_alloc->set_reloc(0, POINTER_SIZE, ::std::move(direct.reloc_0));
+    }
+
+    new(&m_inner.alloc) Inner::Alloc(::std::move(new_alloc));
 }
 void Value::check_bytes_valid(size_t ofs, size_t size) const
 {
     if( size == 0 )
         return ;
-    if( this->allocation )
-    {
-        this->allocation->check_bytes_valid(ofs, size);
+    if( !in_bounds(ofs, size, this->size()) ) {
+        LOG_ERROR("Read out of bounds " << ofs+size << " >= " << this->size());
+        throw "ERROR";
     }
-    else
+    const auto* mask = this->get_mask();
+    for(size_t i = ofs; i < ofs + size; i++)
     {
-        if( size == 0 && this->direct_data.size > 0 ) {
-            return ;
-        }
-        if( !in_bounds(ofs, size, this->direct_data.size) ) {
-            LOG_ERROR("Read out of bounds " << ofs+size << " >= " << int(this->direct_data.size));
-            throw "ERROR";
-        }
-        for(size_t i = ofs; i < ofs + size; i++)
+        if( !get_bit(mask, i) )
         {
-            if( !(this->direct_data.mask[i/8] & (1 << i%8)) )
-            {
-                LOG_ERROR("Accessing invalid bytes in value");
-                throw "ERROR";
-            }
+            LOG_ERROR("Accessing invalid bytes in value, offset " << i << " of " << *this);
         }
     }
 }
 void Value::mark_bytes_valid(size_t ofs, size_t size)
 {
-    if( this->allocation )
+    if( m_inner.is_alloc )
     {
-        this->allocation->mark_bytes_valid(ofs, size);
+        m_inner.alloc.alloc->mark_bytes_valid(ofs, size);
     }
     else
     {
         for(size_t i = ofs; i < ofs+size; i++)
         {
-            this->direct_data.mask[i/8] |= (1 << i%8);
+            m_inner.direct.mask[i/8] |= (1 << i%8);
         }
     }
 }
@@ -760,18 +731,28 @@ void Value::mark_bytes_valid(size_t ofs, size_t size)
 Value Value::read_value(size_t ofs, size_t size) const
 {
     Value   rv;
-    //TRACE_FUNCTION_R(ofs << ", " << size << ") - " << *this, rv);
-    if( this->allocation )
+    TRACE_FUNCTION_R(ofs << ", " << size << " - " << *this, rv);
+    if( m_inner.is_alloc )
     {
-        rv = this->allocation->read_value(ofs, size);
+        rv = m_inner.alloc.alloc->read_value(ofs, size);
     }
     else
     {
         // Inline always fits in inline.
-        rv.direct_data.size = static_cast<uint8_t>(size);
-        rv.write_bytes(0, this->direct_data.data+ofs, size);
-        rv.direct_data.mask[0] = this->direct_data.mask[0];
-        rv.direct_data.mask[1] = this->direct_data.mask[1];
+        if( ofs == 0 && size == this->size() )
+        {
+            rv = Value(*this);
+        }
+        else
+        {
+            rv.m_inner.direct.size = static_cast<uint8_t>(size);
+            memcpy(rv.m_inner.direct.data, this->data_ptr() + ofs, size);
+            copy_bits(rv.m_inner.direct.mask, 0, this->get_mask(), ofs, size);
+            if( ofs == 0 )
+            {
+                rv.m_inner.direct.reloc_0 = RelocationPtr(m_inner.direct.reloc_0);
+            }
+        }
     }
     return rv;
 }
@@ -779,20 +760,14 @@ void Value::read_bytes(size_t ofs, void* dst, size_t count) const
 {
     if(count == 0)
         return ;
-    if( this->allocation )
+    if( m_inner.is_alloc )
     {
-        this->allocation->read_bytes(ofs, dst, count);
+        m_inner.alloc.alloc->read_bytes(ofs, dst, count);
     }
     else
     {
         check_bytes_valid(ofs, count);
-
-        // TODO: Redundant due to above?
-        if( !in_bounds(ofs, count, this->direct_data.size) ) {
-            LOG_ERROR("Out of bounds read, " << ofs << "+" << count << " > " << this->size());
-            throw "ERROR";
-        }
-        ::std::memcpy(dst, this->direct_data.data + ofs, count);
+        ::std::memcpy(dst, m_inner.direct.data + ofs, count);
     }
 }
 
@@ -800,82 +775,65 @@ void Value::write_bytes(size_t ofs, const void* src, size_t count)
 {
     if( count == 0 )
         return ;
-    if( this->allocation )
+    if( m_inner.is_alloc )
     {
-        this->allocation->write_bytes(ofs, src, count);
+        m_inner.alloc.alloc->write_bytes(ofs, src, count);
     }
     else
     {
-        if( !in_bounds(ofs, count, this->direct_data.size) ) {
-            LOG_BUG("Write extends outside value size (" << ofs << "+" << count << " >= " << (int)this->direct_data.size << ")");
+        auto& direct = m_inner.direct;
+        if( !in_bounds(ofs, count, direct.size) ) {
+            LOG_ERROR("Write extends outside value size (" << ofs << "+" << count << " >= " << (int)direct.size << ")");
         }
-        ::std::memcpy(this->direct_data.data + ofs, src, count);
+        ::std::memcpy(direct.data + ofs, src, count);
         mark_bytes_valid(ofs, count);
+        if( 0 <= ofs && ofs < POINTER_SIZE ) {
+            direct.reloc_0 = RelocationPtr();
+        }
     }
 }
 void Value::write_value(size_t ofs, Value v)
 {
-    if( this->allocation )
+    if( m_inner.is_alloc )
     {
-        this->allocation->write_value(ofs, ::std::move(v));
+        m_inner.alloc.alloc->write_value(ofs, ::std::move(v));
     }
     else
     {
-        if( v.allocation && !v.allocation->relocations.empty() )
-        {
-            this->create_allocation();
-            this->allocation->write_value(ofs, ::std::move(v));
-        }
-        else
-        {
-            write_bytes(ofs, v.direct_data.data, v.direct_data.size);
+        write_bytes(ofs, v.data_ptr(), v.size());
+        // - Copy mask
+        copy_bits(this->get_mask_mut(), ofs,  v.get_mask(), 0,  v.size());
 
-            // Lazy way, sets/clears individual bits
-            for(size_t i = 0; i < v.direct_data.size; i ++)
+        // TODO: Faster way of knowing where there are relocations
+        for(size_t i = 0; i < v.size(); i ++)
+        {
+            if( auto r = v.get_relocation(i) )
             {
-                uint8_t dbit = 1 << ((ofs+i) % 8);
-                if( v.direct_data.mask[i/8] & (1 << (i %8)) )
-                    this->direct_data.mask[ (ofs+i) / 8 ] |= dbit;
-                else
-                    this->direct_data.mask[ (ofs+i) / 8 ] &= ~dbit;
+                this->set_reloc(ofs + i, POINTER_SIZE, r);
             }
         }
     }
 }
 void Value::write_ptr(size_t ofs, size_t ptr_ofs, RelocationPtr reloc)
 {
-    if( !this->allocation )
+    if( m_inner.is_alloc )
     {
-        LOG_ERROR("Writing a pointer with no allocation");
-    }
-    this->allocation->write_ptr(ofs, ptr_ofs, ::std::move(reloc));
-}
-
-::std::ostream& operator<<(::std::ostream& os, const Value& v)
-{
-    if( v.allocation )
-    {
-        os << *v.allocation;
+        m_inner.alloc.alloc->write_ptr(ofs, ptr_ofs, ::std::move(reloc));
     }
     else
     {
-        auto flags = os.flags();
-        os << ::std::hex;
-        for(size_t i = 0; i < v.direct_data.size; i++)
+        write_usize(ofs, ptr_ofs);
+        if( ofs != 0 )
         {
-            if( i != 0 )
-                os << " ";
-            if( v.direct_data.mask[i/8] & (1 << i%8) )
-            {
-                os << ::std::setw(2) << ::std::setfill('0') << (int)v.direct_data.data[i];
-            }
-            else
-            {
-                os << "--";
-            }
+            LOG_ERROR("Writing a pointer with no allocation");
         }
-        os.setf(flags);
+        m_inner.direct.reloc_0 = ::std::move(reloc);
     }
+}
+
+::std::ostream& operator<<(::std::ostream& os, const Value& v)
+{
+    os << ValueRef(const_cast<Value&>(v), 0, v.size());
     return os;
 }
 extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v)
@@ -940,9 +898,9 @@ extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v)
             break;
         }
     }
-    else if( v.m_value && v.m_value->allocation )
+    else if( v.m_value && v.m_value->m_inner.is_alloc )
     {
-        const auto& alloc = *v.m_value->allocation;
+        const auto& alloc = *v.m_value->m_inner.alloc.alloc;
 
         os << &alloc << "@" << v.m_offset << "+" << v.m_size << " ";
 
@@ -976,7 +934,7 @@ extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v)
     }
     else if( v.m_value )
     {
-        const auto& direct = v.m_value->direct_data;
+        const auto& direct = v.m_value->m_inner.direct;
 
         auto flags = os.flags();
         os << ::std::hex;
@@ -994,6 +952,10 @@ extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v)
             }
         }
         os.setf(flags);
+        if(direct.reloc_0)
+        {
+            os << " { " << direct.reloc_0 << " }";
+        }
     }
     else
     {
diff --git a/tools/standalone_miri/value.hpp b/tools/standalone_miri/value.hpp
index dea4c890..a76e922b 100644
--- a/tools/standalone_miri/value.hpp
+++ b/tools/standalone_miri/value.hpp
@@ -313,39 +313,85 @@ extern ::std::ostream& operator<<(::std::ostream& os, const Allocation& x);
 struct Value:
     public ValueCommonWrite
 {
-    // TODO: Use this union for more compact representation
+//private:
     union Inner {
         bool    is_alloc;
-        struct {
+        struct Alloc {
             bool    is_alloc;
             AllocationHandle    alloc;
+
+            Alloc(AllocationHandle alloc):
+                is_alloc(true),
+                alloc(::std::move(alloc))
+            {
+            }
         } alloc;
         // Sizeof = 4+8+8 = 20b (plus vtable?)
-        struct {
+        struct Direct {
             bool    is_alloc;
             uint8_t size;
             uint8_t mask[2];
             uint8_t data[2*8];
             RelocationPtr   reloc_0;    // Relocation only for 0+POINTER_SIZE
+
+            Direct(size_t size):
+                is_alloc(false),
+                size(static_cast<uint8_t>(size)),
+                mask{0,0}
+            {
+            }
         } direct;
-    };
-    // If NULL, data is direct
-    AllocationHandle    allocation;
-    // TODO: Use an enum and a single relocation slot
-    struct {
-        // NOTE: Can't pack the mask+size tighter, need 4 bits of size (8-15) leaving 12 bits of mask
-        uint8_t data[2*8-3];   // 13 data bytes, plus 16bit mask, plus size = 16 bytes
-        uint8_t mask[2];
-        uint8_t size;
-    } direct_data;
 
+        Inner():
+            direct(0)
+        {
+        }
+        ~Inner() {
+            if(is_alloc) {
+                alloc.~Alloc();
+            }
+            else {
+                direct.~Direct();
+            }
+        }
+        Inner(const Inner& x) {
+            if(x.is_alloc) {
+                new(&this->alloc) Alloc(x.alloc);
+            }
+            else {
+                new(&this->direct) Direct(x.direct);
+            }
+        }
+        Inner(Inner&& x) {
+            if(x.is_alloc) {
+                new(&this->alloc) Alloc(::std::move(x.alloc));
+            }
+            else {
+                new(&this->direct) Direct(::std::move(x.direct));
+            }
+        }
+        Inner& operator=(Inner&& x) {
+            this->~Inner();
+            new(this) Inner(x);
+            return *this;
+        }
+        Inner& operator=(const Inner& x) {
+            this->~Inner();
+            new(this) Inner(x);
+            return *this;
+        }
+    } m_inner;
+
+public:
     Value();
     Value(::HIR::TypeRef ty);
+    ~Value() {
+    }
 
-    //Value(const Value&) = delete;
-    //Value(Value&&) = default;
-    //Value& operator=(const Value&) = delete;
-    //Value& operator=(Value&&) = default;
+    Value(const Value&) = default;
+    Value(Value&&) = default;
+    Value& operator=(const Value& x) = delete;
+    Value& operator=(Value&& x) = default;
 
     static Value with_size(size_t size, bool have_allocation);
     static Value new_fnptr(const ::HIR::Path& fn_path);
@@ -357,16 +403,49 @@ struct Value:
     static Value new_i32(int32_t v);
     static Value new_i64(int64_t v);
 
+    AllocationHandle borrow(::std::string loc) {
+        if( !m_inner.is_alloc )
+            create_allocation(/*loc*/);
+        return m_inner.alloc.alloc;
+    }
+    void ensure_allocation() {
+        if( !m_inner.is_alloc )
+            create_allocation();
+    }
     void create_allocation();
-    size_t size() const { return allocation ? allocation->size() : direct_data.size; }
-    const uint8_t* data_ptr() const { return allocation ? allocation->data_ptr() : direct_data.data; }
-          uint8_t* data_ptr()       { return allocation ? allocation->data_ptr() : direct_data.data; }
+    size_t size() const { return m_inner.is_alloc ? m_inner.alloc.alloc->size() : m_inner.direct.size; }
+    const uint8_t* data_ptr() const { return m_inner.is_alloc ? m_inner.alloc.alloc->data_ptr() : m_inner.direct.data; }
+          uint8_t* data_ptr()       { return m_inner.is_alloc ? m_inner.alloc.alloc->data_ptr() : m_inner.direct.data; }
+    const uint8_t* get_mask() const { return m_inner.is_alloc ? m_inner.alloc.alloc->m_mask.data() : m_inner.direct.mask; }
+          uint8_t* get_mask_mut()   { return m_inner.is_alloc ? m_inner.alloc.alloc->m_mask.data() : m_inner.direct.mask; }
 
     RelocationPtr get_relocation(size_t ofs) const override {
-        if( this->allocation && this->allocation )
-            return this->allocation->get_relocation(ofs);
+        if( m_inner.is_alloc )
+            return m_inner.alloc.alloc->get_relocation(ofs);
+        else if(ofs == 0)
+        {
+            return m_inner.direct.reloc_0;
+        }
         else
+        {
             return RelocationPtr();
+        }
+    }
+    void set_reloc(size_t ofs, size_t size, RelocationPtr p) /*override*/ {
+        if( m_inner.is_alloc )
+        {
+            m_inner.alloc.alloc->set_reloc(ofs, size, ::std::move(p));
+        }
+        else if( ofs == 0 /*&& size == POINTER_SIZE*/ )
+        {
+            m_inner.direct.reloc_0 = ::std::move(p);
+        }
+        else
+        {
+            this->create_allocation();
+            assert( m_inner.is_alloc );
+            m_inner.alloc.alloc->set_reloc(ofs, size, ::std::move(p));
+        }
     }
 
     void check_bytes_valid(size_t ofs, size_t size) const;
@@ -557,3 +636,7 @@ struct ValueRef:
     bool compare(size_t offset, const void* other, size_t other_len) const;
 };
 extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v);
+//struct ValueRefMut:
+//    public ValueCommonWrite
+//{
+//};