Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic!() calls in stubs could be made easier to debug #2

Open
bcantrill opened this issue Dec 18, 2021 · 4 comments
Open

panic!() calls in stubs could be made easier to debug #2

bcantrill opened this issue Dec 18, 2021 · 4 comments

Comments

@bcantrill
Copy link
Contributor

Currently, stubs will call panic!() without an argument to cut down on ROM size. This is sensible, but in running into #1, the failure was in fact difficult to debug. Here is the panic from #1 as observed:

humility: attached to dump
system time = 55965
ID TASK                 GEN PRI STATE    
 8 hiffy                  1   3 FAULT:  (was: ready)
   |
   +--->  0x20008440 0x0800b53a userlib::sys_panic_stub
                     @ /home/bmc/hubris/sys/userlib/src/lib.rs:686
          0x200084f0 0x0800b582 userlib::sys_panic
                     @ /home/bmc/hubris/sys/userlib/src/lib.rs:678
          0x200084f0 0x0800b588 rust_begin_unwind
                     @ /home/bmc/hubris/sys/userlib/src/lib.rs:863
          0x20008508 0x0800a4ba core::panicking::panic_fmt
                     @ /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c//library/core/src/panicking.rs:88
          0x20008530 0x0800aa34 core::panicking::panic
                     @ /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c//library/core/src/panicking.rs:39
          0x20008580 0x08008b2c task_hiffy::common::qspi_page_program
                     @ /home/bmc/hubris/task/hiffy/src/common.rs:202
          0x20008800 0x0800928c hif::execute::function
                     @ /home/bmc/.cargo/git/checkouts/hif-766e4be28bfdbf05/e512e4c/src/lib.rs:297
          0x20008800 0x08008fe8 hif::execute
                     @ /home/bmc/.cargo/git/checkouts/hif-766e4be28bfdbf05/e512e4c/src/lib.rs:259
          0x20008800 0x0800928c main
                     @ /home/bmc/hubris/task/hiffy/src/main.rs:84

There's a Humility issue here in that the empty panic!() string isn't handled particularly well -- but it's also not overly clear from the stack trace what has happened. One way to achieve the desired results with respect to ROM but also make it much clearer as to the root cause is to inject a frame specific to the error, e.g.:

diff --git a/src/client.rs b/src/client.rs
index 62e95bd..8f7c422 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -51,6 +51,16 @@ pub fn generate_client_stub(
     writeln!(out)?;
 
     writeln!(out, "impl {} {{", iface.name)?;
+    writeln!(out, r##"
+    fn bad_lease_panic() {{
+        // Note: we're not generating a panic message in the client to
+        // save ROM space -- but we give this panic its own stack frame
+        // to indicate from the symbol that the client has erred; if the
+        // user chases the line number into the client stub source file
+        // the error should be clear.
+        panic!();
+    }}"##)?;
+
     for (idx, (name, op)) in iface.ops.iter().enumerate() {
         writeln!(out, "    // operation: {} ({})", name, idx)?;
         if op.idempotent {
@@ -132,10 +142,7 @@ pub fn generate_client_stub(
         for (leasename, lease) in &op.leases {
             if let Some(n) = lease.max_len {
                 writeln!(out, "        if arg_{}.len() >= {} {{", leasename, n)?;
-                // Note: we're not generating a panic message in the client to
-                // save ROM space. If the user chases the line number into the
-                // client stub source file the error should be clear.
-                writeln!(out, "            panic!();")?;
+                writeln!(out, "            {}::bad_lease_panic();", iface.name)?;
                 writeln!(out, "        }}")?;
             }
         }

This results in a much clearer stack:

humility: attached via ST-Link
system time = 25639
ID TASK                 GEN PRI STATE    
 8 hiffy                  1   3 FAULT:  (was: ready)
   |
   +--->  0x20008420 0x0800b722 userlib::sys_panic_stub
                     @ /home/bmc/hubris/sys/userlib/src/lib.rs:686
          0x200084d0 0x0800b76a userlib::sys_panic
                     @ /home/bmc/hubris/sys/userlib/src/lib.rs:678
          0x200084d0 0x0800b770 rust_begin_unwind
                     @ /home/bmc/hubris/sys/userlib/src/lib.rs:863
          0x200084e8 0x0800a692 core::panicking::panic_fmt
                     @ /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c//library/core/src/panicking.rs:88
          0x20008510 0x0800ac0c core::panicking::panic
                     @ /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c//library/core/src/panicking.rs:39
          0x20008518 0x0800aefe drv_gimlet_hf_api::HostFlash::bad_lease_panic
                     @ /home/bmc/hubris/target/thumbv7em-none-eabihf/release/build/drv-gimlet-hf-api-afda7c131dd13e6f/out/client_stub.rs:25
          0x20008580 0x08008d04 drv_gimlet_hf_api::HostFlash::page_program
                     @ /home/bmc/hubris/target/thumbv7em-none-eabihf/release/build/drv-gimlet-hf-api-afda7c131dd13e6f/out/client_stub.rs:194
          0x20008580 0x08008d08 task_hiffy::common::qspi_page_program
                     @ /home/bmc/hubris/task/hiffy/src/common.rs:206
          0x20008800 0x08009464 hif::execute::function
                     @ /home/bmc/.cargo/git/checkouts/hif-766e4be28bfdbf05/e512e4c/src/lib.rs:297
          0x20008800 0x080091c0 hif::execute
                     @ /home/bmc/.cargo/git/checkouts/hif-766e4be28bfdbf05/e512e4c/src/lib.rs:259
          0x20008800 0x08009464 main
                     @ /home/bmc/hubris/task/hiffy/src/main.rs:84

@cbiffle
Copy link
Collaborator

cbiffle commented Dec 19, 2021

Hm. What do you get for addr2line for the address 0x08008b2c? Because if I fix the Humility line number decoding, this might become less annoying.

I'm also surprised that the panic! has no message at all, because normally you'd get the text explicit panic at <file>:<line>. It's possible to compile out panic messages from userlib by unsetting the panic-messages feature, but hiffy does not appear to be doing this.

@bcantrill
Copy link
Contributor Author

Sorry, meant to add that detail: addr2line just offers up a question mark -- even when run with -i. 😐

@bcantrill
Copy link
Contributor Author

And... yes, I was also surprised about not having additional information.

@cbiffle
Copy link
Collaborator

cbiffle commented Dec 20, 2021

Sorry, meant to add that detail: addr2line just offers up a question mark -- even when run with -i. neutral_face

Buh. I'll see what that address correlates to in the DWARF line number tables. addr2line should be using them correctly but that sure sounds bogus.

bcantrill added a commit that referenced this issue Dec 20, 2021
bcantrill added a commit that referenced this issue Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants