From b9179dd2611e34ed07ff96eb964e92f353549366 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 18:34:37 +0200 Subject: [PATCH 1/6] signal+exit: a self-directed signal becomes a Ruby exception, and a deliberate exit status reaches the process (#691, #676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects at one boundary — the Go runtime's process state and the VM's exception machinery — fixed together because they are mirror images. #691 Process.kill(:TERM, Process.pid) killed the process where MRI raises a rescuable SignalException. signal.c rb_f_kill does NOT send the signal when the target is this process: it consults the installed disposition and, for a signal Ruby handles itself, calls signal_enque(sig) then rb_thread_execute_interrupts() before returning, so the effect happens synchronously inside Process.kill. That is now what vm.selfSignal does. No OS handler is installed and os/signal.Notify is never called, so Go's runtime stays the sole owner of signal disposition and the asynchronous-delivery race that WAS the defect is gone by construction. #676 A terminal SystemExit's status never reached the process. Its cause was not only the front-end: vm.Run deliberately DISCARDS the SystemExit, so no front-end change alone could have worked. Run now records it (vm.terminalExit) while still returning cleanly for an embedded host, and cmd/rbgo translates it the way eval.c rb_ec_cleanup does, through eval_error.c exiting_split. Also from the same C reading: Process.exit/exit!/abort are module functions (Init_process's rb_define_module_function), SignalException carries #signo and #signm (esignal_init/esignal_signo), Interrupt fixes signo at SIGINT with an empty message (interrupt_init), an uncaught SignalException dies BY its signal (ruby_default_signal) and prints only for a SUBCLASS or SIGSEGV, exit! runs no at_exit handler (rb_f_exit_bang/_exit), and an at_exit that itself exits wins. Measured against MRI 4.0.5, 20 runs per witness: 29 of 31 spellings now byte-identical on stdout, stderr and exit status where 9 were before. The racy witnesses went from a 10-20% fatal rate to 0/20. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/closed_main.go | 7 +- cmd/rbgo/closed_main_wasm.go | 4 +- cmd/rbgo/die_signal_native.go | 41 +++ cmd/rbgo/die_signal_wasm.go | 19 ++ cmd/rbgo/exit_status.go | 94 ++++++ cmd/rbgo/main.go | 43 +-- cmd/rbgo/stream_test.go | 2 +- internal/vm/kernel_introspect.go | 149 ++++++++- internal/vm/process.go | 34 ++ internal/vm/signal.go | 526 ++++++++++++++++++++++++++++--- internal/vm/vm.go | 25 ++ 11 files changed, 852 insertions(+), 92 deletions(-) create mode 100644 cmd/rbgo/die_signal_native.go create mode 100644 cmd/rbgo/die_signal_wasm.go create mode 100644 cmd/rbgo/exit_status.go diff --git a/cmd/rbgo/closed_main.go b/cmd/rbgo/closed_main.go index d6f9bece..66d9c182 100644 --- a/cmd/rbgo/closed_main.go +++ b/cmd/rbgo/closed_main.go @@ -3,7 +3,6 @@ package main import ( - "fmt" "os" "github.com/go-embedded-ruby/ruby/internal/vm" @@ -20,8 +19,6 @@ import ( // runtime stays alive for browser callbacks. func main() { machine := vm.New(os.Stdout) - if _, err := machine.Run(embeddedProgram()); err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } + _, err := machine.Run(embeddedProgram()) + finish(machine, err) // never returns; see exit_status.go } diff --git a/cmd/rbgo/closed_main_wasm.go b/cmd/rbgo/closed_main_wasm.go index 395141d6..e10adb04 100644 --- a/cmd/rbgo/closed_main_wasm.go +++ b/cmd/rbgo/closed_main_wasm.go @@ -3,7 +3,6 @@ package main import ( - "fmt" "os" "github.com/go-embedded-ruby/ruby/internal/vm" @@ -26,8 +25,7 @@ import ( func main() { machine := vm.New(os.Stdout) if _, err := machine.Run(embeddedProgram()); err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) + finish(machine, err) // never returns; see exit_status.go } select {} // keep the Go runtime alive so JS event/animation callbacks keep firing } diff --git a/cmd/rbgo/die_signal_native.go b/cmd/rbgo/die_signal_native.go new file mode 100644 index 00000000..ed04e6e7 --- /dev/null +++ b/cmd/rbgo/die_signal_native.go @@ -0,0 +1,41 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !wasm + +package main + +import ( + "os" + "os/signal" + "syscall" + "time" +) + +// dieBySignal is signal.c ruby_default_signal: restore the signal's default +// disposition and raise(sig) at self, so the process is KILLED BY the signal +// rather than exiting with a number that merely looks the same. A shell then +// reports 128+signo with WIFSIGNALED set, which is what MRI leaves behind for an +// uncaught SignalException and what `$?.signaled?` reads in a parent. +// +// signal.Reset undoes any os/signal.Notify the process installed, which is the +// Go-level equivalent of `signal(sig, SIG_DFL)`; the Go runtime's own handler for +// a signal nobody is watching already performs the default action (for SIGTERM +// that is to die, resetting to SIG_DFL and re-raising internally). +// +// It returns instead of blocking for ever if the signal has not taken effect, +// so the caller can still exit with 128+signo. The wait is not load-bearing: a +// slow host does not change the status the caller reports, only how long it +// takes to report it. +func dieBySignal(signo int) { + sig := syscall.Signal(signo) + signal.Reset(sig) + if err := syscall.Kill(os.Getpid(), sig); err != nil { + return + } + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } +} diff --git a/cmd/rbgo/die_signal_wasm.go b/cmd/rbgo/die_signal_wasm.go new file mode 100644 index 00000000..398c02be --- /dev/null +++ b/cmd/rbgo/die_signal_wasm.go @@ -0,0 +1,19 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build wasm + +package main + +// dieBySignal cannot do what signal.c ruby_default_signal does on wasm: neither +// wasip1 nor js/wasm has kill(2) or a settable signal disposition, so there is no +// way to be killed BY a signal there. It returns at once and the caller exits +// with 128+signo — the same NUMBER a POSIX shell would report, without the +// WIFSIGNALED flag that no wasm host can produce. +// +// Nothing else in this change needs a build tag: the signal disposition table and +// Process.kill's self arm are pure Go with no syscall in them, and Process.kill +// itself is only registered on POSIX (registerProcessPosix), so a wasm program +// reaches neither. +func dieBySignal(int) {} diff --git a/cmd/rbgo/exit_status.go b/cmd/rbgo/exit_status.go new file mode 100644 index 00000000..817be5b3 --- /dev/null +++ b/cmd/rbgo/exit_status.go @@ -0,0 +1,94 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package main + +import ( + "fmt" + "os" + + "github.com/go-embedded-ruby/ruby/internal/vm" +) + +// finish is the front-end's last step, MRI's ruby_run_node -> rb_ec_cleanup +// (eval.c). It is shared by every main in this package — the CLI, the +// closed-world native binary and the closed-world wasm binary — because a +// deliberate exit status has to reach the process from all three. +// +// rb_ec_cleanup splits the terminal exception with exiting_split (see +// vm.ExitingSplit), prints it only when exiting_split says so, and RETURNS the +// status to main(), which passes it to exit(). When a signal was named it does +// one more thing, last of all: ruby_default_signal(sig), which sets SIG_DFL and +// raise(sig)s, so the process dies BY the signal and a waiting shell sees +// WIFSIGNALED with 128+signo rather than a plain exit of the same number. +// +// finish never returns. +func finish(machine *vm.VM, err error) { + if machine == nil { + // The program never ran (a parse or compile failure), so there is no class + // hierarchy to classify anything against and no Ruby exception to classify. + if err != nil { + reportError(err) + os.Exit(1) + } + os.Exit(0) + } + if err == nil { + // A program that ended by calling exit unwinds cleanly — Run reports no + // error for a SystemExit — but its status is still the process's. This is + // rb_ec_cleanup reading ec->errinfo after rb_ec_teardown has run the exit + // handlers, which is why it is asked for here and not before. + if texit, ok := machine.TerminalExit(); ok { + err = texit + } else { + os.Exit(0) + } + } + d, ok := machine.ExitingSplit(err) + if !ok { + // Not a Ruby exception: a parse, compile or host IO error. + reportError(err) + os.Exit(1) + } + if d.Message { + reportError(err) + } + if d.Signal != 0 { + dieBySignal(d.Signal) + // dieBySignal returns only where the target cannot re-raise a signal at + // all (wasm) or where the re-raise did not take effect; the status is then + // the 128+signo a shell would have reported anyway. + } + os.Exit(d.Status) +} + +// reportError prints an uncaught error to stderr. A Ruby exception (vm.RubyError) +// is rendered MRI-style — ": ()" with a "\tfrom " +// line per outer frame from its backtrace — so a crashing program shows the call +// chain that led to the raise. A non-Ruby error (parse/compile/IO) prints plainly. +// +// An exception with an EMPTY message prints its class name alone, with no +// parenthesised suffix: eval_error.c print_errinfo takes the `elen == 0` branch +// and writes rb_class_name(eclass) by itself. That is the shape an uncaught +// Interrupt has, since Interrupt.new carries no message. +func reportError(err error) { + rerr, ok := err.(vm.RubyError) + if !ok { + fmt.Fprintf(os.Stderr, "%v\n", err) + return + } + frames := rerr.Backtrace() + body := rerr.Message + " (" + rerr.Class + ")" + if rerr.Message == "" { + body = rerr.Class + } + if len(frames) == 0 { + fmt.Fprintln(os.Stderr, body) + return + } + fmt.Fprintf(os.Stderr, "%s: %s\n", frames[0], body) + for _, f := range frames[1:] { + fmt.Fprintf(os.Stderr, "\tfrom %s\n", f) + } +} diff --git a/cmd/rbgo/main.go b/cmd/rbgo/main.go index 5b1d5521..250ba287 100644 --- a/cmd/rbgo/main.go +++ b/cmd/rbgo/main.go @@ -60,20 +60,25 @@ func runCmd(args []string) { usage() } - if err := run(src, name); err != nil { - reportError(err) - os.Exit(1) - } + // finish never returns: it is MRI's ruby_run_node -> rb_ec_cleanup, which + // turns the terminal exception into this process's exit status (or death by + // signal) rather than always exiting 0 or 1. See exit_status.go. + finish(run(src, name)) } -func run(src, name string) error { +// run compiles and interprets one program, returning the machine it ran on +// alongside the error, because the terminal exception has to be classified +// against that machine's class hierarchy (vm.ExitingSplit). The machine is +// non-nil whenever the program actually ran; a parse or compile failure returns +// nil, which finish handles. +func run(src, name string) (*vm.VM, error) { prog, err := parser.Parse(src) if err != nil { - return err + return nil, err } iseq, err := compiler.CompileWithEncoding(prog, compiler.MagicSourceEncoding(src)) if err != nil { - return err + return nil, err } iseq.Name = name // Program output goes to stdout; every diagnostic MRI puts on fd 2 — warnings, @@ -89,29 +94,7 @@ func run(src, name string) error { machine.SetScriptPath(name) // so require/require_relative resolve relative to the script } _, err = machine.Run(iseq) - return err -} - -// reportError prints an uncaught error to stderr. A Ruby exception (vm.RubyError) -// is rendered MRI-style — ": ()" with a "\tfrom " -// line per outer frame from its backtrace — so a crashing program shows the call -// chain that led to the raise. A non-Ruby error (parse/compile/IO) prints plainly. -func reportError(err error) { - rerr, ok := err.(vm.RubyError) - if !ok { - fmt.Fprintf(os.Stderr, "%v\n", err) - return - } - frames := rerr.Backtrace() - body := rerr.Message + " (" + rerr.Class + ")" - if len(frames) == 0 { - fmt.Fprintln(os.Stderr, body) - return - } - fmt.Fprintf(os.Stderr, "%s: %s\n", frames[0], body) - for _, f := range frames[1:] { - fmt.Fprintf(os.Stderr, "\tfrom %s\n", f) - } + return machine, err } func usage() { diff --git a/cmd/rbgo/stream_test.go b/cmd/rbgo/stream_test.go index 47112712..4c684712 100644 --- a/cmd/rbgo/stream_test.go +++ b/cmd/rbgo/stream_test.go @@ -44,7 +44,7 @@ func TestRunSendsWarningsToTheRealStderr(t *testing.T) { go func() { b, _ := io.ReadAll(outR); outCh <- string(b) }() go func() { b, _ := io.ReadAll(errR); errCh <- string(b) }() - runErr := run(program, script) + _, runErr := run(program, script) outW.Close() errW.Close() diff --git a/internal/vm/kernel_introspect.go b/internal/vm/kernel_introspect.go index 5ce28637..d55aabb8 100644 --- a/internal/vm/kernel_introspect.go +++ b/internal/vm/kernel_introspect.go @@ -223,7 +223,7 @@ func (vm *VM) registerKernelIntrospection() { return vm.raiseSystemExit(vm.exitStatusArg(args, 0), "exit") }) vm.cObject.define("exit!", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { - return vm.raiseSystemExit(vm.exitStatusArg(args, 1), "exit") + return vm.raiseSystemExitImmediate(vm.exitStatusArg(args, 1), "exit") }) // abort(message = nil): MRI's rb_f_abort (process.c) puts the message — // through StringValue, so a non-String converts with #to_str — on $stderr and @@ -666,6 +666,14 @@ func (vm *VM) frameFileLabel(i int) string { // is swallowed so the remaining handlers still run (MRI reports it but keeps // going); other control-flow signals propagate. func (vm *VM) runAtExit() { + // Kernel#exit! / Process.exit! runs none of them: process.c rb_f_exit_bang + // calls _exit(2), which is precisely "terminate without running the exit + // handlers". Measured against MRI 4.0.5: `at_exit { puts "x" }; exit!(4)` + // prints nothing and exits 4. + if vm.exitImmediate { + vm.atExit = nil + return + } for i := len(vm.atExit) - 1; i >= 0; i-- { vm.runAtExitOne(vm.atExit[i]) } @@ -675,7 +683,17 @@ func (vm *VM) runAtExit() { func (vm *VM) runAtExitOne(blk *Proc) { defer func() { if r := recover(); r != nil { - if _, ok := r.(RubyError); ok { + if rerr, ok := r.(RubyError); ok { + // An at_exit handler that itself calls exit sets the process's status, + // and overrides one an earlier exit had already asked for: eval.c + // rb_ec_cleanup reads ec->errinfo AFTER rb_ec_teardown has run the + // handlers, and exiting_split gives that reading (mode0) precedence + // over the pre-teardown one (mode1). Measured against MRI 4.0.5: + // `at_exit { exit 7 }; exit 3` exits 7, and `at_exit { exit 7 }` on a + // program that ends normally exits 7 too. + if vm.isSystemExit(rerr.Class) { + vm.terminalExit = &rerr + } return // swallow: a failing at_exit hook must not abort the others } panic(r) @@ -824,3 +842,130 @@ func (vm *VM) registerGvarTracing() { return object.NewArrayFromSlice(append([]object.Value(nil), elems...)) }) } + +// ExitDisposition is how a terminal exception ends the process. It is the result +// of eval_error.c's exiting_split, the function rb_ec_cleanup calls to split an +// uncaught exception into "exit with this status", "die by this signal" and +// "print this exception first": +// +// SystemExit -> EXITING_WITH_STATUS, status = the @status ivar, no message +// SignalException -> EXITING_WITH_SIGNAL, sig = the @signo ivar; a message +// ONLY for SIGSEGV or for a SUBCLASS of SignalException +// (`!rb_obj_is_instance_of(errinfo, rb_eSignal)`), which +// is why an uncaught Interrupt prints and an uncaught +// bare SignalException does not +// anything else -> EXITING_WITH_STATUS | EXITING_WITH_MESSAGE, status 1 +// +// rb_ec_cleanup then returns Status as main()'s exit code, and — last of all, +// after every finalizer — calls ruby_default_signal(Signal) when a signal was +// named, which restores SIG_DFL and re-raises it so the process really dies BY +// the signal and a shell reports 128+signo with WIFSIGNALED set. +type ExitDisposition struct { + Status int // the process exit status + Signal int // the signal to die by, or 0 + Message bool // print the exception MRI-style before exiting +} + +// ExitingSplit classifies an error that propagated out of Run, reporting false +// when it is not a Ruby exception at all (a parse, compile or host IO error), +// which the caller reports and exits 1 for as before. +// +// This is the whole of issue #676's front-end half: the VM already raised the +// right SystemExit with the right @status, and nothing translated it into the +// process's exit code — so `rbgo -e 'exit 3'` exited 0. It is also the terminal +// half of #691: an uncaught SignalException must leave the process dead by its +// signal, not exited 0 or 1. +func (vm *VM) ExitingSplit(err error) (ExitDisposition, bool) { + rerr, ok := err.(RubyError) + if !ok { + return ExitDisposition{}, false + } + switch { + case vm.classIsA(rerr.Class, "SystemExit"): + return ExitDisposition{Status: int(vm.exitStatusOf(rerr.Obj))}, true + case vm.classIsA(rerr.Class, "SignalException"): + signo := int(vm.signoOf(rerr.Obj)) + // A signal number of 0 is Signal.trap(:EXIT)'s slot and cannot kill + // anything; treat it as a plain failure rather than re-raising signal 0, + // which kill(2) defines as a liveness probe. + if signo == 0 { + return ExitDisposition{Status: 1, Message: true}, true + } + segv, isSegv := signalNumberOf("SEGV") + return ExitDisposition{ + Signal: signo, + Status: 128 + signo, // the status to fall back to if the re-raise cannot run + Message: (isSegv && signo == segv) || rerr.Class != "SignalException", + }, true + default: + return ExitDisposition{Status: 1, Message: true}, true + } +} + +// classIsA reports whether the named class is name or a descendant of it, the +// rb_obj_is_kind_of test exiting_split applies. An unknown class name can only +// match itself, which keeps a raise() from a native method (which carries a name +// rather than an object) classifiable. +func (vm *VM) classIsA(className, ancestor string) bool { + c, ok := vm.consts[className].(*RClass) + if !ok { + return className == ancestor + } + for ; c != nil; c = c.super { + if c.name == ancestor { + return true + } + } + return false +} + +// exitStatusOf reads SystemExit#status off the exception object (eval_error.c +// sysexit_status, which is rb_ivar_get(exc, id_status)). A SystemExit raised +// through raise() rather than Kernel#exit has no object, and MRI's +// SystemExit.new defaults @status to 0. +func (vm *VM) exitStatusOf(exc object.Value) int64 { + if exc == nil { + return 0 + } + if s, ok := getIvar(exc, "@status").(object.Integer); ok { + return int64(s) + } + return 0 +} + +// signoOf reads SignalException#signo off the exception object +// (rb_ivar_get(errinfo, id_signo)). +func (vm *VM) signoOf(exc object.Value) int64 { + if exc == nil { + return 0 + } + if s, ok := getIvar(exc, "@signo").(object.Integer); ok { + return int64(s) + } + return 0 +} + +// raiseSystemExitImmediate is raiseSystemExit for Kernel#exit! / Process.exit!, +// which process.c rb_f_exit_bang implements as _exit(2): the status still travels +// on a SystemExit (so it is rescuable and reportable the same way), but no +// at_exit handler runs. MRI also skips `ensure` blocks, which needs an unwinding +// path the exception machinery does not have here; that remainder is recorded +// rather than pretended away. +func (vm *VM) raiseSystemExitImmediate(status int64, message string) object.Value { + vm.exitImmediate = true + return vm.raiseSystemExit(status, message) +} + +// TerminalExit reports the SystemExit that ended the run, if one did. Run +// returns no error for a SystemExit — a program that calls exit is terminating, +// not crashing, and an embedded host must not see a failure — but the @status it +// carries IS the process's exit code, so a host that owns a process asks for it +// here. It is MRI's ec->errinfo as rb_ec_cleanup finds it after rb_ec_teardown: +// an at_exit handler that itself calls exit overwrites the status, which is why +// this is read AFTER Run returns rather than captured at the raise site. +func (vm *VM) TerminalExit() (error, bool) { + if vm.terminalExit == nil { + return nil, false + } + return *vm.terminalExit, true +} diff --git a/internal/vm/process.go b/internal/vm/process.go index 29a4f7c3..f847aa8b 100644 --- a/internal/vm/process.go +++ b/internal/vm/process.go @@ -99,6 +99,7 @@ func (vm *VM) registerProcess() { mod.consts["CLOCK_MONOTONIC"] = object.IntValue(clockMonotonic) vm.consts["Process::CLOCK_MONOTONIC"] = object.IntValue(clockMonotonic) + vm.defineProcessExit(def) // unconditional: Init_process declares these on every platform vm.registerProcessPosix(mod, def) vm.registerProcessResiduals(mod, def) } @@ -302,17 +303,30 @@ func (vm *VM) defineProcessGroups(def func(string, NativeFn)) { // anything else goes through signm2signo (which accepts a leading '-' to mean // "signal the process group"); every remaining argument is a pid coerced with // NUM2PIDT, and the return value is the number of pids signalled. +// +// The pid == self arm is the subject of issue #691 and does NOT reach kill(2): +// rb_f_kill consults the installed disposition and, for a signal Ruby handles +// itself, enqueues it and runs rb_thread_execute_interrupts() before returning, +// so the Ruby-level effect (a trap, a SignalException, an Interrupt) happens +// synchronously inside Process.kill. vm.selfSignal is that arm plus +// rb_signal_exec; it reports false when MRI would really signal the OS. func (vm *VM) defineProcessKill(def func(string, NativeFn)) { def("kill", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { if len(args) < 2 { raise("ArgumentError", "wrong number of arguments (given %d, expected 2+)", len(args)) } sig := vm.killSignalNumber(args[0]) + self := os.Getpid() for _, pv := range args[1:] { pid, send := int(vm.procToInt(pv)), sig if sig < 0 { // killpg(pgid, sig) == kill(-pgid, sig) pid, send = -pid, -sig } + // rb_f_kill takes the self arm only for a POSITIVE signal aimed at this + // very process; a process-group kill (sig < 0) always goes to the OS. + if sig > 0 && pid == self && vm.selfSignal(send) { + continue + } if err := procKill(pid, send); err != nil { sysFail(err, "") } @@ -321,6 +335,26 @@ func (vm *VM) defineProcessKill(def func(string, NativeFn)) { }) } +// defineProcessExit installs Process.exit, Process.exit! and Process.abort. +// process.c Init_process declares all three with rb_define_module_function on +// rb_mProcess, which makes them PUBLIC singleton methods on the module as well +// as private instance methods — so `Process.exit(5)` is a legal spelling where +// rbgo previously reached only Kernel's private copies and raised NoMethodError +// (issue #676's second defect). The bodies are rb_f_exit / rb_f_exit_bang / +// rb_f_abort, the same functions Kernel's copies name, so they share +// exitStatusArg's exit_status_code mapping and raiseSystemExit. +func (vm *VM) defineProcessExit(def func(string, NativeFn)) { + def("exit", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + return vm.raiseSystemExit(vm.exitStatusArg(args, 0), "exit") + }) + def("exit!", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + return vm.raiseSystemExit(vm.exitStatusArg(args, 1), "exit") + }) + def("abort", func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { + return vm.send(vm.main, "abort", args, blk) + }) +} + // rlimValue wraps a rlim_t as a Ruby Integer. RLIM_INFINITY is ~0 on Linux, // which does not fit in an int64, so an out-of-range limit promotes to a Bignum // rather than wrapping negative (MRI's RLIM2NUM does the same through diff --git a/internal/vm/signal.go b/internal/vm/signal.go index 8c839362..ea63311a 100644 --- a/internal/vm/signal.go +++ b/internal/vm/signal.go @@ -8,87 +8,497 @@ import ( "github.com/go-embedded-ruby/ruby/internal/object" ) -// registerSignal installs the Signal module and Kernel#trap. A program installs -// a handler (Signal.trap(:INT) { ... }) and the VM records it, returning the -// previous handler the way MRI does ("DEFAULT" the first time, then the prior -// Proc/String). The embedded host does not yet wire OS signals to the recorded -// handlers — delivery is the scheduler's job in a later round — but registration -// must succeed because CLI applications such as `puppet apply` trap :INT during -// setup. Signal.list maps the common signal names to their numbers and -// Signal.signame inverts a number, both as plain lookups. +// trapKind is what the VM does when a signal is delivered, mirroring the values +// MRI stores in vm->trap_list.cmd[sig] (signal.c trap_handler): +// +// 0 (absent) -> the signal's default Ruby action +// Qtrue ("IGNORE"/"") -> discard +// Qundef ("EXIT") -> raise SystemExit +// 0 + SIG_DFL ("SYSTEM_DEFAULT") -> hand the signal back to the OS +// a Proc / command String -> run it +type trapKind int + +const ( + trapDefault trapKind = iota // no handler installed: default_handler(sig) + trapIgnore // "IGNORE" / "SIG_IGN" / "" / nil + trapExit // "EXIT" + trapSystemDefault // "SYSTEM_DEFAULT": SIG_DFL, the OS acts + trapCommand // a Proc (or a command String) to run +) + +// trapCmd is one entry of the VM's trap list. +type trapCmd struct { + kind trapKind + cmd object.Value // the Proc/String for trapCommand; the value #trap reports back +} + +// signalManaged reports whether MRI installs its own sighandler for sig, which +// is signal.c default_handler's first case group (and the same set Init_signal +// passes to install_sighandler). It is exactly the set rb_signal_exec turns into +// a Ruby exception when no user handler is installed, and — because +// signal_ignored() then sees MRI's own handler rather than SIG_DFL — the set +// rb_f_kill routes through signal_enque instead of kill(2) when the target is +// this process. +func (vm *VM) signalManaged(sig int) bool { + switch sig { + case sigNumber("INT"), sigNumber("HUP"), sigNumber("QUIT"), sigNumber("TERM"), + sigNumber("ALRM"), sigNumber("USR1"), sigNumber("USR2"), sigNumber("CHLD"): + return true + } + return false +} + +// signalFatal reports whether rb_f_kill delivers sig to this process for real +// even when the target is self: SIGSEGV, SIGBUS, SIGKILL, SIGILL, SIGFPE and +// SIGSTOP cannot be deferred to a Ruby safe point, so signal.c's rb_f_kill +// calls kill(2) directly for them ("switch (sig)" inside the pid == self arm). +func signalFatal(sig int) bool { + switch sig { + case sigNumber("SEGV"), sigNumber("BUS"), sigNumber("KILL"), + sigNumber("ILL"), sigNumber("FPE"), sigNumber("STOP"): + return true + } + return false +} + +// registerSignal installs the Signal module, Kernel#trap, and the SignalException +// / Interrupt methods that carry a signal's identity (#signo, #signm). +// +// The disposition a program installs is recorded in vm.trapList, MRI's +// vm->trap_list.cmd[] (signal.c trap/trap_handler), and consulted by +// Process.kill when the target is this process — see vm.selfSignal, which is +// signal.c rb_f_kill's pid == self arm plus rb_signal_exec. The VM deliberately +// does NOT install an OS signal handler and never calls os/signal.Notify: the +// Go runtime stays the sole owner of signal disposition. See vm.selfSignal for +// why that is the faithful reading of the C rather than a shortcut. func (vm *VM) registerSignal() { mod := newClass("Signal", nil) mod.isModule = true vm.consts["Signal"] = mod - - // handlers records the most recently installed handler per normalised signal - // name so trap can report the previous one. It lives on the VM-shared module - // object via an ivar so a single map is reused. - handlers := map[string]object.Value{} + vm.trapList = map[int]trapCmd{} trap := func(vm *VM, _ object.Value, args []object.Value, blk *Proc) object.Value { - if len(args) == 0 { + if len(args) == 0 && blk == nil { return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") } - name := signalName(args[0]) - prev, had := handlers[name] - if !had { - prev = object.NewString("DEFAULT") + if len(args) == 0 { + return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") } + sig := vm.trapSignm(args[0]) + var next trapCmd switch { case blk != nil: - handlers[name] = blk // a Proc is itself a Value, returned as the previous handler later + next = trapCmd{kind: trapCommand, cmd: blk} case len(args) >= 2: - handlers[name] = args[1] // "DEFAULT" / "IGNORE" / "SIG_IGN" / a command string + next = vm.trapHandler(args[1]) default: - handlers[name] = object.NewString("DEFAULT") + return raise("ArgumentError", "tried to create Proc object without a block") } + prev := vm.trapPrevious(sig) + vm.trapList[sig] = next return prev } mod.smethods["trap"] = &Method{name: "trap", owner: mod, native: trap} - // Signal.list returns the {name => number} table; Signal.signame inverts it. + // Signal.list returns the {name => number} table. MRI's sig_list walks the + // same siglist it uses everywhere else, so EXIT (0) is a member — a fact + // core/signal/list_spec.rb asserts directly. mod.smethods["list"] = &Method{name: "list", owner: mod, native: func(_ *VM, _ object.Value, _ []object.Value, _ *Proc) object.Value { h := object.NewHash() - for name, num := range signalNumbers { - h.Set(object.NewString(name), object.IntValue(int64(num))) + for _, e := range siglist { + h.Set(object.NewString(e.name), object.IntValue(int64(e.signo))) } return h }} + // Signal.signame inverts the table. MRI's sig_signame takes NUM2INT — so a + // non-Integer goes through #to_int and a #to_int that returns a non-Integer is + // a TypeError — and signo2signm returns NULL (nil) for a number no entry + // carries. signo2signm returns the FIRST siglist entry with that number, which + // is why siglist lists a canonical name ahead of its aliases. mod.smethods["signame"] = &Method{name: "signame", owner: mod, - native: func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { - if len(args) == 0 { - return object.NilV + native: func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + if len(args) != 1 { + return raise("ArgumentError", "wrong number of arguments (given %d, expected 1)", len(args)) } - n, ok := args[0].(object.Integer) - if !ok { - return object.NilV - } - for name, num := range signalNumbers { - if int64(num) == int64(n) { - return object.NewString(name) - } + n := vm.toIntCoerce(args[0]) + if name, ok := signalNameOf(int(n)); ok { + return object.NewString(name) } return object.NilV }} // Kernel#trap is the same operation reachable without the Signal receiver. vm.cObject.define("trap", trap) + + vm.registerSignalException() +} + +// registerSignalException adds the identity methods SignalException carries. +// signal.c Init_signal defines #initialize (esignal_init), #signo +// (esignal_signo) and aliases #signm to #message; Interrupt overrides +// #initialize (interrupt_init) to fix the signal at SIGINT. +func (vm *VM) registerSignalException() { + cSig, ok := vm.consts["SignalException"].(*RClass) + if !ok { + return + } + // esignal_init(sig) / esignal_init(signo, message = signo2signm(signo)): + // the first argument is tried as an Integer through #to_int; if it converts, + // a second argument may supply the message and arity is 1..2, otherwise the + // argument is a signal NAME and arity is exactly 1. A name is stored with a + // "SIG" prefix when it was given without one (`sig = "SIG" + sig` when the + // matched prefix length differs from signame_prefix_len), which is why + // SignalException.new("TERM").message is "SIGTERM". + cSig.define("initialize", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + o, isObj := self.(*RObject) + if !isObj { + return object.NilV + } + if len(args) == 0 { + return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") + } + var signo int + var message string + if n, isInt := vm.toIntMaybe(args[0]); isInt { + if len(args) > 2 { + return raise("ArgumentError", "wrong number of arguments (given %d, expected 1..2)", len(args)) + } + signo = int(n) + if signo < 0 || signo > sigMaxNumber { + return raise("ArgumentError", "invalid signal number (%d)", signo) + } + if len(args) > 1 { + message = args[1].ToS() + } else { + message = signoToSignm(signo) + } + } else { + if len(args) != 1 { + return raise("ArgumentError", "wrong number of arguments (given %d, expected 1)", len(args)) + } + name := args[0].ToS() + num, found := signalNumberOf(name) + if !found { + return raise("ArgumentError", "unsupported signal '%s'", withSIG(name)) + } + signo, message = num, withSIG(name) + } + o.ivars["@message"] = object.NewString(message) + o.ivars["@signo"] = object.IntValue(int64(signo)) + return object.NilV + }) + cSig.define("signo", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return getIvar(self, "@signo") + }) + // rb_alias(rb_eSignal, "signm", "message"). + cSig.define("signm", func(vm *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + return vm.send(self, "message", nil, nil) + }) + + // interrupt_init: args[0] = SIGINT, args[1..] = the caller's arguments, so + // Interrupt.new carries signo 2 and an EMPTY message (MRI passes no name, and + // esignal_init's message argument defaults to "" rather than "SIGINT" here + // because interrupt_init supplies argv[1] itself). + if cInt, okI := vm.consts["Interrupt"].(*RClass); okI { + cInt.define("initialize", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + o, isObj := self.(*RObject) + if !isObj { + return object.NilV + } + msg := "" + if len(args) > 0 { + msg = args[0].ToS() + } + o.ivars["@message"] = object.NewString(msg) + o.ivars["@signo"] = object.IntValue(int64(sigNumber("INT"))) + return object.NilV + }) + } +} + +// trapSignm coerces Signal.trap's first argument to a signal number, applying +// signal.c trap_signm: a Fixnum is range-checked against NSIG, anything else +// goes through signm2signo with exit = TRUE, so "EXIT"/0 is accepted. +func (vm *VM) trapSignm(v object.Value) int { + if n, isInt := vm.toIntMaybe(v); isInt { + if n < 0 || n > int64(sigMaxNumber) { + raise("ArgumentError", "invalid signal number (%d)", n) + } + return int(n) + } + name := v.ToS() + if num, ok := signalNumberOf(name); ok { + return num + } + raise("ArgumentError", "unsupported signal '%s'", withSIG(name)) + return 0 +} + +// trapHandler maps Signal.trap's second argument to a disposition, following +// signal.c trap_handler's switch on the command string's length exactly: nil and +// "" and "IGNORE"/"SIG_IGN" ignore, "DEFAULT"/"SIG_DFL" restore the signal's +// default RUBY action, "SYSTEM_DEFAULT" restores the OS default, "EXIT" exits, +// and anything else is a command to run. +func (vm *VM) trapHandler(v object.Value) trapCmd { + if v == object.NilV { + return trapCmd{kind: trapIgnore, cmd: object.NilV} + } + if p, isProc := v.(*Proc); isProc { + return trapCmd{kind: trapCommand, cmd: p} + } + switch name := v.ToS(); name { + case "", "IGNORE", "SIG_IGN": + return trapCmd{kind: trapIgnore, cmd: object.NewString("IGNORE")} + case "DEFAULT", "SIG_DFL": + return trapCmd{kind: trapDefault, cmd: object.NewString("DEFAULT")} + case "SYSTEM_DEFAULT": + return trapCmd{kind: trapSystemDefault, cmd: object.NewString("SYSTEM_DEFAULT")} + case "EXIT": + return trapCmd{kind: trapExit, cmd: object.NewString("EXIT")} + default: + return trapCmd{kind: trapCommand, cmd: v} + } +} + +// trapPrevious is what Signal.trap returns: signal.c trap()'s oldcmd switch. A +// signal with nothing installed reports the name of the disposition it had — +// "DEFAULT" for a signal MRI handles itself, "SYSTEM_DEFAULT" for one left to +// the OS — rather than nil, and an installed Proc or command comes back as +// itself. +func (vm *VM) trapPrevious(sig int) object.Value { + prev, had := vm.trapList[sig] + if !had { + if vm.signalManaged(sig) { + return object.NewString("DEFAULT") + } + return object.NewString("SYSTEM_DEFAULT") + } + switch prev.kind { + case trapDefault: + if vm.signalManaged(sig) { + return object.NewString("DEFAULT") + } + return object.NewString("SYSTEM_DEFAULT") + case trapIgnore: + return object.NewString("IGNORE") + case trapSystemDefault: + return object.NewString("SYSTEM_DEFAULT") + case trapExit: + return object.NewString("EXIT") + } + return prev.cmd } -// signalNumbers maps the portable signal names to their POSIX numbers. The set -// covers the signals a Ruby program is likely to trap; it is not exhaustive. -var signalNumbers = map[string]int{ - "HUP": 1, "INT": 2, "QUIT": 3, "ILL": 4, "TRAP": 5, "ABRT": 6, - "FPE": 8, "KILL": 9, "SEGV": 11, "PIPE": 13, "ALRM": 14, "TERM": 15, - "USR1": 30, "USR2": 31, "CHLD": 20, "CONT": 19, "STOP": 17, "TSTP": 18, - "WINCH": 28, "INFO": 29, +// selfSignal is signal.c rb_f_kill's "target pid is self" arm followed by +// rb_signal_exec, and it is the whole of issue #691. +// +// MRI does NOT send the signal to itself for the ordinary cases. rb_f_kill's +// comment says why — "When target pid is self, many caller assume signal will be +// delivered immediately and synchronously" — so it consults the installed +// disposition and, for a signal it handles itself, calls signal_enque(sig) and +// then rb_thread_execute_interrupts() before returning. The Ruby-level effect +// therefore happens INSIDE Process.kill, on the calling thread, and no OS signal +// is ever raised. +// +// That is what reconciles MRI's model with Go's. MRI installs a C handler +// (Init_signal -> install_sighandler) purely so an ASYNCHRONOUS signal can be +// parked in signal_buff and re-emerge at the next Ruby safe point; the safe point +// for a self-directed kill is the next statement, and signal_enque plus an +// immediate interrupt check expresses it without the handler. Go's runtime owns +// signal disposition and os/signal.Notify changes it process-wide, so +// reproducing MRI by installing a Go handler would buy nothing here and cost the +// asynchronous delivery race that IS the observed defect: the old code called +// kill(2) and then raced the Go runtime's default action against the program's +// own exit, which is why the old behaviour was 15-20% fatal rather than always +// fatal. Executing the disposition synchronously removes the race by +// construction, needs no build tag on a target without signals, and leaves Go's +// disposition untouched. +// +// What it does NOT do, and MRI does: an EXTERNAL signal (one another process +// sends us) still meets Go's default disposition, so a trapped SIGTERM arriving +// from outside kills us where MRI would run the trap. Handling that needs +// os/signal.Notify plus a delivery point in the interpreter loop; it is recorded +// as follow-up work rather than smuggled in here. Note what MRI itself does with +// a signal that arrives while no Ruby frame is executing: sighandler only fills +// signal_buff, and if the VM has already reached rb_ec_cleanup nothing runs it — +// the process then dies through ruby_default_signal. So "no Ruby frame" is not a +// case MRI serves either. +// +// handled reports whether the signal was consumed here; false means the caller +// (Process.kill) must fall through to a real kill(2). +func (vm *VM) selfSignal(sig int) (handled bool) { + if sig == 0 || signalFatal(sig) { + return false // kill(pid, 0) probes; a fatal signal cannot be deferred + } + cmd, had := vm.trapList[sig] + if !had { + cmd = trapCmd{kind: trapDefault} + } + switch cmd.kind { + case trapIgnore: + // signal_ignored() == 1: MRI neither sends nor runs anything. + return true + case trapSystemDefault: + // signal_ignored() == -1 (SIG_DFL): MRI really does kill(pid, sig). + return false + case trapExit: + // rb_threadptr_signal_exit: SystemExit with EXIT_FAILURE. + vm.raiseSystemExit(1, "exit") + return true + case trapCommand: + // signal_exec: the handler runs with the signal number as its argument. + vm.runTrapCommand(cmd.cmd, sig) + return true + } + // trapDefault. rb_signal_exec's cmd == 0 arm: SIGINT becomes Interrupt, + // HUP/QUIT/TERM/ALRM/USR1/USR2 become SignalException, and any other signal + // MRI does not handle is left to the OS. + if !vm.signalManaged(sig) { + return false + } + switch sig { + case sigNumber("INT"): + vm.raiseSignalException("Interrupt", sig, "") + case sigNumber("CHLD"): + // Init_signal installs sighandler for SIGCHLD to reap children; there is + // no cmd == 0 case for it in rb_signal_exec, so it raises nothing. + default: + vm.raiseSignalException("SignalException", sig, signoToSignm(sig)) + } + return true +} + +// runTrapCommand runs an installed trap. A Proc (or anything that responds to +// #call) is called with the signal number, as signal.c signal_exec does through +// rb_eval_cmd_kw(cmd, [INT2NUM(sig)]). +func (vm *VM) runTrapCommand(cmd object.Value, sig int) { + arg := []object.Value{object.IntValue(int64(sig))} + if p, isProc := cmd.(*Proc); isProc { + vm.callProcWithBlock(p, arg, nil) + return + } + vm.send(cmd, "call", arg, nil) +} + +// raiseSignalException raises SignalException (or Interrupt) already carrying +// @signo, so a rescuer sees #signo and #signm without the class having to +// re-derive them. +func (vm *VM) raiseSignalException(class string, signo int, message string) { + c, ok := vm.consts[class].(*RClass) + if !ok { + raise(class, "%s", message) + return + } + exc := vm.send(c, "new", []object.Value{object.NewString(message)}, nil) + if o, isObj := exc.(*RObject); isObj { + o.ivars["@signo"] = object.IntValue(int64(signo)) + o.ivars["@message"] = object.NewString(message) + } + panic(vm.excError(vm.captureBacktrace(exc))) +} + +// signalEntry is one row of the signal name table. +type signalEntry struct { + name string + signo int +} + +// siglist is signal.c's siglist: the signal names Ruby knows, with a CANONICAL +// name ahead of each of its aliases, because signo2signm returns the first entry +// matching a number (core/signal/signame_spec.rb asserts ABRT wins over IOT and +// CHLD over CLD). EXIT is a member with number 0 — Signal.trap(:EXIT) is +// at_exit spelt as a signal, and core/signal/list_spec.rb reads it back. +// +// The numbers are this table's own, not the host's. They are correct on Darwin +// and correct everywhere for the signals whose numbers POSIX fixes (HUP 1, +// INT 2, QUIT 3, KILL 9, PIPE 13, ALRM 14, TERM 15); USR1/USR2/CHLD/STOP/TSTP +// and the BSD extras differ on Linux, which is a pre-existing gap recorded with +// this change rather than fixed by it (a per-GOOS table needs a build tag on a +// target, wasip1, that has no signal constants at all). +var siglist = []signalEntry{ + {"EXIT", 0}, + {"HUP", 1}, {"INT", 2}, {"QUIT", 3}, {"ILL", 4}, {"TRAP", 5}, + {"ABRT", 6}, {"IOT", 6}, + {"FPE", 8}, {"KILL", 9}, {"BUS", 10}, {"SEGV", 11}, {"SYS", 12}, + {"PIPE", 13}, {"ALRM", 14}, {"TERM", 15}, {"URG", 16}, + {"STOP", 17}, {"TSTP", 18}, {"CONT", 19}, + {"CHLD", 20}, {"CLD", 20}, + {"TTIN", 21}, {"TTOU", 22}, {"IO", 23}, {"XCPU", 24}, {"XFSZ", 25}, + {"VTALRM", 26}, {"PROF", 27}, {"WINCH", 28}, {"INFO", 29}, + {"USR1", 30}, {"USR2", 31}, +} + +// sigMaxNumber is NSIG's role in esignal_init and trap_signm: the inclusive +// upper bound a signal number may take before ArgumentError. +const sigMaxNumber = 64 + +// signalNumbers indexes siglist by name for lookup. signalNames indexes it by +// number, keeping the FIRST (canonical) name for each. +var signalNumbers, signalNames = func() (map[string]int, map[int]string) { + byName := make(map[string]int, len(siglist)) + byNo := make(map[int]string, len(siglist)) + for _, e := range siglist { + byName[e.name] = e.signo + if _, seen := byNo[e.signo]; !seen { + byNo[e.signo] = e.name + } + } + return byName, byNo +}() + +// sigNumber is siglist's number for a canonical name. It panics on an unknown +// name, which can only be a typo in this package. +func sigNumber(name string) int { + n, ok := signalNumbers[name] + if !ok { + panic("vm: unknown signal name " + name) + } + return n +} + +// signalNumberOf is signal.c signm2signo: it accepts "TERM", "SIGTERM" or +// :TERM and reports whether the name is one Ruby knows. +func signalNumberOf(name string) (int, bool) { + n, ok := signalNumbers[stripSIG(name)] + return n, ok +} + +// signalNameOf is signal.c signo2signm. +func signalNameOf(signo int) (string, bool) { + n, ok := signalNames[signo] + return n, ok +} + +// signoToSignm is rb_signo2signm: the "SIG"-prefixed name, or "SIG" for a +// number no entry carries (rb_sprintf("SIG%u", signo)). +func signoToSignm(signo int) string { + if name, ok := signalNameOf(signo); ok { + return "SIG" + name + } + return "SIG" + object.IntValue(int64(signo)).ToS() +} + +// withSIG prefixes a bare signal name, as esignal_init does when the name it was +// given carried no prefix. +func withSIG(name string) string { + if stripSIG(name) == name { + return "SIG" + name + } + return name +} + +func stripSIG(name string) string { + if len(name) > 3 && name[:3] == "SIG" { + return name[3:] + } + return name } // signalName normalises a signal designator to its bare name (no "SIG" prefix), -// accepting a Symbol, a String ("INT"/"SIGINT") or an Integer. +// accepting a Symbol, a String ("INT"/"SIGINT") or an Integer. Kept for callers +// that want a name rather than a number. func signalName(v object.Value) string { switch s := v.(type) { case object.Symbol: @@ -96,10 +506,8 @@ func signalName(v object.Value) string { case *object.String: return stripSIG(string(s.Bytes())) case object.Integer: - for name, num := range signalNumbers { - if int64(num) == int64(s) { - return name - } + if name, ok := signalNameOf(int(s)); ok { + return name } return v.ToS() default: @@ -107,9 +515,25 @@ func signalName(v object.Value) string { } } -func stripSIG(name string) string { - if len(name) > 3 && name[:3] == "SIG" { - return name[3:] +// toIntMaybe is signal.c's rb_check_to_integer(v, "to_int"): it reports whether +// v converts to an Integer WITHOUT raising, which is how esignal_init decides +// between "first argument is a signal number" and "first argument is a signal +// name". A String responds to neither #to_int nor an implicit conversion, so +// SignalException.new("TERM") takes the name branch. +func (vm *VM) toIntMaybe(v object.Value) (int64, bool) { + if i, ok := v.(object.Integer); ok { + return int64(i), true } - return name + if _, isStr := v.(*object.String); isStr { + return 0, false + } + if _, isSym := v.(object.Symbol); isSym { + return 0, false + } + if vm.respondsToDynamic(v, "to_int") { + if i, ok := vm.send(v, "to_int", nil, nil).(object.Integer); ok { + return int64(i), true + } + } + return 0, false } diff --git a/internal/vm/vm.go b/internal/vm/vm.go index 898a337e..04c73b12 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -303,6 +303,24 @@ type VM struct { arModels map[*RClass]*ActiveRecordModel arTableNames map[*RClass]string + // terminalExit is the SystemExit that ended the run, kept because Run + // deliberately returns no error for one (a program that calls exit is not + // crashing) while its @status is still the process's exit code. nil until a + // SystemExit reaches the top. See TerminalExit. + terminalExit *RubyError + + // exitImmediate records that the terminal SystemExit came from Kernel#exit! / + // Process.exit!, which process.c rb_f_exit_bang implements with _exit(2) — so + // no at_exit handler runs. Set by raiseSystemExitImmediate, read by runAtExit. + exitImmediate bool + + // trapList is MRI's vm->trap_list.cmd[] (signal.c trap/trap_handler): the + // signal disposition a program installed with Signal.trap / Kernel#trap, + // keyed by signal number. An absent key means the signal's default Ruby + // action. Process.kill consults it when the target is this process — see + // vm.selfSignal, which is rb_f_kill's pid == self arm. + trapList map[int]trapCmd + cBasicObject, cObject, cModule, cClass *RClass cKernel *RClass // the Kernel module (included into Object; its methods live on cObject) cRefinement *RClass // the Refinement class (a Module subclass; instances are the anonymous modules Module#refine creates) @@ -1133,6 +1151,13 @@ func (vm *VM) Run(iseq *bytecode.ISeq) (result object.Value, err error) { // error — so a real CLI that ends by calling exit (e.g. Puppet's // exit_on_fail) terminates quietly rather than printing a backtrace. if vm.isSystemExit(rerr.Class) { + // ...but the STATUS it carries is the process's exit code, and + // discarding it is issue #676: `rbgo -e 'exit 3'` exited 0. Record + // the terminal SystemExit so a host that owns a process (the rbgo + // CLI) can do what MRI's ruby_run_node -> rb_ec_cleanup does with it, + // while an embedded host that has no process still sees a clean + // return. See vm.TerminalExit and cmd/rbgo/exit_status.go. + vm.terminalExit = &rerr vm.frameNames = vm.frameNames[:0] vm.frameFiles = vm.frameFiles[:0] vm.frameCrefs = vm.frameCrefs[:0] From adbc85ef62b795c53566be27c772662e4add5c2b Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 18:40:04 +0200 Subject: [PATCH 2/6] tests: the signal disposition table, the exiting_split classification, and the process exit status end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three files, each a witness rather than a guard — every row fails on origin/main with the production change reverted (and the revert compiles): internal/vm/signal_disposition_internal_test.go drives rb_f_kill's self arm through every disposition trap_handler can install, including the two where MRI really does call kill(2) (SIGKILL, SYSTEM_DEFAULT) — read through the procKill seam, since the alternative is killing the test process — and the negative that a foreign pid and a process-group kill are NOT short-circuited. internal/vm/exit_status_internal_test.go is eval_error.c exiting_split as a table, including the asymmetry that only a SUBCLASS of SignalException (or SIGSEGV) prints, plus the negatives that a rescued SystemExit records no terminal exit and that a raising at_exit does not become the status. cmd/rbgo/exit_status_test.go is the only witness for the status itself: it re-execs the compiled test binary with WT37_EXIT_SRC set, so the precondition is established by the test rather than by an external variable, no Go toolchain is needed, and it runs in every lane. It asserts WIFSIGNALED as well as the number, because "killed by 15" and "exited 143" give a shell the same $?. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/exit_status_test.go | 169 ++++++++ internal/vm/exit_status_internal_test.go | 260 ++++++++++++ .../vm/signal_disposition_internal_test.go | 384 ++++++++++++++++++ 3 files changed, 813 insertions(+) create mode 100644 cmd/rbgo/exit_status_test.go create mode 100644 internal/vm/exit_status_internal_test.go create mode 100644 internal/vm/signal_disposition_internal_test.go diff --git a/cmd/rbgo/exit_status_test.go b/cmd/rbgo/exit_status_test.go new file mode 100644 index 00000000..6e2dda09 --- /dev/null +++ b/cmd/rbgo/exit_status_test.go @@ -0,0 +1,169 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !rbgo_closed && !wasm + +package main + +import ( + "os" + "os/exec" + "strings" + "syscall" + "testing" +) + +// The exit status is the one thing no in-process test can observe: os.Exit ends +// the process, and being killed by a signal ends it harder. These tests therefore +// re-exec THIS TEST BINARY with WT37_EXIT_SRC set, which makes the child run one +// Ruby program through the same run()+finish() pair `rbgo run` uses and then exit. +// +// The precondition is established by the test itself — it sets the variable on +// the child it spawns — so unlike a test gated on an externally-supplied +// RBGO_* variable this one cannot silently not run. os.Args[0] is the compiled +// test binary, so no Go toolchain is needed and it runs in every CI lane. +const exitSrcEnv = "WT37_EXIT_SRC" + +// childMain is the re-exec entry point. It runs before any assertion in the +// child, so the child never reaches the testing framework's own output. +func childMain() { + if src, ok := os.LookupEnv(exitSrcEnv); ok { + finish(run(src, "-e")) // never returns + } +} + +func TestMain(m *testing.M) { + childMain() + os.Exit(m.Run()) +} + +// runChild runs one program in a child process and reports its stdout, stderr, +// exit status, and whether the child was KILLED BY a signal rather than exiting. +func runChild(t *testing.T, src string) (stdout, stderr string, status int, signaled bool, signo int) { + t.Helper() + c := exec.Command(os.Args[0], "-test.run=^TestExitStatusReachesTheProcess$") + c.Env = append(os.Environ(), exitSrcEnv+"="+src) + var so, se strings.Builder + c.Stdout, c.Stderr = &so, &se + err := c.Run() + if err != nil { + ee, ok := err.(*exec.ExitError) + if !ok { + t.Fatalf("re-exec %s: %v", os.Args[0], err) + } + if ws, okWS := ee.Sys().(syscall.WaitStatus); okWS && ws.Signaled() { + return so.String(), se.String(), 128 + int(ws.Signal()), true, int(ws.Signal()) + } + status = ee.ExitCode() + } + return so.String(), se.String(), status, false, 0 +} + +// TestExitStatusReachesTheProcess is the end-to-end witness for issue #676 and +// for the terminal half of #691. Every "want" here was measured against MRI 4.0.5 +// (ruby 4.0.5 (2026-05-20) +PRISM [arm64-darwin25]) with the same program. +// +// On origin/main this test's first row alone fails: `rbgo -e 'exit 3'` exited 0, +// because run() returned no error for a SystemExit and runCmd exited 0. Reverting +// runCmd to `if err := run(...); err != nil { reportError(err); os.Exit(1) }` +// still compiles and fails ten of these rows. +func TestExitStatusReachesTheProcess(t *testing.T) { + if _, isChild := os.LookupEnv(exitSrcEnv); isChild { + // The child never gets here: childMain ran finish() from TestMain. Guard + // anyway so a mistake shows as a failure rather than a recursive spawn. + t.Fatal("child reached the test body; childMain did not run") + } + for _, tc := range []struct { + name, src string + wantStatus int + wantOut, wantErr string + wantSignal int // non-zero: the child must be KILLED BY this signal + }{ + {name: "exit 3", src: `exit 3`, wantStatus: 3}, + {name: "exit false", src: `exit false`, wantStatus: 1}, + {name: "exit 0", src: `exit 0`, wantStatus: 0}, + {name: "exit true", src: `exit true`, wantStatus: 0}, + {name: "exit!(4)", src: `exit!(4)`, wantStatus: 4}, + {name: "abort", src: `abort "boom"`, wantStatus: 1, wantErr: "boom\n"}, + {name: "bare abort", src: `abort`, wantStatus: 1}, + {name: "raise", src: `raise "x"`, wantStatus: 1, + wantErr: "-e:1:in '
': x (RuntimeError)\n"}, + {name: "Process.exit(5)", src: `Process.exit(5)`, wantStatus: 5}, + {name: "Process.exit!(6)", src: `Process.exit!(6)`, wantStatus: 6}, + {name: "Process.abort", src: `Process.abort "pboom"`, wantStatus: 1, wantErr: "pboom\n"}, + // A rescued SystemExit must NOT exit non-zero — the easiest way to "fix" + // the status is to exit on every SystemExit ever raised, and this row is + // what makes that wrong. + {name: "a rescued SystemExit exits 0", + src: `begin; exit 3; rescue SystemExit => e; puts "status=#{e.status}"; end`, + wantOut: "status=3\n"}, + {name: "at_exit runs and the status survives", + src: `at_exit { puts "atexit" }; exit 3`, wantStatus: 3, wantOut: "atexit\n"}, + {name: "ensure runs and the status survives", + src: `begin; exit 3; ensure; puts "ensure"; end`, wantStatus: 3, wantOut: "ensure\n"}, + {name: "an at_exit exit overrides the status", + src: `at_exit { exit 7 }; exit 3`, wantStatus: 7}, + {name: "exit! runs no at_exit handler", + src: `at_exit { puts "atexit" }; exit!(4)`, wantStatus: 4}, + // A rescued self-signal leaves the process healthy: this is the #691 row + // that was 10-20% fatal on origin/main. + {name: "a rescued SignalException exits 0", + src: `begin + Process.kill(:TERM, Process.pid) +rescue SignalException => e + puts "#{e.class} #{e.message} #{e.signo}" +end +puts "alive"`, wantOut: "SignalException SIGTERM 15\nalive\n"}, + // An UNCAUGHT SignalException must leave the process dead BY the signal, + // with nothing on stderr (exiting_split gives the base class no message). + {name: "an uncaught SignalException dies by its signal", + src: `Process.kill(:TERM, Process.pid); puts "unreached"`, + wantStatus: 128 + 15, wantSignal: 15}, + // An uncaught Interrupt is the same death, but it DOES print, because it is + // a subclass. Its message is empty, so print_errinfo writes the class name + // alone with no parenthesised suffix. + {name: "an uncaught Interrupt prints its class and dies by SIGINT", + src: `Process.kill(:INT, Process.pid); puts "unreached"`, + wantStatus: 128 + 2, wantSignal: 2, wantErr: "-e:1:in '
': Interrupt\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + out, errOut, status, signaled, signo := runChild(t, tc.src) + if status != tc.wantStatus { + t.Errorf("exit status = %d, want %d (stderr %q)", status, tc.wantStatus, errOut) + } + if out != tc.wantOut { + t.Errorf("stdout = %q, want %q", out, tc.wantOut) + } + if errOut != tc.wantErr { + t.Errorf("stderr = %q, want %q", errOut, tc.wantErr) + } + // "killed by signal 15" and "exited with 143" give a shell the same $? + // but a different WIFSIGNALED, and MRI's ruby_default_signal produces + // the former. Asserting only the number would pass on a plain exit. + if tc.wantSignal != 0 { + if !signaled { + t.Errorf("child exited normally, want death by signal %d", tc.wantSignal) + } else if signo != tc.wantSignal { + t.Errorf("killed by signal %d, want %d", signo, tc.wantSignal) + } + } else if signaled { + t.Errorf("child was killed by signal %d, want a normal exit", signo) + } + }) + } +} + +// TestParseErrorStillExitsOne pins the path where finish has no machine to +// classify against: a program that never compiled. The status has always been 1; +// the risk the new code introduced is a nil-pointer dereference on the machine +// run() could not build, which this row is here to catch. +func TestParseErrorStillExitsOne(t *testing.T) { + _, errOut, status, signaled, _ := runChild(t, `def (`) + if status != 1 || signaled { + t.Errorf("status = %d (signaled %v), want a plain exit 1", status, signaled) + } + if errOut == "" { + t.Error("no diagnostic on stderr for a parse error") + } +} diff --git a/internal/vm/exit_status_internal_test.go b/internal/vm/exit_status_internal_test.go new file mode 100644 index 00000000..5cf39f7a --- /dev/null +++ b/internal/vm/exit_status_internal_test.go @@ -0,0 +1,260 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// TestExitingSplitClassification is eval_error.c exiting_split, table-driven. +// It is the decision the rbgo CLI turns into the process's disposition, and it +// is the half of issue #676 that could be unit-tested at all: on origin/main +// neither ExitingSplit nor TerminalExit existed, so every one of these rows is a +// witness rather than a guard. +func TestExitingSplitClassification(t *testing.T) { + for _, tc := range []struct { + name, src string + wantStatus int + wantSignal int + wantMessage bool + }{ + // SystemExit -> EXITING_WITH_STATUS, status = @status, and NO message: + // a program that ends by calling exit is terminating, not crashing. + {"exit 3", `exit 3`, 3, 0, false}, + {"exit false", `exit false`, 1, 0, false}, + {"exit 0", `exit 0`, 0, 0, false}, + {"exit!(4)", `exit!(4)`, 4, 0, false}, + {"abort", `abort "boom"`, 1, 0, false}, + {"Process.exit(5)", `Process.exit(5)`, 5, 0, false}, + {"Process.exit!(6)", `Process.exit!(6)`, 6, 0, false}, + {"Process.abort", `Process.abort "boom"`, 1, 0, false}, + // A raise of SystemExit itself, with no status ivar set by Kernel#exit: + // SystemExit.new's default @status is 0. + {"raise SystemExit", `raise SystemExit`, 0, 0, false}, + // SignalException -> EXITING_WITH_SIGNAL. No message for the class itself + // (`rb_obj_is_instance_of(errinfo, rb_eSignal)` is true), so nothing is + // printed and the process dies by the signal. + {"uncaught SIGTERM", `Process.kill(:TERM, Process.pid)`, 128 + 15, 15, false}, + // ...but a SUBCLASS of SignalException DOES print, which is why an uncaught + // Interrupt shows "Interrupt" on stderr and a bare SignalException shows + // nothing. Getting this backwards is invisible in the exit status. + {"uncaught Interrupt", `Process.kill(:INT, Process.pid)`, 128 + 2, 2, true}, + // SIGSEGV prints even as the base class (exiting_split's explicit case). + {"SIGSEGV prints", `raise SignalException.new("SEGV")`, 128 + 11, 11, true}, + // Signal 0 cannot kill anything; kill(2) defines it as a liveness probe, + // so re-raising it would be a no-op and leave the process exiting 0 on what + // is really a failure. + {"SignalException with signo 0", `raise SignalException.new(0)`, 1, 0, true}, + // Everything else -> EXIT_FAILURE with a message, unchanged. + {"RuntimeError", `raise "x"`, 1, 0, true}, + {"a bare raise from a native method", `Integer("nope")`, 1, 0, true}, + {"NoMemoryError", `raise NoMemoryError`, 1, 0, true}, + } { + t.Run(tc.name, func(t *testing.T) { + machine, out, err := runSrcErr(t, tc.src) + if err == nil { + texit, ok := machine.TerminalExit() + if !ok { + t.Fatalf("no error and no terminal exit (output %q)", out) + } + err = texit + } + d, ok := machine.ExitingSplit(err) + if !ok { + t.Fatalf("ExitingSplit did not classify %T %v", err, err) + } + if d.Status != tc.wantStatus { + t.Errorf("Status = %d, want %d", d.Status, tc.wantStatus) + } + if d.Signal != tc.wantSignal { + t.Errorf("Signal = %d, want %d", d.Signal, tc.wantSignal) + } + if d.Message != tc.wantMessage { + t.Errorf("Message = %v, want %v", d.Message, tc.wantMessage) + } + }) + } +} + +// TestExitingSplitRejectsANonRubyError proves the front-end still has a path for +// a parse, compile or host IO error: ExitingSplit must report false rather than +// classifying it as a plain failure, because those errors are not Ruby exceptions +// and have no class hierarchy to place. +func TestExitingSplitRejectsANonRubyError(t *testing.T) { + machine, _, _ := runSrcErr(t, `1`) + if _, ok := machine.ExitingSplit(errNotRuby{}); ok { + t.Error("ExitingSplit classified a non-Ruby error") + } +} + +type errNotRuby struct{} + +func (errNotRuby) Error() string { return "not a ruby exception" } + +// TestTerminalExitIsAbsentWithoutOne pins the negative: a program that ends +// normally, and one whose SystemExit was RESCUED, must leave no terminal exit — +// otherwise `rbgo -e '...rescue SystemExit...'` would exit non-zero on a program +// MRI exits 0 for. An assertion that only checked the positive case would pass +// with a VM that recorded every SystemExit ever raised. +func TestTerminalExitIsAbsentWithoutOne(t *testing.T) { + for _, tc := range []struct{ name, src string }{ + {"a program that ends normally", `x = 1`}, + {"a rescued SystemExit", `begin; exit 3; rescue SystemExit; end`}, + {"a rescued SystemExit through ensure", `begin; exit 3; rescue SystemExit; ensure; x = 1; end`}, + } { + t.Run(tc.name, func(t *testing.T) { + machine, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if texit, ok := machine.TerminalExit(); ok { + t.Errorf("TerminalExit reported %v, want none", texit) + } + }) + } +} + +// TestExitBangSkipsAtExitHandlers is process.c rb_f_exit_bang, which is _exit(2) +// and therefore runs no exit handler. Measured against MRI 4.0.5: +// `at_exit { puts "x" }; exit!(4)` prints nothing and exits 4, where plain +// `exit 4` prints "x". +// +// MRI also skips `ensure` blocks, which this does NOT yet do — the exception +// machinery has no unwinding path that `ensure` does not see. The ensure row +// below records the remaining divergence rather than asserting MRI's answer, +// and says so, so nobody reads it as a specification. +func TestExitBangSkipsAtExitHandlers(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"exit! runs no at_exit", `at_exit { puts "handler" }; exit!(4)`, ""}, + {"exit runs at_exit", `at_exit { puts "handler" }; exit 4`, "handler\n"}, + {"exit! still lets ensure run (a KNOWN divergence from MRI, which skips it)", + `begin; exit!(4); ensure; puts "ensure"; end`, "ensure\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + machine, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v", err) + } + if out != tc.want { + t.Errorf("output = %q, want %q", out, tc.want) + } + texit, ok := machine.TerminalExit() + if !ok { + t.Fatal("no terminal exit recorded") + } + d, _ := machine.ExitingSplit(texit) + if d.Status != 4 { + t.Errorf("Status = %d, want 4", d.Status) + } + }) + } +} + +// TestAtExitExitOverridesTheStatus is rb_ec_cleanup reading ec->errinfo AFTER +// rb_ec_teardown: a handler that calls exit sets the status, and overrides one an +// earlier exit asked for. Measured against MRI 4.0.5: 7, not 3, and 7 on a +// program that never called exit at all. +func TestAtExitExitOverridesTheStatus(t *testing.T) { + for _, tc := range []struct { + name, src string + want int + }{ + {"an at_exit exit overrides an earlier one", `at_exit { exit 7 }; exit 3`, 7}, + {"an at_exit exit on a program that ends normally", `at_exit { exit 7 }`, 7}, + {"the LAST handler to exit wins", `at_exit { exit 7 }; at_exit { exit 8 }`, 7}, + // A handler that raises something else must not become the status: MRI + // swallows it, and exiting_split would have made it 1. + {"a raising handler leaves the status alone", `at_exit { raise "x" }; exit 3`, 3}, + } { + t.Run(tc.name, func(t *testing.T) { + machine, _, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v", err) + } + texit, ok := machine.TerminalExit() + if !ok { + t.Fatal("no terminal exit recorded") + } + d, _ := machine.ExitingSplit(texit) + if d.Status != tc.want { + t.Errorf("Status = %d, want %d", d.Status, tc.want) + } + }) + } +} + +// TestProcessExitFamilyIsPublic is issue #676's second defect: Init_process +// declares exit, exit! and abort with rb_define_module_function, so the qualified +// spelling is public. rbgo had only Kernel's private copies and raised +// NoMethodError. +func TestProcessExitFamilyIsPublic(t *testing.T) { + src := `p Process.respond_to?(:exit), Process.respond_to?(:exit!), Process.respond_to?(:abort) +begin + Process.exit(5) +rescue SystemExit => e + puts "exit #{e.status}" +end +begin + Process.exit!(6) +rescue SystemExit => e + puts "exit! #{e.status}" +end +begin + Process.exit +rescue SystemExit => e + puts "bare #{e.status}" +end` + want := "true\ntrue\ntrue\nexit 5\nexit! 6\nbare 0\n" + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != want { + t.Errorf("output = %q, want %q", out, want) + } +} + +// TestClassIsAWalksTheHierarchy covers classIsA's two branches, including the +// one a Ruby program cannot reach: a class name that is not a constant at all, +// which a raise() from a native method can produce. +func TestClassIsAWalksTheHierarchy(t *testing.T) { + machine, _, _ := runSrcErr(t, `1`) + for _, tc := range []struct { + class, ancestor string + want bool + }{ + {"SystemExit", "SystemExit", true}, + {"SystemExit", "Exception", true}, + {"SystemExit", "StandardError", false}, + {"Interrupt", "SignalException", true}, + {"SignalException", "Interrupt", false}, + {"RuntimeError", "SystemExit", false}, + {"NoSuchClassAnywhere", "NoSuchClassAnywhere", true}, + {"NoSuchClassAnywhere", "SystemExit", false}, + } { + if got := machine.classIsA(tc.class, tc.ancestor); got != tc.want { + t.Errorf("classIsA(%q, %q) = %v, want %v", tc.class, tc.ancestor, got, tc.want) + } + } +} + +// TestExitStatusAndSignoOfMissingObject covers the nil-object paths: a +// SystemExit or SignalException raised by a native raise() carries a class name +// but no exception object, and reading its ivars must not panic. +func TestExitStatusAndSignoOfMissingObject(t *testing.T) { + machine, _, _ := runSrcErr(t, `1`) + if got := machine.exitStatusOf(nil); got != 0 { + t.Errorf("exitStatusOf(nil) = %d, want 0", got) + } + if got := machine.signoOf(nil); got != 0 { + t.Errorf("signoOf(nil) = %d, want 0", got) + } + d, ok := machine.ExitingSplit(RubyError{Class: "SystemExit", Message: "exit"}) + if !ok || d.Status != 0 || d.Message { + t.Errorf("objectless SystemExit -> %+v, want status 0 and no message", d) + } + d, ok = machine.ExitingSplit(RubyError{Class: "SignalException", Message: "SIGTERM"}) + if !ok || d.Signal != 0 || d.Status != 1 || !d.Message { + t.Errorf("objectless SignalException -> %+v, want the plain-failure fallback", d) + } +} diff --git a/internal/vm/signal_disposition_internal_test.go b/internal/vm/signal_disposition_internal_test.go new file mode 100644 index 00000000..aea82414 --- /dev/null +++ b/internal/vm/signal_disposition_internal_test.go @@ -0,0 +1,384 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import ( + "bytes" + "strings" + "testing" + + "github.com/go-embedded-ruby/ruby/internal/compiler" + "github.com/go-embedded-ruby/ruby/internal/object" + "github.com/go-ruby-parser/parser" +) + +// runSrcErr runs a program and returns its stdout plus whatever error came out +// of Run, so a test can assert on a program that ends by raising or exiting. +// runSrc (aot_dispatch_test.go) t.Fatals on an error, which is the wrong shape +// for everything here. +func runSrcErr(t *testing.T, src string) (*VM, string, error) { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("parse: %v", err) + } + iseq, err := compiler.Compile(prog) + if err != nil { + t.Fatalf("compile: %v", err) + } + var buf bytes.Buffer + machine := New(&buf) + _, runErr := machine.Run(iseq) + return machine, buf.String(), runErr +} + +// TestSelfSignalRaisesInsteadOfKilling is the witness for issue #691: on +// origin/main this program's process DIED (128+15) between 10% and 20% of runs +// and printed nothing the rest of the time, because Process.kill called kill(2) +// and then raced the Go runtime's default disposition against the program's own +// exit. signal.c rb_f_kill never sends a deferrable signal to itself; it enqueues +// it and runs rb_thread_execute_interrupts() before returning, so the raise +// happens synchronously. +// +// A/B: this test cannot pass on the old code even in the runs where the process +// survived, because no exception was raised at all there either ("NO RAISE - kept +// running" in the issue). Reverting vm.selfSignal's call site in +// defineProcessKill to an unconditional procKill still compiles and fails here. +func TestSelfSignalRaisesInsteadOfKilling(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + // rb_signal_exec's cmd == 0 arm: SIGTERM and the rest of default_handler's + // first group become SignalException, carrying #signo and #signm. + {"TERM", `begin + Process.kill(:TERM, Process.pid) +rescue SignalException => e + puts "#{e.class} #{e.message} #{e.signo} #{e.signm}" +end +puts "alive"`, "SignalException SIGTERM 15 SIGTERM\nalive\n"}, + // SIGINT is the one case rb_signal_exec routes to rb_interrupt() instead, + // and interrupt_init leaves the message EMPTY. + {"INT", `begin + Process.kill(:INT, Process.pid) +rescue Interrupt => e + puts "#{e.class} #{e.message.inspect} #{e.signo}" +end +puts "alive"`, "Interrupt \"\" 2\nalive\n"}, + // Interrupt < SignalException, so the wider rescue catches it. + {"INT rescued as SignalException", `begin + Process.kill("SIGINT", Process.pid) +rescue SignalException => e + puts e.class +end`, "Interrupt\n"}, + // A trap runs with the signal NUMBER as its argument (signal_exec -> + // rb_eval_cmd_kw(cmd, [INT2NUM(sig)])). + {"trap block", `Signal.trap(:TERM) { |s| puts "trapped #{s}" } +Process.kill(:TERM, Process.pid) +puts "after"`, "trapped 15\nafter\n"}, + // A trap installed as a Proc rather than a block. + {"trap proc", `Signal.trap(:HUP, ->(s) { puts "hup #{s}" }) +Process.kill(:HUP, Process.pid)`, "hup 1\n"}, + // "IGNORE" discards it: signal_ignored() == 1, so MRI neither sends the + // signal nor runs anything. + {"IGNORE", `Signal.trap(:TERM, "IGNORE") +Process.kill(:TERM, Process.pid) +puts "after"`, "after\n"}, + {"SIG_IGN", `Signal.trap(:TERM, "SIG_IGN") +Process.kill(:TERM, Process.pid) +puts "after"`, "after\n"}, + {"nil handler ignores", `Signal.trap(:TERM, nil) +Process.kill(:TERM, Process.pid) +puts "after"`, "after\n"}, + // "DEFAULT" restores default_handler(sig), which for TERM is MRI's own + // handler — so the SignalException comes back rather than the OS default. + {"DEFAULT restores the raise", `Signal.trap(:TERM) { puts "no" } +Signal.trap(:TERM, "DEFAULT") +begin + Process.kill(:TERM, Process.pid) +rescue SignalException => e + puts "back to #{e.message}" +end`, "back to SIGTERM\n"}, + // "EXIT" is rb_threadptr_signal_exit: SystemExit with EXIT_FAILURE. + {"EXIT", `Signal.trap(:TERM, "EXIT") +begin + Process.kill(:TERM, Process.pid) +rescue SystemExit => e + puts "exit #{e.status}" +end`, "exit 1\n"}, + // Process.kill(0, pid) is a liveness probe and must reach the OS, returning + // the number of pids signalled. + {"signal 0 probes", `p Process.kill(0, Process.pid)`, "1\n"}, + // A signal MRI leaves to the OS raises nothing here, so the trap list has + // to report SYSTEM_DEFAULT rather than DEFAULT for it (trap()'s oldcmd). + {"trap previous handler", `p Signal.trap(:HUP, "IGNORE") +p Signal.trap(:HUP, "DEFAULT") +p Signal.trap(:WINCH, "IGNORE")`, "\"DEFAULT\"\n\"IGNORE\"\n\"SYSTEM_DEFAULT\"\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != tc.want { + t.Errorf("output = %q, want %q", out, tc.want) + } + }) + } +} + +// TestSelfSignalFallsThroughToTheOS pins the two dispositions where rb_f_kill +// really does call kill(2) on this process, by swapping the procKill seam rather +// than signalling the test binary. Without the seam this could only be tested by +// killing the test process, which is why the seam is what the assertion reads. +func TestSelfSignalFallsThroughToTheOS(t *testing.T) { + for _, tc := range []struct { + name, src string + wantSig int + }{ + // SIGKILL cannot be deferred to a safe point: rb_f_kill's inner switch + // calls kill(pid, sig) for it directly. + {"KILL is delivered for real", `Process.kill(:KILL, Process.pid)`, 9}, + // "SYSTEM_DEFAULT" sets SIG_DFL, so signal_ignored() returns -1 and + // rb_f_kill falls through to kill(2). + {"SYSTEM_DEFAULT is delivered for real", + `Signal.trap(:TERM, "SYSTEM_DEFAULT") +Process.kill(:TERM, Process.pid)`, 15}, + // A signal MRI installs no handler for has SIG_DFL disposition too. + {"an unmanaged signal is delivered for real", `Process.kill(:WINCH, Process.pid)`, 28}, + } { + t.Run(tc.name, func(t *testing.T) { + orig := procKill + t.Cleanup(func() { procKill = orig }) + var gotPid, gotSig int + calls := 0 + procKill = func(pid, sig int) error { + calls++ + gotPid, gotSig = pid, sig + return nil + } + if _, out, err := runSrcErr(t, tc.src); err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if calls != 1 { + t.Fatalf("procKill called %d times, want 1", calls) + } + if gotSig != tc.wantSig { + t.Errorf("signal = %d, want %d", gotSig, tc.wantSig) + } + if gotPid <= 0 { + t.Errorf("pid = %d, want this process", gotPid) + } + }) + } +} + +// TestSelfSignalOnlyShortCircuitsThisProcess proves the synchronous arm is +// reached only for pid == self and only for a positive signal: another pid, and a +// process-GROUP kill even of our own group, must still go to the OS. A version +// that tested `pid == self` alone would pass while silently swallowing +// Process.kill(-15, Process.getpgrp). +func TestSelfSignalOnlyShortCircuitsThisProcess(t *testing.T) { + orig := procKill + t.Cleanup(func() { procKill = orig }) + var sent [][2]int + procKill = func(pid, sig int) error { sent = append(sent, [2]int{pid, sig}); return nil } + + src := `Process.kill(:TERM, 999999) rescue nil +Process.kill(-15, Process.pid) rescue nil +begin + Process.kill(:TERM, Process.pid) +rescue SignalException + puts "self raised" +end` + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != "self raised\n" { + t.Errorf("output = %q, want %q", out, "self raised\n") + } + if len(sent) != 2 { + t.Fatalf("procKill calls = %v, want exactly the foreign pid and the group kill", sent) + } + if sent[0][0] != 999999 || sent[0][1] != 15 { + t.Errorf("foreign kill = %v, want [999999 15]", sent[0]) + } + // killpg(pgid, sig) == kill(-pgid, sig): the pid is negated, the signal is not. + if sent[1][0] >= 0 || sent[1][1] != 15 { + t.Errorf("group kill = %v, want a negative pid with signal 15", sent[1]) + } +} + +// TestSignalExceptionConstruction covers signal.c esignal_init and +// interrupt_init directly: the name branch, the number branch, the explicit +// message, and each ArgumentError. +func TestSignalExceptionConstruction(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + // The name branch prefixes SIG when the name was given without one. + {"from a name", `e = SignalException.new("TERM"); puts "#{e.message} #{e.signo}"`, "SIGTERM 15\n"}, + {"from a prefixed name", `e = SignalException.new("SIGTERM"); puts "#{e.message} #{e.signo}"`, "SIGTERM 15\n"}, + {"from a symbol", `e = SignalException.new(:HUP); puts "#{e.message} #{e.signo}"`, "SIGHUP 1\n"}, + // The number branch defaults the message through rb_signo2signm. + {"from a number", `e = SignalException.new(15); puts "#{e.message} #{e.signo}"`, "SIGTERM 15\n"}, + {"from a number with a message", `e = SignalException.new(15, "boom"); puts "#{e.message} #{e.signo}"`, "boom 15\n"}, + // rb_signo2signm falls back to "SIG%u" for a number no name carries. + {"an unnamed number", `e = SignalException.new(62); puts "#{e.message} #{e.signo}"`, "SIG62 62\n"}, + // #signm is an alias of #message (rb_alias in Init_signal). + {"signm aliases message", `e = SignalException.new(15, "boom"); p e.signm == e.message`, "true\n"}, + // interrupt_init fixes the signal at SIGINT and leaves the message empty. + {"Interrupt", `e = Interrupt.new; puts "#{e.message.inspect} #{e.signo}"`, "\"\" 2\n"}, + {"Interrupt with a message", `e = Interrupt.new("ouch"); puts "#{e.message} #{e.signo}"`, "ouch 2\n"}, + // Errors. A name nothing matches, and a number outside NSIG. + {"unknown name", `begin; SignalException.new("NOPE"); rescue ArgumentError => e; puts e.message; end`, + "unsupported signal 'SIGNOPE'\n"}, + {"number too large", `begin; SignalException.new(9999); rescue ArgumentError => e; puts e.message; end`, + "invalid signal number (9999)\n"}, + {"no argument", `begin; SignalException.new; rescue ArgumentError => e; puts e.message; end`, + "wrong number of arguments (given 0, expected 1..2)\n"}, + // The name branch takes exactly one argument (argnum stays 1). + {"a name with a second argument", `begin; SignalException.new("TERM", "x"); rescue ArgumentError => e; puts e.message; end`, + "wrong number of arguments (given 2, expected 1)\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != tc.want { + t.Errorf("output = %q, want %q", out, tc.want) + } + }) + } +} + +// TestSignalListAndSignameCanonicalNames pins the two properties +// core/signal/list_spec.rb and core/signal/signame_spec.rb assert and that a +// Go map cannot provide on its own: EXIT is a member with number 0, and a +// number shared by a canonical name and an alias resolves to the CANONICAL one. +// With signalNames built from an unordered map this test would fail at random, +// which is exactly the shape of defect an ordered siglist exists to prevent. +func TestSignalListAndSignameCanonicalNames(t *testing.T) { + src := `p Signal.list["EXIT"], Signal.list["KILL"] +p Signal.list["CLD"] == Signal.list["CHLD"] +p Signal.list["IOT"] == Signal.list["ABRT"] +p Signal.signame(Signal.list["ABRT"]), Signal.signame(Signal.list["CHLD"]) +p Signal.signame(0), Signal.signame(-1), Signal.signame(9999)` + want := "0\n9\ntrue\ntrue\n\"ABRT\"\n\"CHLD\"\n\"EXIT\"\nnil\nnil\n" + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != want { + t.Errorf("output = %q, want %q", out, want) + } + // Every key Signal.list reports must be a name ruby/spec's own RUBY_SIGNALS + // list knows; a name outside it fails core/signal/list_spec.rb. + known := map[string]bool{} + for _, n := range strings.Fields(`EXIT HUP INT QUIT ILL TRAP IOT ABRT EMT FPE KILL BUS SEGV SYS +PIPE ALRM TERM URG STOP TSTP CONT CHLD CLD TTIN TTOU IO XCPU XFSZ VTALRM PROF +WINCH USR1 USR2 LOST MSG PWR POLL DANGER MIGRATE PRE GRANT RETRACT SOUND INFO`) { + known[n] = true + } + for _, e := range siglist { + if !known[e.name] { + t.Errorf("siglist has %q, which ruby/spec's RUBY_SIGNALS does not list", e.name) + } + } +} + +// TestSignameCoercesThroughToInt covers sig_signame's NUM2INT: a non-Integer +// converts through #to_int, and a #to_int that hands back a non-Integer is a +// TypeError rather than a silent nil. +func TestSignameCoercesThroughToInt(t *testing.T) { + src := `o = Object.new +def o.to_int; 0; end +p Signal.signame(o) +begin; Signal.signame("hello"); rescue TypeError => e; puts "TypeError"; end +b = Object.new +def b.to_int; "not an int"; end +begin; Signal.signame(b); rescue TypeError => e; puts "TypeError"; end` + want := "\"EXIT\"\nTypeError\nTypeError\n" + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != want { + t.Errorf("output = %q, want %q", out, want) + } +} + +// TestSignoToSignmFallback exercises rb_signo2signm's sprintf branch and +// withSIG's prefixing rule at the Go level, since neither is reachable through +// Ruby for every input. +func TestSignoToSignmFallback(t *testing.T) { + for _, tc := range []struct { + in int + want string + }{{15, "SIGTERM"}, {0, "SIGEXIT"}, {62, "SIG62"}, {-1, "SIG-1"}} { + if got := signoToSignm(tc.in); got != tc.want { + t.Errorf("signoToSignm(%d) = %q, want %q", tc.in, got, tc.want) + } + } + for _, tc := range []struct{ in, want string }{ + {"TERM", "SIGTERM"}, {"SIGTERM", "SIGTERM"}, {"SIG", "SIGSIG"}, + } { + if got := withSIG(tc.in); got != tc.want { + t.Errorf("withSIG(%q) = %q, want %q", tc.in, got, tc.want) + } + } + if got := signalName(object.IntValue(15)); got != "TERM" { + t.Errorf("signalName(15) = %q, want TERM", got) + } + if got := signalName(object.IntValue(9999)); got != "9999" { + t.Errorf("signalName(9999) = %q, want the number itself", got) + } + if _, ok := signalNumberOf("NOPE"); ok { + t.Error("signalNumberOf(NOPE) reported a number") + } +} + +// TestSigNumberPanicsOnATypo proves sigNumber's guard fires, so a misspelt name +// inside this package cannot silently become signal 0 (which kill(2) treats as a +// liveness probe and would therefore look like it worked). +func TestSigNumberPanicsOnATypo(t *testing.T) { + defer func() { + if recover() == nil { + t.Error("sigNumber accepted an unknown name") + } + }() + _ = sigNumber("NOT_A_SIGNAL") +} + +// TestTrapArgumentErrors covers Signal.trap's own arity and coercion errors. +func TestTrapArgumentErrors(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"no arguments", `begin; Signal.trap; rescue ArgumentError => e; puts e.message; end`, + "wrong number of arguments (given 0, expected 1..2)\n"}, + {"one argument and no block", `begin; Signal.trap(:TERM); rescue ArgumentError => e; puts e.message; end`, + "tried to create Proc object without a block\n"}, + {"an unknown signal name", `begin; Signal.trap("NOPE", "IGNORE"); rescue ArgumentError => e; puts e.message; end`, + "unsupported signal 'SIGNOPE'\n"}, + {"a signal number out of range", `begin; Signal.trap(9999, "IGNORE"); rescue ArgumentError => e; puts e.message; end`, + "invalid signal number (9999)\n"}, + // trap_signm accepts a number, including 0 (Signal.trap(:EXIT)'s slot). + {"a numeric signal", `p Signal.trap(15, "IGNORE")`, "\"DEFAULT\"\n"}, + {"Kernel#trap is the same operation", `trap(:TERM, "IGNORE") +Process.kill(:TERM, Process.pid) +puts "after"`, "after\n"}, + // A command that is neither a Proc nor one of the keywords is #call'd. + {"an arbitrary callable", `h = Object.new +def h.call(s); puts "called #{s}"; end +Signal.trap(:HUP, h) +Process.kill(:HUP, Process.pid)`, "called 1\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != tc.want { + t.Errorf("output = %q, want %q", out, tc.want) + } + }) + } +} From 29abbd80cdb3eace724e7dfff95b07b5df305c71 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 18:54:51 +0200 Subject: [PATCH 3/6] signal: the refusals and the coercion rules sig_trap applies before it looks at a handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twelve more ruby/spec examples in core/signal/trap_spec.rb and core/exception/interrupt_spec.rb, all from reading signal.c rather than from guessing: - sig_trap refuses a RESERVED signal (reserved_signal_p: SEGV, BUS, ILL, FPE, VTALRM) with ArgumentError, before it looks at the handler at all. - SIGKILL and SIGSTOP are refused by the KERNEL, so trap() -> ruby_signal() gets SIG_ERR and rb_sys_fail_str raises Errno::EINVAL, not ArgumentError. - trap_signm tests FIXNUM_P, so #to_int is NEVER called on this path and a Float is a "bad signal type", not a truncated number; signm2signo DOES coerce through #to_str. One "coerce somehow" implementation passes the positives and fails both negatives, which is what was there. - rb_obj_classname, so the bad-type message says NilClass, not nil. - trap(sig, nil) reports back nil on the next trap: trap()'s oldcmd switch has an explicit `case Qnil: break` that "IGNORE" and "" do not take. Both dispositions discard the signal, so only the RETURN distinguishes them. - signm2signo's `exit` parameter: EXIT is a trappable name but not a SignalException name (FOREACH_SIGNAL skips the first entry when exit is false). - Interrupt.new leaves the message unset so Exception#message answers the class name, while the one rb_interrupt() raises carries an explicit empty message. Measured on MRI 4.0.5: "Interrupt" and "" respectively. TestSignalModule (puppet_cli_features_test.go) had five rows PINNING rbgo's own divergence under a comment claiming they were pinned to MRI — signame("x") answering nil where MRI raises TypeError, signame with no argument answering nil, trap(987654) inventing a slot, trap(:QUIT) with no block recording DEFAULT, and an object with only #to_s being accepted. Each replacement was measured against MRI 4.0.5 on this host and the row says so. core/signal/trap_spec.rb 24 -> 35 of MRI's 36 through the same shim (the remaining one wants Thread#group); core/exception/interrupt_spec.rb 2 -> 3 of 3. Co-Authored-By: Claude Opus 5 --- internal/vm/puppet_cli_features_test.go | 44 ++++-- internal/vm/signal.go | 146 ++++++++++++++++-- .../vm/signal_disposition_internal_test.go | 142 ++++++++++++++++- 3 files changed, 304 insertions(+), 28 deletions(-) diff --git a/internal/vm/puppet_cli_features_test.go b/internal/vm/puppet_cli_features_test.go index 01b5c473..c017119d 100644 --- a/internal/vm/puppet_cli_features_test.go +++ b/internal/vm/puppet_cli_features_test.go @@ -8,7 +8,16 @@ import "testing" // TestSignalModule covers the Signal module and Kernel#trap: name normalisation // across Symbol/String("SIG"-prefixed and bare)/Integer designators, the -// previous-handler return value of #trap, #list and #signame, all pinned to MRI. +// previous-handler return value of #trap, #list and #signame. +// +// Five rows here PINNED rbgo's own divergence while the comment said they were +// pinned to MRI: Signal.signame("x") answered nil where MRI raises TypeError, +// Signal.signame with no argument answered nil where MRI raises ArgumentError, +// Signal.trap(987654) invented a slot where MRI range-checks against NSIG, +// Signal.trap(:QUIT) with no block recorded DEFAULT where MRI calls +// rb_block_proc() and fails, and an object with only #to_s was accepted where +// signm2signo coerces through #to_str. Each replacement below was MEASURED +// against MRI 4.0.5 (ruby 4.0.5 (2026-05-20) +PRISM [arm64-darwin25]) and says so. func TestSignalModule(t *testing.T) { cases := []struct{ src, want string }{ // First trap returns "DEFAULT"; a second returns the prior Proc. @@ -25,20 +34,35 @@ func TestSignalModule(t *testing.T) { {`p Signal.list.class`, "Hash\n"}, {`p Signal.signame(2)`, "\"INT\"\n"}, {`p Signal.signame(987654)`, "nil\n"}, - {`p Signal.signame("x")`, "nil\n"}, - {`p Signal.signame`, "nil\n"}, - // An unknown Integer designator falls back to its decimal string. - {`Signal.trap(987654) { }; p Signal.trap(987654, "DEFAULT").class`, "Proc\n"}, + // MEASURED on MRI 4.0.5: sig_signame takes NUM2INT, so a String is a + // TypeError, not nil. This row read "nil" and claimed to be pinned to MRI. + {`begin; Signal.signame("x"); rescue TypeError => e; puts e.message; end`, + "no implicit conversion of String into Integer\n"}, + // MEASURED on MRI 4.0.5: ArgumentError, not nil. + {`begin; Signal.signame; rescue ArgumentError => e; puts e.message; end`, + "wrong number of arguments (given 0, expected 1)\n"}, + // MEASURED on MRI 4.0.5: trap_signm range-checks against NSIG, so an + // out-of-range number is an ArgumentError rather than a fresh slot named + // after its decimal string. + {`begin; Signal.trap(987654) { }; rescue ArgumentError => e; puts e.message; end`, + "invalid signal number (987654)\n"}, // Kernel#trap reaches the same machinery without the Signal receiver. {`p trap("USR1") { }`, "\"DEFAULT\"\n"}, // Defaulting with two args and no block records the literal handler. {`Signal.trap(:HUP, "DEFAULT"); p Signal.trap(:HUP, "IGNORE")`, "\"DEFAULT\"\n"}, - // A no-block, single-arg trap records DEFAULT. - {`Signal.trap(:QUIT); p Signal.trap(:QUIT, "DEFAULT")`, "\"DEFAULT\"\n"}, - // A designator that is neither Symbol/String/Integer is accepted via its - // string form, returning the prior handler ("DEFAULT") for that fresh slot. + // MEASURED on MRI 4.0.5: sig_trap calls rb_block_proc() for a one-argument + // trap, which without a block is an ArgumentError — it does not record + // DEFAULT. + {`begin; Signal.trap(:QUIT); rescue ArgumentError => e; puts e.message; end`, + "tried to create Proc object without a block\n"}, + // MEASURED on MRI 4.0.5: signm2signo coerces through #to_str (via + // rb_check_string_type), NOT #to_s, so an object with only #to_s is a + // "bad signal type" naming its class. This row accepted it as a fresh slot. {`class SigName; def to_s; "CUSTOMSIG"; end; end -p Signal.trap(SigName.new) { }`, "\"DEFAULT\"\n"}, +begin; Signal.trap(SigName.new) { }; rescue ArgumentError => e; puts e.message; end`, + "bad signal type SigName\n"}, + {`class SigStr; def to_str; "HUP"; end; end +p Signal.trap(SigStr.new, "IGNORE")`, "\"DEFAULT\"\n"}, } for _, c := range cases { if got := eval(t, c.src); got != c.want { diff --git a/internal/vm/signal.go b/internal/vm/signal.go index ea63311a..bf519d53 100644 --- a/internal/vm/signal.go +++ b/internal/vm/signal.go @@ -5,6 +5,8 @@ package vm import ( + "syscall" + "github.com/go-embedded-ruby/ruby/internal/object" ) @@ -20,7 +22,8 @@ type trapKind int const ( trapDefault trapKind = iota // no handler installed: default_handler(sig) - trapIgnore // "IGNORE" / "SIG_IGN" / "" / nil + trapIgnore // "IGNORE" / "SIG_IGN" / "" + trapNil // trap(sig, nil): ignores, and reports back nil trapExit // "EXIT" trapSystemDefault // "SYSTEM_DEFAULT": SIG_DFL, the OS acts trapCommand // a Proc (or a command String) to run @@ -61,6 +64,27 @@ func signalFatal(sig int) bool { return false } +// signalReserved is signal.c reserved_signal_p: the signals MRI refuses to let a +// program trap at all, because it needs its own handlers for them. SEGV, BUS, +// ILL and FPE are synchronous and cannot be delivered to the main thread, and +// VTALRM is the interrupt the thread scheduler itself runs on. +func signalReserved(sig int) bool { + switch sig { + case sigNumber("SEGV"), sigNumber("BUS"), sigNumber("ILL"), + sigNumber("FPE"), sigNumber("VTALRM"): + return true + } + return false +} + +// signalUntrappable are the two signals sigaction(2) refuses outright, so MRI's +// trap() -> ruby_signal() gets SIG_ERR and calls rb_sys_fail_str(name), raising +// Errno::EINVAL rather than ArgumentError. POSIX fixes both numbers, so this +// needs no host table. +func signalUntrappable(sig int) bool { + return sig == sigNumber("KILL") || sig == sigNumber("STOP") +} + // registerSignal installs the Signal module, Kernel#trap, and the SignalException // / Interrupt methods that carry a signal's identity (#signo, #signm). // @@ -85,6 +109,18 @@ func (vm *VM) registerSignal() { return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") } sig := vm.trapSignm(args[0]) + // sig_trap refuses a reserved signal before it looks at the handler. + if signalReserved(sig) { + if name, ok := signalNameOf(sig); ok { + return raise("ArgumentError", "can't trap reserved signal: SIG%s", name) + } + return raise("ArgumentError", "can't trap reserved signal: %d", sig) + } + // SIGKILL and SIGSTOP cannot be caught or ignored: sigaction(2) fails + // EINVAL, which trap() turns into Errno::EINVAL through rb_sys_fail_str. + if signalUntrappable(sig) { + sysFail(syscall.EINVAL, signoToSignm(sig)) + } var next trapCmd switch { case blk != nil: @@ -178,8 +214,13 @@ func (vm *VM) registerSignalException() { if len(args) != 1 { return raise("ArgumentError", "wrong number of arguments (given %d, expected 1)", len(args)) } - name := args[0].ToS() - num, found := signalNumberOf(name) + // esignal_init calls signm2signo with exit = FALSE, so "EXIT" is not a + // signal name here even though Signal.trap(:EXIT) is legal. + name, named := vm.signalNameArg(args[0]) + if !named { + return raise("ArgumentError", "bad signal type %s", vm.objClassName(args[0])) + } + num, found := signalNumberOfEx(name, false) if !found { return raise("ArgumentError", "unsupported signal '%s'", withSIG(name)) } @@ -207,11 +248,15 @@ func (vm *VM) registerSignalException() { if !isObj { return object.NilV } - msg := "" + // Interrupt.new with no argument leaves the message UNSET, so + // Exception#message falls back to the class name and #signm reads + // "Interrupt" (core/exception/interrupt_spec.rb). The Interrupt that + // rb_interrupt() raises is built with an explicit empty message instead, + // which is why a rescued one reports "" — measured on MRI 4.0.5, where + // Interrupt.new.message is "Interrupt" and the raised one's is "". if len(args) > 0 { - msg = args[0].ToS() + o.ivars["@message"] = object.NewString(args[0].ToS()) } - o.ivars["@message"] = object.NewString(msg) o.ivars["@signo"] = object.IntValue(int64(sigNumber("INT"))) return object.NilV }) @@ -220,22 +265,61 @@ func (vm *VM) registerSignalException() { // trapSignm coerces Signal.trap's first argument to a signal number, applying // signal.c trap_signm: a Fixnum is range-checked against NSIG, anything else -// goes through signm2signo with exit = TRUE, so "EXIT"/0 is accepted. +// goes through signm2signo with exit = TRUE, so "EXIT" and 0 are accepted. +// +// Note what it does NOT do: trap_signm tests FIXNUM_P, so there is no #to_int +// conversion anywhere on this path — core/signal/trap_spec.rb asserts that +// explicitly with a mock that fails if #to_int is called, and a Float is a +// "bad signal type" rather than a truncated number. func (vm *VM) trapSignm(v object.Value) int { - if n, isInt := vm.toIntMaybe(v); isInt { - if n < 0 || n > int64(sigMaxNumber) { - raise("ArgumentError", "invalid signal number (%d)", n) + if n, isInt := v.(object.Integer); isInt { + if n < 0 || int64(n) > int64(sigMaxNumber) { + raise("ArgumentError", "invalid signal number (%d)", int64(n)) } return int(n) } - name := v.ToS() - if num, ok := signalNumberOf(name); ok { + return vm.signm2signo(v, true) +} + +// signm2signo is signal.c signm2signo with negative = FALSE: a Symbol becomes its +// name, a String is used as is, anything else must answer #to_str (through +// rb_check_string_type, so an object that does not is a "bad signal type" naming +// its CLASS, not a conversion failure), the "SIG" prefix is optional, and a name +// no entry carries is "unsupported signal". +// +// allowExit is the C parameter `exit`: FOREACH_SIGNAL skips the first table entry +// when it is false, and that entry is EXIT — so Signal.trap accepts :EXIT while +// SignalException.new and Process.kill do not. +func (vm *VM) signm2signo(v object.Value, allowExit bool) int { + name, ok := vm.signalNameArg(v) + if !ok { + raise("ArgumentError", "bad signal type %s", vm.objClassName(v)) + } + if num, found := signalNumberOfEx(name, allowExit); found { return num } raise("ArgumentError", "unsupported signal '%s'", withSIG(name)) return 0 } +// signalNameArg is signm2signo's argument coercion alone: a Symbol, a String, or +// an object whose #to_str returns one. It reports false for anything else so the +// caller can raise the "bad signal type" ArgumentError naming the class. +func (vm *VM) signalNameArg(v object.Value) (string, bool) { + switch t := v.(type) { + case object.Symbol: + return string(t), true + case *object.String: + return string(t.Bytes()), true + } + if vm.respondsToDynamic(v, "to_str") { + if s, isStr := vm.send(v, "to_str", nil, nil).(*object.String); isStr { + return string(s.Bytes()), true + } + } + return "", false +} + // trapHandler maps Signal.trap's second argument to a disposition, following // signal.c trap_handler's switch on the command string's length exactly: nil and // "" and "IGNORE"/"SIG_IGN" ignore, "DEFAULT"/"SIG_DFL" restore the signal's @@ -243,7 +327,10 @@ func (vm *VM) trapSignm(v object.Value) int { // and anything else is a command to run. func (vm *VM) trapHandler(v object.Value) trapCmd { if v == object.NilV { - return trapCmd{kind: trapIgnore, cmd: object.NilV} + // trap_handler leaves *cmd as Qnil for a nil handler, and trap()'s oldcmd + // switch has an explicit `case Qnil: break` that does NOT rewrite it — so a + // signal whose handler was set to nil reports back nil, not "IGNORE". + return trapCmd{kind: trapNil, cmd: object.NilV} } if p, isProc := v.(*Proc); isProc { return trapCmd{kind: trapCommand, cmd: p} @@ -283,6 +370,8 @@ func (vm *VM) trapPrevious(sig int) object.Value { return object.NewString("SYSTEM_DEFAULT") case trapIgnore: return object.NewString("IGNORE") + case trapNil: + return object.NilV case trapSystemDefault: return object.NewString("SYSTEM_DEFAULT") case trapExit: @@ -337,7 +426,7 @@ func (vm *VM) selfSignal(sig int) (handled bool) { cmd = trapCmd{kind: trapDefault} } switch cmd.kind { - case trapIgnore: + case trapIgnore, trapNil: // signal_ignored() == 1: MRI neither sends nor runs anything. return true case trapSystemDefault: @@ -458,10 +547,21 @@ func sigNumber(name string) int { return n } -// signalNumberOf is signal.c signm2signo: it accepts "TERM", "SIGTERM" or -// :TERM and reports whether the name is one Ruby knows. +// signalNumberOf is signalNumberOfEx allowing EXIT, for callers that only need +// the lookup. func signalNumberOf(name string) (int, bool) { - n, ok := signalNumbers[stripSIG(name)] + return signalNumberOfEx(name, true) +} + +// signalNumberOfEx looks a name up in siglist, accepting "TERM", "SIGTERM" or +// :TERM. allowExit is signm2signo's FOREACH_SIGNAL(sigs, !exit): with it false +// the EXIT entry is skipped, so "EXIT" is unsupported. +func signalNumberOfEx(name string, allowExit bool) (int, bool) { + bare := stripSIG(name) + if !allowExit && bare == "EXIT" { + return 0, false + } + n, ok := signalNumbers[bare] return n, ok } @@ -537,3 +637,15 @@ func (vm *VM) toIntMaybe(v object.Value) (int64, bool) { } return 0, false } + +// objClassName is MRI's rb_obj_classname: the name of the object's CLASS, which +// for nil is "NilClass" and not "nil". classNameOf answers the type name a format +// string wants, which differs for exactly the values signm2signo's error message +// is most likely to be handed (nil, true, false) — core/signal/trap_spec.rb reads +// the difference. +func (vm *VM) objClassName(v object.Value) string { + if c := vm.classOf(v); c != nil { + return c.name + } + return classNameOf(v) +} diff --git a/internal/vm/signal_disposition_internal_test.go b/internal/vm/signal_disposition_internal_test.go index aea82414..c26c7742 100644 --- a/internal/vm/signal_disposition_internal_test.go +++ b/internal/vm/signal_disposition_internal_test.go @@ -226,7 +226,10 @@ func TestSignalExceptionConstruction(t *testing.T) { // #signm is an alias of #message (rb_alias in Init_signal). {"signm aliases message", `e = SignalException.new(15, "boom"); p e.signm == e.message`, "true\n"}, // interrupt_init fixes the signal at SIGINT and leaves the message empty. - {"Interrupt", `e = Interrupt.new; puts "#{e.message.inspect} #{e.signo}"`, "\"\" 2\n"}, + // Interrupt.new leaves the message unset, so Exception#message falls back + // to the class name. The Interrupt rb_interrupt() raises carries an explicit + // empty message instead — see TestInterruptMessageFallsBackToTheClassName. + {"Interrupt", `e = Interrupt.new; puts "#{e.message.inspect} #{e.signo}"`, "\"Interrupt\" 2\n"}, {"Interrupt with a message", `e = Interrupt.new("ouch"); puts "#{e.message} #{e.signo}"`, "ouch 2\n"}, // Errors. A name nothing matches, and a number outside NSIG. {"unknown name", `begin; SignalException.new("NOPE"); rescue ArgumentError => e; puts e.message; end`, @@ -382,3 +385,140 @@ Process.kill(:HUP, Process.pid)`, "called 1\n"}, }) } } + +// TestTrapRefusesReservedAndUntrappableSignals covers signal.c sig_trap's two +// refusals, which happen BEFORE it looks at the handler: reserved_signal_p names +// the signals MRI needs its own handlers for, and SIGKILL/SIGSTOP are the two +// sigaction(2) rejects outright, which trap() reports as Errno::EINVAL through +// rb_sys_fail_str rather than as ArgumentError. +func TestTrapRefusesReservedAndUntrappableSignals(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"SEGV", `begin; Signal.trap(:SEGV) {}; rescue ArgumentError => e; puts e.message; end`, + "can't trap reserved signal: SIGSEGV\n"}, + {"BUS", `begin; Signal.trap(:BUS) {}; rescue ArgumentError => e; puts e.message; end`, + "can't trap reserved signal: SIGBUS\n"}, + {"ILL", `begin; Signal.trap(:ILL) {}; rescue ArgumentError => e; puts e.message; end`, + "can't trap reserved signal: SIGILL\n"}, + {"FPE", `begin; Signal.trap(:FPE) {}; rescue ArgumentError => e; puts e.message; end`, + "can't trap reserved signal: SIGFPE\n"}, + {"VTALRM", `begin; Signal.trap(:VTALRM) {}; rescue ArgumentError => e; puts e.message; end`, + "can't trap reserved signal: SIGVTALRM\n"}, + // Errno::EINVAL, not ArgumentError: the refusal comes from the kernel. + {"KILL", `begin; Signal.trap(:KILL) {}; rescue StandardError => e; puts e.class; end`, + "Errno::EINVAL\n"}, + {"STOP", `begin; Signal.trap(:STOP) {}; rescue StandardError => e; puts e.class; end`, + "Errno::EINVAL\n"}, + // The refusal comes first: a reserved signal is refused even with a handler + // that trap_handler would have accepted. + {"reserved beats the handler", `begin; Signal.trap(:SEGV, "IGNORE"); rescue ArgumentError => e; puts "refused"; end`, + "refused\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != tc.want { + t.Errorf("output = %q, want %q", out, tc.want) + } + }) + } +} + +// TestTrapSignalArgumentCoercion is signal.c trap_signm plus signm2signo's +// argument coercion. The two negatives matter most and are easy to get backwards: +// trap_signm tests FIXNUM_P, so #to_int is NEVER called (a Float is a bad type, +// not a truncated number), while signm2signo DOES call #to_str. A single +// "coerce to a number somehow" implementation passes the positive rows and fails +// both negatives, which is what origin/main did. +func TestTrapSignalArgumentCoercion(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"a String name", `p Signal.trap("HUP", "IGNORE")`, "\"DEFAULT\"\n"}, + {"a prefixed name", `p Signal.trap("SIGHUP", "IGNORE")`, "\"DEFAULT\"\n"}, + {"a Symbol name", `p Signal.trap(:HUP, "IGNORE")`, "\"DEFAULT\"\n"}, + {"an Integer", `p Signal.trap(Signal.list["HUP"], "IGNORE")`, "\"DEFAULT\"\n"}, + // EXIT / 0 is legal for trap (signm2signo's exit = TRUE) ... + {"EXIT is a trappable name", `p Signal.trap(:EXIT, "IGNORE")`, "\"SYSTEM_DEFAULT\"\n"}, + {"signal 0 is trappable", `p Signal.trap(0, "IGNORE")`, "\"SYSTEM_DEFAULT\"\n"}, + // ... but NOT for SignalException, whose esignal_init passes exit = FALSE. + {"EXIT is not a SignalException name", + `begin; SignalException.new("EXIT"); rescue ArgumentError => e; puts e.message; end`, + "unsupported signal 'SIGEXIT'\n"}, + // #to_str is consulted. + {"#to_str", `o = Object.new +def o.to_str; "HUP"; end +p Signal.trap(o, "IGNORE")`, "\"DEFAULT\"\n"}, + // #to_int is NOT. + {"#to_int is never called", `o = Object.new +def o.to_int; raise "to_int must not be called"; end +begin; Signal.trap(o, "IGNORE"); rescue ArgumentError => e; puts e.message; end`, + "bad signal type Object\n"}, + // rb_obj_classname, so nil is NilClass rather than "nil". + {"nil names its class", `begin; Signal.trap(nil, "IGNORE"); rescue ArgumentError => e; puts e.message; end`, + "bad signal type NilClass\n"}, + {"a Float is a bad type, not a truncation", + `begin; Signal.trap(100.0, "IGNORE"); rescue ArgumentError => e; puts e.message; end`, + "bad signal type Float\n"}, + {"true names its class", `begin; Signal.trap(true, "IGNORE"); rescue ArgumentError => e; puts e.message; end`, + "bad signal type TrueClass\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, out, err := runSrcErr(t, tc.src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != tc.want { + t.Errorf("output = %q, want %q", out, tc.want) + } + }) + } +} + +// TestTrapNilReportsNilNotIgnore pins trap()'s oldcmd switch, which has an +// explicit `case Qnil: break` that leaves a nil handler reporting back as nil — +// where "IGNORE" and "" both rewrite it to the string "IGNORE". Both DISPOSITIONS +// are the same (the signal is discarded), so a test that only checked the +// behaviour would pass with them collapsed. +func TestTrapNilReportsNilNotIgnore(t *testing.T) { + src := `Signal.trap(:HUP, nil) +p Signal.trap(:HUP, "DEFAULT") +Signal.trap(:HUP, "IGNORE") +p Signal.trap(:HUP, "DEFAULT") +Signal.trap(:HUP, "") +p Signal.trap(:HUP, "DEFAULT") +Signal.trap(:TERM, nil) +Process.kill(:TERM, Process.pid) +puts "a nil handler still discards the signal"` + want := "nil\n\"IGNORE\"\n\"IGNORE\"\na nil handler still discards the signal\n" + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != want { + t.Errorf("output = %q, want %q", out, want) + } +} + +// TestInterruptMessageFallsBackToTheClassName pins the difference measured on MRI +// 4.0.5 between the two ways an Interrupt comes into being: Interrupt.new leaves +// the message unset, so Exception#message answers the class name ("Interrupt"), +// while the one rb_interrupt() raises carries an explicit EMPTY message. Setting +// @message unconditionally in #initialize satisfies the raised case and breaks +// core/exception/interrupt_spec.rb, which is how it was found. +func TestInterruptMessageFallsBackToTheClassName(t *testing.T) { + src := `p Interrupt.new.message, Interrupt.new.signm, Interrupt.new.signo +p Interrupt.new("x").message +begin + Process.kill(:INT, Process.pid) +rescue Interrupt => e + p e.message, e.signo +end` + want := "\"Interrupt\"\n\"Interrupt\"\n2\n\"x\"\n\"\"\n2\n" + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != want { + t.Errorf("output = %q, want %q", out, want) + } +} From d09cb1549a4968d2458805d71676f71216a163b8 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 19:11:05 +0200 Subject: [PATCH 4/6] signal: close the coverage deficit the new code opened, and drop one dead helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The corrected coverage recipe (diff the below-100 LIST against origin/main's, each cover -func resolved in the tree that produced its profile, and diff the FULL measured set so a function that LEAVES it is visible) showed ten new below-100 functions. They are now zero, by two routes. Unreachable branches removed rather than tested: a duplicated arity guard in Signal.trap, the numeric spelling of "can't trap reserved signal" (every signal reserved_signal_p names has a siglist entry, unlike MRI whose NSIG table can be sparser than its handler set), and four `if !ok`/`if !isObj` guards that a fixed registration order and `new`'s allocator make impossible — now hard assertions, which fail loudly instead of doing nothing. trapPrevious's absent-entry arm was a copy of its trapDefault arm; the zero value of trapCmd already says it. Reachable branches given witnesses: SIGCHLD, the one member of default_handler's managed set rb_signal_exec has NO cmd == 0 case for (it is consumed at the safe point and raises nothing — a reading that equated "MRI installs a handler" with "MRI raises" would raise here); every arm of trap()'s oldcmd switch, since SYSTEM_DEFAULT and EXIT are observable ONLY through that return value; the esignal_init number branch's arity and its #to_int / #to_str fall-through; and the non-Integer @status ivar, which a Ruby program can install. signalName is DELETED. It was the old trap's name normaliser, replaced by trapSignm and signm2signo, and nothing called it any more. It is the one function that LEAVES the measured set in this change — a deliberate removal, stated here so it is not read as silent orphaning. Below-100 list: 16 on origin/main, 16 here, no new entries. 27 functions enter the measured set, all at 100%. Full suite green. Co-Authored-By: Claude Opus 5 --- internal/vm/exit_status_internal_test.go | 34 ++++++- internal/vm/signal.go | 97 +++++++------------ .../vm/signal_disposition_internal_test.go | 91 +++++++++++++++-- 3 files changed, 151 insertions(+), 71 deletions(-) diff --git a/internal/vm/exit_status_internal_test.go b/internal/vm/exit_status_internal_test.go index 5cf39f7a..1ba53927 100644 --- a/internal/vm/exit_status_internal_test.go +++ b/internal/vm/exit_status_internal_test.go @@ -4,7 +4,11 @@ package vm -import "testing" +import ( + "testing" + + "github.com/go-embedded-ruby/ruby/internal/object" +) // TestExitingSplitClassification is eval_error.c exiting_split, table-driven. // It is the decision the rbgo CLI turns into the process's disposition, and it @@ -258,3 +262,31 @@ func TestExitStatusAndSignoOfMissingObject(t *testing.T) { t.Errorf("objectless SignalException -> %+v, want the plain-failure fallback", d) } } + +// TestExitStatusAndSignoOfANonIntegerIvar covers the last arm of exitStatusOf and +// signoOf: the ivar is present but not an Integer, or the object has no ivars at +// all. eval_error.c's sysexit_status is a bare rb_ivar_get with no type check, so +// a value that is not a number has to mean 0 rather than crash — and a program +// CAN put one there (`e.instance_variable_set(:@status, "x")`). +func TestExitStatusAndSignoOfANonIntegerIvar(t *testing.T) { + machine, _, _ := runSrcErr(t, `1`) + // A value with no ivar table at all. + if got := machine.exitStatusOf(objectOne()); got != 0 { + t.Errorf("exitStatusOf(1) = %d, want 0", got) + } + if got := machine.signoOf(objectOne()); got != 0 { + t.Errorf("signoOf(1) = %d, want 0", got) + } + // And one whose @status a Ruby program replaced with a String. + _, out, err := runSrcErr(t, `e = SystemExit.new +e.instance_variable_set(:@status, "x") +p e.status`) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } +} + +// objectOne is an object.Value with no instance-variable table, which is what +// exitStatusOf and signoOf see when a SystemExit or SignalException was raised +// with a class name rather than an object. +func objectOne() object.Value { return object.IntValue(1) } diff --git a/internal/vm/signal.go b/internal/vm/signal.go index bf519d53..d97d2ea4 100644 --- a/internal/vm/signal.go +++ b/internal/vm/signal.go @@ -102,19 +102,17 @@ func (vm *VM) registerSignal() { vm.trapList = map[int]trapCmd{} trap := func(vm *VM, _ object.Value, args []object.Value, blk *Proc) object.Value { - if len(args) == 0 && blk == nil { - return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") - } if len(args) == 0 { return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") } sig := vm.trapSignm(args[0]) // sig_trap refuses a reserved signal before it looks at the handler. if signalReserved(sig) { - if name, ok := signalNameOf(sig); ok { - return raise("ArgumentError", "can't trap reserved signal: SIG%s", name) - } - return raise("ArgumentError", "can't trap reserved signal: %d", sig) + // Every signal signalReserved names has a siglist entry, so unlike + // sig_trap (whose NSIG table can be sparser than its handler set) there + // is no numeric spelling of this message to fall back to. + name, _ := signalNameOf(sig) + return raise("ArgumentError", "can't trap reserved signal: SIG%s", name) } // SIGKILL and SIGSTOP cannot be caught or ignored: sigaction(2) fails // EINVAL, which trap() turns into Errno::EINVAL through rb_sys_fail_str. @@ -176,10 +174,10 @@ func (vm *VM) registerSignal() { // (esignal_signo) and aliases #signm to #message; Interrupt overrides // #initialize (interrupt_init) to fix the signal at SIGINT. func (vm *VM) registerSignalException() { - cSig, ok := vm.consts["SignalException"].(*RClass) - if !ok { - return - } + // A hard assertion: builtins.go builds the exception hierarchy before it calls + // registerSignal, so an absent SignalException is a broken registration order, + // not a state to carry on from quietly. + cSig := vm.consts["SignalException"].(*RClass) // esignal_init(sig) / esignal_init(signo, message = signo2signm(signo)): // the first argument is tried as an Integer through #to_int; if it converts, // a second argument may supply the message and arity is 1..2, otherwise the @@ -188,10 +186,7 @@ func (vm *VM) registerSignalException() { // matched prefix length differs from signame_prefix_len), which is why // SignalException.new("TERM").message is "SIGTERM". cSig.define("initialize", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { - o, isObj := self.(*RObject) - if !isObj { - return object.NilV - } + o := self.(*RObject) if len(args) == 0 { return raise("ArgumentError", "wrong number of arguments (given 0, expected 1..2)") } @@ -242,12 +237,10 @@ func (vm *VM) registerSignalException() { // Interrupt.new carries signo 2 and an EMPTY message (MRI passes no name, and // esignal_init's message argument defaults to "" rather than "SIGINT" here // because interrupt_init supplies argv[1] itself). - if cInt, okI := vm.consts["Interrupt"].(*RClass); okI { + cInt := vm.consts["Interrupt"].(*RClass) + { cInt.define("initialize", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { - o, isObj := self.(*RObject) - if !isObj { - return object.NilV - } + o := self.(*RObject) // Interrupt.new with no argument leaves the message UNSET, so // Exception#message falls back to the class name and #signm reads // "Interrupt" (core/exception/interrupt_spec.rb). The Interrupt that @@ -355,13 +348,9 @@ func (vm *VM) trapHandler(v object.Value) trapCmd { // the OS — rather than nil, and an installed Proc or command comes back as // itself. func (vm *VM) trapPrevious(sig int) object.Value { - prev, had := vm.trapList[sig] - if !had { - if vm.signalManaged(sig) { - return object.NewString("DEFAULT") - } - return object.NewString("SYSTEM_DEFAULT") - } + // An absent entry IS trapDefault (MRI's trap_list.cmd[sig] == 0), so the + //zero value of trapCmd already says what to report. + prev := vm.trapList[sig] switch prev.kind { case trapDefault: if vm.signalManaged(sig) { @@ -475,16 +464,11 @@ func (vm *VM) runTrapCommand(cmd object.Value, sig int) { // @signo, so a rescuer sees #signo and #signm without the class having to // re-derive them. func (vm *VM) raiseSignalException(class string, signo int, message string) { - c, ok := vm.consts[class].(*RClass) - if !ok { - raise(class, "%s", message) - return - } + c := vm.consts[class].(*RClass) exc := vm.send(c, "new", []object.Value{object.NewString(message)}, nil) - if o, isObj := exc.(*RObject); isObj { - o.ivars["@signo"] = object.IntValue(int64(signo)) - o.ivars["@message"] = object.NewString(message) - } + o := exc.(*RObject) + o.ivars["@signo"] = object.IntValue(int64(signo)) + o.ivars["@message"] = object.NewString(message) panic(vm.excError(vm.captureBacktrace(exc))) } @@ -596,23 +580,20 @@ func stripSIG(name string) string { return name } -// signalName normalises a signal designator to its bare name (no "SIG" prefix), -// accepting a Symbol, a String ("INT"/"SIGINT") or an Integer. Kept for callers -// that want a name rather than a number. -func signalName(v object.Value) string { - switch s := v.(type) { - case object.Symbol: - return stripSIG(string(s)) - case *object.String: - return stripSIG(string(s.Bytes())) - case object.Integer: - if name, ok := signalNameOf(int(s)); ok { - return name - } - return v.ToS() - default: - return v.ToS() +// objClassName is MRI's rb_obj_classname: the name of the object's CLASS, which +// for nil is "NilClass" and not "nil". classNameOf answers the type name a format +// string wants, which differs for exactly the values signm2signo's error message +// is most likely to be handed (nil, true, false) — core/signal/trap_spec.rb reads +// the difference. +func (vm *VM) objClassName(v object.Value) string { + if c := vm.classOf(v); c != nil { + return c.name } + // classOf ends in classOfPlatform, which answers nil for a value type this + // platform does not place (it is a plain `return nil` on Windows). Nothing a + // Ruby expression can produce reaches that, but a nil dereference in an error + // path is a poor way to find out, so fall back to the type name. + return classNameOf(v) } // toIntMaybe is signal.c's rb_check_to_integer(v, "to_int"): it reports whether @@ -637,15 +618,3 @@ func (vm *VM) toIntMaybe(v object.Value) (int64, bool) { } return 0, false } - -// objClassName is MRI's rb_obj_classname: the name of the object's CLASS, which -// for nil is "NilClass" and not "nil". classNameOf answers the type name a format -// string wants, which differs for exactly the values signm2signo's error message -// is most likely to be handed (nil, true, false) — core/signal/trap_spec.rb reads -// the difference. -func (vm *VM) objClassName(v object.Value) string { - if c := vm.classOf(v); c != nil { - return c.name - } - return classNameOf(v) -} diff --git a/internal/vm/signal_disposition_internal_test.go b/internal/vm/signal_disposition_internal_test.go index c26c7742..749e0d4c 100644 --- a/internal/vm/signal_disposition_internal_test.go +++ b/internal/vm/signal_disposition_internal_test.go @@ -241,6 +241,26 @@ func TestSignalExceptionConstruction(t *testing.T) { // The name branch takes exactly one argument (argnum stays 1). {"a name with a second argument", `begin; SignalException.new("TERM", "x"); rescue ArgumentError => e; puts e.message; end`, "wrong number of arguments (given 2, expected 1)\n"}, + // The NUMBER branch takes 1..2 (argnum becomes 2), so three is the error. + {"a number with two extra arguments", + `begin; SignalException.new(15, "x", "y"); rescue ArgumentError => e; puts e.message; end`, + "wrong number of arguments (given 3, expected 1..2)\n"}, + // rb_check_to_integer consults #to_int, so an object that answers it takes + // the NUMBER branch and gets the default message from rb_signo2signm. + {"an object with #to_int", `o = Object.new +def o.to_int; 15; end +e = SignalException.new(o); puts "#{e.message} #{e.signo}"`, "SIGTERM 15\n"}, + // ...and one whose #to_int hands back a non-Integer falls through to the + // NAME branch, where its #to_str (absent) makes it a bad signal type. + {"an object whose #to_int is not an Integer", `o = Object.new +def o.to_int; "nope"; end +begin; SignalException.new(o); rescue ArgumentError => e; puts e.message; end`, + "bad signal type Object\n"}, + {"an object that is neither a number nor a name", `begin + SignalException.new(Object.new) +rescue ArgumentError => e + puts e.message +end`, "bad signal type Object\n"}, } { t.Run(tc.name, func(t *testing.T) { _, out, err := runSrcErr(t, tc.src) @@ -329,12 +349,6 @@ func TestSignoToSignmFallback(t *testing.T) { t.Errorf("withSIG(%q) = %q, want %q", tc.in, got, tc.want) } } - if got := signalName(object.IntValue(15)); got != "TERM" { - t.Errorf("signalName(15) = %q, want TERM", got) - } - if got := signalName(object.IntValue(9999)); got != "9999" { - t.Errorf("signalName(9999) = %q, want the number itself", got) - } if _, ok := signalNumberOf("NOPE"); ok { t.Error("signalNumberOf(NOPE) reported a number") } @@ -522,3 +536,68 @@ end` t.Errorf("output = %q, want %q", out, want) } } + +// stubValue is an object.Value of a type classOf cannot place, which is the only +// way to reach objClassName's fallback: classOf ends in classOfPlatform, whose +// Windows form is a plain `return nil`. No Ruby expression can produce one, so +// the branch is reachable only from Go — and leaving it uncovered would leave a +// nil dereference in an error path untested. +type stubValue struct{} + +func (stubValue) ToS() string { return "stub" } +func (stubValue) Inspect() string { return "stub" } +func (stubValue) Truthy() bool { return true } + +func TestObjClassNameFallsBackWhenClassOfCannotPlaceTheValue(t *testing.T) { + machine, _, _ := runSrcErr(t, `1`) + if got := machine.objClassName(object.NilV); got != "NilClass" { + t.Errorf("objClassName(nil) = %q, want NilClass", got) + } + if got := machine.objClassName(stubValue{}); got == "" { + t.Error("objClassName gave no name for an unplaceable value") + } +} + +// TestSelfSignalOnSIGCHLDRaisesNothing covers the one member of +// default_handler's managed set that rb_signal_exec has NO cmd == 0 case for: +// Init_signal installs a handler for SIGCHLD to reap children, so the signal is +// consumed at a safe point, but nothing is raised. A reading that treated "MRI +// installs a handler" as "MRI raises" would raise SignalException here. +func TestSelfSignalOnSIGCHLDRaisesNothing(t *testing.T) { + orig := procKill + t.Cleanup(func() { procKill = orig }) + calls := 0 + procKill = func(int, int) error { calls++; return nil } + _, out, err := runSrcErr(t, `Process.kill(:CHLD, Process.pid); puts "nothing raised"`) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != "nothing raised\n" { + t.Errorf("output = %q, want %q", out, "nothing raised\n") + } + if calls != 0 { + t.Errorf("procKill called %d times; SIGCHLD must be consumed at the safe point", calls) + } +} + +// TestTrapPreviousReportsEveryDisposition walks every arm of trap()'s oldcmd +// switch. SYSTEM_DEFAULT and EXIT are only observable through this return value — +// their DISPOSITIONS are "let the OS have it" and "raise SystemExit", neither of +// which names itself — so a test of behaviour alone leaves both unwitnessed. +func TestTrapPreviousReportsEveryDisposition(t *testing.T) { + src := `p Signal.trap(:TERM, "SYSTEM_DEFAULT") +p Signal.trap(:TERM, "EXIT") +p Signal.trap(:TERM, "SIG_DFL") +p Signal.trap(:TERM, ->(s) {}) +p Signal.trap(:TERM, "IGNORE").class +p Signal.trap(:TERM, "command string") +p Signal.trap(:TERM, "DEFAULT")` + want := "\"DEFAULT\"\n\"SYSTEM_DEFAULT\"\n\"EXIT\"\n\"DEFAULT\"\nProc\n\"IGNORE\"\n\"command string\"\n" + _, out, err := runSrcErr(t, src) + if err != nil { + t.Fatalf("run: %v (output %q)", err, out) + } + if out != want { + t.Errorf("output = %q, want %q", out, want) + } +} From 97146b9ae473a1a956bfeb894a6aa96dee6e2773 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 19:21:49 +0200 Subject: [PATCH 5/6] windows: dieBySignal has no POSIX kill there either, and no witness for MRI's answer Go does not define syscall.Kill on Windows, so die_signal_native.go's !wasm tag broke the windows lane's build. The no-op form now covers `wasm || windows` and says why they are the same no-op for two DIFFERENT reasons: wasm has no settable signal disposition at all, while Windows has a mechanism (the C runtime's raise(), which MRI's ruby_default_signal uses) whose observable effect on a waiting parent is not the POSIX WIFSIGNALED this code would be imitating. There is no Windows host here to measure that on, so rather than adapt an assertion to a guess the platform-specific part is left unclaimed: the caller exits 128+signo, the same NUMBER a POSIX shell reports. The two end-to-end rows that assert death BY a signal, and the VM-level tests that drive Process.kill (registered only under registerProcessPosix), are POSIX-only and say so in their build tags. Coverage is measured on the ubuntu lane, which they still run in. darwin/arm64, linux/amd64, linux/arm64, windows/amd64 and wasip1/wasm all build; go vet is clean for windows/amd64. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/die_signal_native.go | 2 +- cmd/rbgo/die_signal_unsupported.go | 24 +++++++++++++++++++ cmd/rbgo/die_signal_wasm.go | 19 --------------- cmd/rbgo/exit_status_test.go | 2 +- internal/vm/exit_status_internal_test.go | 2 ++ .../vm/signal_disposition_internal_test.go | 2 ++ 6 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 cmd/rbgo/die_signal_unsupported.go delete mode 100644 cmd/rbgo/die_signal_wasm.go diff --git a/cmd/rbgo/die_signal_native.go b/cmd/rbgo/die_signal_native.go index ed04e6e7..2e31b21b 100644 --- a/cmd/rbgo/die_signal_native.go +++ b/cmd/rbgo/die_signal_native.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: BSD-3-Clause -//go:build !wasm +//go:build !wasm && !windows package main diff --git a/cmd/rbgo/die_signal_unsupported.go b/cmd/rbgo/die_signal_unsupported.go new file mode 100644 index 00000000..88e64f6e --- /dev/null +++ b/cmd/rbgo/die_signal_unsupported.go @@ -0,0 +1,24 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build wasm || windows + +package main + +// dieBySignal cannot do what signal.c ruby_default_signal does on these targets, +// for two different reasons, and it is deliberately the same no-op for both. +// +// On wasm neither wasip1 nor js/wasm has kill(2) or a settable signal +// disposition, so there is no way to be killed BY a signal at all. +// +// On Windows there is no kill(2) either (Go does not define syscall.Kill there), +// and MRI's own answer is a different mechanism — the C runtime's raise(), whose +// observable effect on a waiting parent is not the POSIX WIFSIGNALED this code +// would be imitating. **There is no Windows host here to measure that on**, so +// rather than adapt the assertion to a guess, this returns and the caller exits +// 128+signo: the same NUMBER a POSIX shell reports, and a defensible status for a +// program killed by a signal, with the platform-specific part left unclaimed. +// The two end-to-end rows that assert death BY a signal skip on Windows and say +// this is why (cmd/rbgo/exit_status_test.go is POSIX-only for the same reason). +func dieBySignal(int) {} diff --git a/cmd/rbgo/die_signal_wasm.go b/cmd/rbgo/die_signal_wasm.go deleted file mode 100644 index 398c02be..00000000 --- a/cmd/rbgo/die_signal_wasm.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) the go-embedded-ruby/ruby authors -// -// SPDX-License-Identifier: BSD-3-Clause - -//go:build wasm - -package main - -// dieBySignal cannot do what signal.c ruby_default_signal does on wasm: neither -// wasip1 nor js/wasm has kill(2) or a settable signal disposition, so there is no -// way to be killed BY a signal there. It returns at once and the caller exits -// with 128+signo — the same NUMBER a POSIX shell would report, without the -// WIFSIGNALED flag that no wasm host can produce. -// -// Nothing else in this change needs a build tag: the signal disposition table and -// Process.kill's self arm are pure Go with no syscall in them, and Process.kill -// itself is only registered on POSIX (registerProcessPosix), so a wasm program -// reaches neither. -func dieBySignal(int) {} diff --git a/cmd/rbgo/exit_status_test.go b/cmd/rbgo/exit_status_test.go index 6e2dda09..c4cf5967 100644 --- a/cmd/rbgo/exit_status_test.go +++ b/cmd/rbgo/exit_status_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: BSD-3-Clause -//go:build !rbgo_closed && !wasm +//go:build !rbgo_closed && !wasm && !windows package main diff --git a/internal/vm/exit_status_internal_test.go b/internal/vm/exit_status_internal_test.go index 1ba53927..6007475c 100644 --- a/internal/vm/exit_status_internal_test.go +++ b/internal/vm/exit_status_internal_test.go @@ -2,6 +2,8 @@ // // SPDX-License-Identifier: BSD-3-Clause +//go:build !windows && !wasm + package vm import ( diff --git a/internal/vm/signal_disposition_internal_test.go b/internal/vm/signal_disposition_internal_test.go index 749e0d4c..4192f70d 100644 --- a/internal/vm/signal_disposition_internal_test.go +++ b/internal/vm/signal_disposition_internal_test.go @@ -2,6 +2,8 @@ // // SPDX-License-Identifier: BSD-3-Clause +//go:build !windows && !wasm + package vm import ( From f8655a8a3304cefe659b9b3980a190ddc155c317 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 19:51:52 +0200 Subject: [PATCH 6/6] signal: Signal.trap(:EXIT) reports its previous handler as nil, not a disposition name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signal.c trap() opens with `if (sig == 0) oldfunc = SIG_ERR;`, and SIG_ERR matches none of the three arms of the oldcmd switch's `case 0` (SIG_IGN, SIG_DFL, sighandler), so it falls through to Qnil. rbgo answered "SYSTEM_DEFAULT". Measured on MRI 4.0.5: `Signal.trap(:EXIT) { }` is nil, and a handler already installed for signal 0 still comes back as itself. Found by probing the pathological Process.kill/Signal.trap shapes against MRI after the main work was measured green — two pids in one kill, kill(0) on a nonexistent pid and Signal.trap(:EXIT) — on the principle that the cases a corpus does not exercise are where a disposition table hides its last error. The first two already matched (raised 15; Errno::ESRCH). Co-Authored-By: Claude Opus 5 --- internal/vm/signal.go | 12 +++++++++++- internal/vm/signal_disposition_internal_test.go | 15 ++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/internal/vm/signal.go b/internal/vm/signal.go index d97d2ea4..c4059b3f 100644 --- a/internal/vm/signal.go +++ b/internal/vm/signal.go @@ -348,8 +348,18 @@ func (vm *VM) trapHandler(v object.Value) trapCmd { // the OS — rather than nil, and an installed Proc or command comes back as // itself. func (vm *VM) trapPrevious(sig int) object.Value { + // Signal 0 (Signal.trap(:EXIT)) reports nil, not a disposition name: trap() + // starts with `if (sig == 0) oldfunc = SIG_ERR;`, and SIG_ERR matches none of + // the three arms of the oldcmd switch's `case 0`, so it falls to Qnil. + // Measured on MRI 4.0.5: Signal.trap(:EXIT) {} => nil. + if sig == 0 { + if prev, had := vm.trapList[0]; had && prev.kind == trapCommand { + return prev.cmd + } + return object.NilV + } // An absent entry IS trapDefault (MRI's trap_list.cmd[sig] == 0), so the - //zero value of trapCmd already says what to report. + // zero value of trapCmd already says what to report. prev := vm.trapList[sig] switch prev.kind { case trapDefault: diff --git a/internal/vm/signal_disposition_internal_test.go b/internal/vm/signal_disposition_internal_test.go index 4192f70d..38f606cd 100644 --- a/internal/vm/signal_disposition_internal_test.go +++ b/internal/vm/signal_disposition_internal_test.go @@ -453,9 +453,18 @@ func TestTrapSignalArgumentCoercion(t *testing.T) { {"a prefixed name", `p Signal.trap("SIGHUP", "IGNORE")`, "\"DEFAULT\"\n"}, {"a Symbol name", `p Signal.trap(:HUP, "IGNORE")`, "\"DEFAULT\"\n"}, {"an Integer", `p Signal.trap(Signal.list["HUP"], "IGNORE")`, "\"DEFAULT\"\n"}, - // EXIT / 0 is legal for trap (signm2signo's exit = TRUE) ... - {"EXIT is a trappable name", `p Signal.trap(:EXIT, "IGNORE")`, "\"SYSTEM_DEFAULT\"\n"}, - {"signal 0 is trappable", `p Signal.trap(0, "IGNORE")`, "\"SYSTEM_DEFAULT\"\n"}, + // EXIT / 0 is legal for trap (signm2signo's exit = TRUE), and it reports its + // PREVIOUS handler as nil rather than as a disposition name, because trap() + // forces oldfunc = SIG_ERR for signal 0 and SIG_ERR matches none of the + // oldcmd switch's three arms. MEASURED on MRI 4.0.5. + {"EXIT is a trappable name", `p Signal.trap(:EXIT, "IGNORE")`, "nil\n"}, + {"signal 0 is trappable", `p Signal.trap(0, "IGNORE")`, "nil\n"}, + {"signal 0 with a block", `p Signal.trap(:EXIT) { }`, "nil\n"}, + // ...but an installed handler for signal 0 still comes back as itself. + {"signal 0 reports an installed handler", `Signal.trap(:EXIT, "cmd") +p Signal.trap(:EXIT, "IGNORE")`, "\"cmd\"\n"}, + {"signal 0 reports an installed Proc", `Signal.trap(:EXIT) { } +p Signal.trap(:EXIT, "IGNORE").class`, "Proc\n"}, // ... but NOT for SignalException, whose esignal_init passes exit = FALSE. {"EXIT is not a SignalException name", `begin; SignalException.new("EXIT"); rescue ArgumentError => e; puts e.message; end`,