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

runtime: allow panic on syscall.Exit instead of halting #4010

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

rminnich
Copy link

In some environments, such as u-root, code can be imported which provides a shell. The shell calls functions, instead of trying to use exec.Command (which is not available). An example can be found in github.com:u-root/u-root/tools/tinygobb.

When the command exits, the shell needs to resume operation, which is not possible when runtime.Exit calls halt.

On tamago, we achieve this by setting a normally nil function, runtime.Exit, to a funtion which panics. The shell has a recover() to catch the panics.

There are many ways in which setting runtime.Exit can go wrong, including different functions setting it to different functions.

tinygo allows a simpler, and hence more robust, model: instead of halting, we can panic. But in most cases, halting is ok.

Add a function, runtime.PanicOnExit, which enables a panic on exit. This function is idempotent, and once panic on exit is enabled, it can not be disabled. This is restrictive by design. It is hoped that by making it simple and non-revocable, we can avoid the problems that can occur with more complex mechanisms, but still provide an option to halting.

A function is provided, instead of just a variable, to allow simpler access from assembly, i.e., both variables and functions have been used in bare metal environments and, for several reasons, functions have proved more flexible.

In some environments, such as u-root, code can be imported which
provides a shell. The shell calls functions, instead of trying
to use exec.Command (which is not available). An example
can be found in github.com:u-root/u-root/tools/tinygobb.

When the command exits, the shell needs to resume operation,
which is not possible when runtime.Exit calls halt.

On tamago, we achieve this by setting a normally nil
function, runtime.Exit, to a funtion which panics. The
shell has a recover() to catch the panics.

There are many ways in which setting runtime.Exit can go wrong,
including different functions setting it to different functions.

tinygo allows a simpler, and hence more robust, model: instead
of halting, we can panic. But in most cases, halting is ok.

Add a function, runtime.PanicOnExit, which enables a panic
on exit. This function is idempotent, and once panic on exit
is enabled, it can not be disabled. This is restrictive
by design. It is hoped that by making it simple and non-revocable,
we can avoid the problems that can occur with more complex
mechanisms, but still provide an option to halting.

A function is provided, instead of just a variable, to allow
simpler access from assembly, i.e., both variables and
functions have been used in bare metal environments and, for
several reasons, functions have proved more flexible.

Signed-off-by: Ronald G. Minnich <[email protected]>
@rminnich
Copy link
Author

rminnich commented Nov 25, 2023

This program can be run on the microbit2 with: tinygo flash -target microbit-v2 -monitor .

// blink program for the BBC micro:bit
package main

import (
	"fmt"
	"machine"
	"runtime"
	"syscall"
	"time"
)

// The LED matrix in the micro:bit is a multiplexed display: https://en.wikipedia.org/wiki/Multiplexed_display
// Driver for easier control: https://github.com/tinygo-org/drivers/tree/master/microbitmatrix
func main() {
	runtime.PanicOnExit()
	for {
		err := g()
		fmt.Printf("did it %v\n", err)
	}
}

func g() error {

	defer func() {
		if r := recover(); r != nil {
			fmt.Println("Recovered in f", r)
		}
	}()
	ledrow := machine.LED_ROW_1
	ledrow.Configure(machine.PinConfig{Mode: machine.PinOutput})
	ledcol := machine.LED_COL_1
	ledcol.Configure(machine.PinConfig{Mode: machine.PinOutput})
	ledcol.Low()
	for {
		ledrow.Low()
		fmt.Printf("low\n")
		time.Sleep(time.Millisecond * 500)

		ledrow.High()
		fmt.Printf("high\n")
		time.Sleep(time.Millisecond * 500)
		syscall.Exit(1)
	}
}

produces the following:

low
high
Recovered in f errno 1
did it <nil>
low
high
Recovered in f errno 1
did it <nil>
low
high
Recovered in f errno 1
did it <nil>
low
high
Recovered in f errno 1
did it <nil>

Thus, u-root programs which now call syscall.Exit can be run as functions, on bare metal, and the shell will work.

[aside: OMG this tinygo environment is wonderful. Best firmware language I've ever used, and I've used many.
Thanks!]

@dgryski
Copy link
Member

dgryski commented Dec 17, 2023

I'm not a firmware person but this seems reasonable.

@deadprogram deadprogram requested a review from aykevl July 21, 2024 09:18
@deadprogram
Copy link
Member

Just wondering if this is something that would be better off as a compile-time flag instead of a runtime callable function?

@deadprogram
Copy link
Member

Reminder to @aykevl to PTAL

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be doing this. It introduces a new feature that's pretty core to Go and I strongly doubt upstream Go would ever accept such a change.

(Also, what about goroutines that were started? They won't be exited with panic).

What if you create a new package for use by u-root that implements exactly this behavior? It can export the same PanicOnExit function and an Exit function that is called everywhere that is needed. It also has the added benefit of not requiring TinyGo to work.

@deadprogram
Copy link
Member

What if you create a new package for use by u-root that implements exactly this behavior? It can export the same PanicOnExit function and an Exit function that is called everywhere that is needed. It also has the added benefit of not requiring TinyGo to work.

That is a very good suggestion, IMO. @rminnich what do you think?

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

Successfully merging this pull request may close these issues.

4 participants