Add ByteRepr for Ptr#225
Conversation
In C and Rust refcount struct sizes are different. Make sure that the pointer serialization uses the C size.
|
The code seems to be a bit inefficient. I think a single tree is sufficient. Cleanup should happen during traversal and be limited to the visited nodes. We shouldn't visit the whole tree. |
| Self { | ||
| // Increase this if you need higher alignment. In general, malloc returns | ||
| // 16-bits-aligned pointers, but that's not relevant for now. | ||
| cursor: 1, |
There was a problem hiding this comment.
this cursor seems like an unnecessary hack
|
Also, this implementation doesn't work for the +1 OOB case. The only way to do this properly is with a new Ptr kind. |
I don't see how this evicts all dangling references. Only evicting during traversal means that some dangling references will leave in the tree forever, especially in the current implementation where RangeAllocator does not reuse addresses. Besides this the cost of whole-tree-eviction is O(1) amortized.
The cursor is here because neither Weak::as_ptr nor &T from
So the cursor is there to allow ranges to work over C sizes, not over Rust sizes.
Can you eleborate on that? I don't see how a new Ptr kind can solve this because Ptr does not store any information about layout, only about the data it points to. The current refcount model does not preserve struct layout because each field is a Value. If we want to preserve C layout, I think a possible solution would be something similar to unions where we have a backing byte storage that preserves C layout + reinterpreted view for each member. |
I was thinking a bit more about this and I think I have a (not 100% correct, but practical) solution. RangeAllocator now hands addresses that have a gap of 1 byte between them. Because of this is easy to overflow an object and jump into a new one. Instead of having a 1 byte gap. We should have a 4GB gap, effectively using a SyntheticAddr with the upper 32 bytes as address (beginning of range), and lower 32 bytes as offset in that range. This way it will be harder for objects to overflow because they will jump into the gap which is direct panic. The limitation of this is that it limits object size to at most 4GB and if a C program wants to do Ideally we would have 64 bytes for address and 64 bytes for offset. But that's harder to achieve because the size of the serialized pointer must stay the same as in C, which is 64 bytes. |
ByteRepr::to_bytes and ByteRepr::from_bytes are implemented using the new PtrRegistry. PtrRegistry is a global collection that keeps mappings between Ptr and
[base, base + byte_len]. The right side of the interval includes base + byte_len because it's valid to have a pointer +1 OOB.When switching from Ptr to the integer representation, the program is free to do arithmetic on the integer representation. When switching back form integer to Ptr, we construct a Ptr if the integer representation is describing a valid Ptr inside PtrRegistry, otherwise panic.
The PtrRegistry interface is defined as follows:
ByteRepr::to_bytes becomes a call to:
ByteRepr::from_bytes becomes a call to PtrRegistry::get + reconstructing the Ptr with the correct offset and the correct type.
Notes about this implementation:
&a is serialized as a Ptr with
[base_a, base_a + 4], then on the integer representation of the Ptr, the program doesbase_a + 50which happens to fit into[base_b, base_b + 100]. Ptr::from_bytes happily deserializesbase_a + 50as being part of the b range. This is not ideal, a panic should be generated instead. @nunoplopes should we focus on fixing this now or can we consider it a known limitation?[base, base + size]is inclusive in order to accept creating a +1 OOB pointer