Standalone MIRI - TLS and some other messing about

4 files changed, 228 insertions, 57 deletions

diff --git a/src/trans/codegen_mmir.cpp b/src/trans/codegen_mmir.cpp
index db538842..19574814 100644
--- a/src/trans/codegen_mmir.cpp
+++ b/src/trans/codegen_mmir.cpp
@@ -157,7 +157,7 @@ namespace
         {
             if( is_executable )
             {
-                m_of << "fn ::main#(i32, *const *const i8): i32 {\n";
+                m_of << "fn ::main#(isize, *const *const i8): i32 {\n";
                 auto c_start_path = m_resolve.m_crate.get_lang_item_path_opt("mrustc-start");
                 if( c_start_path == ::HIR::SimplePath() )
                 {
diff --git a/tools/standalone_miri/main.cpp b/tools/standalone_miri/main.cpp
index b7f62252..1b33087a 100644
--- a/tools/standalone_miri/main.cpp
+++ b/tools/standalone_miri/main.cpp
@@ -19,9 +19,31 @@ struct ProgramOptions
     int parse(int argc, const char* argv[]);
 };
 
-Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> args);
-Value MIRI_Invoke_Extern(const ::std::string& link_name, const ::std::string& abi, ::std::vector<Value> args);
-Value MIRI_Invoke_Intrinsic(ModuleTree& modtree, const ::std::string& name, const ::HIR::PathParams& ty_params, ::std::vector<Value> args);
+struct ThreadState
+{
+    static unsigned s_next_tls_key;
+    unsigned call_stack_depth;
+    ::std::vector<uint64_t> tls_values;
+
+    ThreadState():
+        call_stack_depth(0)
+    {
+    }
+
+    struct DecOnDrop {
+        unsigned* p;
+        ~DecOnDrop() { (*p) --; }
+    };
+    DecOnDrop enter_function() {
+        this->call_stack_depth ++;
+        return DecOnDrop { &this->call_stack_depth };
+    }
+};
+unsigned ThreadState::s_next_tls_key = 1;
+
+Value MIRI_Invoke(ModuleTree& modtree, ThreadState& thread, ::HIR::Path path, ::std::vector<Value> args);
+Value MIRI_Invoke_Extern(ThreadState& thread, const ::std::string& link_name, const ::std::string& abi, ::std::vector<Value> args);
+Value MIRI_Invoke_Intrinsic(ModuleTree& modtree, ThreadState& thread,  const ::std::string& name, const ::HIR::PathParams& ty_params, ::std::vector<Value> args);
 
 int main(int argc, const char* argv[])
 {
@@ -36,20 +58,21 @@ int main(int argc, const char* argv[])
 
     tree.load_file(opts.infile);
 
-    auto val_argc = Value( ::HIR::TypeRef{RawType::I32} );
+    auto val_argc = Value( ::HIR::TypeRef{RawType::ISize} );
     ::HIR::TypeRef  argv_ty { RawType::I8 };
     argv_ty.wrappers.push_back(TypeWrapper { TypeWrapper::Ty::Pointer, 0 });
     argv_ty.wrappers.push_back(TypeWrapper { TypeWrapper::Ty::Pointer, 0 });
     auto val_argv = Value(argv_ty);
-    val_argc.write_bytes(0, "\0\0\0", 4);
+    val_argc.write_bytes(0, "\0\0\0\0\0\0\0", 8);
     val_argv.write_bytes(0, "\0\0\0\0\0\0\0", argv_ty.get_size());
 
     try
     {
+        ThreadState ts;
         ::std::vector<Value>    args;
         args.push_back(::std::move(val_argc));
         args.push_back(::std::move(val_argv));
-        auto rv = MIRI_Invoke( tree, tree.find_lang_item("start"), ::std::move(args) );
+        auto rv = MIRI_Invoke( tree, ts, tree.find_lang_item("start"), ::std::move(args) );
         ::std::cout << rv << ::std::endl;
     }
     catch(const DebugExceptionTodo& /*e*/)
@@ -291,7 +314,7 @@ struct Ops {
 namespace
 {
 
-    void drop_value(ModuleTree& modtree, Value ptr, const ::HIR::TypeRef& ty)
+    void drop_value(ModuleTree& modtree, ThreadState& thread, Value ptr, const ::HIR::TypeRef& ty)
     {
         if( ty.wrappers.empty() )
         {
@@ -301,7 +324,7 @@ namespace
                 {
                     LOG_DEBUG("Drop - " << ty);
 
-                    MIRI_Invoke(modtree, ty.composite_type->drop_glue, { ptr });
+                    MIRI_Invoke(modtree, thread, ty.composite_type->drop_glue, { ptr });
                 }
                 else
                 {
@@ -338,7 +361,7 @@ namespace
 
 }
 
-Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> args)
+Value MIRI_Invoke(ModuleTree& modtree, ThreadState& thread, ::HIR::Path path, ::std::vector<Value> args)
 {
     Value   ret;
 
@@ -373,17 +396,22 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
     if( fcn.external.link_name != "" )
     {
         // External function!
-        ret = MIRI_Invoke_Extern(fcn.external.link_name, fcn.external.link_abi, ::std::move(args));
+        ret = MIRI_Invoke_Extern(thread, fcn.external.link_name, fcn.external.link_abi, ::std::move(args));
         LOG_DEBUG(path << " = " << ret);
         return ret;
     }
 
-    // TODO: Recursion limit.
+    // Recursion limit.
+    if( thread.call_stack_depth > 40 ) {
+        LOG_ERROR("Recursion limit exceeded");
+    }
+    auto _ = thread.enter_function();
 
     TRACE_FUNCTION_R(path, path << " = " << ret);
     for(size_t i = 0; i < args.size(); i ++)
     {
         LOG_DEBUG("- Argument(" << i << ") = " << args[i]);
+        // TODO: Check argument sizes against prototype?
     }
 
     ret = Value(fcn.ret_ty == RawType::Unreachable ? ::HIR::TypeRef() : fcn.ret_ty);
@@ -489,7 +517,7 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                 ::HIR::TypeRef  ptr_ty;
                 auto val = get_value_and_type(*e.val, ptr_ty);
                 ty = ptr_ty.get_inner();
-                LOG_DEBUG("val = " << val);
+                LOG_DEBUG("val = " << val << ", (inner) ty=" << ty);
 
                 LOG_ASSERT(val.m_size >= POINTER_SIZE, "Deref of a value that doesn't fit a pointer - " << ty);
                 size_t ofs = val.read_usize(0);
@@ -500,7 +528,7 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                 LOG_ASSERT(val_alloc.is_alloc(), "Deref of a value with a non-data allocation");
                 LOG_TRACE("Deref " << val_alloc.alloc() << " + " << ofs << " to give value of type " << ty);
                 auto alloc = val_alloc.alloc().get_relocation(val.m_offset);
-                LOG_ASSERT(alloc, "Deref of a value with no relocation");
+                // NOTE: No alloc can happen when dereferencing a zero-sized pointer
                 if( alloc.is_alloc() )
                 {
                     LOG_DEBUG("> " << lv << " alloc=" << alloc.alloc());
@@ -517,15 +545,26 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                     meta_val = ::std::make_shared<Value>( val.read_value(POINTER_SIZE, meta_size) );
 
                     // TODO: Get a more sane size from the metadata
-                    LOG_DEBUG("> Meta " << *meta_val << ", size = " << alloc.get_size() << " - " << ofs);
-                    size = alloc.get_size() - ofs;
+                    if( alloc )
+                    {
+                        LOG_DEBUG("> Meta " << *meta_val << ", size = " << alloc.get_size() << " - " << ofs);
+                        size = alloc.get_size() - ofs;
+                    }
+                    else
+                    {
+                        size = 0;
+                    }
                 }
                 else
                 {
                     LOG_ASSERT(val.m_size == POINTER_SIZE, "Deref of a value that isn't a pointer-sized value (size=" << val.m_size << ") - " << val << ": " << ptr_ty);
                     size = ty.get_size();
+                    if( !alloc ) {
+                        LOG_ERROR("Deref of a value with no relocation - " << val);
+                    }
                 }
 
+                LOG_DEBUG("alloc=" << alloc << ", ofs=" << ofs << ", size=" << size);
                 auto rv = ValueRef(::std::move(alloc), ofs, size);
                 rv.m_metadata = ::std::move(meta_val);
                 return rv;
@@ -709,7 +748,7 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                     ::HIR::TypeRef  src_ty;
                     ValueRef src_base_value = state.get_value_and_type(re.val, src_ty);
                     auto alloc = src_base_value.m_alloc;
-                    if( !alloc )
+                    if( !alloc && src_base_value.m_value )
                     {
                         if( !src_base_value.m_value->allocation )
                         {
@@ -1422,7 +1461,7 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                     ptr_val.write_usize(0, ofs);
                     ptr_val.allocation.alloc().relocations.push_back(Relocation { 0, ::std::move(alloc) });
 
-                    drop_value(modtree, ptr_val, ty);
+                    drop_value(modtree, thread, ptr_val, ty);
                     // TODO: Clear validity on the entire inner value.
                     //alloc.mark_as_freed();
                 }
@@ -1515,11 +1554,12 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
             for(const auto& a : te.args)
             {
                 sub_args.push_back( state.param_to_value(a) );
+                LOG_DEBUG("#" << (sub_args.size() - 1) << " " << sub_args.back());
             }
             if( te.fcn.is_Intrinsic() )
             {
                 const auto& fe = te.fcn.as_Intrinsic();
-                state.write_lvalue(te.ret_val, MIRI_Invoke_Intrinsic(modtree, fe.name, fe.params, ::std::move(sub_args)));
+                state.write_lvalue(te.ret_val, MIRI_Invoke_Intrinsic(modtree, thread, fe.name, fe.params, ::std::move(sub_args)));
             }
             else
             {
@@ -1534,7 +1574,7 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                     LOG_DEBUG("> Indirect call " << v);
                     // TODO: Assert type
                     // TODO: Assert offset/content.
-                    assert(v.read_usize(v.m_offset) == 0);
+                    assert(v.read_usize(0) == 0);
                     auto& alloc_ptr = v.m_alloc ? v.m_alloc : v.m_value->allocation;
                     LOG_ASSERT(alloc_ptr, "Calling value that can't be a pointer (no allocation)");
                     fcn_alloc_ptr = alloc_ptr.alloc().get_relocation(v.m_offset);
@@ -1545,7 +1585,7 @@ Value MIRI_Invoke(ModuleTree& modtree, ::HIR::Path path, ::std::vector<Value> ar
                 }
 
                 LOG_DEBUG("Call " << *fcn_p);
-                auto v = MIRI_Invoke(modtree, *fcn_p, ::std::move(sub_args));
+                auto v = MIRI_Invoke(modtree, thread, *fcn_p, ::std::move(sub_args));
                 LOG_DEBUG(te.ret_val << " = " << v << " (resume " << path << ")");
                 state.write_lvalue(te.ret_val, ::std::move(v));
             }
@@ -1562,7 +1602,7 @@ extern "C" {
     long sysconf(int);
 }
 
-Value MIRI_Invoke_Extern(const ::std::string& link_name, const ::std::string& abi, ::std::vector<Value> args)
+Value MIRI_Invoke_Extern(ThreadState& thread, const ::std::string& link_name, const ::std::string& abi, ::std::vector<Value> args)
 {
     if( link_name == "__rust_allocate" )
     {
@@ -1712,7 +1752,45 @@ Value MIRI_Invoke_Extern(const ::std::string& link_name, const ::std::string& ab
         rv.write_i32(0, 0);
         return rv;
     }
-    else if( link_name == "pthread_key_create" || link_name == "pthread_key_delete" )
+    else if( link_name == "pthread_key_create" )
+    {
+        size_t  size;
+        auto key_ref = args.at(0).read_pointer_valref_mut(0, 4);
+
+        auto key = ThreadState::s_next_tls_key ++;
+        key_ref.m_alloc.alloc().write_u32( key_ref.m_offset, key );
+        
+        auto rv = Value(::HIR::TypeRef(RawType::I32));
+        rv.write_i32(0, 0);
+        return rv;
+    }
+    else if( link_name == "pthread_getspecific" )
+    {
+        auto key = args.at(0).read_u32(0);
+
+        // Get a pointer-sized value from storage
+        uint64_t v = key < thread.tls_values.size() ? thread.tls_values[key] : 0;
+
+        auto rv = Value(::HIR::TypeRef(RawType::USize));
+        rv.write_usize(0, v);
+        return rv;
+    }
+    else if( link_name == "pthread_setspecific" )
+    {
+        auto key = args.at(0).read_u32(0);
+        auto v = args.at(1).read_u64(0);
+
+        // Get a pointer-sized value from storage
+        if( key >= thread.tls_values.size() ) {
+            thread.tls_values.resize(key+1);
+        }
+        thread.tls_values[key] = v;
+
+        auto rv = Value(::HIR::TypeRef(RawType::I32));
+        rv.write_i32(0, 0);
+        return rv;
+    }
+    else if( link_name == "pthread_key_delete" )
     {
         auto rv = Value(::HIR::TypeRef(RawType::I32));
         rv.write_i32(0, 0);
@@ -1758,7 +1836,7 @@ Value MIRI_Invoke_Extern(const ::std::string& link_name, const ::std::string& ab
     }
     throw "";
 }
-Value MIRI_Invoke_Intrinsic(ModuleTree& modtree, const ::std::string& name, const ::HIR::PathParams& ty_params, ::std::vector<Value> args)
+Value MIRI_Invoke_Intrinsic(ModuleTree& modtree, ThreadState& thread, const ::std::string& name, const ::HIR::PathParams& ty_params, ::std::vector<Value> args)
 {
     Value rv;
     TRACE_FUNCTION_R(name, rv);
@@ -1798,6 +1876,24 @@ Value MIRI_Invoke_Intrinsic(ModuleTree& modtree, const ::std::string& name, cons
         const auto& ty = ty_params.tys.at(0);
         rv = alloc.alloc().read_value(ofs, ty.get_size());
     }
+    else if( name == "atomic_cxchg" )
+    {
+        const auto& ty_T = ty_params.tys.at(0);
+        // TODO: Get a ValueRef to the target location
+        auto data_ref = args.at(0).read_pointer_valref_mut(0, ty_T.get_size());
+        const auto& old_v = args.at(1);
+        const auto& new_v = args.at(1);
+        rv = Value::with_size( ty_T.get_size() + 1, false );
+        rv.write_value(0, data_ref.read_value(0, old_v.size()));
+        if( data_ref.compare(old_v.data_ptr(), old_v.size()) == 0 ) {
+            data_ref.m_alloc.alloc().write_value( data_ref.m_offset, new_v );
+            rv.write_u8( old_v.size(), 1 );
+        }
+        else {
+            rv.write_u8( old_v.size(), 0 );
+        }
+        return rv;
+    }
     else if( name == "transmute" )
     {
         // Transmute requires the same size, so just copying the value works
@@ -1919,7 +2015,7 @@ Value MIRI_Invoke_Intrinsic(ModuleTree& modtree, const ::std::string& name, cons
             auto ptr = val.read_value(0, POINTER_SIZE);;
             for(size_t i = 0; i < item_count; i ++)
             {
-                drop_value(modtree, ptr, ity);
+                drop_value(modtree, thread, ptr, ity);
                 ptr.write_usize(0, ptr.read_usize(0) + item_size);
             }
         }
diff --git a/tools/standalone_miri/value.cpp b/tools/standalone_miri/value.cpp
index af462e40..468425e9 100644
--- a/tools/standalone_miri/value.cpp
+++ b/tools/standalone_miri/value.cpp
@@ -211,6 +211,20 @@ void* ValueCommon::read_pointer_unsafe(size_t rd_ofs, size_t req_valid, size_t&
         throw "";
     }
 }
+ValueRef ValueCommon::read_pointer_valref_mut(size_t rd_ofs, size_t size)
+{
+    auto ofs = read_usize(rd_ofs);
+    auto reloc = get_relocation(rd_ofs);
+    if( !reloc )
+    {
+        LOG_ERROR("Getting ValRef to null pointer (no relocation)");
+    }
+    else
+    {
+        // TODO: Validate size
+        return ValueRef(reloc, ofs, size);
+    }
+}
 
 
 void Allocation::resize(size_t new_size)
@@ -231,7 +245,7 @@ void Allocation::check_bytes_valid(size_t ofs, size_t size) const
     {
         if( !(this->mask[i/8] & (1 << i%8)) )
         {
-            ::std::cerr << "ERROR: Invalid bytes in value" << ::std::endl;
+            LOG_ERROR("Invalid bytes in value");
             throw "ERROR";
         }
     }
@@ -749,7 +763,7 @@ extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v)
         {
         case AllocationPtr::Ty::Allocation: {
             const auto& alloc = alloc_ptr.alloc();
-            
+
             auto flags = os.flags();
             os << ::std::hex;
             for(size_t i = v.m_offset; i < ::std::min(alloc.size(), v.m_offset + v.m_size); i++)
@@ -823,6 +837,41 @@ extern ::std::ostream& operator<<(::std::ostream& os, const ValueRef& v)
     return os;
 }
 
+Value ValueRef::read_value(size_t ofs, size_t size) const
+{
+    if( size == 0 )
+        return Value();
+    if( !(ofs < m_size && size <= m_size && ofs + size <= m_size) ) {
+        LOG_ERROR("Read exceeds bounds, " << ofs << " + " << size << " > " << m_size << " - from " << *this);
+    }
+    if( m_alloc ) {
+        switch(m_alloc.get_ty())
+        {
+        case AllocationPtr::Ty::Allocation:
+            return m_alloc.alloc().read_value(m_offset + ofs, size);
+        case AllocationPtr::Ty::StdString: {
+            auto rv = Value::with_size(size, false);
+            //ASSERT_BUG(ofs <= m_alloc.str().size(), "");
+            //ASSERT_BUG(size <= m_alloc.str().size(), "");
+            //ASSERT_BUG(ofs+size <= m_alloc.str().size(), "");
+            assert(m_offset+ofs <= m_alloc.str().size() && size <= m_alloc.str().size() && m_offset+ofs+size <= m_alloc.str().size());
+            rv.write_bytes(0, m_alloc.str().data() + m_offset + ofs, size);
+            return rv;
+            }
+        default:
+            //ASSERT_BUG(m_alloc.is_alloc(), "read_value on non-data backed Value - " << );
+            throw "TODO";
+        }
+    }
+    else {
+        return m_value->read_value(m_offset + ofs, size);
+    }
+}
+bool ValueRef::compare(const void* other, size_t other_len) const
+{
+    check_bytes_valid(0, other_len);
+    return ::std::memcmp(data_ptr(), other, other_len) == 0;
+}
 uint64_t ValueRef::read_usize(size_t ofs) const
 {
     uint64_t    v = 0;
diff --git a/tools/standalone_miri/value.hpp b/tools/standalone_miri/value.hpp
index 95f8b508..2b3bd4e6 100644
--- a/tools/standalone_miri/value.hpp
+++ b/tools/standalone_miri/value.hpp
@@ -15,6 +15,7 @@ namespace HIR {
 }
 class Allocation;
 struct Value;
+struct ValueRef;
 
 struct FFIPointer
 {
@@ -160,6 +161,8 @@ struct ValueCommon
             //LOG_FATAL("Attempting to get an uninit pointer to immutable data");
         return rv;
     }
+    /// Read a pointer and return a ValueRef to it (mutable data)
+    ValueRef read_pointer_valref_mut(size_t rd_ofs, size_t size);
 };
 
 class Allocation:
@@ -223,6 +226,8 @@ struct Value:
 
     void create_allocation();
     size_t size() const { return allocation ? allocation.alloc().size() : direct_data.size; }
+    const uint8_t* data_ptr() const { return allocation ? allocation.alloc().data_ptr() : direct_data.data; }
+          uint8_t* data_ptr()       { return allocation ? allocation.alloc().data_ptr() : direct_data.data; }
 
     AllocationPtr get_relocation(size_t ofs) const override {
         if( this->allocation && this->allocation.is_alloc() )
@@ -259,20 +264,23 @@ struct ValueRef
         m_offset(ofs),
         m_size(size)
     {
-        switch(m_alloc.get_ty())
+        if( m_alloc )
         {
-        case AllocationPtr::Ty::Allocation:
-            assert(ofs < m_alloc.alloc().size());
-            assert(size <= m_alloc.alloc().size());
-            assert(ofs+size <= m_alloc.alloc().size());
-            break;
-        case AllocationPtr::Ty::StdString:
-            assert(ofs < m_alloc.str().size());
-            assert(size <= m_alloc.str().size());
-            assert(ofs+size <= m_alloc.str().size());
-            break;
-        default:
-            throw "TODO";
+            switch(m_alloc.get_ty())
+            {
+            case AllocationPtr::Ty::Allocation:
+                assert(ofs < m_alloc.alloc().size());
+                assert(size <= m_alloc.alloc().size());
+                assert(ofs+size <= m_alloc.alloc().size());
+                break;
+            case AllocationPtr::Ty::StdString:
+                assert(ofs < m_alloc.str().size());
+                assert(size <= m_alloc.str().size());
+                assert(ofs+size <= m_alloc.str().size());
+                break;
+            default:
+                throw "TODO";
+            }
         }
     }
     ValueRef(Value& val):
@@ -294,7 +302,7 @@ struct ValueRef
             else
                 return AllocationPtr();
         }
-        else if( m_value->allocation )
+        else if( m_value && m_value->allocation )
         {
             if( m_value->allocation.is_alloc() )
                 return m_value->allocation.alloc().get_relocation(ofs);
@@ -306,9 +314,30 @@ struct ValueRef
             return AllocationPtr();
         }
     }
-    Value read_value(size_t ofs, size_t size) const {
+    Value read_value(size_t ofs, size_t size) const;
+    const uint8_t* data_ptr() const {
+        if( m_alloc ) {
+            switch(m_alloc.get_ty())
+            {
+            case AllocationPtr::Ty::Allocation:
+                return m_alloc.alloc().data_ptr() + m_offset;
+                break;
+            case AllocationPtr::Ty::StdString:
+                return reinterpret_cast<const uint8_t*>(m_alloc.str().data() + m_offset);
+            default:
+                throw "TODO";
+            }
+        }
+        else if( m_value ) {
+            return m_value->data_ptr() + m_offset;
+        }
+        else {
+            return nullptr;
+        }
+    }
+    void read_bytes(size_t ofs, void* dst, size_t size) const {
         if( size == 0 )
-            return Value();
+            return ;
         assert(ofs < m_size);
         assert(size <= m_size);
         assert(ofs+size <= m_size);
@@ -316,26 +345,22 @@ struct ValueRef
             switch(m_alloc.get_ty())
             {
             case AllocationPtr::Ty::Allocation:
-                return m_alloc.alloc().read_value(m_offset + ofs, size);
-            case AllocationPtr::Ty::StdString: {
-                auto rv = Value::with_size(size, false);
-                //ASSERT_BUG(ofs <= m_alloc.str().size(), "");
-                //ASSERT_BUG(size <= m_alloc.str().size(), "");
-                //ASSERT_BUG(ofs+size <= m_alloc.str().size(), "");
+                m_alloc.alloc().read_bytes(m_offset + ofs, dst, size);
+                break;
+            case AllocationPtr::Ty::StdString:
                 assert(m_offset+ofs <= m_alloc.str().size() && size <= m_alloc.str().size() && m_offset+ofs+size <= m_alloc.str().size());
-                rv.write_bytes(0, m_alloc.str().data() + m_offset + ofs, size);
-                return rv;
-                }
+                ::std::memcpy(dst, m_alloc.str().data() + m_offset + ofs, size);
+                break;
             default:
                 //ASSERT_BUG(m_alloc.is_alloc(), "read_value on non-data backed Value - " << );
                 throw "TODO";
             }
         }
         else {
-            return m_value->read_value(m_offset + ofs, size);
+            m_value->read_bytes(m_offset + ofs, dst, size);
         }
     }
-    void read_bytes(size_t ofs, void* dst, size_t size) const {
+    void check_bytes_valid(size_t ofs, size_t size) const {
         if( size == 0 )
             return ;
         assert(ofs < m_size);
@@ -345,11 +370,10 @@ struct ValueRef
             switch(m_alloc.get_ty())
             {
             case AllocationPtr::Ty::Allocation:
-                m_alloc.alloc().read_bytes(m_offset + ofs, dst, size);
+                m_alloc.alloc().check_bytes_valid(m_offset + ofs, size);
                 break;
             case AllocationPtr::Ty::StdString:
                 assert(m_offset+ofs <= m_alloc.str().size() && size <= m_alloc.str().size() && m_offset+ofs+size <= m_alloc.str().size());
-                ::std::memcpy(dst, m_alloc.str().data() + m_offset + ofs, size);
                 break;
             default:
                 //ASSERT_BUG(m_alloc.is_alloc(), "read_value on non-data backed Value - " << );
@@ -357,10 +381,12 @@ struct ValueRef
             }
         }
         else {
-            m_value->read_bytes(m_offset + ofs, dst, size);
+            m_value->check_bytes_valid(m_offset + ofs, size);
         }
     }
 
+    bool compare(const void* other, size_t other_len) const;
+
     // TODO: Figure out how to make this use `ValueCommon` when it can't write.
     uint8_t read_u8(size_t ofs) const { uint8_t rv; read_bytes(ofs, &rv, 1); return rv; }
     uint16_t read_u16(size_t ofs) const { uint16_t rv; read_bytes(ofs, &rv, 2); return rv; }