aboutsummaryrefslogtreecommitdiff
path: root/src/os
AgeCommit message (Collapse)Author
2023-01-25os: eliminate arbitrary timeout in testClosewithBlockingReadBryan C. Mills
The 1-second timeout on execution of this test is empirically too short on some platforms. Rather than trying to tune the timeout, allow the test to time out on its own (and dump goroutines) if it deadlocks. Fixes #57993. Fixes #57994. Change-Id: I69ee86c75034469e4b4cd391b8dc5616b93468b8 Reviewed-on: https://go-review.googlesource.com/c/go/+/463180 Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com>
2023-01-25os: make Lstat for symlinks on Windows consistent with POSIXBryan C. Mills
This also makes path/filepath.Walk more consistent between Windows and POSIX platforms. According to https://pubs.opengroup.org/onlinepubs/9699919799.2013edition/basedefs/V1_chap04.html#tag_04_12 symlinks in a path that includes a trailing slash must be resolved before a function acts on that path. POSIX defines an lstat function, whereas the Win32 API does not, so Go's os.Lstat should follow the (defined) POSIX semantics instead of doing something arbitrarily different. CL 134195 added a test for the correct POSIX behavior when os.Lstat is called on a symlink. However, the test turned out to be broken on Windows, and when it was fixed (in CL 143578) it was fixed with different Lstat behavior on Windows than on all other platforms that support symlinks. In #50807 we are attempting to provide consistent symlink behavior for cmd/go. This unnecessary platform difference, if left uncorrected, will make that fix much more difficult. CL 460595 reworked the implementation of Stat and Lstat on Windows, and with the new implementation this fix is straightforward. For #50807. Updates #27225. Change-Id: Ia28821aa4aab6cefa021da2d9b803506cdb2621b Reviewed-on: https://go-review.googlesource.com/c/go/+/463177 Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com>
2023-01-25os: have RemoveAll loop on EINTRNigel Tao
Fixes #57966 Change-Id: Ia732d499ff9bd6e70030daab8fac42d1e204be37 Reviewed-on: https://go-review.googlesource.com/c/go/+/463076 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Nigel Tao <nigeltao@golang.org> Reviewed-by: Nigel Tao (INACTIVE; USE @golang.org INSTEAD) <nigeltao@google.com>
2023-01-24os: use handle based APIs to read directories on windowsqmuntal
This CL updates File.readdir() on windows so it uses GetFileInformationByHandleEx with FILE_ID_BOTH_DIR_INFO instead of Find* APIs. The former is more performant because it allows us to buffer IO calls and reduces the number of system calls, passing from 1 per file to 1 every ~100 files (depending on the size of the file name and the size of the buffer). This change improve performance of File.ReadDir by 20-30%. name old time/op new time/op delta ReadDir-12 562µs ±14% 385µs ± 9% -31.60% (p=0.000 n=9+9) name old alloc/op new alloc/op delta ReadDir-12 29.7kB ± 0% 29.5kB ± 0% -0.88% (p=0.000 n=8+10) name old allocs/op new allocs/op delta ReadDir-12 399 ± 0% 397 ± 0% -0.50% (p=0.000 n=10+10) This change also speeds up calls to os.SameFile when using FileStats returned from File.readdir(), as their file ID can be inferred while reading the directory. Change-Id: Id56a338ee66c39656b564105cac131099218fb5d Reviewed-on: https://go-review.googlesource.com/c/go/+/452995 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com>
2023-01-23os: treat non-symlink reparse points as irregular filesBryan C. Mills
Prior to this change (as of CL 143578), our stat function attempted to resolve all reparse points as if they were symlinks. This results in an additional call to CreateFile when statting a symlink file: we use CreateFile once to obtain the reparse tag and check whether the file is actually a symlink, and if it is we call CreateFile again without FILE_FLAG_OPEN_REPARSE_POINT to stat the link target. Fortunately, since symlinks are rare on Windows that overhead shouldn't be a big deal in practice. Fixes #42919. Change-Id: If453930c6e98040cd6525ac4aea60a84498c9579 Reviewed-on: https://go-review.googlesource.com/c/go/+/460595 Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2023-01-19internal/godebug: export non-default-behavior counters in runtime/metricsRuss Cox
Allow GODEBUG users to report how many times a setting resulted in non-default behavior. Record non-default-behaviors for all existing GODEBUGs. Also rework tests to ensure that runtime is in sync with runtime/metrics.All, and generate docs mechanically from metrics.All. For #56986. Change-Id: Iefa1213e2a5c3f19ea16cd53298c487952ef05a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/453618 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2023-01-19os: clean up testsBryan C. Mills
- Use testenv.Command instead of exec.Command to try to get more useful timeout behavior. - Parallelize tests that appear not to require global state. (And add explanatory comments for a few that are not parallelizable for subtle reasons.) - Consolidate some “Helper” tests with their parent tests. - Use t.TempDir instead of os.MkdirTemp when appropriate. - Factor out subtests for repeated test helpers. For #36107. Updates #22315. Change-Id: Ic24b6957094dcd40908a59f48e44c8993729222b Reviewed-on: https://go-review.googlesource.com/c/go/+/458015 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>
2023-01-19os: deflake TestPipeEOF and TestFifoEOFBryan C. Mills
- Consolidate the two test bodies as one helper function. - Eliminate arbitrary timeout. - Shorten arbitrary sleeps in short mode. - Simplify goroutines. - Mark the tests as parallel. Fixes #36107. Updates #24164. Change-Id: I14fe4395963a7256cb6d2d743d348a1ade077d5b Reviewed-on: https://go-review.googlesource.com/c/go/+/457336 Reviewed-by: Rob Pike <r@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2023-01-17os: document that Rename is not atomic on non-Unix platformsAlan Donovan
Windows provides no reliable way to rename files atomically. The Plan 9 implementation of os.Rename performs a deletion if the target exists. Change-Id: Ife5f9c97b21f48c11e300cd76d8c7f715db09fd4 Reviewed-on: https://go-review.googlesource.com/c/go/+/462395 Auto-Submit: Alan Donovan <adonovan@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Alan Donovan <adonovan@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
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-19os: reenable TestReaddirSmallSeek on windowsqmuntal
TestReaddirSmallSeek should have been reenabled as part of CL 405275, but didn't. Do it now. Updates #36019 Change-Id: I5676eee4e63675d30e9d48ac708e72bd036b6aee Reviewed-on: https://go-review.googlesource.com/c/go/+/458336 Reviewed-by: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
2022-12-16os/user,net: add -fno-stack-protector to CFLAGSThan McIntosh
Some compilers default to having -fstack-protector on, which breaks when using internal linking because the linker doesn't know how to find the support functions. Updates #52919. Updates #54313. Fixes #57261. Change-Id: Iaae731851407af4521fff2dfefc5b7e3e92cf284 Reviewed-on: https://go-review.googlesource.com/c/go/+/456855 Run-TryBot: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
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-12-10os: skip size test in TestLstat if the file is a symlinkIan Lance Taylor
Tested by temporarily changing sysdir to use a directory where the expected files were all symlinks. We should consider using a different approach that doesn't rely on sysdir, but for now do a minimal fix. Fixes #57210 Change-Id: Ifb1becef03e014ceb48290ce13527b3e103c0e07 Reviewed-on: https://go-review.googlesource.com/c/go/+/456557 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-12-09os/user: zero-initialize C structs returned to GoBryan C. Mills
In the wrappers for getgrnam_r and similar, the structs to be returned are allocated on the C stack and may be uninitialized. If the call to the wrapped C function returns an error (such as ERANGE), it may leave the struct uninitialized, expecting that the caller will not read it. However, when that struct is returned to Go, it may be read by the Go garbage collector. If the uninitialized struct fields happen to contain wild pointers, the Go garbage collector will throw an error. (Prior to CL 449335, the Go runtime would not scan the struct fields because they did not reside in Go memory.) Fix this by always zeroing the struct before the C call. Fixes #57170. Change-Id: I241ae8e4added6f9a406dac37a7f6452341aa0cf Reviewed-on: https://go-review.googlesource.com/c/go/+/456121 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-12-08os/user: on AIX getpwuid_r seems to return -1 on overflowIan Lance Taylor
The getpwuid_r function is expected to return ERANGE on overflow. Accept -1 on AIX as we see that in practice. This problem was uncovered by, but not caused by, CL 455815, which introduced a test that forced a buffer overflow. Change-Id: I3ae94faf1257d2c73299b1478e49769bb807fc4d Reviewed-on: https://go-review.googlesource.com/c/go/+/456075 Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-12-07os/user: fix buffer retry loop on macOSRuss Cox
getpwnam_r and friends return the errno as the result, not in the global errno. The code changes in CL 449316 inadvertently started using the global errno. So if a lookup didn't fit in the first buffer size, it was treated as not found instead of growing the buffer. Fixes #56942. Change-Id: Ic5904fbeb31161bccd858e5adb987e919fb3e9d9 Reviewed-on: https://go-review.googlesource.com/c/go/+/455815 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Russ Cox <rsc@golang.org>
2022-12-06os, net/http: avoid escapes from os.DirFS and http.Dir on WindowsDamien Neil
Do not permit access to Windows reserved device names (NUL, COM1, etc.) via os.DirFS and http.Dir filesystems. Avoid escapes from os.DirFS(`\`) on Windows. DirFS would join the the root to the relative path with a path separator, making os.DirFS(`\`).Open(`/foo/bar`) open the path `\\foo\bar`, which is a UNC name. Not only does this not open the intended file, but permits reference to any file on the system rather than only files on the current drive. Make os.DirFS("") invalid, with all file access failing. Previously, a root of "" was interpreted as "/", which is surprising and probably unintentional. Fixes CVE-2022-41720 Fixes #56694 Change-Id: I275b5fa391e6ad7404309ea98ccc97405942e0f0 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1663834 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Julie Qiu <julieqiu@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/455362 Reviewed-by: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Jenny Rakoczy <jenny@golang.org> Reviewed-on: https://go-review.googlesource.com/c/go/+/455716 Reviewed-by: Jenny Rakoczy <jenny@golang.org> Run-TryBot: Damien Neil <dneil@google.com>
2022-11-18all: add missing periods in commentscui fliter
Change-Id: I69065f8adf101fdb28682c55997f503013a50e29 Reviewed-on: https://go-review.googlesource.com/c/go/+/449757 Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Joedian Reid <joedian@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Joedian Reid <joedian@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-11-17os: use testenv.Command instead of exec.Command in testscui fliter
testenv.Command sets a default timeout based on the test's deadline and sends SIGQUIT (where supported) in case of a hang. Change-Id: I32ea9ca11c30d8af3d5490f2db1674314962cc80 Reviewed-on: https://go-review.googlesource.com/c/go/+/451195 Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Joedian Reid <joedian@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
2022-11-16internal/testpty: move from os/signal/internal/ptyAustin Clements
We're going to use this for another test, so make it more accessible. Preparation for #37486. Change-Id: If194cc4244c4b9e1b1f253759b813555b39ad67e Reviewed-on: https://go-review.googlesource.com/c/go/+/449502 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-14os,syscall: File.Stat to use file handle for directories on Windowsqmuntal
Updates syscall.Open to support opening directories via CreateFileW. CreateFileW handles are more versatile than FindFirstFile handles. They can be used in Win32 APIs like GetFileInformationByHandle and SetFilePointerEx, which are needed by some Go APIs. Fixes #52747 Fixes #36019 Change-Id: I26a00cef9844fb4abeeb18d2f9d854162a146651 Reviewed-on: https://go-review.googlesource.com/c/go/+/405275 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Patrik Nyblom <pnyb@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Quim Muntal <quimmuntal@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-14os: don't request read access from CreateFile in StatDamien Neil
CL 448897 changed os.Stat to request GENERIC_READ access when using CreateFile to examine a file. This is unnecessary; access flags of 0 will permit examining file metadata even if the file isn't readable. Revert to the old behavior here. For #56217 Change-Id: I09220b3bbee304bd89f4a94ec9b0af42042b7773 Reviewed-on: https://go-review.googlesource.com/c/go/+/450296 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
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-11os/user: use libc (not cgo) on macOSRuss Cox
With net converted to libc, os/user is the last remaining cgo code in the standard libary on macOS. Convert it to libc too. Now only plugin remains as a cgo-using package on macOS. Change-Id: Ibb518b5c62ef9ec1e6ab6191f4b576f7c5a4501c Reviewed-on: https://go-review.googlesource.com/c/go/+/449316 Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-10os/signal/internal/pty: use libc (not cgo) on macOSRuss Cox
This package is only used by tests anyway, but might as well remove the cgo use on macOS so that it doesn't show up as a cgo user, as part of our overall strategy to remove cgo use in the standard library on macOS. Change-Id: I5a1a39ed56373385f9d43a5e17098035dc1a451a Reviewed-on: https://go-review.googlesource.com/c/go/+/449315 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-11-10os: document that WriteFile is not atomicRichard Tweed
Fixes #56173 Change-Id: I03a3ad769c99c0bdb78b1d757173d630879fd4dd GitHub-Last-Rev: e3e31fa0b95aba363b13c45f562e3a4c8b31f2cc GitHub-Pull-Request: golang/go#56282 Reviewed-on: https://go-review.googlesource.com/c/go/+/443495 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com>
2022-11-10os/user: allocate buffers in Go memory, not C memoryIan Lance Taylor
Since the first implementation of os/user, it's called C malloc to allocate memory for buffers. However, the buffers are just used for temporary storage, and we can just a []byte instead. To make this work without causing cgo pointer errors, we move the pwd and grp structs into C stack memory, and just return them. It's OK to store a Go pointer on the C stack temporarily. Change-Id: I9f8ffb6e51df1e585276c259fe99359d7835df87 Reviewed-on: https://go-review.googlesource.com/c/go/+/449335 Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-11-09os: remove special casing of NUL in Windows file operationsDamien Neil
Some file operations, notably Stat and Mkdir, special cased their behavior when operating on a file named "NUL" (case-insensitive). This check failed to account for the many other names of the NUL device, as well as other non-NUL device files: "./nul", "//./nul", "nul.txt" (on some Windows versions), "con", etc. Remove the special case. os.Mkdir("NUL") now returns no error. This is consonant with the operating system's behavior: CreateDirectory("NUL") succeeds, as does "MKDIR NUL" on the command line. os.Stat("NUL") now follows the existing path for FILE_TYPE_CHAR devices, returning a FileInfo which correctly reports the file as being a character device. os.Stat and os.File.Stat have common elements of their logic unified. For #24482. For #24556. For #56217. Change-Id: I7e70f45901127c9961166dd6dbfe0c4a10b4ab64 Reviewed-on: https://go-review.googlesource.com/c/go/+/448897 Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
2022-11-04all: fix function names in commentscui fliter
Change-Id: I871a747b4b47bccc889f2fdc93a2bcebb041b719 Reviewed-on: https://go-review.googlesource.com/c/go/+/447895 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: 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-31os: in TestDirFS only check returned path for unexpected stringIan Lance Taylor
The test added in CL 446115 was failing on Plan 9, on which the error returned by a failed open includes the path that failed. Change-Id: If27222596c3cb0366a030bb49ae41c4c869c3db2 Reviewed-on: https://go-review.googlesource.com/c/go/+/446641 Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David du Colombier <0intro@gmail.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-10-28os: don't include DirFS argument in DirFS errorsIan Lance Taylor
Otherwise we wind up mixing GOOS paths with slash separated paths. Change-Id: I63dd733cbdb0668effbc030cfd58945008732d9e Reviewed-on: https://go-review.googlesource.com/c/go/+/446115 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Ian Lance Taylor <iant@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-17os/signal: pass *int32 to ioctl that expects pid_tIan Lance Taylor
Fixes #56233 Change-Id: I1cf176bc2f39c5e41d5a390ec6893426cdd39be0 Reviewed-on: https://go-review.googlesource.com/c/go/+/443175 Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Joel Sing <joel@sing.id.au> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
2022-10-14os/signal: add missing newlines to TestTerminalSignalMichael Pratt
For #37329. For #56233. Change-Id: Iafcddaddafd2d27fa5d535b57aaefec387f0b3f0 Reviewed-on: https://go-review.googlesource.com/c/go/+/443066 Run-TryBot: Michael Pratt <mpratt@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-14os/signal: rewrite TestTerminalSignal without bashMichael Pratt
The existing version of this test contains several races it tries to control with sleeps. Unfortunately, it is still flaky on darwin because writing `fg` in bash too early can apparently result in failure to actually continue the stopped child. Rather than continuing to get perfect timing with bash, rewrite this to eliminate bash and instead perform the same PTY operations that bash would do. This test is still quite complex because psuedo-terminals are interminably complicated, but I believe it is no longer racy. Technically there are still two races (waiting for child to enter read() and waiting for the darwin kernel to wake the read after TIOCSPGRP), but loss of either of these races should only mean we fail to test the desired darwin EINTR case, not failure. This test is skipped on DragonflyBSD, as it tickles a Wait hang bug (#56132). Updates #56132. Fixes #37329. Change-Id: I0ceaf5aa89f6be0f1bf68b2140f47db673cedb33 Reviewed-on: https://go-review.googlesource.com/c/go/+/440220 Run-TryBot: Michael Pratt <mpratt@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
2022-10-13os: split wait6 syscall wrapper into per-platform filesBryan C. Mills
There are getting to be enough special cases in this wrapper that the increase in clarity from having a single file is starting to be outweighed by the complexity from chained conditionals. Updates #50138. Updates #13987. Change-Id: If4f1be19c0344e249aa6092507c28363ca6c8438 Reviewed-on: https://go-review.googlesource.com/c/go/+/442575 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-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-12os/signal: document behavior of SIGPIPE on non-Go threadIan Lance Taylor
Fixes #56150 Change-Id: Id990783562950ba8be7ce9526b7a811625f2190a Reviewed-on: https://go-review.googlesource.com/c/go/+/442415 Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org>
2022-10-12os: use the correct constant for P_PID on NetBSDBryan C. Mills
Dragonfly and FreeBSD both used numerical values for these constants chosen to be the same as on Solaris. For some reason, NetBSD did not, and happens to interpret value 0 as P_ALL instead of P_PID (see https://github.com/NetBSD/src/blob/3323ceb7822f98b3d2693aa26fd55c4ded6d8ba4/sys/sys/idtype.h#L43-L44). Using the correct value for P_PID should cause wait6 to wait for the correct process, which may help to avoid the deadlocks reported in For #50138. Updates #13987. Change-Id: I0eacd1faee4a430d431fe48f9ccf837f49c42f39 Reviewed-on: https://go-review.googlesource.com/c/go/+/442478 Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com> Run-TryBot: 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-08os: use poll.fdMutex for Plan 9 filesIan Lance Taylor
This permits us to safely support concurrent access to files on Plan 9. Concurrent access was already safe on other systems. This does introduce a change: if one goroutine calls a blocking read on a pipe, and another goroutine closes the pipe, then before this CL the close would occur. Now the close will be delayed until the blocking read completes. Also add tests that concurrent I/O and Close on a pipe are OK. For #50436 For #56043 Change-Id: I969c869ea3b8c5c2f2ef319e441a56a3c64e7bf5 Reviewed-on: https://go-review.googlesource.com/c/go/+/438347 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David du Colombier <0intro@gmail.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: Rob Pike <r@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>