betrusted-io / xous-core

The Xous microkernel
Apache License 2.0
532 stars 85 forks source link

hard to reproduce: possible rkyv issue with String? #48

Closed bunnie closed 3 years ago

bunnie commented 3 years ago

I noticed a weird bug today where shifting the order of the text String in the TextView struct fixed a bug where it seems like some items after the text was being overwritten. It's hard to reproduce -- the symptom is that a single word in a Gid appears to be set to "0", but it changes based on factors unknown. Once it happens, it's repeatable, but it only shows up on real hardware and not hosted mode. Here is an example:

INFO - IMEF: pc canvas Gid { gid: [2467029514, 1791361364, 3307595540, 289361412] }
INFO - GAM: bogus GID Gid { gid: [2467029514, 0, 3307595540, 289361412] } in TextView Ready for input..., not doing anything in response to draw request.

This is the commit used to make this bug: https://github.com/betrusted-io/xous-core/commit/307df04e8b232ef305e0fd4b2d1f8c67328d59b1

And this commit "fixes" it: https://github.com/betrusted-io/xous-core/commit/685f36af410f29d04338ec11d3049bdf46c3eb5d

The issue only seems to happen after lending the TextView to another process. I suspect I probably did not do something quite right with the implementation of String in rkyv.

For now, moving the String type to the end seems to work around this issue, but making a note of it here to look into this more carefully sometime in the future. This is very likely related to a problem with the unsafe serialization code around the TextView struct!

xobs commented 3 years ago

This commit also fixes it:

--- a/services/graphics-server/src/api/text.rs
+++ b/services/graphics-server/src/api/text.rs
@@ -44,6 +44,7 @@ impl From<usize> for TextOp {
 // roughly 168 bytes to represent the rest of the struct, and we want to fill out the 4096 byte page with text
 const TEXTVIEW_LEN: usize = 3072;

+#[repr(C)]
 #[derive(Copy, Clone, rkyv::Archive, rkyv::Unarchive)]
 pub struct TextView {
     // this is the operation as specified for the GAM. Note this is different from the "op" when sent to graphics-server

At this point I'm betting it has to do with how Unarchive is implemented on String.

bunnie commented 3 years ago

This was fixed in Xous 0.8