Kevin Burke [Mon, 31 Jul 2017 04:14:10 +0000 (21:14 -0700)]
os/user: fix darwin GetGroupIds for n > 256
If a Mac user has more than 256 groups, getGroupList returns -1 but
does not correctly set n. We need to retry the syscall with an
ever-increasing group size until we get all of the user's groups.
The easiest way to test this change is to set n to a value lower than
the current user's number of groups, test on a Mac and observe
a failure, then apply the patch and test that it passes.
Austin Clements [Tue, 21 Nov 2017 22:19:43 +0000 (17:19 -0500)]
runtime/debug: make SetGCPercent(-1) wait for concurrent GC
Currently, SetGCPercent(-1) disables GC, but doesn't wait for any
currently running concurrent GC to finish, so GC can still be running
when it returns. This is a change in behavior from Go 1.8, probably
defies user expectations, and can break various runtime tests that
depend on SetGCPercent(-1) to disable garbage collection in order to
prevent preemption deadlocks.
Fix this by making SetGCPercent(-1) block until any concurrently
running GC cycle finishes.
Fixes #22443.
Change-Id: I904133a34acf97a7942ef4531ace0647b13930ef
Reviewed-on: https://go-review.googlesource.com/79195
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 21 Nov 2017 15:14:11 +0000 (07:14 -0800)]
cmd/compile: fix mapassign_fast* routines for pointer keys
The signature of the mapassign_fast* routines need to distinguish
the pointerness of their key argument. If the affected routines
suspend part way through, the object pointed to by the key might
get garbage collected because the key is typed as a uint{32,64}.
This is not a problem for mapaccess or mapdelete because the key
in those situations do not live beyond the call involved. If the
object referenced by the key is garbage collected prematurely, the
code still works fine. Even if that object is subsequently reallocated,
it can't be written to the map in time to affect the lookup/delete.
Fixes #22781
Change-Id: I0bbbc5e9883d5ce702faf4e655348be1191ee439
Reviewed-on: https://go-review.googlesource.com/79018
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Tim Wright [Wed, 22 Nov 2017 03:11:56 +0000 (19:11 -0800)]
syscall: add missing fs locking in Link, Rename on nacl
Per the comments at the head of fs_nacl.go, unexported methods expect
the fs mutex to have been taken by the caller.
This change brings Link and Rename into line with the other exported
functions wrt fs locking.
Brad Fitzpatrick [Wed, 22 Nov 2017 01:38:40 +0000 (01:38 +0000)]
doc: update the URL of the latest go1.4 source snapshot
Updates golang/go#20672
Change-Id: I3c62b1606aec93e188255f1701c0af569d540016
Reviewed-on: https://go-review.googlesource.com/79276 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Tue, 21 Nov 2017 22:55:18 +0000 (17:55 -0500)]
test: make inline_callers.go test not inline the runtime
CL 76551 modified inline_callers.go to build everything, including the
runtime, with -l=4. While that works in most places (and ideally
should work everywhere), it blows out the nosplit stack on
solaris/amd64.
Fix this by only building the test itself with -l=4.
This undoes some of the changes to this test from CL 73212, which
originally changed the go tool to rebuild all packages with the given
flags. This change modified the expected output of this test, so now
that we can go back to building only the test itself with inlining, we
revert these changes to the expected output. (That CL also changed
log.Fatalf to log.Printf, but didn't add "\n" to the end of the lines,
so this CL fixes that, too.)
isharipo [Mon, 20 Nov 2017 17:56:05 +0000 (20:56 +0300)]
cmd/internal/obj/x86: fix /is4 encoding for VBLEND
Fixes VBLENDVP{D/S}, VPBLENDVB encoding for /is4 imm8[7:4]
encoded register operand.
Explanation:
`reg[r]+regrex[r]+1` will yield correct values for 8..15 reg indexes,
but for 0..7 it gives `index+1` results.
There was no test that used lower 8 register with /is4 encoding,
so the bug passed the tests.
The proper solution is to get 4th bit from regrex with a proper shift:
`reg[r]|(regrex[r]<<1)`.
Instead of inlining `reg[r]|(regrex[r]<<1)` expr,
using new `regIndex(r)` function.
Test that reproduces this issue is added to
amd64enc_extra.s test suite.
Michael Pratt [Mon, 20 Nov 2017 19:01:00 +0000 (11:01 -0800)]
runtime: skip netpoll check if there are no waiters
If there are no netpoll waiters then calling netpoll will never find any
goroutines. The later blocking netpoll in findrunnable already has this
optimization.
With golang.org/cl/78538 also applied, this change has a small impact on
latency:
Jamie Liu [Wed, 15 Nov 2017 20:47:22 +0000 (12:47 -0800)]
runtime: only sleep before stealing work from a running P
The sleep in question does not make sense if the stolen-from P cannot
run the stolen G. The usleep(3) has been observed delaying execution of
woken G's by ~60us; skipping it reduces the wakeup-to-execution latency
to ~7us in these cases, improving CPU utilization.
Florian Uekermann [Fri, 6 Oct 2017 15:16:43 +0000 (17:16 +0200)]
time: enable Location loading from user provided timezone data
The return values of the LoadLocation are inherently dependent
on the runtime environment. Add LoadLocationFromTZData, whose
results depend only on the timezone data provided as arguments.
Than McIntosh [Tue, 14 Nov 2017 18:56:18 +0000 (13:56 -0500)]
cmd/compile: ignore RegKill ops for non-phi after phi check
Relax the 'phi after non-phi' SSA sanity check to allow
RegKill ops interspersed with phi ops in a block. This fixes
a sanity check failure when -dwarflocationlists is enabled.
Updates #22694.
Change-Id: Iaae604ab6f1a8b150664dd120003727a6fb2f698
Reviewed-on: https://go-review.googlesource.com/77610
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Heschi Kreinick [Thu, 16 Nov 2017 21:19:19 +0000 (16:19 -0500)]
cmd/link: use . as DW_AT_comp_dir
Go's DWARF usually has absolute paths, in which case DW_AT_comp_dir
doesn't matter. But the -trimpath flag produces relative paths, and
then the spec says that they are relative to _comp_dir.
There's no way to know what the "right" value of _comp_dir is without
more user input, but we can at least leave the paths alone rather than
making them absolute.
After this change, Delve can find sources to a program built with
-gcflags=-trimpath=$(pwd) as long as it's run in the right directory.
fanzha02 [Tue, 13 Jun 2017 11:07:34 +0000 (11:07 +0000)]
cmd/internal/obj/arm64: fix assemble msr/mrs bug
The arguments <pstatefield> is a struct that includes two elements,
element reg is special register, elememt enc is pstate field values.
The current code compares two different type values and get a incorrect
result.
The fix follows pstate field to create a systemreg struct,
each system register has a vaule to use in instruction.
When instruction has only one argument, Go parser saves the
argument value into prog.From without any special handling.
But assembler gets the argument value from prog.To.
The fix adds special handling for CLREX and puts other instructions
arguments value into prog.From.
Uncomment hlt/hvc/smc/brk/dcps1/dcps2/dcps3/clrex test cases.
Daniel Martí [Sun, 19 Nov 2017 17:28:46 +0000 (17:28 +0000)]
cmd/doc: print a symbol error on "bytes Foo"
In golang.org/cl/59413, the two-argument behavior of cmd/doc was changed
to use findPackage instead of build.Import, meaning that the tool was
more consistent and useful.
However, it introduced a regression:
$ go doc bytes Foo
doc: no such package: bytes
This is because the directory list search would not find Foo in bytes,
and reach the end of the directory list - thus resulting in a "no such
package" error, since no directory matched our first argument.
Move the "no such package" error out of parseArgs, so that the "loop
until something is printed" loop can have control over it. In
particular, it is useful to know when we have reached the end of the
list without any exact match, yet we did find one package matching
"bytes":
$ go doc bytes Foo
doc: no symbol Foo in package bytes
While at it, make the "no such package" error not be fatal so that we
may test for it. It is important to have the test, as parseArgs may now
return a nil package instead of exiting the entire program, potentially
meaning a nil pointer dereference panic.
Fixes #22810.
Change-Id: I90cc6fd755e2d1675bea6d49a1c13cc18ac9bfb9
Reviewed-on: https://go-review.googlesource.com/78677
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Kunpei Sakai [Mon, 23 Oct 2017 06:27:04 +0000 (15:27 +0900)]
testing: fix invalid error message about argument of TestMain
Also, this commit adds a test for ensuring that TestMain(t *testing.T) is a normal test.
Fixes #22388
Change-Id: Iffcb1db5cdcf34b9c822fcdb58f8926535415177
Reviewed-on: https://go-review.googlesource.com/72591
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Sat, 18 Nov 2017 15:53:53 +0000 (15:53 +0000)]
bytes: make all readOp constants actually typed
This is a regression introduced in golang.org/cl/28817. That change got
rid of the iota, which meant that the type was no longer applied to all
the constant names.
Re-add the iota starting at -1, simplifying the code and adding the
types once more.
Ian Lance Taylor [Tue, 24 Oct 2017 14:22:26 +0000 (07:22 -0700)]
cmd/go: always copy files on Windows
Copying ensures that we respect the NTFS permissions of the parent folder.
I don't know if there is a way to tell when it is safe to simply rename.
Fixes #22343
Change-Id: I424bfe655b53b0e0fe425ce92bbc15450d52d851
Reviewed-on: https://go-review.googlesource.com/72910
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Daniel Theophanes [Thu, 9 Nov 2017 22:14:44 +0000 (14:14 -0800)]
database/sql: check for arg counts after eliminating arguments
Check for the expected number of arguments in a SQL statement
after arguments are eliminated in the argument converter.
This situation was already tested for in TestNamedValueChecker.
However the test used Exec which didn't have any check for
NumInput on it at all, thus this issue was never caught.
In addition to moving the NumInput check on the Query
methods after the converter, add the NumInput check
to the Exec methods as well.
Fixes #22630
Change-Id: If45920c6e1cf70dca63822a0cedec2cdc5cc611c
Reviewed-on: https://go-review.googlesource.com/76732
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Lynn Boger [Tue, 14 Nov 2017 20:50:38 +0000 (15:50 -0500)]
cmd/dist: return dropped tests from misc/cgo/test
In a previous change to cmd/dist/test.go to fix some pie
testcases, a few other tests were incorrectly dropped.
This returns the testcases that shouldn't have been removed.
Fixes #22708
Change-Id: I2f735f4fd3a378f0f45d12a99768638aeb4787c7
Reviewed-on: https://go-review.googlesource.com/77650
Run-TryBot: Russ Cox <rsc@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 26 Sep 2017 22:14:50 +0000 (15:14 -0700)]
cmd/cgo: special case C ptr types to use uintptr
Some C types are declared as pointers, but C code
stores non-pointers in them. When the Go garbage
collector sees such a pointer, it gets unhappy.
Instead, for these types represent them on the Go
side with uintptr.
We need this change to handle Apple's CoreFoundation
CF*Ref types. Users of these types might need to
update their code like we do in root_cgo_darwin.go.
The only change that is required under normal
circumstances is converting some nils to 0.
A go fix module is provided to help.
Fixes #21897
RELNOTE=yes
Change-Id: I9716cfb255dc918792625f42952aa171cd31ec1b
Reviewed-on: https://go-review.googlesource.com/66332
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Munday [Fri, 17 Nov 2017 10:01:06 +0000 (10:01 +0000)]
cmd/compile: delete temporary files when test finishes
I noticed some files prefixed with ssa_fg_tmp in the /tmp folder of
the s390x builder. runGenTest (a helper for TestGenFlowGraph) wasn't
deleting its temporary files. The distinct prefix made this easy to
figure out.
isharipo [Wed, 15 Nov 2017 17:56:10 +0000 (20:56 +0300)]
cmd/internal/obj/x86: add AVX2 gather and VSIB
Enables AVX2 gather instructions and VSIB support,
which makes vm32{x,y} vm64{x,y} operands encodable.
AXXX constants placed with respect to sorting order.
New VEX optabs inserted near non-VEX entries to simplify
potential transition to auto-generated VSIB optabs.
Tests go into new AMD64 encoder test file (amd64enc_extra.s)
to avoid unnecessary interactions with auto-generated "amd64enc.s".
Side note: x86avxgen did not produced these instructions
because x86.v0.2.csv misses them.
This also explains why x86 test suite have no AVX2 gather
instructions tests.
List of new instructions:
VGATHERPDP
VGATHERDPS
VGATHERQPD
VGATHERQPS
VPGATHERDD
VPGATHERDQ
VPGATHERQD
VPGATHERQQ
Adam Langley [Fri, 13 Oct 2017 22:36:01 +0000 (15:36 -0700)]
crypto/x509: always emit a critical SAN extension if the Subject is empty.
The RFC is a little ambiguous here: “the subject field contains an empty
sequence” could mean that it's a non-empty sequence where one of the
sets contains an empty sequence. But, in context, I think it means “the
subject field is an empty sequence”.
Adam Langley [Mon, 13 Nov 2017 18:38:05 +0000 (10:38 -0800)]
crypto/x509: relax EKU checking in some cases.
CL 71030 enforced EKU nesting at verification time, to go along with the
change in name constraints behaviour. From scanning the Certificate
Transparency logs, it's clear that some CAs are not getting EKU nesting
correct.
This change relaxes the EKU rules in a few ways:
∙ EKUs in roots are no longer checked.
∙ Any CA certificate may issue OCSP responder certificates.
∙ The ServerAuth and SGC EKUs are treated as a single EKU when
checking nesting.
∙ ServerAuth in a CA can now authorise ClientAuth.
∙ The generic CodeSigning EKU can now authorise two, Microsoft-specific
code-signing EKUs.
Adam Langley [Mon, 13 Nov 2017 18:56:54 +0000 (10:56 -0800)]
crypto/x509: don't fail to parse addition elements in GeneralSubtree.
The GeneralSubtree structure can have additional elements after the name
(minimum and maximum, which are unused). Previously these fields, if
present, would cause a parse error. This change allows trailing data in
the GeneralSubtrees structure.
David Chase [Thu, 16 Nov 2017 02:25:01 +0000 (18:25 -0800)]
cmd/compile: leave Pos unset for racewalk enter/exit
The locations chosen for racewalking inserted code can
be wrong and thus cause unwanted next/step behavior in
debuggers. Forcing the positions to be unset results in
better behavior.
Test added, and test harness corrected to deal with
changes to gdb's output caused by -racewalk.
Incidental changes in Delve (not part of the usual testing,
but provided because we care about Delve) also reflected
in this CL.
Fixes #22600.
Change-Id: Idd0218afed52ab8c68efd9eabbdff3c92ea2b996
Reviewed-on: https://go-review.googlesource.com/78336
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
Bill O'Farrell [Tue, 31 Oct 2017 23:18:48 +0000 (19:18 -0400)]
cmd/link: enable c-shared and c-archive mode on s390x
Adding s390x to the list of architectures that support c-shared and c-archive.
Required adding load-time initialization (via _rt0_s390x_linux_lib) and adding s390x
to the c-shared and c-archive tests.
Change-Id: I75883b2891c310fe8ce7f08c27b06895c074e123
Reviewed-on: https://go-review.googlesource.com/74910 Reviewed-by: Michael Munday <mike.munday@ibm.com>
Austin Clements [Wed, 15 Nov 2017 22:43:05 +0000 (14:43 -0800)]
runtime: remove another TODO
I experimented with having the compiler spill the two registers that
are clobbered by the write barrier fast path, but it slightly slows
down compilebench, which is a good write barrier benchmark:
Austin Clements [Wed, 15 Nov 2017 22:26:33 +0000 (14:26 -0800)]
runtime: remove TODO
I experimented with changing the write barrier to take the value in SI
rather than AX to improve register allocation. It had no effect on
performance and only made the "hello world" text 0.07% smaller, so
let's just remove the comment.
Daniel Theophanes [Tue, 14 Nov 2017 16:53:56 +0000 (08:53 -0800)]
database/sql: allow OpenConnector in a driver.Driver interface
While driver.Connector was previously added to allow non-string
connection arguments and access to the context, most users of
the sql package will continue to rely on a string DSN.
Allow drivers to implement a string DSN to Connector interface
that both allows a single parsing of the string DSN and uses
the Connector interface which passes available context to
the driver dialer.
Fixes #22713
Change-Id: Ia0b862262f4c4670effe2538d0d6d43733fea18d
Reviewed-on: https://go-review.googlesource.com/77550 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
I realized this simplification was possible when writing the vet loop
(just above the code being modified here) but never circled back
to make the compiler loop match.
Russ Cox [Tue, 14 Nov 2017 15:57:14 +0000 (10:57 -0500)]
cmd/go/internal/work: do not write trivial.c when testing gcc flags
CL 61111 disabled the writing of trivial.c in -n mode, which
made -n mode at least inconsistent with regular mode in
how it was testing for flags. We think that both were getting
the same answer, so avoid creating the file in both modes
to make sure.
If this CL turns out to be wrong, then when we revert it we
should make sure that the empty file is written even in -n mode,
because this check affects the command-line flags printed
by other commands in that mode.
Change-Id: I0a050bfc148fe5a9d430a153d7816b2821277f0d
Reviewed-on: https://go-review.googlesource.com/78115
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Thu, 16 Nov 2017 02:14:21 +0000 (21:14 -0500)]
cmd/go: fix swig support and run swig tests during run.bash
(The tests only run when swig is already installed on the local system.)
Change-Id: I172d106a68cfc746a1058f5a4bcf6761bab88912
Reviewed-on: https://go-review.googlesource.com/78175 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Joe Tsai [Wed, 15 Nov 2017 19:27:10 +0000 (11:27 -0800)]
archive/tar: partially revert sparse file support
This CL removes the following APIs:
type SparseEntry struct{ ... }
type Header struct{ SparseHoles []SparseEntry; ... }
func (*Header) DetectSparseHoles(f *os.File) error
func (*Header) PunchSparseHoles(f *os.File) error
func (*Reader) WriteTo(io.Writer) (int, error)
func (*Writer) ReadFrom(io.Reader) (int, error)
This API was added during the Go1.10 dev cycle, and are safe to remove.
The rationale for reverting is because Header.DetectSparseHoles and
Header.PunchSparseHoles are functionality that probably better belongs in
the os package itself.
The other API like Header.SparseHoles, Reader.WriteTo, and Writer.ReadFrom
perform no OS specific logic and only perform the actual business logic of
reading and writing sparse archives. Since we do know know what the API added to
package os may look like, we preemptively revert these non-OS specific changes
as well by simply commenting them out.
Russ Cox [Thu, 16 Nov 2017 16:49:33 +0000 (11:49 -0500)]
cmd/go/internal/cache: fix build failure introduced in CL 78176
Maybe a bad git merge - not sure.
In any event, I do miss the trybots.
Noticed while fixing: change print-to-stderr+panic
to pure panic, just so that the test (which catches the panic)
does not print any errors before passing.
Russ Cox [Sun, 12 Nov 2017 00:50:19 +0000 (19:50 -0500)]
cmd/go: implement go clean -testcache
Ian suggested that since test caching is not expected to be perfect
in all cases, we should allow users to clear the test cache separately
from clearing the entire build cache.
This CL adds 'go clean -testcache' to do that. The implementation
does not actually delete files (for that, use 'go clean -cache').
Instead, it writes down the current time, and future go tests will
ignore any cached test results written before that time.
Change-Id: I4f84065d7dfc2499fa3f203e9ab62e68d7f367c5
Reviewed-on: https://go-review.googlesource.com/78176 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 10 Nov 2017 20:58:16 +0000 (15:58 -0500)]
cmd/go: run cover before cgo
If we're running coverage on a package using cgo, we need to
apply both cmd/cover and cmd/cgo as source transformers.
To date we've applied cgo, then cover.
Cover is very sensitive to the exact character position of
expressions in its input, though, and cgo is not, so swap
them, applying first cover and then cgo.
The only drawback here is that coverage formerly applied
to SWIG-generated cgo files, and now it does not.
I am not convinced anyone depended critically on that,
and probably the later analysis with go tool cover would
have tried to parse the original .swig file as a Go file and
gotten very confused.
Fixes #8726.
Fixes #9212.
Fixes #9479.
Change-Id: I777c8b64f7726cb117d59e03073954abc6dfa34d
Reviewed-on: https://go-review.googlesource.com/77155 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 10 Nov 2017 20:46:56 +0000 (15:46 -0500)]
cmd/go: make file:line for cgo files look like non-cgo files
Passing the absolute path to cgo puts the absolute path in the
generated file's //line directives, which then shows that path
in the compiler output, which the go command can then
make relative to the current directory, same as it does for
other compiler output.
Change-Id: Ia2064fea40078c46fd97e3a3b8c9fa1488f913e3
Reviewed-on: https://go-review.googlesource.com/77154 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 10 Nov 2017 16:45:16 +0000 (11:45 -0500)]
cmd/cgo: modify source as text, not as AST
Cgo has always operated by rewriting the AST and invoking go/printer.
This CL converts it to use the AST to make decisions but then apply
its edits directly to the underlying source text. This approach worked
better in rsc.io/grind (used during the C to Go conversion) and also
more recently in cmd/cover. It guarantees that all comments and
line numbers are preserved exactly.
This eliminates a lot of special concern about comments and
problems with cgo not preserving meaningful comments.
Combined with the CL changing cmd/cover to use the same
approach, it means that the combination of applying cgo and
applying cover still guarantees all comments and line numbers
are preserved exactly.
This sets us up to fix some cgo vs cover bugs by swapping
the order in which they run during the go command.
This also sets up #16623 a bit: the edit list being
accumulated here is nearly exactly what you'd want
to pass to the compiler for that issue.
Change-Id: I7611815be22e7c5c0d4fc3fa11832c42b32c4eb3
Reviewed-on: https://go-review.googlesource.com/77153 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Thu, 16 Nov 2017 15:29:50 +0000 (10:29 -0500)]
encoding/csv: restore Go 1.9 quoted \r\n handling in Reader
CL 52810 changed Reader to interpret a quoted \r\n as a raw \r\n
when reading fields. This seems likely to break existing users, and
discussion on both #21201 (the original issue that triggered the change)
and #22746 (discussing whether to revert the change) failed to identify
a single motivating example for this change. To avoid breaking existing
users for no clear reason, revert the change.
The Reader has been rewritten in the interim so this is not a git revert
but instead and adjustment (and slight simplification) of the new Reader.
Fixes #22746.
Change-Id: Ie857b2f4b1359a207d085b6d3c3a6d440a997d12
Reviewed-on: https://go-review.googlesource.com/78295 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Russ Cox [Tue, 14 Nov 2017 03:32:19 +0000 (22:32 -0500)]
cmd/internal/obj, cmd/trace: restore bounds checks dropped in CL 56950
CL 56950 correctly identified code with checks that were impossible.
But instead of correcting the checks it deleted them.
This CL corrects the code to check what was meant.
Change-Id: Ic89222184ee4fa5cacccae12d750601a9438ac8d
Reviewed-on: https://go-review.googlesource.com/78113
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Thu, 16 Nov 2017 02:29:04 +0000 (02:29 +0000)]
Revert "math/rand: make Perm match Shuffle"
This reverts CL 55972.
Reason for revert: this changes Perm's behavior unnecessarily.
I asked for this change originally but I now regret it.
Reverting so that I don't have to justify it in Go 1.10 release notes.
Edited to keep the change to rand_test.go, which seems to have
been mostly unrelated.
Fixes #22744.
Change-Id: If8bb1bcde3ced0db2fdcd0aa65ab128613686c66
Reviewed-on: https://go-review.googlesource.com/78195
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reason for revert: the CL moves a parser for what appears to be an
Android-specific file format into the main code and makes it available
on all platforms. Android-specific file formats should be limited to
Android.
Russ Cox [Tue, 14 Nov 2017 19:35:39 +0000 (14:35 -0500)]
io: eliminate full copy of copy loop in CopyN
CL 60630 claimed to and did “improve performance of CopyN”
but in doing so introduced a second copy of the I/O copying loop.
This code is subtle and easy to get wrong and the last thing we
need is of two copies that can drift out of sync. Even the newly
introduced copy contains various subtle changes that are not
obviously semantically equivalent to the original. (They probably
are, but it's not obvious.)
Although the CL description does not explain further what the
important optimization was, it appears that the most critical
one was not allocating a 32kB buffer for CopyN(w, r, 512).
This CL deletes the forked copy of copy and instead applies
the buffer size restriction optimization directly to copy itself.
CL 60630 reported:
name old time/op new time/op delta
CopyNSmall-4 5.09µs ± 1% 2.25µs ±86% -55.91% (p=0.000 n=11+14)
CopyNLarge-4 114µs ±73% 121µs ±72% ~ (p=0.701 n=14+14)
Starting with that CL as the baseline, this CL does not change a ton:
name old time/op new time/op delta
CopyNSmall-8 370ns ± 1% 411ns ± 1% +11.18% (p=0.000 n=16+14)
CopyNLarge-8 18.2µs ± 1% 18.3µs ± 1% +0.63% (p=0.000 n=19+20)
It does give up a small amount of the win of 60630 but preserves
the bulk of it, with the benefit that we will not need to debug these
two copies drifting out of sync in the future.
Russ Cox [Fri, 10 Nov 2017 04:58:42 +0000 (23:58 -0500)]
cmd/cover: do not report coverage for assembly functions
cover -func mode was reporting a coverage for function
declarations without bodies - assembly functions.
Since we are not annotating their code, we have no data
for those functions and should not report them at all.
Fixes #6880.
Change-Id: I4b8cd90805accf61f54e3ee167f54f4dc10c7c59
Reviewed-on: https://go-review.googlesource.com/77152 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 10 Nov 2017 04:55:15 +0000 (23:55 -0500)]
cmd/cover: add //line comment pointing to original file
Now that cover does not modify the formatting of the original file
or add any newline characters, we can make it print a //line comment
pointing back at the original, and compiler errors and panics will
report accurate line numbers.
Fixes #6329.
Fixes #15757.
Change-Id: I7b0e386112c69beafe69e0d47c5f9e9abc87c0f5
Reviewed-on: https://go-review.googlesource.com/77151
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Wed, 15 Nov 2017 20:19:31 +0000 (15:19 -0500)]
database/sql/driver: document that Execer, Queryer need not be implemented
CL 21663 allowed drivers to implement ExecerContext without
also implementing Execer, and similarly QueryerContext without
Queryer, but it did not make that clear in the documentation.
This CL updates the documentation.
Change-Id: I9a4accaac32edfe255fe7c0b0907d4c1014322b4
Reviewed-on: https://go-review.googlesource.com/78129 Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Russ Cox [Tue, 14 Nov 2017 18:43:02 +0000 (13:43 -0500)]
encoding/hex: make Decode, Decoder, DecodeString agree about partial results and errors
CL 70210 added Decoder for #21590, and in doing so it changed
the existing func Decode to return partial results for decoding errors.
That seems like a good change to make to Decode, but it was
untested (except as used by Decoder), inconsistent with DecodeString
in all error cases, and inconsistent with Decoder in not returning
partial results for odd-length input strings.
This CL makes Decode, DecodeString, and Decoder all agree about
the handling of partial results (they are returned) and error
precedence (the error earliest in the input is reported),
and it documents and tests this.
Kevin Burke [Fri, 14 Jul 2017 22:20:27 +0000 (16:20 -0600)]
crypto/cipher: use raw bytes for keys in docs
Using ASCII values for keys is a bad idea since it makes them vastly
easier to guess. Instead, use the same method as the examples in the
golang.org/x/crypto/nacl package to load keys from a hex value.
Changing the key required updating the ciphertext in many of the
examples.
I am still worried about the fact the examples ask the user to
authenticate messages; authentication isn't trivial, and to be honest
it may be better to steer people to a higher level primitive like
secretbox, unless people really need AES.
Troels Thomsen [Tue, 14 Nov 2017 22:22:19 +0000 (23:22 +0100)]
net: Forget lookups for canceled contexts
A sequential lookup using any non-canceled context has a risk of
returning the result of the previous lookup for a canceled context (i.e.
an error).
This is already prevented for timed out context by forgetting the host
immediately and extending this to also compare the error to
`context.Canceled` resolves this issue.
Russ Cox [Wed, 15 Nov 2017 16:47:31 +0000 (11:47 -0500)]
archive/zip: fix handling of replacement rune in UTF8 check
The replacement rune is a valid rune and can appear as itself in valid UTF8
(it encodes as three bytes). To check for invalid UTF8 it is necessary to
look for utf8.DecodeRune returning the replacement rune and size==1.
Russ Cox [Wed, 15 Nov 2017 20:07:44 +0000 (15:07 -0500)]
database/sql/driver: rename ResetSessioner to SessionResetter
Originally we tried the strict -er suffix as the rule in this case
but eventually we decided it was too awkward: io.WriteByter
became io.ByteWriter. By analogy, here the interface should be
named SessionResetter instead of the awkward ResetSessioner.
This change should not affect any drivers that have already
implemented the interface, because the method name is not changing.
(This was added during the Go 1.10 cycle and has not been
released yet, so we can change it.)
Russ Cox [Tue, 14 Nov 2017 19:01:11 +0000 (14:01 -0500)]
encoding/xml: move unexported const out of exported const block
CL 58210 introduced this constant for reasons I don't understand.
It should not be in the exported const block, which will pollute
godoc output with a "... unexported" notice.
Also since we already have a constant named xmlnsPrefix for "xmlns",
it is very confusing to also have xmlNamespacePrefix for "xml".
If we must have the constant at all, rename it to xmlPrefix.
Russ Cox [Tue, 14 Nov 2017 18:23:10 +0000 (13:23 -0500)]
encoding/csv: rename ParseError.RecordLine to .StartLine
A record can span multiple lines (the whole reason for the extra field),
so the important fact is that it's the _start_ of the record.
Make that clear in the name.
(This API was added during the Go 1.10 cycle so it can still be cleaned up.)
Russ Cox [Tue, 14 Nov 2017 17:38:11 +0000 (12:38 -0500)]
encoding/binary: make new example a bit more idiomatic
Mainly get rid of the weird zero-value struct literal,
but while we're here also group and order things a bit better:
first the reader, then the data, then the call (which takes reader then data).
Russ Cox [Tue, 14 Nov 2017 02:30:22 +0000 (21:30 -0500)]
bytes, strings: restore O(1) behavior of IndexAny(s, "") and LastIndexAny(s, "")
CL 65851 (bytes) and CL 65910 (strings) “improve[d] readability”
by removing the special case that bypassed the whole function body
when chars == "". In doing so, yes, the function was unindented a
level, which is nice, but the runtime of that case went from O(1) to O(n)
where n = len(s).
I don't know if anyone's code depends on the O(1) behavior in this case,
but quite possibly someone's does.
This CL adds the special case back, with a comment to prevent future
deletions, and without reindenting each function body in full.