From 0db9a3e1806b438620712399dd1e8dd0c29fad6f Mon Sep 17 00:00:00 2001 From: David Negstad Date: Mon, 21 Sep 2026 13:18:51 -0700 Subject: [PATCH 1/6] Apply Go runtime signal fix through build overlay Generate a targeted overlay from the selected Go toolchain using the upstream fix for golang/go#81009. Remove the DCP-specific exec shim and add a Darwin child-process regression test that proves the overlay is active. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Makefile | 82 +++++-- internal/dcp/commands/root.go | 6 - internal/dcpproc/commands/fork_process.go | 139 +----------- .../commands/fork_process_darwin_test.go | 133 ------------ .../dcpproc/commands/fork_process_exec.go | 135 ------------ .../dcpproc/commands/fork_process_test.go | 128 ----------- internal/dcpproc/commands/root.go | 6 - internal/dcpproc/fork_process_darwin_test.go | 113 ---------- .../tools/goruntimeoverlay/go-81009.patch | 17 ++ internal/tools/goruntimeoverlay/main.go | 204 ++++++++++++++++++ internal/tools/goruntimeoverlay/main_test.go | 147 +++++++++++++ pkg/process/os_executor_darwin_test.go | 47 ++++ pkg/process/signal_disposition_darwin.go | 152 ------------- pkg/process/signal_disposition_darwin_test.go | 171 --------------- pkg/process/signal_disposition_other.go | 36 ---- test/signaldisposition/main.go | 60 ++++++ 16 files changed, 535 insertions(+), 1041 deletions(-) delete mode 100644 internal/dcpproc/commands/fork_process_darwin_test.go delete mode 100644 internal/dcpproc/commands/fork_process_exec.go delete mode 100644 internal/dcpproc/commands/fork_process_test.go delete mode 100644 internal/dcpproc/fork_process_darwin_test.go create mode 100644 internal/tools/goruntimeoverlay/go-81009.patch create mode 100644 internal/tools/goruntimeoverlay/main.go create mode 100644 internal/tools/goruntimeoverlay/main_test.go create mode 100644 pkg/process/os_executor_darwin_test.go delete mode 100644 pkg/process/signal_disposition_darwin.go delete mode 100644 pkg/process/signal_disposition_darwin_test.go delete mode 100644 pkg/process/signal_disposition_other.go create mode 100644 test/signaldisposition/main.go diff --git a/Makefile b/Makefile index 0f990a25..310d42a0 100644 --- a/Makefile +++ b/Makefile @@ -116,8 +116,19 @@ PARROT_TOOL ?= $(TOOL_BIN)/parrot$(exe_suffix) PARROT_TOOL_CONTAINER_BINARY ?= $(TOOL_BIN)/parrot_c CONTAINER_PROBE_TOOL_CONTAINER_BINARY ?= $(TOOL_BIN)/container_probe_c TERMCHILD_TOOL ?= $(TOOL_BIN)/termchild$(exe_suffix) +SIGNAL_DISPOSITION_TOOL ?= $(TOOL_BIN)/signal-disposition$(exe_suffix) GO_LICENSES ?= $(TOOL_BIN)/go-licenses$(exe_suffix) PROTOC ?= $(TOOL_BIN)/protoc/bin/protoc$(exe_suffix) +GO_RUNTIME_OVERLAY_DIR ?= $(TOOL_BIN)/go-runtime-overlay +GO_RUNTIME_OVERLAY_FILE ?= $(GO_RUNTIME_OVERLAY_DIR)/overlay.json + +ifeq ($(build_os),darwin) + GO_RUNTIME_OVERLAY_ARG := -overlay="$(GO_RUNTIME_OVERLAY_FILE)" + GO_RUNTIME_OVERLAY_PREREQ := $(GO_RUNTIME_OVERLAY_FILE) +else + GO_RUNTIME_OVERLAY_ARG := + GO_RUNTIME_OVERLAY_PREREQ := +endif # Tool Versions PROTOC_VERSION ?= 33.5 @@ -167,6 +178,9 @@ help: ## Display this help. ##@ Code generation +.PHONY: generate-go-runtime-overlay +generate-go-runtime-overlay: $(GO_RUNTIME_OVERLAY_PREREQ) ## Generate the Darwin Go runtime overlay used by DCP builds + .PHONY: generate generate: generate-object-methods generate-openapi generate-goversioninfo generate-grpc ## Generate artifacts needed for DCP binary build: object copy methods, OpenAPI definitions, binary version info, and gRPC files. @@ -293,16 +307,16 @@ build-ci: generate-ci release ## Runs codegen, including license/notice files, t .PHONY: build-dcp build-dcp: $(DCP_BINARY) ## Builds DCP CLI binary -$(DCP_BINARY): $(GO_SOURCES) go.mod | ${OUTPUT_BIN} - $(GO_BIN) build -o $(DCP_BINARY) $(BUILD_ARGS) ./cmd/dcp +$(DCP_BINARY): $(GO_SOURCES) go.mod $(GO_RUNTIME_OVERLAY_PREREQ) | ${OUTPUT_BIN} + $(GO_BIN) build -o $(DCP_BINARY) $(GO_RUNTIME_OVERLAY_ARG) $(BUILD_ARGS) ./cmd/dcp .PHONY: build-dcptun-containerexe build-dcptun-containerexe: $(DCPTUN_CLIENT_BINARY) ## Builds DCP reverse network tunnel client binary for Linux (to be used in containers) -$(DCPTUN_CLIENT_BINARY): $(GO_SOURCES) go.mod | $(OUTPUT_BIN) +$(DCPTUN_CLIENT_BINARY): $(GO_SOURCES) go.mod $(GO_RUNTIME_OVERLAY_PREREQ) | $(OUTPUT_BIN) ifeq ($(detected_OS),windows) - $$env:GOOS = "linux"; $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(BUILD_ARGS) ./cmd/dcptun + $$env:GOOS = "linux"; $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(GO_RUNTIME_OVERLAY_ARG) $(BUILD_ARGS) ./cmd/dcptun else - GOOS=linux $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(BUILD_ARGS) ./cmd/dcptun + GOOS=linux $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(GO_RUNTIME_OVERLAY_ARG) $(BUILD_ARGS) ./cmd/dcptun endif .PHONY: clean @@ -344,6 +358,9 @@ TEST_PREREQS := generate-grpc .WAIT build-dcp build-dcptun-containerexe containe else TEST_PREREQS := generate-grpc build-dcp build-dcptun-containerexe container-probe-tool-containerexe delay-tool lfwriter-tool parrot-tool parrot-tool-containerexe termchild-tool endif +ifeq ($(build_os),darwin) +TEST_PREREQS := $(TEST_PREREQS) signal-disposition-tool +endif .PHONY: test-prereqs test-prereqs: BUILD_ARGS := $(BUILD_ARGS) -gcflags="all=-N -l" -ldflags "$(version_values)" @@ -361,13 +378,13 @@ TEST_OPTS := $(COMMON_TEST_OPTS) -race endif .PHONY: test -test: test-prereqs ## Run all tests in the repository - $(GO_BIN) test ./... $(TEST_OPTS) -parallel 32 +test: test-prereqs $(GO_RUNTIME_OVERLAY_PREREQ) ## Run all tests in the repository + $(GO_BIN) test ./... $(GO_RUNTIME_OVERLAY_ARG) $(TEST_OPTS) -parallel 32 # NOTE: Keep scripts/test-ci.ps1 in sync with test-ci (see comment above TEST_PREREQS). .PHONY: test-ci -test-ci: test-ci-prereqs ## Runs tests in a way appropriate for CI pipeline, with linting etc. - $(GO_BIN) test ./... $(TEST_OPTS) +test-ci: test-ci-prereqs $(GO_RUNTIME_OVERLAY_PREREQ) ## Runs tests in a way appropriate for CI pipeline, with linting etc. + $(GO_BIN) test ./... $(GO_RUNTIME_OVERLAY_ARG) $(TEST_OPTS) ## Development and test support targets @@ -381,6 +398,16 @@ ${OUTPUT_BIN}/ext/bin/: | ${OUTPUT_BIN} $(TOOL_BIN): $(mkdir) $(TOOL_BIN) +ifeq ($(build_os),darwin) +.PHONY: force-go-runtime-overlay +force-go-runtime-overlay: + +# Apply the upstream fix for golang/go#81009 to the selected toolchain without +# replacing unrelated standard-library source. +$(GO_RUNTIME_OVERLAY_FILE): force-go-runtime-overlay $(wildcard ./internal/tools/goruntimeoverlay/*) | $(TOOL_BIN) + $(CLEAR_GOARGS) $(GO_BIN) run ./internal/tools/goruntimeoverlay --output-dir "$(GO_RUNTIME_OVERLAY_DIR)" +endif + $(DCP_DIR): $(mkdir) $(DCP_DIR) @@ -405,45 +432,54 @@ endif # delay-tool is used for process package testing .PHONY: delay-tool delay-tool: $(DELAY_TOOL) -$(DELAY_TOOL): $(wildcard ./test/delay/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(DELAY_TOOL) github.com/microsoft/dcp/test/delay +$(DELAY_TOOL): $(wildcard ./test/delay/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(DELAY_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/delay # termchild-tool is used for internal/termpty pseudo-terminal tests .PHONY: termchild-tool termchild-tool: $(TERMCHILD_TOOL) -$(TERMCHILD_TOOL): $(wildcard ./test/termchild/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(TERMCHILD_TOOL) github.com/microsoft/dcp/test/termchild +$(TERMCHILD_TOOL): $(wildcard ./test/termchild/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(TERMCHILD_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/termchild + +# signal-disposition captures the signal state inherited by an exec'd child before +# its Go runtime initializes. +ifeq ($(build_os),darwin) +.PHONY: signal-disposition-tool +signal-disposition-tool: $(SIGNAL_DISPOSITION_TOOL) +$(SIGNAL_DISPOSITION_TOOL): $(wildcard ./test/signaldisposition/*.go) | $(TOOL_BIN) + CGO_ENABLED=1 $(GO_BIN) build -o $(SIGNAL_DISPOSITION_TOOL) github.com/microsoft/dcp/test/signaldisposition +endif # lfwriter tool is used for testing lockfile package .PHONY: lfwriter-tool lfwriter-tool: $(LFWRITER_TOOL) -$(LFWRITER_TOOL): $(wildcard ./test/lfwriter/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(LFWRITER_TOOL) github.com/microsoft/dcp/test/lfwriter +$(LFWRITER_TOOL): $(wildcard ./test/lfwriter/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(LFWRITER_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/lfwriter # parrot tool is used for testing network connectivity .PHONY: parrot-tool parrot-tool: $(PARROT_TOOL) -$(PARROT_TOOL): $(wildcard ./test/parrot/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(PARROT_TOOL) github.com/microsoft/dcp/test/parrot +$(PARROT_TOOL): $(wildcard ./test/parrot/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(PARROT_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/parrot # Builds a static parrot binary suitable for the scratch-based test container image. .PHONY: parrot-tool-containerexe parrot-tool-containerexe: $(PARROT_TOOL_CONTAINER_BINARY) -$(PARROT_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/parrot/*.go) | $(TOOL_BIN) +$(PARROT_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/parrot/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) ifeq ($(detected_OS),windows) - $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/parrot + $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/parrot else - CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/parrot + CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/parrot endif # Builds a static probe binary for the scratch-based container conformance image. .PHONY: container-probe-tool-containerexe container-probe-tool-containerexe: $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) -$(CONTAINER_PROBE_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/containerprobe/*.go) | $(TOOL_BIN) +$(CONTAINER_PROBE_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/containerprobe/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) ifeq ($(detected_OS),windows) - $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/containerprobe + $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/containerprobe else - CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/containerprobe + CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/containerprobe endif .PHONY: httpcontent-stream-repro diff --git a/internal/dcp/commands/root.go b/internal/dcp/commands/root.go index de9944f3..8c4bd106 100644 --- a/internal/dcp/commands/root.go +++ b/internal/dcp/commands/root.go @@ -93,12 +93,6 @@ func NewRootCmd(log *logger.Logger) (*cobra.Command, error) { rootCmd.AddCommand(cmd) } - if cmd, err = dcpproc_cmds.NewForkProcessExecCommand(log.Logger); err != nil { - return nil, fmt.Errorf("could not set up '%s' command: %w", dcpproc_cmds.ForkProcessExecCmdName, err) - } else { - rootCmd.AddCommand(cmd) - } - // Add dcptun sub-commands rootCmd.AddCommand(dcptun_cmds.NewRunServerCommand(log.Logger)) diff --git a/internal/dcpproc/commands/fork_process.go b/internal/dcpproc/commands/fork_process.go index 1e06b45e..4d0bb725 100644 --- a/internal/dcpproc/commands/fork_process.go +++ b/internal/dcpproc/commands/fork_process.go @@ -8,12 +8,8 @@ package commands import ( "context" "fmt" - "io" "os" "os/exec" - "strconv" - "strings" - "syscall" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -61,14 +57,6 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error logger.WithSessionId(childCmd) process.ForkFromParent(childCmd) - execShim, shimErr := useExecShim(childCmd) - if shimErr != nil { - return shimErr - } - if execShim != nil { - defer execShim.close() - } - monitorEnabled := cmd.Flags().Changed("monitor") var monitorCtx context.Context var monitorCtxCancel context.CancelFunc @@ -83,7 +71,7 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error } } - pid, childExitInfoCh, disposeChildExecutor, startErr := startForkedProcess(cmd, childCmd, execShim, monitorEnabled, log) + pid, childExitInfoCh, disposeChildExecutor, startErr := startForkedProcess(cmd, childCmd, monitorEnabled, log) if startErr != nil { return startErr } @@ -127,7 +115,6 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error func startForkedProcess( cmd *cobra.Command, childCmd *exec.Cmd, - execShim *execShimHandshake, observeExit bool, log logr.Logger, ) (process.Pid_t, <-chan process.ProcessExitInfo, func(), error) { @@ -154,18 +141,6 @@ func startForkedProcess( return process.UnknownPID, nil, nil, fmt.Errorf("could not start forked process: %w", startErr) } - // Starting the shim only means dcp itself started. The PID must not be reported before the - // requested program is known to be running, so that a program which cannot be executed is - // still reported as a start failure. - if execShim != nil { - if execErr := execShim.wait(); execErr != nil { - // The logger already carries the command and arguments. - log.Error(execErr, "Failed to execute forked process") - executor.Dispose() - return process.UnknownPID, nil, nil, fmt.Errorf("could not start forked process: %w", execErr) - } - } - pid := handle.Pid if _, writeErr := fmt.Fprintln(cmd.OutOrStdout(), pid); writeErr != nil { log.Error(writeErr, "Failed to write forked process PID", "PID", pid) @@ -188,115 +163,3 @@ func trimForkProcessArgSeparator(args []string) []string { return args } - -// Redirects the child through the 'fork-process-exec' command on platforms where the child would -// otherwise inherit an invalid SIGUSR1 disposition from the Go runtime. The shim cleans that -// disposition and then execs the original program, which keeps the process ID, session, standard -// streams, and exit code that the caller of 'fork-process' expects. -// -// The reset cannot be done here: the Go runtime restores its own signal dispositions in the -// forked child before it reaches execve, so it has to happen in the process that calls exec. -// -// Returns the handshake that reports whether the shim reached the requested program, or nil when -// the child is started directly. The caller owns the returned handshake and must close it. -func useExecShim(childCmd *exec.Cmd) (*execShimHandshake, error) { - if !process.NeedsExecSignalDispositionWorkaround() { - return nil, nil - } - - if childCmd.Err != nil { - // The program could not be located. Leave the command untouched so that starting it - // reports that original failure rather than one from the shim. - return nil, nil - } - - callerSIGUSR1Ignored, dispositionErr := process.InheritedSIGUSR1Ignored() - if dispositionErr != nil { - return nil, fmt.Errorf("could not determine the inherited SIGUSR1 disposition: %w", dispositionErr) - } - - return useExecShimWithDisposition(childCmd, callerSIGUSR1Ignored) -} - -func useExecShimWithDisposition( - childCmd *exec.Cmd, - callerSIGUSR1Ignored bool, -) (*execShimHandshake, error) { - dcpPath, dcpPathErr := os.Executable() - if dcpPathErr != nil { - return nil, fmt.Errorf("could not determine the path of the current executable: %w", dcpPathErr) - } - - statusR, statusW, pipeErr := os.Pipe() - if pipeErr != nil { - return nil, fmt.Errorf("could not create the exec status pipe: %w", pipeErr) - } - - shimArgs := []string{ - dcpPath, - ForkProcessExecCmdName, - "--" + execPathFlagName, childCmd.Path, - "--" + callerSIGUSR1IgnoredFlagName + "=" + strconv.FormatBool(callerSIGUSR1Ignored), - } - shimArgs = append(shimArgs, "--") - childCmd.Args = append(shimArgs, childCmd.Args...) - childCmd.Path = dcpPath - - // The shim reports the outcome of the exec on this descriptor. It is the only extra file, so - // the shim sees it as execStatusFd. - childCmd.ExtraFiles = append(childCmd.ExtraFiles, statusW) - - return &execShimHandshake{statusR: statusR, statusW: statusW}, nil -} - -// execShimHandshake reports whether the shim managed to exec the requested program. Starting the -// shim only proves that dcp itself could be started, so without this the caller would be told -// that a program which never ran had started successfully. -// -// The shim inherits the write end. A successful execve closes it and the read end reports EOF, -// while a failure sends the errno before the shim exits. -type execShimHandshake struct { - statusR *os.File - statusW *os.File -} - -// wait blocks until the shim either replaces itself with the requested program or reports why it -// could not. It returns the failure that a direct start would have reported. -func (h *execShimHandshake) wait() error { - // The write end is now owned by the shim. The parent's copy has to go, because the read below - // only reports EOF once every writer is closed. - h.closeWriteEnd() - - status, readErr := io.ReadAll(h.statusR) - if readErr != nil { - return fmt.Errorf("could not read the exec status: %w", readErr) - } - - if len(status) == 0 { - // EOF with nothing written: the descriptor was closed by a successful execve. - return nil - } - - errnoValue, parseErr := strconv.Atoi(strings.TrimSpace(string(status))) - if parseErr != nil { - return fmt.Errorf("the exec status %q could not be parsed: %w", status, parseErr) - } - - return syscall.Errno(errnoValue) -} - -func (h *execShimHandshake) closeWriteEnd() { - if h.statusW != nil { - _ = h.statusW.Close() - h.statusW = nil - } -} - -func (h *execShimHandshake) close() { - h.closeWriteEnd() - - if h.statusR != nil { - _ = h.statusR.Close() - h.statusR = nil - } -} diff --git a/internal/dcpproc/commands/fork_process_darwin_test.go b/internal/dcpproc/commands/fork_process_darwin_test.go deleted file mode 100644 index 35eab395..00000000 --- a/internal/dcpproc/commands/fork_process_darwin_test.go +++ /dev/null @@ -1,133 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package commands - -import ( - "bytes" - "fmt" - "os" - "os/exec" - "os/signal" - "syscall" - "testing" - "time" - - "github.com/go-logr/logr" - "github.com/stretchr/testify/require" - - "github.com/microsoft/dcp/pkg/process" - "github.com/microsoft/dcp/pkg/testutil" -) - -const ( - execShimIgnoredEndToEndEnvVar = "DCP_TEST_EXEC_SHIM_IGNORED_END_TO_END_HELPER" - execShimIgnoredEndToEndTestName = "TestExecShimPreservesIgnoredSIGUSR1Helper" -) - -func TestMain(m *testing.M) { - if len(os.Args) > 1 && os.Args[1] == ForkProcessExecCmdName { - os.Exit(runForkProcessExecTestCommand()) - } - - os.Exit(m.Run()) -} - -func runForkProcessExecTestCommand() int { - forkProcessExecCmd, commandErr := NewForkProcessExecCommand(logr.Discard()) - if commandErr != nil { - _, _ = fmt.Fprintf(os.Stderr, "could not create fork-process-exec test command: %v\n", commandErr) - return 1 - } - - forkProcessExecCmd.SetArgs(os.Args[2:]) - executeErr := forkProcessExecCmd.Execute() - if executeErr != nil { - _, _ = fmt.Fprintf(os.Stderr, "fork-process-exec test command failed: %v\n", executeErr) - return 1 - } - - return 0 -} - -func TestUseExecShimCarriesIgnoredSIGUSR1(t *testing.T) { - t.Parallel() - - childCmd := exec.Command("/bin/sh", "-c", "exit 0") - execShim, shimErr := useExecShimWithDisposition(childCmd, true) - require.NoError(t, shimErr) - require.NotNil(t, execShim) - t.Cleanup(execShim.close) - - require.Contains( - t, - childCmd.Args, - "--"+callerSIGUSR1IgnoredFlagName+"=true", - "the shim invocation should carry the caller's ignored disposition", - ) -} - -func TestExecShimPreservesIgnoredSIGUSR1ForNonGoTarget(t *testing.T) { - t.Parallel() - - runExecShimTestHelper(t, execShimIgnoredEndToEndTestName, execShimIgnoredEndToEndEnvVar) -} - -func TestExecShimPreservesIgnoredSIGUSR1Helper(t *testing.T) { - if os.Getenv(execShimIgnoredEndToEndEnvVar) == "" { - t.Skip("helper for TestExecShimPreservesIgnoredSIGUSR1ForNonGoTarget") - } - - // Only this subprocess changes its signal state; the parallel parent test remains untouched. - signal.Ignore(syscall.SIGUSR1) - ignoredByCaller, dispositionErr := process.IsSIGUSR1Ignored() - require.NoError(t, dispositionErr) - require.True(t, ignoredByCaller) - - childCmd := exec.Command("/bin/sh", "-c", `kill -USR1 $$; printf ignored`) - var childOutput bytes.Buffer - childCmd.Stdout = &childOutput - childCmd.Stderr = &childOutput - - execShim, shimErr := useExecShimWithDisposition(childCmd, true) - require.NoError(t, shimErr) - require.NotNil(t, execShim) - t.Cleanup(execShim.close) - - // The exec shim starts from this ignored disposition, then its Go runtime replaces it. - childStartErr := childCmd.Start() - require.NoError(t, childStartErr) - - childFinished := false - t.Cleanup(func() { - if !childFinished { - _ = childCmd.Process.Kill() - _ = childCmd.Wait() - } - }) - - handshakeErr := execShim.wait() - require.NoError(t, handshakeErr, "the Go exec shim should reach the non-Go target") - - childWaitErr := childCmd.Wait() - childFinished = true - require.NoError(t, childWaitErr, "the target should survive SIGUSR1; output:\n%s", childOutput.String()) - require.Equal(t, "ignored", childOutput.String()) -} - -func runExecShimTestHelper(t *testing.T, testName string, envVarName string) { - t.Helper() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - helperCmd := exec.CommandContext(testCtx, os.Args[0], "-test.run=^"+testName+"$", "-test.v") - helperCmd.Env = append(os.Environ(), envVarName+"=1") - - output, runErr := helperCmd.CombinedOutput() - require.NoError(t, runErr, "helper process failed; output:\n%s", output) -} diff --git a/internal/dcpproc/commands/fork_process_exec.go b/internal/dcpproc/commands/fork_process_exec.go deleted file mode 100644 index 7bc43f50..00000000 --- a/internal/dcpproc/commands/fork_process_exec.go +++ /dev/null @@ -1,135 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package commands - -import ( - "errors" - "fmt" - "os" - "syscall" - - "github.com/go-logr/logr" - "github.com/spf13/cobra" - - cmds "github.com/microsoft/dcp/internal/commands" - "github.com/microsoft/dcp/pkg/process" -) - -const ( - // The name of the command, also used when 'fork-process' builds an invocation of it. - ForkProcessExecCmdName = "fork-process-exec" - - // The flag carrying the resolved path of the image to execute. It is passed separately from - // the arguments so that the child keeps the argv[0] the caller asked for. - execPathFlagName = "exec-path" - - // The hidden flag carrying the SIGUSR1 disposition captured before this Go process started. - callerSIGUSR1IgnoredFlagName = "caller-sigusr1-ignored" - - // The descriptor 'fork-process' passes as the only extra file, on which this command reports - // whether the exec succeeded. It is the first descriptor after the standard streams. - execStatusFd = 3 - - // Reported when the image cannot be executed, matching the shell convention for a command - // that could not be run. 'fork-process' reports the underlying errno itself, so this is only - // a fallback for anything that inspects the shim's own exit code. - execFailedExitCode = 127 -) - -var ( - execPath string - callerSIGUSR1Ignored bool -) - -// NewForkProcessExecCommand creates the 'fork-process-exec' command, which installs the clean -// SIGUSR1 disposition requested by 'fork-process' and replaces itself with the requested image. -// It is an implementation detail and is not meant to be invoked directly. -func NewForkProcessExecCommand(log logr.Logger) (*cobra.Command, error) { - forkProcessExecCmd := &cobra.Command{ - Use: ForkProcessExecCmdName + " --" + execPathFlagName + " path -- command [args...]", - Short: "Replaces this process with another program.", - Long: "Installs a clean SIGUSR1 disposition captured by 'fork-process' and then replaces this process with the requested program, keeping the same process ID. This prevents children from inheriting signal handler flags that confuse other language runtimes.", - RunE: forkProcessExec(log), - Args: validateForkProcessExecArgs, - - Hidden: true, - SilenceUsage: true, - } - - forkProcessExecCmd.Flags().StringVar(&execPath, execPathFlagName, "", "Resolved path of the program to execute") - forkProcessExecCmd.Flags().BoolVar( - &callerSIGUSR1Ignored, - callerSIGUSR1IgnoredFlagName, - false, - "Whether the caller ignored SIGUSR1 before starting the exec shim", - ) - hideDispositionFlagErr := forkProcessExecCmd.Flags().MarkHidden(callerSIGUSR1IgnoredFlagName) - if hideDispositionFlagErr != nil { - return nil, fmt.Errorf("could not hide --%s: %w", callerSIGUSR1IgnoredFlagName, hideDispositionFlagErr) - } - - return forkProcessExecCmd, nil -} - -func validateForkProcessExecArgs(_ *cobra.Command, args []string) error { - if len(trimForkProcessArgSeparator(args)) == 0 { - return fmt.Errorf("command is required") - } - - return nil -} - -func forkProcessExec(log logr.Logger) func(cmd *cobra.Command, args []string) error { - return func(_ *cobra.Command, args []string) error { - args = trimForkProcessArgSeparator(args) - - if execPath == "" { - return fmt.Errorf("--%s is required", execPathFlagName) - } - - log = log.WithName("ForkProcessExec").WithValues( - "Path", execPath, - "Args", args[1:], - ) - - // 'fork-process' waits for this descriptor to close, which is how a successful execve is - // reported, so it must not survive into the new program. It is always supplied, because - // this command is only ever started by 'fork-process'. - statusFile := os.NewFile(execStatusFd, "exec-status") - syscall.CloseOnExec(execStatusFd) - - targetEnv := os.Environ() - - prepareErr := process.PrepareSIGUSR1ForExec(callerSIGUSR1Ignored) - if prepareErr != nil { - writeForkProcessExecFailure(statusFile, prepareErr) - log.Error(prepareErr, "Could not prepare SIGUSR1 disposition for executed program") - return cmds.NewExitCodeError( - fmt.Errorf("could not prepare SIGUSR1 disposition for %q: %w", execPath, prepareErr), - execFailedExitCode, - ) - } - - // Exec must immediately follow the signal change. It only returns when it fails; on - // success this process becomes the requested program. - execErr := syscall.Exec(execPath, args, targetEnv) - - writeForkProcessExecFailure(statusFile, execErr) - - log.Error(execErr, "Could not execute the requested program") - return cmds.NewExitCodeError(fmt.Errorf("could not execute %q: %w", execPath, execErr), execFailedExitCode) - } -} - -func writeForkProcessExecFailure(statusFile *os.File, failureErr error) { - var failureErrno syscall.Errno - if !errors.As(failureErr, &failureErrno) { - failureErrno = syscall.EINVAL - } - - _, _ = fmt.Fprintf(statusFile, "%d", int(failureErrno)) - _ = statusFile.Close() -} diff --git a/internal/dcpproc/commands/fork_process_test.go b/internal/dcpproc/commands/fork_process_test.go deleted file mode 100644 index a1e64f90..00000000 --- a/internal/dcpproc/commands/fork_process_test.go +++ /dev/null @@ -1,128 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package commands - -import ( - "fmt" - "os" - "os/exec" - "strconv" - "syscall" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/microsoft/dcp/pkg/process" -) - -// Verifies that the child is redirected through the 'fork-process-exec' command on platforms -// that need it, and left alone everywhere else. The redirection is what cleans the Go runtime's -// SIGUSR1 disposition before the real program starts. -func TestUseExecShim(t *testing.T) { - t.Parallel() - - childCmd := exec.Command("sh", "-c", "exit 0") - childCmd.Env = []string{"EXISTING=value", "GODEBUG=gctrace=1,asyncpreemptoff=0,schedtrace=1000"} - originalPath := childCmd.Path - originalArgs := childCmd.Args - originalEnv := childCmd.Env - - execShim, shimErr := useExecShim(childCmd) - require.NoError(t, shimErr) - if execShim != nil { - defer execShim.close() - } - - if !process.NeedsExecSignalDispositionWorkaround() { - require.Nil(t, execShim, "no handshake is needed on this platform") - require.Equal(t, originalPath, childCmd.Path, "the command should not be redirected on this platform") - require.Equal(t, originalArgs, childCmd.Args, "the arguments should not be rewritten on this platform") - require.Equal(t, originalEnv, childCmd.Env, "the environment should not be rewritten on this platform") - return - } - - dcpPath, dcpPathErr := os.Executable() - require.NoError(t, dcpPathErr) - - ignoredByCaller, dispositionErr := process.InheritedSIGUSR1Ignored() - require.NoError(t, dispositionErr) - - expectedArgs := append( - []string{ - dcpPath, - ForkProcessExecCmdName, - "--" + execPathFlagName, - originalPath, - "--" + callerSIGUSR1IgnoredFlagName + "=" + strconv.FormatBool(ignoredByCaller), - "--", - }, - originalArgs..., - ) - - require.Equal(t, dcpPath, childCmd.Path, "the command should run the current executable") - require.Equal(t, expectedArgs, childCmd.Args, "the original program and arguments should be passed to the shim") - require.Equal(t, originalEnv, childCmd.Env, "the target environment should not be rewritten") - - require.NotNil(t, execShim, "the shim should report whether the exec succeeded") - require.Len(t, childCmd.ExtraFiles, 1, "the status descriptor should be passed to the shim") -} - -// Verifies that a command that could not be resolved is left untouched, so that starting it -// reports the original lookup failure instead of one produced by the shim. -func TestUseExecShimLeavesUnresolvedCommand(t *testing.T) { - t.Parallel() - - childCmd := exec.Command("dcp-command-that-does-not-exist") - require.Error(t, childCmd.Err, "the test requires a command that cannot be resolved") - - originalPath := childCmd.Path - originalArgs := childCmd.Args - - execShim, shimErr := useExecShim(childCmd) - require.NoError(t, shimErr) - require.Nil(t, execShim, "an unresolved command should not be redirected through the shim") - - require.Equal(t, originalPath, childCmd.Path, "an unresolved command should not be redirected") - require.Equal(t, originalArgs, childCmd.Args, "an unresolved command should not have its arguments rewritten") -} - -// Verifies that the handshake reports a successful exec, which the shim signals by closing the -// status descriptor without writing to it. -func TestExecShimHandshakeReportsSuccess(t *testing.T) { - t.Parallel() - - handshake := newTestExecShimHandshake(t) - - // Stand in for the shim: a successful execve closes the inherited descriptor. - require.NoError(t, handshake.statusW.Close()) - - require.NoError(t, handshake.wait()) -} - -// Verifies that the errno the shim reports is surfaced to the caller. Without this the caller -// would be handed the PID of a process that never became the requested program. -func TestExecShimHandshakeReportsExecFailure(t *testing.T) { - t.Parallel() - - handshake := newTestExecShimHandshake(t) - - // Stand in for the shim reporting a failed execve. - writeForkProcessExecFailure(handshake.statusW, fmt.Errorf("executing child: %w", syscall.ENOENT)) - - require.ErrorIs(t, handshake.wait(), syscall.ENOENT) -} - -func newTestExecShimHandshake(t *testing.T) *execShimHandshake { - t.Helper() - - statusR, statusW, pipeErr := os.Pipe() - require.NoError(t, pipeErr) - - handshake := &execShimHandshake{statusR: statusR, statusW: statusW} - t.Cleanup(handshake.close) - - return handshake -} diff --git a/internal/dcpproc/commands/root.go b/internal/dcpproc/commands/root.go index e9d6f84c..e7254060 100644 --- a/internal/dcpproc/commands/root.go +++ b/internal/dcpproc/commands/root.go @@ -68,12 +68,6 @@ func NewRootCmd(log *logger.Logger) (*cobra.Command, error) { rootCmd.AddCommand(cmd) } - if cmd, err = NewForkProcessExecCommand(log.Logger); err != nil { - return nil, fmt.Errorf("could not set up '%s' command: %w", ForkProcessExecCmdName, err) - } else { - rootCmd.AddCommand(cmd) - } - return rootCmd, nil } diff --git a/internal/dcpproc/fork_process_darwin_test.go b/internal/dcpproc/fork_process_darwin_test.go deleted file mode 100644 index b1014e85..00000000 --- a/internal/dcpproc/fork_process_darwin_test.go +++ /dev/null @@ -1,113 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package dcpproc_test - -import ( - "bytes" - "io" - "os" - "os/exec" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/require" - - usvc_io "github.com/microsoft/dcp/pkg/io" - "github.com/microsoft/dcp/pkg/testutil" -) - -func TestForkProcessPreservesInheritedIgnoredSIGUSR1(t *testing.T) { - t.Parallel() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - dcpProc, dcpProcErr := getDcpProcExecutablePath() - require.NoError(t, dcpProcErr) - - outputPath := filepath.Join(t.TempDir(), "target-sigusr1") - cmdArgs := forkProcessArgsForCurrentProcess( - t, - "/bin/sh", - "-c", - `kill -USR1 $$; printf ignored > "$1"`, - "sh", - outputPath, - ) - launcherArgs := append( - []string{"-c", `trap '' USR1; "$@"; status=$?; exit "$status"`, "sh", dcpProc}, - cmdArgs..., - ) - dcpProcCmd := exec.CommandContext(testCtx, "/bin/sh", launcherArgs...) - var stdout, stderr bytes.Buffer - dcpProcCmd.Stdout = &stdout - dcpProcCmd.Stderr = &stderr - - runErr := dcpProcCmd.Run() - require.NoError(t, runErr, "dcp fork-process should preserve ignored SIGUSR1; stderr: %s", stderr.String()) - _ = parseForkedPid(t, stdout.String()) - - outputFile, openErr := usvc_io.OpenFileReadOnly(outputPath) - require.NoError(t, openErr) - output, readErr := io.ReadAll(outputFile) - closeErr := outputFile.Close() - require.NoError(t, readErr) - require.NoError(t, closeErr) - require.Equal(t, "ignored", string(output)) -} - -func TestForkProcessExecShimPreservesTargetGoDebug(t *testing.T) { - t.Parallel() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - dcpProc, dcpProcErr := getDcpProcExecutablePath() - require.NoError(t, dcpProcErr) - - const originalGoDebug = "asyncpreemptoff=0,tracebackancestors=7" - outputPath := filepath.Join(t.TempDir(), "target-godebug") - cmdArgs := forkProcessArgsForCurrentProcess( - t, - "/bin/sh", - "-c", - `printf '%s' "$GODEBUG" > "$1"`, - "sh", - outputPath, - ) - dcpProcCmd := exec.CommandContext(testCtx, dcpProc, cmdArgs...) - dcpProcCmd.Env = environmentWithValue(os.Environ(), "GODEBUG", originalGoDebug) - var stdout, stderr bytes.Buffer - dcpProcCmd.Stdout = &stdout - dcpProcCmd.Stderr = &stderr - - runErr := dcpProcCmd.Run() - require.NoError(t, runErr, "dcp fork-process should exit cleanly; stderr: %s", stderr.String()) - _ = parseForkedPid(t, stdout.String()) - - outputFile, openErr := usvc_io.OpenFileReadOnly(outputPath) - require.NoError(t, openErr) - output, readErr := io.ReadAll(outputFile) - closeErr := outputFile.Close() - require.NoError(t, readErr) - require.NoError(t, closeErr) - require.Equal(t, originalGoDebug, string(output), "the requested program should observe the caller's GODEBUG") -} - -func environmentWithValue(env []string, name string, value string) []string { - prefix := name + "=" - updatedEnv := make([]string, 0, len(env)+1) - for _, entry := range env { - if !strings.HasPrefix(entry, prefix) { - updatedEnv = append(updatedEnv, entry) - } - } - return append(updatedEnv, prefix+value) -} diff --git a/internal/tools/goruntimeoverlay/go-81009.patch b/internal/tools/goruntimeoverlay/go-81009.patch new file mode 100644 index 00000000..bfb8a2d4 --- /dev/null +++ b/internal/tools/goruntimeoverlay/go-81009.patch @@ -0,0 +1,17 @@ +diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go +--- a/src/runtime/os_darwin.go ++++ b/src/runtime/os_darwin.go +@@ -394,7 +394,12 @@ var sigset_all = ^sigset(0) + //go:nowritebarrierrec + func setsig(i uint32, fn uintptr) { + var sa usigactiont +- sa.sa_flags = _SA_SIGINFO | _SA_ONSTACK | _SA_RESTART ++ ++ sa.sa_flags = _SA_ONSTACK | _SA_RESTART ++ // SA_SIGINFO should not be set when assigning SIG_DFL or SIG_IGN ++ if fn != _SIG_DFL && fn != _SIG_IGN { ++ sa.sa_flags |= _SA_SIGINFO ++ } + sa.sa_mask = ^uint32(0) + if fn == abi.FuncPCABIInternal(sighandler) { // abi.FuncPCABIInternal(sighandler) matches the callers in signal_unix.go + if iscgo { diff --git a/internal/tools/goruntimeoverlay/main.go b/internal/tools/goruntimeoverlay/main.go new file mode 100644 index 00000000..4a3974cc --- /dev/null +++ b/internal/tools/goruntimeoverlay/main.go @@ -0,0 +1,204 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package main + +import ( + "bytes" + "context" + _ "embed" + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + + dcpio "github.com/microsoft/dcp/pkg/io" + "github.com/microsoft/dcp/pkg/osutil" +) + +//go:embed go-81009.patch +var darwinRuntimePatch []byte + +type overlayConfig struct { + Replace map[string]string +} + +func main() { + outputDirectory := flag.String("output-dir", "", "Directory where the runtime overlay is generated") + flag.Parse() + + if *outputDirectory == "" { + _, _ = fmt.Fprintln(os.Stderr, "--output-dir is required") + os.Exit(2) + } + + generateErr := generateRuntimeOverlay(context.Background(), runtime.GOROOT(), *outputDirectory) + if generateErr != nil { + _, _ = fmt.Fprintf(os.Stderr, "could not generate Go runtime overlay: %v\n", generateErr) + os.Exit(1) + } +} + +func generateRuntimeOverlay(ctx context.Context, goRoot string, outputDirectory string) error { + runtimeSourcePath := filepath.Join(goRoot, "src", "runtime", "os_darwin.go") + runtimeSource, readErr := readFile(runtimeSourcePath) + if readErr != nil { + return fmt.Errorf("reading %q: %w", runtimeSourcePath, readErr) + } + + patchedSource, patchRequired, patchErr := applyDarwinRuntimePatch(ctx, runtimeSource) + if patchErr != nil { + return fmt.Errorf("patching %q: %w", runtimeSourcePath, patchErr) + } + + makeOutputDirectoryErr := os.MkdirAll(outputDirectory, osutil.PermissionDirectoryOthersRead) + if makeOutputDirectoryErr != nil { + return fmt.Errorf("creating output directory %q: %w", outputDirectory, makeOutputDirectoryErr) + } + + replacements := make(map[string]string, 1) + if patchRequired { + patchedSourceDirectory := filepath.Join(outputDirectory, "runtime") + makeSourceDirectoryErr := os.MkdirAll( + patchedSourceDirectory, + osutil.PermissionDirectoryOthersRead, + ) + if makeSourceDirectoryErr != nil { + return fmt.Errorf("creating patched runtime directory %q: %w", patchedSourceDirectory, makeSourceDirectoryErr) + } + + patchedSourcePath := filepath.Join(patchedSourceDirectory, "os_darwin.go") + writeSourceErr := writeFileIfChanged(patchedSourcePath, patchedSource) + if writeSourceErr != nil { + return fmt.Errorf("writing patched runtime source %q: %w", patchedSourcePath, writeSourceErr) + } + + replacements[runtimeSourcePath] = patchedSourcePath + } + + config := overlayConfig{Replace: replacements} + configContents, marshalErr := json.MarshalIndent(config, "", " ") + if marshalErr != nil { + return fmt.Errorf("encoding overlay configuration: %w", marshalErr) + } + configContents = append(configContents, '\n') + + configPath := filepath.Join(outputDirectory, "overlay.json") + writeConfigErr := writeFileIfChanged(configPath, configContents) + if writeConfigErr != nil { + return fmt.Errorf("writing overlay configuration %q: %w", configPath, writeConfigErr) + } + + return nil +} + +func applyDarwinRuntimePatch(ctx context.Context, source []byte) ([]byte, bool, error) { + stagingDirectory, createTempErr := os.MkdirTemp("", "dcp-go-runtime-overlay-") + if createTempErr != nil { + return nil, false, fmt.Errorf("creating patch staging directory: %w", createTempErr) + } + defer func() { + _ = os.RemoveAll(stagingDirectory) + }() + + stagedSourceDirectory := filepath.Join(stagingDirectory, "src", "runtime") + makeSourceDirectoryErr := os.MkdirAll(stagedSourceDirectory, osutil.PermissionDirectoryOthersRead) + if makeSourceDirectoryErr != nil { + return nil, false, fmt.Errorf("creating patch source directory: %w", makeSourceDirectoryErr) + } + + stagedSourcePath := filepath.Join(stagedSourceDirectory, "os_darwin.go") + writeSourceErr := dcpio.WriteFile( + stagedSourcePath, + source, + osutil.PermissionOwnerReadWriteOthersRead, + ) + if writeSourceErr != nil { + return nil, false, fmt.Errorf("writing patch source: %w", writeSourceErr) + } + + forwardCheckOutput, forwardCheckErr := runGitApply(ctx, stagingDirectory, false, true) + if forwardCheckErr == nil { + applyOutput, applyErr := runGitApply(ctx, stagingDirectory, false, false) + if applyErr != nil { + return nil, false, fmt.Errorf("applying upstream patch: %w: %s", applyErr, applyOutput) + } + + patchedSource, patchedSourceErr := readFile(stagedSourcePath) + if patchedSourceErr != nil { + return nil, false, fmt.Errorf("reading patched runtime source: %w", patchedSourceErr) + } + + return patchedSource, true, nil + } + + reverseCheckOutput, reverseCheckErr := runGitApply(ctx, stagingDirectory, true, true) + if reverseCheckErr == nil { + return source, false, nil + } + + return nil, false, fmt.Errorf( + "upstream patch applies neither forward nor in reverse (forward: %v: %s; reverse: %v: %s)", + forwardCheckErr, + forwardCheckOutput, + reverseCheckErr, + reverseCheckOutput, + ) +} + +func runGitApply( + ctx context.Context, + workingDirectory string, + reverse bool, + check bool, +) ([]byte, error) { + args := []string{"-C", workingDirectory, "apply"} + if reverse { + args = append(args, "--reverse") + } + if check { + args = append(args, "--check") + } + args = append(args, "-") + + applyCommand := exec.CommandContext(ctx, "git", args...) + applyCommand.Stdin = bytes.NewReader(darwinRuntimePatch) + output, applyErr := applyCommand.CombinedOutput() + return output, applyErr +} + +func readFile(path string) ([]byte, error) { + file, openErr := dcpio.OpenFileReadOnly(path) + if openErr != nil { + return nil, openErr + } + + contents, readErr := io.ReadAll(file) + closeErr := file.Close() + if fileErr := errors.Join(readErr, closeErr); fileErr != nil { + return nil, fileErr + } + + return contents, nil +} + +func writeFileIfChanged(path string, contents []byte) error { + existingContents, readErr := readFile(path) + switch { + case readErr == nil && bytes.Equal(existingContents, contents): + return nil + case readErr == nil: + case errors.Is(readErr, os.ErrNotExist): + default: + return readErr + } + + return dcpio.WriteFile(path, contents, osutil.PermissionOwnerReadWriteOthersRead) +} diff --git a/internal/tools/goruntimeoverlay/main_test.go b/internal/tools/goruntimeoverlay/main_test.go new file mode 100644 index 00000000..aabdef82 --- /dev/null +++ b/internal/tools/goruntimeoverlay/main_test.go @@ -0,0 +1,147 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + dcpio "github.com/microsoft/dcp/pkg/io" + "github.com/microsoft/dcp/pkg/osutil" +) + +const ( + unpatchedRuntimeSource = `package runtime + +func before() {} + +type sigset uint32 + +var sigset_all = ^sigset(0) + +//go:nosplit +//go:nowritebarrierrec +func setsig(i uint32, fn uintptr) { + var sa usigactiont + sa.sa_flags = _SA_SIGINFO | _SA_ONSTACK | _SA_RESTART + sa.sa_mask = ^uint32(0) + if fn == abi.FuncPCABIInternal(sighandler) { // abi.FuncPCABIInternal(sighandler) matches the callers in signal_unix.go + if iscgo { + } + } +} + +func after() {} +` + patchedRuntimeSource = `package runtime + +func before() {} + +type sigset uint32 + +var sigset_all = ^sigset(0) + +//go:nosplit +//go:nowritebarrierrec +func setsig(i uint32, fn uintptr) { + var sa usigactiont + + sa.sa_flags = _SA_ONSTACK | _SA_RESTART + // SA_SIGINFO should not be set when assigning SIG_DFL or SIG_IGN + if fn != _SIG_DFL && fn != _SIG_IGN { + sa.sa_flags |= _SA_SIGINFO + } + sa.sa_mask = ^uint32(0) + if fn == abi.FuncPCABIInternal(sighandler) { // abi.FuncPCABIInternal(sighandler) matches the callers in signal_unix.go + if iscgo { + } + } +} + +func after() {} +` +) + +func TestGenerateRuntimeOverlay(t *testing.T) { + t.Parallel() + + goRoot := t.TempDir() + runtimeDirectory := filepath.Join(goRoot, "src", "runtime") + require.NoError(t, os.MkdirAll(runtimeDirectory, osutil.PermissionDirectoryOthersRead)) + + runtimeSourcePath := filepath.Join(runtimeDirectory, "os_darwin.go") + runtimeSource := []byte(unpatchedRuntimeSource) + require.NoError(t, dcpio.WriteFile( + runtimeSourcePath, + runtimeSource, + osutil.PermissionOwnerReadWriteOthersRead, + )) + + outputDirectory := filepath.Join(t.TempDir(), "overlay") + require.NoError(t, generateRuntimeOverlay(context.Background(), goRoot, outputDirectory)) + + patchedSourcePath := filepath.Join(outputDirectory, "runtime", "os_darwin.go") + patchedSource, patchedSourceErr := readFile(patchedSourcePath) + require.NoError(t, patchedSourceErr) + require.Equal(t, patchedRuntimeSource, string(patchedSource)) + + configContents, configReadErr := readFile(filepath.Join(outputDirectory, "overlay.json")) + require.NoError(t, configReadErr) + + var config overlayConfig + require.NoError(t, json.Unmarshal(configContents, &config)) + require.Equal(t, map[string]string{runtimeSourcePath: patchedSourcePath}, config.Replace) +} + +func TestGenerateRuntimeOverlayIsEmptyWhenToolchainContainsFix(t *testing.T) { + t.Parallel() + + goRoot := t.TempDir() + runtimeDirectory := filepath.Join(goRoot, "src", "runtime") + require.NoError(t, os.MkdirAll(runtimeDirectory, osutil.PermissionDirectoryOthersRead)) + + runtimeSourcePath := filepath.Join(runtimeDirectory, "os_darwin.go") + require.NoError(t, dcpio.WriteFile( + runtimeSourcePath, + []byte(patchedRuntimeSource), + osutil.PermissionOwnerReadWriteOthersRead, + )) + + outputDirectory := filepath.Join(t.TempDir(), "overlay") + require.NoError(t, generateRuntimeOverlay(context.Background(), goRoot, outputDirectory)) + + configContents, configReadErr := readFile(filepath.Join(outputDirectory, "overlay.json")) + require.NoError(t, configReadErr) + + var config overlayConfig + require.NoError(t, json.Unmarshal(configContents, &config)) + require.Empty(t, config.Replace) +} + +func TestGenerateRuntimeOverlayRejectsUnexpectedSource(t *testing.T) { + t.Parallel() + + goRoot := t.TempDir() + runtimeDirectory := filepath.Join(goRoot, "src", "runtime") + require.NoError(t, os.MkdirAll(runtimeDirectory, osutil.PermissionDirectoryOthersRead)) + + runtimeSourcePath := filepath.Join(runtimeDirectory, "os_darwin.go") + require.NoError(t, dcpio.WriteFile( + runtimeSourcePath, + []byte("package runtime\n"), + osutil.PermissionOwnerReadWriteOthersRead, + )) + + outputDirectory := filepath.Join(t.TempDir(), "overlay") + generateErr := generateRuntimeOverlay(context.Background(), goRoot, outputDirectory) + + require.ErrorContains(t, generateErr, "upstream patch applies neither forward nor in reverse") +} diff --git a/pkg/process/os_executor_darwin_test.go b/pkg/process/os_executor_darwin_test.go new file mode 100644 index 00000000..09fbdb7f --- /dev/null +++ b/pkg/process/os_executor_darwin_test.go @@ -0,0 +1,47 @@ +//go:build darwin + +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package process_test + +import ( + "bytes" + "os/exec" + "testing" + "time" + + "github.com/stretchr/testify/require" + + int_testutil "github.com/microsoft/dcp/internal/testutil" + "github.com/microsoft/dcp/pkg/process" + "github.com/microsoft/dcp/pkg/testutil" +) + +func TestStartProcessChildDoesNotInheritSIGINFOWithDefaultHandler(t *testing.T) { + t.Parallel() + + signalDispositionTool, toolPathErr := int_testutil.GetTestToolPath("signal-disposition") + require.NoError( + t, + toolPathErr, + "could not locate signal-disposition test tool (did you run `make test-prereqs`?)", + ) + + childCmd := exec.Command(signalDispositionTool) + var childOutput bytes.Buffer + childCmd.Stdout = &childOutput + childCmd.Stderr = &childOutput + + executor := process.NewOSExecutor(log) + defer executor.Dispose() + + testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) + defer testCancel() + + exitCode, runErr := process.RunToCompletion(testCtx, executor, childCmd) + require.NoError(t, runErr, "child process inherited an invalid signal disposition:\n%s", childOutput.String()) + require.Zero(t, exitCode, "child process reported an invalid signal disposition:\n%s", childOutput.String()) +} diff --git a/pkg/process/signal_disposition_darwin.go b/pkg/process/signal_disposition_darwin.go deleted file mode 100644 index c3eec72f..00000000 --- a/pkg/process/signal_disposition_darwin.go +++ /dev/null @@ -1,152 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package process - -import ( - "fmt" - "os" - "syscall" - "unsafe" - - "golang.org/x/sys/unix" -) - -const ( - // Darwin's NSIG. Valid signal numbers are 1 through darwinNumSignals-1. - darwinNumSignals = 32 - - // SIG_DFL and SIG_IGN, which the syscall package does not define. - darwinSigDfl = uintptr(0) - darwinSigIgn = uintptr(1) -) - -// InheritedSIGUSR1Ignored reports whether SIGUSR1 was ignored when this process started. -// -// The Go runtime replaces an inherited SIG_IGN disposition for SIGUSR1 during startup, so the -// original state is recovered from the live launcher process that supplied it. -func InheritedSIGUSR1Ignored() (bool, error) { - parentPID := os.Getppid() - if parentPID <= 0 { - return false, fmt.Errorf("invalid launcher process ID %d", parentPID) - } - - parentInfo, parentInfoErr := unix.SysctlKinfoProc("kern.proc.pid", parentPID) - if parentInfoErr != nil { - return false, fmt.Errorf("reading launcher process %d signal dispositions: %w", parentPID, parentInfoErr) - } - if parentInfo == nil { - return false, fmt.Errorf("launcher process %d returned no process information", parentPID) - } - if parentInfo.Proc.P_pid != int32(parentPID) { - return false, fmt.Errorf( - "launcher process query returned process %d instead of %d", - parentInfo.Proc.P_pid, - parentPID, - ) - } - - signalNumber := uint32(syscall.SIGUSR1) - if signalNumber == 0 || signalNumber >= darwinNumSignals { - return false, fmt.Errorf("SIGUSR1 number %d cannot be represented in Darwin sigset_t", signalNumber) - } - - // Darwin's sigmask macro assigns signal N to bit N-1. - signalMask := uint32(1) << (signalNumber - 1) - return parentInfo.Proc.P_sigignore&signalMask != 0, nil -} - -// darwinSigactionNew mirrors Darwin's `struct __sigaction`, which is the layout the -// sigaction(2) system call expects for the new disposition. It differs from the userspace -// `struct sigaction` by the sa_tramp field that libc fills in with the signal trampoline. -// A nil trampoline is only safe when the handler is SIG_DFL or SIG_IGN, because the kernel -// stores the trampoline but never invokes it in those cases. -type darwinSigactionNew struct { - handler uintptr - tramp uintptr - mask uint32 - flags int32 -} - -// darwinSigactionOld mirrors Darwin's userspace `struct sigaction`, which is the layout the -// sigaction(2) system call uses when reporting the previous disposition. -type darwinSigactionOld struct { - handler uintptr - mask uint32 - flags int32 -} - -// NeedsExecSignalDispositionWorkaround reports whether an exec'd child can inherit a SIGUSR1 -// disposition that is incompatible with other language runtimes. -// -// Darwin's execve(2) resets signal handlers to SIG_DFL but preserves sa_flags. Linux clears -// sa_flags along with the handler, and Windows has no signal dispositions at all. -func NeedsExecSignalDispositionWorkaround() bool { - return true -} - -// IsSIGUSR1Ignored reports whether SIGUSR1 currently has the SIG_IGN disposition. -func IsSIGUSR1Ignored() (bool, error) { - current, currentErr := signalDisposition(int(syscall.SIGUSR1)) - if currentErr != nil { - return false, fmt.Errorf("reading SIGUSR1 disposition: %w", currentErr) - } - - return current.handler == darwinSigIgn, nil -} - -// PrepareSIGUSR1ForExec gives SIGUSR1 the requested disposition with no trampoline, flags, or -// mask. The requested disposition must be captured before starting this Go process because the -// Go runtime may replace the inherited disposition during startup. -// -// Affected Go releases install handlers with SA_SIGINFO|SA_ONSTACK|SA_RESTART and restore them -// to SIG_DFL before exec without clearing those flags. Because Darwin's execve(2) preserves -// sa_flags, children can inherit SIG_DFL together with SA_SIGINFO. .NET misinterprets that as a -// handler being present and jumps to address zero when it first uses SIGUSR1 to suspend threads -// for a garbage collection. -// -// Resetting in a process that then forks is not sufficient, because the Go runtime restores its -// own dispositions in the forked child before it reaches execve. The reset has to happen in the -// process that calls exec, which is what the 'fork-process-exec' command exists to do. This -// workaround can be removed once DCP requires a Go release containing golang/go#81009. -func PrepareSIGUSR1ForExec(ignoredByCaller bool) error { - targetHandler := darwinSigDfl - if ignoredByCaller { - targetHandler = darwinSigIgn - } - - act := darwinSigactionNew{ - handler: targetHandler, - tramp: 0, - mask: 0, - flags: 0, - } - - if setErr := setSignalDisposition(int(syscall.SIGUSR1), &act); setErr != nil { - return fmt.Errorf("setting SIGUSR1 disposition for exec: %w", setErr) - } - - return nil -} - -func setSignalDisposition(sig int, act *darwinSigactionNew) error { - if _, _, errno := syscall.Syscall(syscall.SYS_SIGACTION, uintptr(sig), uintptr(unsafe.Pointer(act)), 0); errno != 0 { - return errno - } - - return nil -} - -// signalDisposition reports the current handler and flags for a signal. -func signalDisposition(sig int) (darwinSigactionOld, error) { - var current darwinSigactionOld - if _, _, errno := syscall.Syscall(syscall.SYS_SIGACTION, uintptr(sig), 0, uintptr(unsafe.Pointer(¤t))); errno != 0 { - return current, errno - } - - return current, nil -} diff --git a/pkg/process/signal_disposition_darwin_test.go b/pkg/process/signal_disposition_darwin_test.go deleted file mode 100644 index a9b6cc9e..00000000 --- a/pkg/process/signal_disposition_darwin_test.go +++ /dev/null @@ -1,171 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package process - -import ( - "os" - "os/exec" - "os/signal" - "syscall" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/microsoft/dcp/pkg/testutil" -) - -const ( - prepareSIGUSR1ForExecHelperEnvVar = "DCP_TEST_PREPARE_SIGUSR1_FOR_EXEC_HELPER" - prepareSIGUSR1DefaultMode = "default" - prepareSIGUSR1IgnoredMode = "ignored" - darwinTestSARestart int32 = 0x2 - dirtySignalMask uint32 = 1 -) - -func TestPrepareSIGUSR1ForExecUsesDefaultDisposition(t *testing.T) { - t.Parallel() - - runPrepareSIGUSR1ForExecHelper(t, prepareSIGUSR1DefaultMode) -} - -func TestPrepareSIGUSR1ForExecUsesIgnoredDisposition(t *testing.T) { - t.Parallel() - - runPrepareSIGUSR1ForExecHelper(t, prepareSIGUSR1IgnoredMode) -} - -func runPrepareSIGUSR1ForExecHelper(t *testing.T, mode string) { - t.Helper() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - helper := exec.CommandContext(testCtx, os.Args[0], "-test.run=^TestPrepareSIGUSR1ForExecHelper$", "-test.v") - helper.Env = append(os.Environ(), prepareSIGUSR1ForExecHelperEnvVar+"="+mode) - - output, runErr := helper.CombinedOutput() - require.NoError(t, runErr, "helper process failed; output:\n%s", output) -} - -func TestPrepareSIGUSR1ForExecHelper(t *testing.T) { - mode := os.Getenv(prepareSIGUSR1ForExecHelperEnvVar) - if mode == "" { - t.Skip("helper for TestPrepareSIGUSR1ForExec") - } - - switch mode { - case prepareSIGUSR1DefaultMode: - testPrepareSIGUSR1ForExecDefault(t) - case prepareSIGUSR1IgnoredMode: - testPrepareSIGUSR1ForExecIgnored(t) - default: - t.Fatalf("unknown helper mode %q", mode) - } -} - -func testPrepareSIGUSR1ForExecDefault(t *testing.T) { - t.Helper() - - initiallyIgnored, ignoredErr := IsSIGUSR1Ignored() - require.NoError(t, ignoredErr) - require.False(t, initiallyIgnored, "the Go test process should not start with SIGUSR1 ignored") - - // The requested target disposition, rather than the shim's current disposition, must win. - setDirtySIGUSR1Disposition(t, darwinSigIgn) - before := readSignalDispositions(t) - beforeSIGUSR1 := before[int(syscall.SIGUSR1)] - require.Equal(t, darwinSigIgn, beforeSIGUSR1.handler) - require.NotZero(t, beforeSIGUSR1.flags) - require.NotZero(t, beforeSIGUSR1.mask) - - require.NoError(t, PrepareSIGUSR1ForExec(false)) - - assertPreparedSIGUSR1Disposition(t, before, darwinSigDfl) -} - -func testPrepareSIGUSR1ForExecIgnored(t *testing.T) { - t.Helper() - - signal.Ignore(syscall.SIGUSR1) - ignored, ignoredErr := IsSIGUSR1Ignored() - require.NoError(t, ignoredErr) - require.True(t, ignored, "SIGUSR1 should be reported as ignored") - - // The requested target disposition, rather than the shim's current disposition, must win. - setDirtySIGUSR1Disposition(t, darwinSigDfl) - before := readSignalDispositions(t) - beforeSIGUSR1 := before[int(syscall.SIGUSR1)] - require.Equal(t, darwinSigDfl, beforeSIGUSR1.handler) - require.NotZero(t, beforeSIGUSR1.flags) - require.NotZero(t, beforeSIGUSR1.mask) - - require.NoError(t, PrepareSIGUSR1ForExec(true)) - - assertPreparedSIGUSR1Disposition(t, before, darwinSigIgn) -} - -func setDirtySIGUSR1Disposition(t *testing.T, handler uintptr) { - t.Helper() - - dirtyAction := darwinSigactionNew{ - handler: handler, - mask: dirtySignalMask, - flags: darwinTestSARestart, - } - setErr := setSignalDisposition(int(syscall.SIGUSR1), &dirtyAction) - require.NoError(t, setErr) -} - -func assertPreparedSIGUSR1Disposition( - t *testing.T, - before map[int]darwinSigactionOld, - expectedHandler uintptr, -) { - t.Helper() - - after := readSignalDispositions(t) - afterSIGUSR1 := after[int(syscall.SIGUSR1)] - require.Equal(t, expectedHandler, afterSIGUSR1.handler, "SIGUSR1 should have the requested handler") - require.Zero(t, afterSIGUSR1.flags, "SIGUSR1 should have no flags") - require.Zero(t, afterSIGUSR1.mask, "SIGUSR1 should have an empty mask") - require.Equal( - t, - before[int(syscall.SIGURG)], - after[int(syscall.SIGURG)], - "SIGURG disposition should remain unchanged", - ) - - for sig := 1; sig < darwinNumSignals; sig++ { - if sig == int(syscall.SIGKILL) || - sig == int(syscall.SIGSTOP) || - sig == int(syscall.SIGUSR1) || - sig == int(syscall.SIGURG) { - continue - } - - require.Equal(t, before[sig], after[sig], "signal %d disposition should remain unchanged", sig) - } -} - -func readSignalDispositions(t *testing.T) map[int]darwinSigactionOld { - t.Helper() - - dispositions := make(map[int]darwinSigactionOld, darwinNumSignals-3) - for sig := 1; sig < darwinNumSignals; sig++ { - if sig == int(syscall.SIGKILL) || sig == int(syscall.SIGSTOP) { - continue - } - - disposition, dispositionErr := signalDisposition(sig) - require.NoError(t, dispositionErr, "signal %d disposition should be readable", sig) - dispositions[sig] = disposition - } - - return dispositions -} diff --git a/pkg/process/signal_disposition_other.go b/pkg/process/signal_disposition_other.go deleted file mode 100644 index 30ab51c3..00000000 --- a/pkg/process/signal_disposition_other.go +++ /dev/null @@ -1,36 +0,0 @@ -//go:build !darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package process - -// NeedsExecSignalDispositionWorkaround reports whether an exec'd child can inherit a SIGUSR1 -// disposition that is incompatible with other language runtimes. -// -// Only Darwin is affected: its execve(2) resets signal handlers to SIG_DFL but preserves -// sa_flags. Linux clears sa_flags along with the handler, and Windows has no signal dispositions -// at all. -func NeedsExecSignalDispositionWorkaround() bool { - return false -} - -// InheritedSIGUSR1Ignored reports whether SIGUSR1 was ignored when this process started. It -// reports false on platforms that do not need the Darwin exec signal disposition workaround. -func InheritedSIGUSR1Ignored() (bool, error) { - return false, nil -} - -// IsSIGUSR1Ignored reports whether SIGUSR1 currently has the SIG_IGN disposition. It reports -// false on platforms that do not need the Darwin exec signal disposition workaround. -func IsSIGUSR1Ignored() (bool, error) { - return false, nil -} - -// PrepareSIGUSR1ForExec gives SIGUSR1 a disposition that is safe for an exec'd child. It is a -// no-op wherever NeedsExecSignalDispositionWorkaround reports false. -func PrepareSIGUSR1ForExec(_ bool) error { - return nil -} diff --git a/test/signaldisposition/main.go b/test/signaldisposition/main.go new file mode 100644 index 00000000..b4813140 --- /dev/null +++ b/test/signaldisposition/main.go @@ -0,0 +1,60 @@ +//go:build darwin && cgo + +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package main + +/* +#include + +static int initial_handler_is_default; +static int initial_has_siginfo; +static int initial_flags; + +__attribute__((constructor)) +static void capture_initial_sigusr1(void) { + struct sigaction action; + if (sigaction(SIGUSR1, NULL, &action) == 0) { + initial_handler_is_default = action.sa_handler == SIG_DFL; + initial_has_siginfo = (action.sa_flags & SA_SIGINFO) != 0; + initial_flags = action.sa_flags; + } +} + +static int get_initial_handler_is_default(void) { + return initial_handler_is_default; +} + +static int get_initial_has_siginfo(void) { + return initial_has_siginfo; +} + +static int get_initial_flags(void) { + return initial_flags; +} +*/ +import "C" + +import ( + "fmt" + "os" +) + +func main() { + handlerIsDefault := C.get_initial_handler_is_default() != 0 + hasSIGINFO := C.get_initial_has_siginfo() != 0 + flags := uint32(C.get_initial_flags()) + + if !handlerIsDefault || hasSIGINFO { + _, _ = fmt.Fprintf( + os.Stderr, + "SIGUSR1 at process startup: default=%t flags=%#x\n", + handlerIsDefault, + flags, + ) + os.Exit(1) + } +} From 3d66477630c055d70f91efcb778b92afd276b85b Mon Sep 17 00:00:00 2001 From: David Negstad Date: Mon, 21 Sep 2026 13:50:48 -0700 Subject: [PATCH 2/6] Fix direct Darwin process test runs Build the signal regression launcher with the runtime overlay and have it start the observer through OSExecutor. This keeps the regression effective while allowing direct go test and IDE runs to use an unpatched test binary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Makefile | 4 +-- pkg/process/os_executor_darwin_test.go | 22 ++++++-------- test/signaldisposition/main.go | 40 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index 310d42a0..cc7cb380 100644 --- a/Makefile +++ b/Makefile @@ -446,8 +446,8 @@ $(TERMCHILD_TOOL): $(wildcard ./test/termchild/*.go) $(GO_RUNTIME_OVERLAY_PREREQ ifeq ($(build_os),darwin) .PHONY: signal-disposition-tool signal-disposition-tool: $(SIGNAL_DISPOSITION_TOOL) -$(SIGNAL_DISPOSITION_TOOL): $(wildcard ./test/signaldisposition/*.go) | $(TOOL_BIN) - CGO_ENABLED=1 $(GO_BIN) build -o $(SIGNAL_DISPOSITION_TOOL) github.com/microsoft/dcp/test/signaldisposition +$(SIGNAL_DISPOSITION_TOOL): $(wildcard ./test/signaldisposition/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + CGO_ENABLED=1 $(GO_BIN) build -o $(SIGNAL_DISPOSITION_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/signaldisposition endif # lfwriter tool is used for testing lockfile package diff --git a/pkg/process/os_executor_darwin_test.go b/pkg/process/os_executor_darwin_test.go index 09fbdb7f..f014b5df 100644 --- a/pkg/process/os_executor_darwin_test.go +++ b/pkg/process/os_executor_darwin_test.go @@ -13,11 +13,9 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - int_testutil "github.com/microsoft/dcp/internal/testutil" - "github.com/microsoft/dcp/pkg/process" "github.com/microsoft/dcp/pkg/testutil" + "github.com/stretchr/testify/require" ) func TestStartProcessChildDoesNotInheritSIGINFOWithDefaultHandler(t *testing.T) { @@ -30,18 +28,14 @@ func TestStartProcessChildDoesNotInheritSIGINFOWithDefaultHandler(t *testing.T) "could not locate signal-disposition test tool (did you run `make test-prereqs`?)", ) - childCmd := exec.Command(signalDispositionTool) - var childOutput bytes.Buffer - childCmd.Stdout = &childOutput - childCmd.Stderr = &childOutput - - executor := process.NewOSExecutor(log) - defer executor.Dispose() - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) defer testCancel() - exitCode, runErr := process.RunToCompletion(testCtx, executor, childCmd) - require.NoError(t, runErr, "child process inherited an invalid signal disposition:\n%s", childOutput.String()) - require.Zero(t, exitCode, "child process reported an invalid signal disposition:\n%s", childOutput.String()) + launcherCmd := exec.CommandContext(testCtx, signalDispositionTool) + var launcherOutput bytes.Buffer + launcherCmd.Stdout = &launcherOutput + launcherCmd.Stderr = &launcherOutput + + runErr := launcherCmd.Run() + require.NoError(t, runErr, "child process inherited an invalid signal disposition:\n%s", launcherOutput.String()) } diff --git a/test/signaldisposition/main.go b/test/signaldisposition/main.go index b4813140..748ff0ca 100644 --- a/test/signaldisposition/main.go +++ b/test/signaldisposition/main.go @@ -39,11 +39,30 @@ static int get_initial_flags(void) { import "C" import ( + "context" "fmt" "os" + "os/exec" + "time" + + "github.com/go-logr/logr" + + "github.com/microsoft/dcp/pkg/process" ) +const childArgument = "child" + func main() { + if len(os.Args) == 1 { + runLauncher() + return + } + + if len(os.Args) != 2 || os.Args[1] != childArgument { + _, _ = fmt.Fprintf(os.Stderr, "unexpected arguments: %q\n", os.Args[1:]) + os.Exit(2) + } + handlerIsDefault := C.get_initial_handler_is_default() != 0 hasSIGINFO := C.get_initial_has_siginfo() != 0 flags := uint32(C.get_initial_flags()) @@ -58,3 +77,24 @@ func main() { os.Exit(1) } } + +func runLauncher() { + runCtx, runCancel := context.WithTimeout(context.Background(), 30*time.Second) + defer runCancel() + + childCmd := exec.Command(os.Args[0], childArgument) + childCmd.Stdout = os.Stdout + childCmd.Stderr = os.Stderr + + executor := process.NewOSExecutor(logr.Discard()) + defer executor.Dispose() + + exitCode, runErr := process.RunToCompletion(runCtx, executor, childCmd) + if runErr != nil { + _, _ = fmt.Fprintf(os.Stderr, "could not run signal disposition child: %v\n", runErr) + os.Exit(1) + } + if exitCode != 0 { + os.Exit(1) + } +} From 0d2f024dcff541581d53f5969e242a0c8540a1d5 Mon Sep 17 00:00:00 2001 From: David Negstad Date: Mon, 21 Sep 2026 14:11:07 -0700 Subject: [PATCH 3/6] Mark temporary Go overlay tests Document the generator transition checks and Darwin signal regression as temporary coverage to remove when DCP requires a Go release containing golang/go#81009. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- internal/tools/goruntimeoverlay/main_test.go | 2 ++ pkg/process/os_executor_darwin_test.go | 1 + 2 files changed, 3 insertions(+) diff --git a/internal/tools/goruntimeoverlay/main_test.go b/internal/tools/goruntimeoverlay/main_test.go index aabdef82..cd1a2b9d 100644 --- a/internal/tools/goruntimeoverlay/main_test.go +++ b/internal/tools/goruntimeoverlay/main_test.go @@ -101,6 +101,7 @@ func TestGenerateRuntimeOverlay(t *testing.T) { require.Equal(t, map[string]string{runtimeSourcePath: patchedSourcePath}, config.Replace) } +// Remove this test with the runtime overlay once DCP requires a Go release containing golang/go#81009. func TestGenerateRuntimeOverlayIsEmptyWhenToolchainContainsFix(t *testing.T) { t.Parallel() @@ -126,6 +127,7 @@ func TestGenerateRuntimeOverlayIsEmptyWhenToolchainContainsFix(t *testing.T) { require.Empty(t, config.Replace) } +// Remove this test with the runtime overlay once DCP requires a Go release containing golang/go#81009. func TestGenerateRuntimeOverlayRejectsUnexpectedSource(t *testing.T) { t.Parallel() diff --git a/pkg/process/os_executor_darwin_test.go b/pkg/process/os_executor_darwin_test.go index f014b5df..5091044f 100644 --- a/pkg/process/os_executor_darwin_test.go +++ b/pkg/process/os_executor_darwin_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" ) +// Remove this test with the runtime overlay once DCP requires a Go release containing golang/go#81009. func TestStartProcessChildDoesNotInheritSIGINFOWithDefaultHandler(t *testing.T) { t.Parallel() From 9cd45ce3397966682b33de025187e13c47064535 Mon Sep 17 00:00:00 2001 From: David Negstad Date: Mon, 21 Sep 2026 14:31:24 -0700 Subject: [PATCH 4/6] Avoid Darwin overlay for tunnel client The dcptun container client is built for Linux, so it does not need the Darwin runtime overlay or its generation prerequisite. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index cc7cb380..563887a9 100644 --- a/Makefile +++ b/Makefile @@ -312,11 +312,11 @@ $(DCP_BINARY): $(GO_SOURCES) go.mod $(GO_RUNTIME_OVERLAY_PREREQ) | ${OUTPUT_BIN} .PHONY: build-dcptun-containerexe build-dcptun-containerexe: $(DCPTUN_CLIENT_BINARY) ## Builds DCP reverse network tunnel client binary for Linux (to be used in containers) -$(DCPTUN_CLIENT_BINARY): $(GO_SOURCES) go.mod $(GO_RUNTIME_OVERLAY_PREREQ) | $(OUTPUT_BIN) +$(DCPTUN_CLIENT_BINARY): $(GO_SOURCES) go.mod | $(OUTPUT_BIN) ifeq ($(detected_OS),windows) - $$env:GOOS = "linux"; $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(GO_RUNTIME_OVERLAY_ARG) $(BUILD_ARGS) ./cmd/dcptun + $$env:GOOS = "linux"; $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(BUILD_ARGS) ./cmd/dcptun else - GOOS=linux $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(GO_RUNTIME_OVERLAY_ARG) $(BUILD_ARGS) ./cmd/dcptun + GOOS=linux $(GO_BIN) build -o $(DCPTUN_CLIENT_BINARY) $(BUILD_ARGS) ./cmd/dcptun endif .PHONY: clean From 2ea47ef92646f912c73efe540dddebf84150a92b Mon Sep 17 00:00:00 2001 From: David Negstad Date: Mon, 21 Sep 2026 14:42:57 -0700 Subject: [PATCH 5/6] Remove extra Makefile blank line Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index 563887a9..b5a682fd 100644 --- a/Makefile +++ b/Makefile @@ -401,7 +401,6 @@ $(TOOL_BIN): ifeq ($(build_os),darwin) .PHONY: force-go-runtime-overlay force-go-runtime-overlay: - # Apply the upstream fix for golang/go#81009 to the selected toolchain without # replacing unrelated standard-library source. $(GO_RUNTIME_OVERLAY_FILE): force-go-runtime-overlay $(wildcard ./internal/tools/goruntimeoverlay/*) | $(TOOL_BIN) From 8e251889f30790d23bb2a7cb77c461141201f172 Mon Sep 17 00:00:00 2001 From: David Negstad Date: Mon, 21 Sep 2026 15:17:22 -0700 Subject: [PATCH 6/6] Address runtime overlay review feedback Make Git patch application independent of line-ending configuration, distinguish Git launch failures, remove overlay arguments from Linux-only helper builds, and improve the signal regression launcher's timeout and self-exec handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Makefile | 12 +++++----- internal/tools/goruntimeoverlay/main.go | 30 ++++++++++++++++++++++-- pkg/process/os_executor_darwin_test.go | 14 +++++++++-- test/signaldisposition/main.go | 31 +++++++++++++++++++------ 4 files changed, 70 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index b5a682fd..dab93f4f 100644 --- a/Makefile +++ b/Makefile @@ -464,21 +464,21 @@ $(PARROT_TOOL): $(wildcard ./test/parrot/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $( # Builds a static parrot binary suitable for the scratch-based test container image. .PHONY: parrot-tool-containerexe parrot-tool-containerexe: $(PARROT_TOOL_CONTAINER_BINARY) -$(PARROT_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/parrot/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) +$(PARROT_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/parrot/*.go) | $(TOOL_BIN) ifeq ($(detected_OS),windows) - $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/parrot + $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/parrot else - CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/parrot + CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(PARROT_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/parrot endif # Builds a static probe binary for the scratch-based container conformance image. .PHONY: container-probe-tool-containerexe container-probe-tool-containerexe: $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) -$(CONTAINER_PROBE_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/containerprobe/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) +$(CONTAINER_PROBE_TOOL_CONTAINER_BINARY): Makefile $(wildcard ./test/containerprobe/*.go) | $(TOOL_BIN) ifeq ($(detected_OS),windows) - $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/containerprobe + $$env:CGO_ENABLED = "0"; $$env:GOOS = "linux"; $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/containerprobe else - CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/containerprobe + CGO_ENABLED=0 GOOS=linux $(GO_BIN) build -o $(CONTAINER_PROBE_TOOL_CONTAINER_BINARY) github.com/microsoft/dcp/test/containerprobe endif .PHONY: httpcontent-stream-repro diff --git a/internal/tools/goruntimeoverlay/main.go b/internal/tools/goruntimeoverlay/main.go index 4a3974cc..b2649ab0 100644 --- a/internal/tools/goruntimeoverlay/main.go +++ b/internal/tools/goruntimeoverlay/main.go @@ -138,11 +138,25 @@ func applyDarwinRuntimePatch(ctx context.Context, source []byte) ([]byte, bool, return patchedSource, true, nil } + if !gitApplyRejectedPatch(forwardCheckErr) { + return nil, false, fmt.Errorf( + "checking whether the upstream patch applies: %w: %s", + forwardCheckErr, + forwardCheckOutput, + ) + } reverseCheckOutput, reverseCheckErr := runGitApply(ctx, stagingDirectory, true, true) if reverseCheckErr == nil { return source, false, nil } + if !gitApplyRejectedPatch(reverseCheckErr) { + return nil, false, fmt.Errorf( + "checking whether the upstream patch was already applied: %w: %s", + reverseCheckErr, + reverseCheckOutput, + ) + } return nil, false, fmt.Errorf( "upstream patch applies neither forward nor in reverse (forward: %v: %s; reverse: %v: %s)", @@ -159,7 +173,13 @@ func runGitApply( reverse bool, check bool, ) ([]byte, error) { - args := []string{"-C", workingDirectory, "apply"} + args := []string{ + "-c", "core.autocrlf=false", + "-c", "core.eol=lf", + "-C", workingDirectory, + "apply", + "--whitespace=nowarn", + } if reverse { args = append(args, "--reverse") } @@ -169,11 +189,17 @@ func runGitApply( args = append(args, "-") applyCommand := exec.CommandContext(ctx, "git", args...) - applyCommand.Stdin = bytes.NewReader(darwinRuntimePatch) + normalizedPatch := bytes.ReplaceAll(darwinRuntimePatch, []byte("\r\n"), []byte("\n")) + applyCommand.Stdin = bytes.NewReader(normalizedPatch) output, applyErr := applyCommand.CombinedOutput() return output, applyErr } +func gitApplyRejectedPatch(applyErr error) bool { + var exitErr *exec.ExitError + return errors.As(applyErr, &exitErr) +} + func readFile(path string) ([]byte, error) { file, openErr := dcpio.OpenFileReadOnly(path) if openErr != nil { diff --git a/pkg/process/os_executor_darwin_test.go b/pkg/process/os_executor_darwin_test.go index 5091044f..e924f657 100644 --- a/pkg/process/os_executor_darwin_test.go +++ b/pkg/process/os_executor_darwin_test.go @@ -32,11 +32,21 @@ func TestStartProcessChildDoesNotInheritSIGINFOWithDefaultHandler(t *testing.T) testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) defer testCancel() - launcherCmd := exec.CommandContext(testCtx, signalDispositionTool) + testDeadline, haveTestDeadline := testCtx.Deadline() + require.True(t, haveTestDeadline, "signal disposition test context should have a deadline") + + launcherTimeout := time.Until(testDeadline) + require.Positive(t, launcherTimeout, "signal disposition test deadline should be in the future") + + launcherCmd := exec.CommandContext( + testCtx, + signalDispositionTool, + "--timeout", launcherTimeout.String(), + ) var launcherOutput bytes.Buffer launcherCmd.Stdout = &launcherOutput launcherCmd.Stderr = &launcherOutput runErr := launcherCmd.Run() - require.NoError(t, runErr, "child process inherited an invalid signal disposition:\n%s", launcherOutput.String()) + require.NoError(t, runErr, "signal disposition launcher failed:\n%s", launcherOutput.String()) } diff --git a/test/signaldisposition/main.go b/test/signaldisposition/main.go index 748ff0ca..867c3ea0 100644 --- a/test/signaldisposition/main.go +++ b/test/signaldisposition/main.go @@ -40,6 +40,7 @@ import "C" import ( "context" + "flag" "fmt" "os" "os/exec" @@ -53,16 +54,25 @@ import ( const childArgument = "child" func main() { - if len(os.Args) == 1 { - runLauncher() + if len(os.Args) == 2 && os.Args[1] == childArgument { + validateChildSignalDisposition() return } - if len(os.Args) != 2 || os.Args[1] != childArgument { - _, _ = fmt.Fprintf(os.Stderr, "unexpected arguments: %q\n", os.Args[1:]) + launcherFlags := flag.NewFlagSet("signal-disposition", flag.ContinueOnError) + launcherTimeout := launcherFlags.Duration("timeout", 30*time.Second, "Maximum time to wait for the child") + if parseErr := launcherFlags.Parse(os.Args[1:]); parseErr != nil { os.Exit(2) } + if launcherFlags.NArg() != 0 { + _, _ = fmt.Fprintf(os.Stderr, "unexpected arguments: %q\n", launcherFlags.Args()) + os.Exit(2) + } + + runLauncher(*launcherTimeout) +} +func validateChildSignalDisposition() { handlerIsDefault := C.get_initial_handler_is_default() != 0 hasSIGINFO := C.get_initial_has_siginfo() != 0 flags := uint32(C.get_initial_flags()) @@ -78,11 +88,17 @@ func main() { } } -func runLauncher() { - runCtx, runCancel := context.WithTimeout(context.Background(), 30*time.Second) +func runLauncher(timeout time.Duration) { + executablePath, executablePathErr := os.Executable() + if executablePathErr != nil { + _, _ = fmt.Fprintf(os.Stderr, "could not determine signal disposition executable path: %v\n", executablePathErr) + os.Exit(1) + } + + runCtx, runCancel := context.WithTimeout(context.Background(), timeout) defer runCancel() - childCmd := exec.Command(os.Args[0], childArgument) + childCmd := exec.Command(executablePath, childArgument) childCmd.Stdout = os.Stdout childCmd.Stderr = os.Stderr @@ -95,6 +111,7 @@ func runLauncher() { os.Exit(1) } if exitCode != 0 { + _, _ = fmt.Fprintf(os.Stderr, "signal disposition child exited with code %d\n", exitCode) os.Exit(1) } }