Fix Complex error size (#403)

Idol calls returning a `Complex` error type return a non-zero error code of `1`,
but _also_ serialize their error into the buffer provided by the caller.  Right
now, we're not necessarily sizing that buffer correctly when making HIF and RPC
calls!  If the `ok` type is smaller than the `error` type, then the buffer will
be undersized and bytes will be dropped on the floor during the `sys_reply`
call.

`sys_reply` does not report errors, but it expects the server to check the
buffer size itself.  A separate issue in Idol (idolatry#36) means that the
generated server code makes the same mistake in checking the buffer size;
otherwise, we would have seen this error much sooner!

(Luckily, this all works correctly in task-to-task calls, because Idol's
generated client API correctly computes the maximum `REPLY_SIZE` and passes
correctly-sized buffers)

This PR correctly computes the `reply_size()` for Idol operations using
`Complex` error types.  It also fixes an error in computing serialized size for
`enum` types (we were taking the sum of variant sizes instead of the max, which
is wrong).

Cargo.toml

@@ -17,7 +17,7 @@ name = "humility"
 #
 # Be sure to check in and push all of the files that change.  Happy versioning!
 #
-version = "0.10.24"
+version = "0.10.25"
 authors = ["Bryan Cantrill <bryan@oxide.computer>"]
 edition = "2018"
 license = "MPL-2.0"

cmd/hiffy/src/lib.rs

@@ -133,7 +133,7 @@ pub fn hiffy_list(hubris: &HubrisArchive, filter: Vec<String>) -> Result<()> {
             Ok((_, idol::IdolError::Complex(t))) => match &op.1.reply {
                 Reply::Result { ok, .. } => {
                     println!("{}{:<27} {}", margin, "<ok>", ok.ty.0);
-                    println!("{}{:<27} {}", margin, "<error>", t);
+                    println!("{}{:<27} {}", margin, "<error>", t.name);
                 }
                 _ => warn!("mismatch on reply: found {op:?}"),
             },

cmd/rpc/src/lib.rs

@@ -406,15 +406,7 @@ impl<'a> RpcClient<'a> {
 
         let our_image_id = self.hubris.image_id().unwrap();
 
-        let nreply = match op.operation.encoding {
-            ::idol::syntax::Encoding::Zerocopy => {
-                self.hubris.typesize(op.ok)?
-            }
-            ::idol::syntax::Encoding::Ssmarshal
-            | ::idol::syntax::Encoding::Hubpack => {
-                self.hubris.hubpack_serialized_maxsize(op.ok)?
-            }
-        };
+        let nreply = op.reply_size()?;
 
         let header = RpcHeader {
             image_id: U64::from_bytes(our_image_id.try_into().unwrap()),

humility-core/src/hubris.rs

@@ -3029,9 +3029,8 @@ impl HubrisArchive {
         &self,
         goff: HubrisGoff,
     ) -> Result<usize> {
-        let mut total = 0;
-
         if let Some(v) = self.structs.get(&goff) {
+            let mut total = 0;
             for m in &v.members {
                 total += self.hubpack_serialized_maxsize(m.goff)?;
             }
@@ -3043,13 +3042,13 @@ impl HubrisArchive {
         }
 
         if let Some(v) = self.enums.get(&goff) {
-            total += 1; // hubpack tag
+            let mut max = 0; // largest variant
             for variant in &v.variants {
                 if let Some(goff) = variant.goff {
-                    total += self.hubpack_serialized_maxsize(goff)?;
+                    max = max.max(self.hubpack_serialized_maxsize(goff)?);
                 }
             }
-            return Ok(total);
+            return Ok(max + 1); // the extra byte is for the hubpack tag
         }
 
         if let Some(v) = self.arrays.get(&goff) {

humility-hiffy/src/lib.rs

@@ -778,16 +778,7 @@ impl<'a> HiffyContext<'a> {
         }
 
         ops.push(push(payload.len() as u32));
-        let reply_size = match op.operation.encoding {
-            ::idol::syntax::Encoding::Zerocopy => {
-                self.hubris.typesize(op.ok)?
-            }
-            ::idol::syntax::Encoding::Ssmarshal
-            | ::idol::syntax::Encoding::Hubpack => {
-                self.hubris.hubpack_serialized_maxsize(op.ok)?
-            }
-        };
-        ops.push(push(reply_size as u32));
+        ops.push(push(op.reply_size()? as u32));
         if let Some(lease_size) = lease_size {
             ops.push(push(lease_size));
         }
@@ -1286,8 +1277,8 @@ pub fn hiffy_decode(
                     Err(format!("<Unknown variant {e}>"))
                 }
             }
-            idol::IdolError::Complex(ref t) => {
-                Err(format!("<Complex error: {t}>"))
+            idol::IdolError::Complex(error) => {
+                Err(format!("<Complex error: {}>", error.name))
             }
             _ => Err(format!("<Unhandled error {e:x?}>")),
         },

humility-idol/src/lib.rs

@@ -45,7 +45,7 @@ where
 #[derive(Debug)]
 pub enum IdolError<'a> {
     CLike(&'a HubrisEnum),
-    Complex(String),
+    Complex(&'a HubrisEnum),
     None,
 }
 
@@ -206,6 +206,24 @@ impl<'a> IdolOperation<'a> {
             format!("<Unknown {}.{} error: {}>", self.name.0, self.name.1, code)
         }
     }
+
+    pub fn reply_size(&self) -> Result<usize> {
+        let reply_size = match self.operation.encoding {
+            ::idol::syntax::Encoding::Zerocopy => {
+                self.hubris.typesize(self.ok)?
+            }
+            ::idol::syntax::Encoding::Ssmarshal
+            | ::idol::syntax::Encoding::Hubpack => {
+                let ok = self.hubris.hubpack_serialized_maxsize(self.ok)?;
+                if let IdolError::Complex(e) = self.error {
+                    ok.max(self.hubris.hubpack_serialized_maxsize(e.goff)?)
+                } else {
+                    ok
+                }
+            }
+        };
+        Ok(reply_size)
+    }
 }
 
 //
@@ -683,7 +701,10 @@ pub fn lookup_reply<'a>(
                 }
                 ::idol::syntax::Error::ServerDeath => IdolError::None,
                 ::idol::syntax::Error::Complex(t) => {
-                    IdolError::Complex(t.0.clone())
+                    let t = m.lookup_enum_byname(hubris, &t.0)?.ok_or_else(
+                        || anyhow!("failed to find error type {reply:?}"),
+                    )?;
+                    IdolError::Complex(t)
                 }
             };
             Ok((lookup_ok(&ok.ty.0)?, err))

tests/cmd/chip.trycmd

@@ -13,7 +13,7 @@ For more information try --help
 
 ```
 $ humility --chip this-can-be-anything -V
-humility 0.10.24
+humility 0.10.25
 
 ```
 
@@ -28,7 +28,7 @@ For more information try --help
 
 ```
 $ humility -c apx432 -V
-humility 0.10.24
+humility 0.10.25
 
 ```
 

tests/cmd/version.trycmd

@@ -2,14 +2,14 @@ Long version flag:
 
 ```
 $ humility --version
-humility 0.10.24
+humility 0.10.25
 
 ```
 
 Short version flag:
 
 ```
 $ humility -V
-humility 0.10.24
+humility 0.10.25
 
 ```