fix(cmd): usage exit codes, asm output file and cross-arch ground truth

Assisted-by: GLM 5.3

cmd/gasm/audit.go

@@ -63,7 +63,7 @@ the encodability backlog by frequency rather than by table order.
 	archName := "amd64"
 	switch n := len(fs.Args()); {
 	case n > 1:
-		return fmt.Errorf("audit-instructions takes at most one architecture argument")
+		return &usageError{fmt.Errorf("audit-instructions takes at most one architecture argument")}
 	case n == 1:
 		archName = strings.ToLower(fs.Arg(0))
 	}
@@ -136,7 +136,7 @@ func auditArch(name string) (arch.Arch, error) {
 	case "loong64", "loong":
 		return arch.LOONG64, nil
 	}
-	return arch.Unknown, fmt.Errorf("unknown architecture %q: want amd64, arm64, riscv64 or loong64", name)
+	return arch.Unknown, &usageError{fmt.Errorf("unknown architecture %q: want amd64, arm64, riscv64 or loong64", name)}
 }
 
 // goarchName maps an arch identifier onto its GOARCH spelling.
@@ -217,6 +217,13 @@ func probeGoAsm(goarch string, names []string) (map[string]bool, error) {
 	cmd := exec.Command(asmBin, "-p", "probe", "-o", filepath.Join(dir, "probe.o"), probePath)
 	cmd.Env = append(os.Environ(), "GOARCH="+goarch, "GOOS="+runtime.GOOS)
 	out, _ := cmd.CombinedOutput()
+	// The expected failure mode is a non-zero exit with compiler diagnostics
+	// on stdout; empty output means the probe broke at the exec level (a
+	// killed child, a tool that would not start), and seeding every name as
+	// recognized on that silence would fake a clean audit.
+	if len(out) == 0 {
+		return nil, fmt.Errorf("go tool asm probe for GOARCH=%s produced no output", goarch)
+	}
 
 	result := map[string]bool{}
 	for _, name := range names {
@@ -344,7 +351,7 @@ func (t *corpusTally) fail(path, reason string) {
 // cmdAuditCorpus implements audit-instructions --corpus.
 func cmdAuditCorpus(args []string) error {
 	if len(args) > 1 {
-		return fmt.Errorf("audit-instructions --corpus takes at most one directory argument")
+		return &usageError{fmt.Errorf("audit-instructions --corpus takes at most one directory argument")}
 	}
 	root := ""
 	if len(args) == 1 {

cmd/gasm/main.go

@@ -10,6 +10,7 @@ package main
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -48,6 +49,22 @@ func version() string {
 	return bi.Main.Version
 }
 
+// usageError marks an error the caller's arguments caused, which exits 2
+// instead of the 1 a runtime failure gets.
+type usageError struct{ err error }
+
+func (e *usageError) Error() string { return e.err.Error() }
+func (e *usageError) Unwrap() error { return e.err }
+
+// exitCodeFor maps an error onto the process exit status: 2 for a usage
+// error, 1 for anything else.
+func exitCodeFor(err error) int {
+	if _, ok := errors.AsType[*usageError](err); ok {
+		return 2
+	}
+	return 1
+}
+
 func main() {
 	if len(os.Args) < 2 {
 		usage(os.Stderr)
@@ -77,12 +94,12 @@ func main() {
 	case "audit-instructions":
 		if err := cmdAuditInstructions(os.Args[2:]); err != nil {
 			fmt.Fprintln(os.Stderr, err)
-			os.Exit(1)
+			os.Exit(exitCodeFor(err))
 		}
 	case "scaffold":
 		if err := cmdScaffold(os.Args[2:]); err != nil {
 			fmt.Fprintln(os.Stderr, err)
-			os.Exit(1)
+			os.Exit(exitCodeFor(err))
 		}
 	case "lsp":
 		os.Exit(cmdLSP(os.Args[2:]))
@@ -104,11 +121,11 @@ func cmdVersion() int {
 
 // ANSI colour helpers for terminal output.
 const (
-	colorReset  = "\033[0m"
-	colorBold   = "\033[1m"
-	colorCyan   = "\033[36m"
-	colorYellow = "\033[33m"
-	colorGray   = "\033[90m"
+	colourReset  = "\033[0m"
+	colourBold   = "\033[1m"
+	colourCyan   = "\033[36m"
+	colourYellow = "\033[33m"
+	colourGrey   = "\033[90m"
 )
 
 // isTTY reports whether the writer is a terminal (for colour output).
@@ -122,9 +139,9 @@ func isTTY(w io.Writer) bool {
 
 func usage(w io.Writer) {
 	useColor := isTTY(w)
-	bold, cyan, yellow, gray, reset := "", "", "", "", ""
+	bold, cyan, yellow, grey, reset := "", "", "", "", ""
 	if useColor {
-		bold, cyan, yellow, gray, reset = colorBold, colorCyan, colorYellow, colorGray, colorReset
+		bold, cyan, yellow, grey, reset = colourBold, colourCyan, colourYellow, colourGrey, colourReset
 	}
 
 	fmt.Fprintf(w, "%sgasm %s%s: developer tooling for Go's Plan 9 assembler (GAsm)%s\n\n", bold, version(), reset, reset)
@@ -153,12 +170,12 @@ func usage(w io.Writer) {
 		{"version", "print the version (same as --version)"},
 	}
 	for _, c := range commands {
-		fmt.Fprintf(w, "  %s%-10s%s %s%s%s\n", cyan, c.name, reset, gray, c.desc, reset)
+		fmt.Fprintf(w, "  %s%-10s%s %s%s%s\n", cyan, c.name, reset, grey, c.desc, reset)
 	}
 
 	fmt.Fprintf(w, "\n%sFlags:%s\n", yellow, reset)
-	fmt.Fprintf(w, "  %s-h, --help%s      %sshow this help%s\n", cyan, reset, gray, reset)
-	fmt.Fprintf(w, "  %s-V, --version%s   %sprint the version%s\n", cyan, reset, gray, reset)
+	fmt.Fprintf(w, "  %s-h, --help%s      %sshow this help%s\n", cyan, reset, grey, reset)
+	fmt.Fprintf(w, "  %s-V, --version%s   %sprint the version%s\n", cyan, reset, grey, reset)
 
 	fmt.Fprintf(w, "\nRun \"gasm <command> -h\" for a command's usage and flags.\n\n")
 
@@ -172,7 +189,7 @@ func usage(w io.Writer) {
 	}
 	for _, e := range examples {
 		if e.desc != "" {
-			fmt.Fprintf(w, "  %s%s%s  %s%s%s\n", cyan, e.cmd, reset, gray, e.desc, reset)
+			fmt.Fprintf(w, "  %s%s%s  %s%s%s\n", cyan, e.cmd, reset, grey, e.desc, reset)
 		} else {
 			fmt.Fprintf(w, "  %s%s%s\n", cyan, e.cmd, reset)
 		}
@@ -483,6 +500,14 @@ requires -p, the package path, and the installed Go toolchain).
 		fmt.Fprintln(os.Stderr, "usage: gasm asm [--format raw|elf|goobj] [-p pkg] [-GOARCH arch] [-o out] <file>")
 		return 2
 	}
+	// The format is validated before anything else, so a bogus value exits 2
+	// with or without -o instead of silently dumping the hex of a raw image.
+	switch *format {
+	case "raw", "elf", "goobj":
+	default:
+		fmt.Fprintf(os.Stderr, "gasm asm: unknown format %q (want raw, elf or goobj)\n", *format)
+		return 2
+	}
 	path := fs.Arg(0)
 	targetArch := arch.FromFilename(path)
 	if *archName != "" {
@@ -517,6 +542,9 @@ requires -p, the package path, and the installed Go toolchain).
 		fmt.Fprintln(os.Stderr, "gasm asm: no assemblable TEXT functions or GLOBL data found")
 		return 1
 	}
+	// Without -o the hex dump on stdout is the output; with -o the file is,
+	// and the dump is skipped, as the -o help text promises.
+	if *out == "" {
 		for _, fn := range img.Funcs {
 			code := img.Code[fn.Offset : fn.Offset+fn.Size]
 			fmt.Printf("%s: %d bytes\n", fn.Name, fn.Size)
@@ -551,6 +579,7 @@ requires -p, the package path, and the installed Go toolchain).
 				fmt.Println()
 			}
 		}
+	}
 	if *out != "" {
 		var obj []byte
 		var err error
@@ -586,9 +615,6 @@ requires -p, the package path, and the installed Go toolchain).
 				obj, err = img.GOObject(*pkg, path)
 			}
 			kind = "Go object"
-		default:
-			fmt.Fprintf(os.Stderr, "gasm asm: unknown format %q (want raw, elf or goobj)\n", *format)
-			return 2
 		}
 		if err != nil {
 			fmt.Fprintln(os.Stderr, "gasm asm:", err)
@@ -1012,15 +1038,14 @@ each entry reproduces.
 	// under the available loong64 emulators), so those kernels take the
 	// toolchain-comparison path.
 	if targetArch != hostArch() || targetArch == arch.LOONG64 {
-		// No JIT on this host: ground truth and profile remain available.
-		// (loong64 is ground-truth-only everywhere for now: its trampoline
-		// is implemented but not yet validated against real hardware.)
+		// No JIT on this host: ground truth and profile remain available for
+		// every architecture, because cmdVerifyNonJIT assembles and compares
+		// against the toolchain without executing anything.  (loong64 is
+		// ground-truth-only everywhere for now: its trampoline is implemented
+		// but not yet validated against real hardware.)
 		switch targetArch {
-		case arch.RISCV, arch.LOONG64, arch.ARM64:
+		case arch.AMD64, arch.RISCV, arch.LOONG64, arch.ARM64:
 			return cmdVerifyNonJIT(path, targetArch, *groundTruth, *profile)
-		case arch.AMD64:
-			fmt.Fprintln(os.Stderr, "gasm verify: JIT-based checks need an amd64 host; use --ground-truth here")
-			return 1
 		default:
 			fmt.Fprintln(os.Stderr, "gasm verify: unsupported architecture")
 			return 1

cmd/gasm/main_test.go

@@ -13,6 +13,8 @@ import (
 	"strings"
 	"syscall"
 	"testing"
+
+	"sourcedock.dev/petrbalvin/gasm-devkit/arch"
 )
 
 const clean = "#include \"textflag.h\"\n" +
@@ -242,6 +244,96 @@ func TestCmdArgErrors(t *testing.T) {
 	}
 }
 
+// TestUsageExitCodes pins the exit-code contract for the commands whose main
+// dispatches on a returned error: a wrong argument set exits 2, the same as
+// the commands that count their arguments themselves, while a runtime
+// failure (an unreadable file) keeps exit 1.
+func TestUsageExitCodes(t *testing.T) {
+	for name, err := range map[string]error{
+		"audit-instructions extra argument": cmdAuditInstructions([]string{"amd64", "extra"}),
+		"audit-instructions unknown arch":   cmdAuditInstructions([]string{"mips"}),
+		"audit-instructions corpus extra":   cmdAuditInstructions([]string{"--corpus", "a", "b"}),
+		"scaffold no arguments":             cmdScaffold(nil),
+		"scaffold extra arguments":          cmdScaffold([]string{"differential", "a.s", "b.s"}),
+	} {
+		if err == nil {
+			t.Errorf("%s: expected an error", name)
+			continue
+		}
+		if code := exitCodeFor(err); code != 2 {
+			t.Errorf("%s: exit code = %d, want 2 (err: %v)", name, code, err)
+		}
+	}
+	if err := cmdScaffold([]string{"differential", "/nonexistent/file.s"}); err == nil {
+		t.Error("scaffold on a missing file should fail")
+	} else if code := exitCodeFor(err); code != 1 {
+		t.Errorf("scaffold on a missing file: exit code = %d, want 1", code)
+	}
+}
+
+// TestCmdAsmFormatValidation checks that an unknown --format exits 2 with
+// and without -o, instead of assembling and silently dumping a raw image.
+func TestCmdAsmFormatValidation(t *testing.T) {
+	path := writeTemp(t, "f_amd64.s", clean)
+	out := filepath.Join(t.TempDir(), "f.bin")
+	if _, _, code := capture(func() int { return cmdAsm([]string{"--format", "bogus", path}) }); code != 2 {
+		t.Errorf("asm --format bogus without -o: code = %d, want 2", code)
+	}
+	if _, _, code := capture(func() int { return cmdAsm([]string{"--format", "bogus", "-o", out, path}) }); code != 2 {
+		t.Errorf("asm --format bogus with -o: code = %d, want 2", code)
+	}
+}
+
+// TestCmdAsmOutputFile pins the documented -o behaviour: the output goes to
+// the file and stdout carries no hex dump; without -o the dump is the output.
+func TestCmdAsmOutputFile(t *testing.T) {
+	path := writeTemp(t, "f_amd64.s", clean)
+	out := filepath.Join(t.TempDir(), "f.bin")
+	stdout, _, code := capture(func() int { return cmdAsm([]string{"-o", out, path}) })
+	if code != 0 {
+		t.Fatalf("code = %d", code)
+	}
+	if strings.Contains(stdout, "0000:") {
+		t.Errorf("stdout carries a hex dump despite -o:\n%s", stdout)
+	}
+	if !strings.Contains(stdout, "wrote ") {
+		t.Errorf("stdout misses the wrote line:\n%s", stdout)
+	}
+	b, err := os.ReadFile(out)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(b) == 0 {
+		t.Error("the output file is empty")
+	}
+
+	stdout, _, code = capture(func() int { return cmdAsm([]string{path}) })
+	if code != 0 {
+		t.Fatalf("without -o: code = %d", code)
+	}
+	if !strings.Contains(stdout, "0000:") {
+		t.Errorf("without -o the hex dump is missing:\n%s", stdout)
+	}
+}
+
+// TestVerifyNonJITAMD64GroundTruth drives the cross-architecture
+// ground-truth path for an amd64 kernel: the path a host of any other
+// architecture takes, which must compare against the toolchain rather than
+// refuse to run.
+func TestVerifyNonJITAMD64GroundTruth(t *testing.T) {
+	if testing.Short() {
+		t.Skip("runs go tool asm")
+	}
+	path := writeTemp(t, "f_amd64.s", clean)
+	out, _, code := capture(func() int { return cmdVerifyNonJIT(path, arch.AMD64, true, false) })
+	if code != 0 {
+		t.Fatalf("code = %d (%s)", code, out)
+	}
+	if !strings.Contains(out, "1/1 matched") {
+		t.Errorf("output misses the matched report:\n%s", out)
+	}
+}
+
 // TestVerifySmokeCrashIsolation checks that a function faulting on its
 // zeroed smoke arguments is reported as CRASH by a child process instead of
 // killing `gasm verify` itself.

cmd/gasm/scaffold.go

@@ -44,7 +44,7 @@ bodies, place the file in the kernel's package, and run it in CI.
 		rest = rest[1:]
 	}
 	if len(rest) != 1 {
-		return fmt.Errorf("usage: gasm scaffold differential <file.s>")
+		return &usageError{fmt.Errorf("usage: gasm scaffold differential <file.s>")}
 	}
 	path := rest[0]
 	src, err := os.ReadFile(path)