aboutsummaryrefslogtreecommitdiff
path: root/src/os/exec
AgeCommit message (Collapse)Author
2023-01-10os/exec: avoid leaking an exec.Cmd in TestWaitInterruptBryan C. Mills
In CL 436655 I added a GODEBUG setting to this test process to verify that Wait is eventually called for every exec.Cmd before it becomes unreachable. However, the cmdHang test helpers in TestWaitInterrupt/Exit-hang and TestWaitInterrupt/SIGKILL-hang intentially leak a subprocess in order to simulate a leaky third-party program, as Go users might encounter in practical use. To avoid tripping over the leak check, we call Wait on the leaked subprocess in a background goroutine. Since we expect the process running cmdHang to exit before its subprocess does, the call to Wait should have no effect beyond suppressing the leak check. Fixes #57596. Updates #52580. Updates #50436. Change-Id: Ia4b88ea47fc6b605c27ca6d9d7669c874867a900 Reviewed-on: https://go-review.googlesource.com/c/go/+/460998 Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-12-16os/exec: retry ETXTBSY errors in TestFindExecutableVsNoexecBryan C. Mills
I made this test parallel in CL 439196, which exposed it to the fork/exec race condition described in #22315. The ETXTBSY errors from that race should resolve on their own, so we can simply retry the call to get past them. Fixes #56811. Updates #22315. Change-Id: I2c6aa405bf3a1769d69cf08bf661a9e7f86440b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/458016 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-14internal/godebug: define more efficient APIRuss Cox
We have been expanding our use of GODEBUG for compatibility, and the current implementation forces a tradeoff between freshness and efficiency. It parses the environment variable in full each time it is called, which is expensive. But if clients cache the result, they won't respond to run-time GODEBUG changes, as happened with x509sha1 (#56436). This CL changes the GODEBUG API to provide efficient, up-to-date results. Instead of a single Get function, New returns a *godebug.Setting that itself has a Get method. Clients can save the result of New, which is no more expensive than errors.New, in a global variable, and then call that variable's Get method to get the value. Get costs only two atomic loads in the case where the variable hasn't changed since the last call. Unfortunately, these changes do require importing sync from godebug, which will mean that sync itself will never be able to use a GODEBUG setting. That doesn't seem like such a hardship. If it was really necessary, the runtime could pass a setting to package sync itself at startup, with the caveat that that setting, like the ones used by runtime itself, would not respond to run-time GODEBUG changes. Change-Id: I99a3acfa24fb2a692610af26a5d14bbc62c966ac Reviewed-on: https://go-review.googlesource.com/c/go/+/449504 Run-TryBot: Russ Cox <rsc@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-11os/exec: fix TestWaitInterrupt/WaitDelay error messagescott
As the comments say. Here we expect err to be nil instead of ctx.Err() Change-Id: I4cd02d62ac0a13c9577a567de36742f13d140d36 GitHub-Last-Rev: 6bedfbc9d2511140d088dc9ee5f40015725f68db GitHub-Pull-Request: golang/go#56698 Reviewed-on: https://go-review.googlesource.com/c/go/+/449737 Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-11-04os/exec: allow NUL in environment variables on Plan 9Matthew Dempsky
Plan 9 uses NUL as os.PathListSeparator, so it's almost always going to appear in the environment variable list. Exempt GOOS=plan9 from the check for NUL in environment variables. For #56284. Fixes #56544. Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a Reviewed-on: https://go-review.googlesource.com/c/go/+/447715 Reviewed-by: David du Colombier <0intro@gmail.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
2022-11-03os/exec: allow open descriptors to be closed during TestPipeLookPathLeakBryan C. Mills
In https://build.golang.org/log/d2eb315305bf3d513c490e7f85d56e9a016aacd2, we observe a failure in TestPipeLookPathLeak due to an additional descriptor (7) that was open at the start of the test being closed while the test executes. I haven't dug much into the failure, but it seems plausible to me that the descriptor may have been opened by libc for some reason, and may have been closed due to some sort of idle timeout or the completion of a background initialization routine. Since the test is looking for a leak, and closing an existing descriptor does not indicate a leak, let's not fail the test if an existing descriptor is unexpectedly closed. Updates #5071. Change-Id: I03973ddff6592c454cfcc790d6e56accd051dd52 Reviewed-on: https://go-review.googlesource.com/c/go/+/447235 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-01syscall, os/exec: reject environment variables containing NULsDamien Neil
Check for and reject environment variables containing NULs. The conventions for passing environment variables to subprocesses cause most or all systems to interpret a NUL as a separator. The syscall package rejects environment variables containing a NUL on most systems, but erroniously did not do so on Windows. This causes an environment variable such as "FOO=a\x00BAR=b" to be interpreted as "FOO=a", "BAR=b". Check for and reject NULs in environment variables passed to syscall.StartProcess on Windows. Add a redundant check to os/exec as extra insurance. Fixes #56284 Fixes CVE-2022-41716 Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434 Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com> TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/446916 Reviewed-by: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
2022-10-25os/exec: add the Cancel and WaitDelay fieldsBryan C. Mills
Fixes #50436. Change-Id: I9dff8caa317a04b7b2b605f810b8f12ef8ca485d Reviewed-on: https://go-review.googlesource.com/c/go/+/401835 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-13os/exec: reduce arbitrary sleeps in TestWaitidBryan C. Mills
If we use the "pipetest" helper command instead of "sleep", we can use its stdout pipe to determine when the process is ready to handle a SIGSTOP, and we can additionally check that sending a SIGCONT actually causes the process to continue. This also allows us to remove the "sleep" helper command, making the test file somewhat more concise. Noticed while looking into #50138. Change-Id: If4fdee4b1ddf28c6ed07ec3268c81b73c2600238 Reviewed-on: https://go-review.googlesource.com/c/go/+/442576 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-13os/exec: set traceback to "system" in TestContextCancelBryan C. Mills
This will dump more goroutines if the test happens to fail. For #50138. Change-Id: Ifae30b5ba8bddcdaa9250dd90be8d8ba7d5604d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/442476 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-11os/exec: remove protection against a duplicate Close on StdinPipeBryan C. Mills
As of CL 438347, multiple concurrents calls to Close should be safe. This removes some indirection and may also make some programs that use type-assertions marginally more efficient. For example, if a program calls (*exec.Cmd).StdinPipe to obtain a pipe and then sets that as the Stdout of another command, that program will now allow the second command to inherit the file descriptor directly instead of copying everything through a goroutine. This will also cause calls to Close after the first to return an error wrapping os.ErrClosed instead of nil. However, it seems unlikely that programs will depend on that error behavior: if a program is calling Write in a loop followed by Close, then if a racing Close occurs it is likely that the Write would have already reported an error. (The only programs likely to notice a change are those that call Close — without Write! — after a call to Wait.) Updates #56043. Updates #9307. Updates #6270. Change-Id: Iec734b23acefcc7e7ad0c8bc720085bc45988efb Reviewed-on: https://go-review.googlesource.com/c/go/+/439195 Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-07os/exec: document ProcessState available after a call to Wait or Runhopehook
Wait or Run will populate its ProcessState when the command completes. Fixes #56002. Change-Id: I21547431f5d2d3e0fc0734fd1705421a0ac4209c Reviewed-on: https://go-review.googlesource.com/c/go/+/437996 Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-06os/exec: parallelize more testsBryan C. Mills
This cuts the wall duration for 'go test os/exec' and 'go test -race os/exec' roughly in half on my machine, which is an even more significant speedup with a high '-count'. For better or for worse, it may also increase the repro rate of #34988. Tests that use Setenv or Chdir or check for FDs opened during the test still cannot be parallelized, but they are only a few of those. Change-Id: I8d284d8bff05787853f825ef144aeb7a4126847f Reviewed-on: https://go-review.googlesource.com/c/go/+/439196 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-06os/exec: delete TestExtraFilesFDShuffleBryan C. Mills
This test has been disabled for over nine years (since CL 12869049). Although it still compiles, it seems likely to have rotted since then, and if it was going to detect a real bug it also seems like that bug would have been encountered and reported by users since then (and would presumably have its own regression tests). To eliminate overhead from mainining it (or skipping over it while maintaining other tests), let's just delete it. Fixes #5780. Change-Id: I2a85cba20cba98a1dc6fc82336ae5e22d2242e99 Reviewed-on: https://go-review.googlesource.com/c/go/+/439197 Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-04os/exec: add a GODEBUG setting to diagnose leaked processesBryan C. Mills
Updates #52580. For #50436. Change-Id: I669f13863f1f85d576c3c94500b118e6989000eb Reviewed-on: https://go-review.googlesource.com/c/go/+/436655 Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
2022-10-04Revert "os/exec: make StdoutPipe and StderrPipe safe to Close concurrently"Bryan Mills
This reverts CL 437176. Reason for revert: broke programs that plumb StdoutPipe from one command to Stdin on another and then call Wait on the former. os/exec itself uses a type-assertion to *os.File to determine whether to copy stdin using a goroutine or just pass a file descriptor. An early Wait using a *os.File is benign (because closing the pipe doesn't close the child's inherited file descriptor), but an early Wait using a non-*os.File is not. Updates #50436. Change-Id: I4a2993e290982834f91696d890dfe77364c0cc50 Reviewed-on: https://go-review.googlesource.com/c/go/+/438695 Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-01os/exec: make StdoutPipe and StderrPipe safe to Close concurrentlyBryan C. Mills
For #50436, I want to be able to close the pipes returned by StdoutPipe and StderrPipe after the Context has been canceled and the WaitDelay has subsequently expired. However, the fact that the exec.onceCloser wrapper for StdinPipe (added in CL 13329043) was retained in CL 65490 suggests to me that (*os.File).Close is still not safe to call concurrently. This may cause type assertions of these ReadClosers to *os.File that once succeeded to no longer do so. However, the StdoutPipe and StderrPipe methods return interfaces, not concrete *os.Files, so callers already should not have been relying on that implementation detail — and as far as I can tell the closeOnce wrapper does not mask any (*os.File) methods, so assertions to any interface type that previously succeeded will continue to do so. This change is logically part of CL 401835, but since it may expose fragile type-assertions in callers I want to keep it separate for clearer bisection of any new test failures. For #50436. Change-Id: I58de1d48fb6fd788502f13657d8d4484516271cf Reviewed-on: https://go-review.googlesource.com/c/go/+/437176 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-01os/exec: recombine goroutinePipes and userPipesBryan C. Mills
This change undoes CL 401894, because on further consideration it turns out not to be needed. This also makes (*Cmd).closeDescriptors a free function, since it does not actually use the receiver in any way and is not needed to satisfy any interfaces. For #50436. Change-Id: I7915265b0e6398ed5a34fae0c12873ab08a14194 Reviewed-on: https://go-review.googlesource.com/c/go/+/437175 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-09-29os/exec: avoid leaking a process in TestDoubleStartLeavesPipesOpenBryan C. Mills
Updates #52580. For #50436. Change-Id: I0929055ffca1ca429f6ebec7d877f4268bd1fda2 Reviewed-on: https://go-review.googlesource.com/c/go/+/436656 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
2022-09-29os/exec: refactor goroutine error reportingBryan C. Mills
Use a separate channel to report the final error of the copying goroutines, receiving a value only when all of the copying goroutines have completed. In a followup change (CL 401835), that will allow waiters to select on goroutine completion alongside other events (such as Context cancellation). Also mildly refactor the watchCtx helper method so that its structure better matches what will be needed to implement WaitDelay. For #50436. Change-Id: I54b3997fb6931d204814d8382f0a388a67b520f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/435995 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
2022-09-29os/exec: do not close pipes on a double-Start errorBryan C. Mills
This fixes a bug introduced in CL 401834 in which calling Start twice with pipes attached to a command would spuriously close those pipes when returning the error from the second Start call. For #50436. Change-Id: I3563cc99c0a0987752190fef95da3e9927a76fda Reviewed-on: https://go-review.googlesource.com/c/go/+/436095 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-09-28all: fix some typoscui fliter
Change-Id: I8b28aebbb9494b2c877139a4584a5a42253e3bea GitHub-Last-Rev: e3703fd3a50b811785df75751472aa3ab098b3d1 GitHub-Pull-Request: golang/go#55902 Reviewed-on: https://go-review.googlesource.com/c/go/+/435617 Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-09-26os/exec: split parent I/O pipes by whether they are pumped by user code or ↵Bryan C. Mills
internal goroutines The pipes pumped by goroutines can be closed as soon as their respective goroutines are done. The pipes pumped by user code, however, are documented to be closed in Wait. When we add the WaitDelay field, it isn't obvious that we should terminate the user-pumped pipes when the WaitDelay expires, since Wait itself isn't going to wait for those user-controlled goroutines to complete. (It's a bit more complicated than that because the documentation currently states that Wait must not be called while the pipes are being read — but it isn't obvious to me that that advice is entirely correct.) For #50436. Change-Id: I97909c91d2097fb75138a360747168c08609696d Reviewed-on: https://go-review.googlesource.com/c/go/+/401894 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
2022-09-26os/exec: clean up pipe-closing logicBryan C. Mills
Change the childFiles field to a local variable, since it was populated during Start and (as far as I can determine) has no purpose after Start returns. Rename closeAfterStart and closeAfterWait to childIOFiles and parentIOPipes respectively. That makes their contents clearer, and also helps to clarify what should happen on error (when, for example, Wait shouldn't be called at all). Use a deferred call instead of individual calls to close child (and, if necessary, pipe) FDs after Start. That helps to clarify the invariants around when they are closed, and also makes the function a bit more robust for future refactoring. Also nil out the slices containing the file closers so that they can be collected earlier. This CL is intended as a pure refactor in preparation for #50436. Change-Id: I05d13fa91d539b95b84b2ba923c1733f9a6203e5 Reviewed-on: https://go-review.googlesource.com/c/go/+/401834 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
2022-09-07os/exec: skip TestFindExecutableVsNoexec if mount failedKir Kolyshkin
Apparently, some testing environments do not allow root to mount tmpfs (due to e.g. AppArmor profile disallowing mount(2) syscall). Always skip the test if the mount has failed. Fixes the test issue introduced by CL 414824. Change-Id: Ic565d2e6f277f2926d85a351be7df2498ffba656 Reviewed-on: https://go-review.googlesource.com/c/go/+/429175 Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2022-09-07os: use strings.Buildercuiweixie
Change-Id: I610509aa35c345ff7fbb1fc94bf177ffbe934731 Reviewed-on: https://go-review.googlesource.com/c/go/+/428274 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
2022-09-07os/exec: LookPath: use eaccess for exec check on linuxKir Kolyshkin
Having an executable bit set for a binary is not enough for it to be executable -- there might be more checks in the kernel. For example, binaries on a filesystem mounted with "noexec" flag couldn't be executed. There might be other scenarios involving ACLs, SELinux, file capabilities, and so on. As a result, LookPath might either find a non-executable (while going over $PATH elements), or return a false positive that the argument provided is an executable. One possible fix would be to perform the check by using access(2) syscall with X_OK flag. Now, since access(2) uses real (rather than effective) uid and gid, when used by a setuid or setgid binary, it checks permissions of the (real) user who started the binary, rather than the actual effective permissions. Therefore, using access with X_OK won't work as expected for setuid/setgid binaries. To fix this, modern platforms added ways to check against effective uid and gid, with the most common being the faccessat(2) call with the AT_EACCESS flag, as described by POSIX.1-2008 (in Linux, only faccessat2(2) supports flags such as AT_EACCESS). Let's use it, and fall back to checking permission bits if faccessat is not available. Wrap the logic into unix.Eaccess, which is currently only implemented on Linux. While many other OSes (Free/Net/OpenBSD, AIX, Solaris/Illumos, and Darwin) do implement faccessat(2) with AT_EACCESS, it is not wired in syscall package (except for AIX), so these platforms are left out for now. In the future, eaccess can be implemented for these OSes, too. Alas, a call to unix.Eaccess is not enough since we have to filter out directories, so use both stat and Eaccess. One minor change introduced by this commit is that LookPath and Command now returns "is a directory" error when the argument contains a slash and is a directory. This is similar to what e.g. bash does on Linux: $ bash -c /etc bash: line 1: /etc: Is a directory Add a test case, which, unfortunately, requires root, is specific to Linux, and needs a relatively new kernel (supporting faccessat2). Other platforms either have different semantics for tmpfs with noexec, or have different ways to set up a binary which has x bit set but nevertheless could not be executed. Change-Id: If49b6ef6bf4dd23b2c32bebec8832d83e511a4bb Reviewed-on: https://go-review.googlesource.com/c/go/+/414824 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
2022-07-28os/exec: add GODEBUG setting to opt out of ErrDot changesRuss Cox
The changes are likely to break users, and we need to make it easy to unbreak without code changes. For #43724. Fixes #53962. Change-Id: I105c5d6c801d354467e0cefd268189c18846858e Reviewed-on: https://go-review.googlesource.com/c/go/+/419794 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Russ Cox <rsc@golang.org>
2022-07-06os/exec: clarify that Wait must be calledIan Lance Taylor
Fixes #52580 Change-Id: Ib2dd8a793b9c6fcb083abb3f7c346f6279adefc9 Reviewed-on: https://go-review.googlesource.com/c/go/+/414056 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-06-28os/exec: on Windows, suppress ErrDot if the implicit path matches the ↵Bryan C. Mills
explicit one If the current directory is also listed explicitly in %PATH%, this changes the behavior of LookPath to prefer the explicit name for it (and thereby avoid ErrDot). However, in order to avoid running a different executable from what would have been run by previous Go versions, we still return the implicit path (and ErrDot) if it refers to a different file entirely. Fixes #53536. Updates #43724. Change-Id: I7ab01074e21a0e8b07a176e3bc6d3b8cf0c873cd Reviewed-on: https://go-review.googlesource.com/c/go/+/414054 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-05-10os/exec: eliminate some arbitrary short timeoutsBryan C. Mills
These tests appear to be using timeouts to check for deadlocks or to cause the test to fail earlier. However, on slower machines these short timeouts can cause spurious failures, and even on faster machines if the test locks up we usually want a goroutine dump instead of a short failure message anyway. Fixes #52818 (maybe). Change-Id: Ib8f18d679f9443721e8a924caef6dc8d214fca1b Reviewed-on: https://go-review.googlesource.com/c/go/+/405434 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-05-10os/exec: return clear error for missing cmd.PathRuss Cox
Following up on CL 403694, there is a bit of confusion about when Path is and isn't set, along with now the exported Err field. Catch the case where Path and Err (and lookPathErr) are all unset and give a helpful error. Fixes #52574 Followup after #43724. Change-Id: I03205172aef3801c3194f5098bdb93290c02b1b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/403759 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
2022-05-06os/exec: refactor goroutine communication in WaitBryan C. Mills
This provides clearer synchronization invariants: if it occurs at all, the call to c.Process.Kill always occurs before Wait returns. It also allows any unexpected errors from the goroutine to be propagated back to Wait. For #50436. Change-Id: I7ddadc73e6e67399596e35393f5845646f6111ab Reviewed-on: https://go-review.googlesource.com/c/go/+/401896 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-05-03os/exec: in Command, update cmd.Path even if LookPath returns an errorBryan C. Mills
Fixes #52666. Updates #43724. Updates #43947. Change-Id: I72cb585036b7e93cd7adbff318b400586ea97bd5 Reviewed-on: https://go-review.googlesource.com/c/go/+/403694 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-05-02os/exec: return error when PATH lookup would use current directoryRuss Cox
CL 381374 was reverted because x/sys/execabs broke. This CL reapplies CL 381374, but adding a lookPathErr error field back, for execabs to manipulate with reflect. That field will just be a bit of scar tissue in this package forever, to keep old code working with new toolchains. CL 403256 fixes x/sys/execabs's test to be ready for the change. Older versions of x/sys/execabs will keep working (that is, will keep rejecting what they should reject), but they will return a slightly different error from LookPath without that CL, and the test fails because of the different error text. For #43724. This reverts commit f2b674756b3b684118e4245627d4ed8c07e518e7. Change-Id: Iee55f8cd9939e1bd31e5cbdada50681cdc505117 Reviewed-on: https://go-review.googlesource.com/c/go/+/403274 Auto-Submit: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-29Revert "os/exec: return error when PATH lookup would use current directory"Bryan Mills
This reverts CL 381374. Reason for revert: broke tests for x/sys/execabs. Updates #43724. Updates #43947. Change-Id: I9eb3adb5728dead66dbd20f6afe1e7a77e2a26f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/403058 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-04-29os/exec: return error when PATH lookup would use current directoryRuss Cox
Following discussion on #43724, change os/exec to take the approach of golang.org/x/sys/execabs, refusing to respect path entries mentioning relative paths by default. Code that insists on being able to find executables in relative directories in the path will need to add a couple lines to override the error. See the updated package docs in exec.go for more details. Fixes #43724. Fixes #43947. Change-Id: I73c1214f322b60b4167a23e956e933d50470fe13 Reviewed-on: https://go-review.googlesource.com/c/go/+/381374 Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com>
2022-04-26os/exec: use a TestMain to avoid hijacking stdout for helper commandsBryan C. Mills
The previous implementation of helperCommand relied on running a well-known Test function which implemented all known commands. That not only added Skip noise in the test's output, but also (and more importantly) meant that the commands could not write directly to stdout in the usual way, since the testing package hijacks os.Stdout for its own use. The new implementation addresses the above issues, and also ensures that all registered commands are actually used, reducing the risk of an unused command sticking around after refactoring. It also sets the subprocess environment variable directly in the test process, instead of on each individual helper command's Env field, allowing helper commands to be used without an explicit Env. Updates #50599. (Also for #50436.) Change-Id: I189c7bed9a07cfe47a084b657b88575b1ee370b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/401934 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-26os/exec: make skipStdinCopyError a function instead of a variableBryan C. Mills
This makes clearer that skipStdinCopyError is always defined and never overridden in tests. Secondarily, it may also help reduce init-time work and allow the linker and/or inliner to better optimize this package. (Noticed while prototyping #50436.) Change-Id: I4f3c1bc146384a98136a4039f82165ed106c14b8 Reviewed-on: https://go-review.googlesource.com/c/go/+/401897 Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-25os/exec: in TestImplicitPWD, explicitly request the logical pathBryan C. Mills
Fixes #52537 Change-Id: I70959881a31f425e940e7adf86b36be2596aafb7 Reviewed-on: https://go-review.googlesource.com/c/go/+/402158 Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-21os/exec: set PWD implicitly if Dir is non-empty and Env is nilBryan C. Mills
Fixes #50599. Change-Id: I4e5dbb3972cdf21ede049567bfb98f2c992c5849 Reviewed-on: https://go-review.googlesource.com/c/go/+/401340 Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-21os/exec: preserve original order of entries in dedupEnvBryan C. Mills
Once #50599 is implemented, the entries will be observable via the Environ method. I find it confusing for later entries in the list to jump arbitrarily far forward based on entries for the same key that no longer exist. This also fixes the deduplication logic for the degenerate Windows keys observed in #49886, which were previously deduplicated as empty keys. (It does not do anything about the even-more-degenerate keys observed in #52436.) For #50599. Change-Id: Ia7cd2200ec34ccc4b9d18631cb513194dc420c25 Reviewed-on: https://go-review.googlesource.com/c/go/+/401339 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-04Revert "os: add handling of os.Interrupt for windows"Bryan Mills
This reverts CL 367495. Reason for revert: broke `x/tools` tests on Windows. Change-Id: Iab6b33259181c9520cf8db1e5b6edfeba763f974 Reviewed-on: https://go-review.googlesource.com/c/go/+/397997 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-04-03os: add handling of os.Interrupt for windowsConstantin Konstantinidis
Add GenerateConsoleCtrlEvent call to internal syscall package. Define ErrProcessDone while reviewing handling of os.Signal(). Update test to run for windows using the added call. Fixes #42311 Fixes #46354 Change-Id: I460955efc76c4febe04b612ac9a0670e62ba5ff3 Reviewed-on: https://go-review.googlesource.com/c/go/+/367495 Trust: Patrik Nyblom <pnyb@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-03-29all: use new "unix" build tag where appropriateIan Lance Taylor
For #20322 For #51572 Change-Id: Id0b4799d097d01128e98ba4cc0092298357bca45 Reviewed-on: https://go-review.googlesource.com/c/go/+/389935 Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
2021-12-17os/exec: skip TestContextCancel on netbsd/arm64Bryan C. Mills
For #42061 Change-Id: I3b4c774ad9e375d4bfef1cfb4336c35ed30a6430 Reviewed-on: https://go-review.googlesource.com/c/go/+/372795 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-12-13all: gofmt -w -r 'interface{} -> any' srcRuss Cox
And then revert the bootstrap cmd directories and certain testdata. And adjust tests as needed. Not reverting the changes in std that are bootstrapped, because some of those changes would appear in API docs, and we want to use any consistently. Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories when preparing the bootstrap copy. A few files changed as a result of running gofmt -w not because of interface{} -> any but because they hadn't been updated for the new //go:build lines. Fixes #49884. Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09 Reviewed-on: https://go-review.googlesource.com/c/go/+/368254 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2021-12-02os/exec: in TestContextCancel, dump goroutines on failureBryan C. Mills
If this test fails, we want to know exactly what the os/exec goroutines are doing. Panicking gives us a goroutine dump, whereas t.Fatal does not. While we're here, use exponential backoff instead of a hard-coded 1ms sleep. We want to give the OS enough time to actually terminate the subprocess. For #42061 Change-Id: I3d50a71ac314853c68a935218e7f97ce18b08b5b Reviewed-on: https://go-review.googlesource.com/c/go/+/368317 Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-11-16os/exec: avoid NewFile on unknown FDsMichael Pratt
exec_test.go's init function uses os.NewFile(fd) + f.Stat as a portable mechanism to determine if an FD is in use. Unfortunately, the current use is racy: if an unused FD becomes used between NewFile and f.Close, then we will unintentionally close an FD we do not use. We cannot simply drop Close, as the finalizer will close the FD. We could hold all of the os.Files in a global for the lifetime of the process, but the need for such a hack is indicative of the larger problem: we should not create an os.File for an FD that we do not own. Instead, the new fdtest.Exists provides a helper that performs the equivalent of fstat(2) on each OS to determine if the FD is valid, without using os.File. We also reuse this helper on a variety of other tests that look at open FDs. Fixes #49533 Change-Id: I36e2bdb15f271ab01e55c18db6564271995a15af Reviewed-on: https://go-review.googlesource.com/c/go/+/364035 Trust: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-10-28all: go fix -fix=buildtag std cmd (except for bootstrap deps, vendor)Russ Cox
When these packages are released as part of Go 1.18, Go 1.16 will no longer be supported, so we can remove the +build tags in these files. Ran go fix -fix=buildtag std cmd and then reverted the bootstrapDirs as defined in src/cmd/dist/buildtool.go, which need to continue to build with Go 1.4 for now. Also reverted src/vendor and src/cmd/vendor, which will need to be updated in their own repos first. Manual changes in runtime/pprof/mprof_test.go to adjust line numbers. For #41184. Change-Id: Ic0f93f7091295b6abc76ed5cd6e6746e1280861e Reviewed-on: https://go-review.googlesource.com/c/go/+/344955 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
generated by cgit v1.3 (git 2.53.0) at 2026-04-15 12:42:08 +0000