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>
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>
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>
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>
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>
- 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>
- 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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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#52747Fixes#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>
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>
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>
As the comments say. Here we expect err to be nil instead of ctx.Err()
Change-Id: I4cd02d62ac0a13c9577a567de36742f13d140d36
GitHub-Last-Rev: 6bedfbc9d2
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>
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>
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>
Fixes#56173
Change-Id: I03a3ad769c99c0bdb78b1d757173d630879fd4dd
GitHub-Last-Rev: e3e31fa0b9
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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 3323ceb782/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>
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>
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>
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>
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>
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>
Bryan C. Mills
Nigel Tao
qmuntal
Russ Cox
Alan Donovan
Than McIntosh
Ian Lance Taylor
Damien Neil
cui fliter
Austin Clements
scott
Richard Tweed
Matthew Dempsky
Michael Pratt
hopehook