print macros should not return a result

jonas-schievink commented 4 years ago

The hprint{ln} macros currently all return a Result to the caller. There are two reasons for why this is not ideal:

It doesn't match the print{ln} macros in libstd, which return nothing (()).
It sets the wrong expectation that semihosting failures are reliably reported via Err. This is not the case as semihosting calls will just crash the application when no debugger is attached (it would be great to fix that regardless).

pftbest commented 4 years ago

I am using this code to detect if debugger is present so my prints don't crash when I don't have JTAG/SWD attached. It works reasonably well for STM32, there are some corner cases, like for example when I have my STLINK plugged in, but don't have openocd running on PC, this function will still report that stlink is present, and semihosting will crash, but that I can live with.

    fn is_debugger_present() -> bool {
        const C_DEBUGEN: u32 = 1;
        let dhcsr = unsafe { read_reg!(stm32ral::dcb, DCB, DHCSR) };
        (dhcsr & C_DEBUGEN) == C_DEBUGEN
    }

    pub fn _print(args: Arguments) {
        if is_debugger_present() {
            interrupt::free(|cs| {
                let itm = unsafe { &mut *ITM::ptr() };
                itm::write_fmt(&mut itm.stim[0], args);
            });
        }
    }

thejpster commented 4 years ago

@jonas-schievink, I agree.

japaric commented 4 years ago

It doesn't match the print{ln} macros in libstd, which return nothing (()).

Does they have to match return type of the macros in libstd? The current implementation is more flexible: it lets the user handle error, ignore it or unwrap it. They can even write a macro that replicates what the std macros do (and call it println!).

It sets the wrong expectation that semihosting failures are reliably reported via Err. This is not the case as semihosting calls will just crash the application when no debugger is attached (it would be great to fix that regardless).

The ARM semihosting spec includes a return code for errors. Perhaps OpenOCD never returns an error but that's only one implementation of a semihosting environment. This crate works within QEMU (IDK if it uses the error exit code at all) and I hope it will work with any other Cortex-M emulator that comes around. My point is that we should not make decisions based on a single semihosting implementation (even if it's the most used one) -- this crate is cortex-m-semihosting not cortex-m-openocd-semihosting.

jonas-schievink commented 4 years ago

Does they have to match return type of the macros in libstd?

No, but I'd say that matching libstd in behavior is less surprising and makes for an easy to learn API. Just like with the libstd macros, if control over error handling is desired, the user can always acquire a handle to the stream and use writeln!.

The ARM semihosting spec includes a return code for errors.

Yes, and debugger hosts may well use them, but that point is moot when no debug probe is attached to the target. In that case, the bkpt instruction to trigger a semihosting call will simply crash the application before any error code can be returned (https://github.com/rust-embedded/cortex-m-semihosting/issues/8).

There seems to be no reliable vendor-independent way of detecting this case, so the best we can do is loudly document this behavior. This is one of those kinds of pitfalls that are impossible to navigate, since it only happens when the thing you'd use to debug them is not attached. I'd love to get rid of this somehow, but it looks like that isn't going to happen.

rust-embedded / cortex-m-semihosting

print macros should not return a result #46