Tested on linux/amd64 too this time.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=dave, golang-codereviews, iant, rsc
https://golang.org/cl/140050043
The net/http package supports setting the HTTP Authorization header
using the Basic Authentication Scheme as defined in RFC 2617, but does
not provide support for extracting the username and password from an
authenticated request using the Basic Authentication Scheme.
Add BasicAuth method to *http.Request that returns the username and
password from authenticated requests using the Basic Authentication
Scheme.
Fixes#6779.
LGTM=bradfitz
R=golang-codereviews, josharian, bradfitz, alberto.garcia.hierro, blakesgentry
CC=golang-codereviews
https://golang.org/cl/76540043
In CL 131450043, which raised it to 160,
I'd raise it to 192 if necessary.
Apparently it is necessary on windows/amd64.
One note for those concerned about the growth:
in the old segmented stack world, we wasted this much
space at the bottom of every stack segment.
In the new contiguous stack world, each goroutine has
only one stack segment, so we only waste this much space
once per goroutine. So even raising the limit further might
still be a net savings.
Fixes windows/amd64 build.
TBR=r
CC=golang-codereviews
https://golang.org/cl/132480043
Renaming the C SysAlloc will let Go define a prototype without exporting it.
For use in cpuprof.goc's translation to Go.
LGTM=mdempsky
R=golang-codereviews, mdempsky
CC=golang-codereviews, iant
https://golang.org/cl/140060043
Actually it mostly deletes code -- alg.print and alg.copy go away.
There was only one usage of alg.print for debug purposes.
Alg.copy is used in chan.goc, but Keith replaces them with
memcopy during conversion, so alg.copy is not needed as well.
Converting them would be significant amount of work
for no visible benefit.
LGTM=crawshaw, rsc, khr
R=golang-codereviews, crawshaw, khr
CC=golang-codereviews, rsc
https://golang.org/cl/139930044
preparing for the syscall package freeze.
the change for issue 8218 is only applied to go.sys/unix.
««« original CL description
syscall: implement setresuid(2) and setresgid(2) on OpenBSD/FreeBSD/DragonflyBSD
Fixes#8218.
LGTM=iant
R=golang-codereviews, iant, minux
CC=golang-codereviews
https://golang.org/cl/107150043
»»»
LGTM=r
R=r, iant, golang-codereviews
CC=golang-codereviews
https://golang.org/cl/138840044
Prevents non-rooted queries with > ndots dots from being tried twice on error.
Fixes#8616.
Benchmark results on linux/amd64
benchmark old ns/op new ns/op delta
BenchmarkGoLookupIPNoSuchHost 8212394 4413293 -46.26%
benchmark old allocs new allocs delta
BenchmarkGoLookupIPNoSuchHost 216 108 -50.00%
benchmark old bytes new bytes delta
BenchmarkGoLookupIPNoSuchHost 17460 8726 -50.02%
LGTM=iant, mikioh.mikioh
R=golang-codereviews, iant, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/137870043
It is anyway, just an allocated one.
Giving it a sized type makes Go access nicer.
LGTM=iant
R=dvyukov, iant
CC=golang-codereviews
https://golang.org/cl/139960043
I had to rename Kevent and Sigaction to avoid the functions of the
same (lowercase) name.
LGTM=iant, r
R=golang-codereviews, r, iant, aram.h
CC=dvyukov, golang-codereviews, khr
https://golang.org/cl/140740043
Signer is an interface to support opaque private keys.
These keys typically result from being kept in special hardware
(i.e. a TPM) although sometimes operating systems provide a
similar interface using process isolation for security rather
than hardware boundaries.
This changes provides interfaces for representing them and
alters crypto/tls so that client certificates can use
opaque keys.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews, jdeprez
https://golang.org/cl/114680043
Needless except that the api tool complains. We could fix that issue instead.
TBR=bradfitz
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/133290043
Run it right before main.init.
There is still some runtime initialization that
happens before runtime.init, and some of that
may call into Go code (for example to acquire locks)
so this timing is not perfect, but I believe it is the
best we can do.
This came up because global variables intialized
to func values are done in the generated init code,
not in the linker.
LGTM=dvyukov
R=dvyukov
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/135210043
As part of the translation of the runtime, we need to rewrite
C printf calls to Go print calls. Consider this C printf:
runtime·printf("[signal %x code=%p addr=%p pc=%p]\n",
g->sig, g->sigcode0, g->sigcode1, g->sigpc);
Today the only way to write that in Go is:
print("[signal ")
printhex(uint64(g->sig))
print(" code=")
printhex(uint64(g->sigcode0))
print(" addr=")
printhex(uint64(g->sigcode1))
print(" pc=")
printhex(uint64(g->sigpc))
print("]\n")
(That's nearly exactly what runtime code looked like in C before
I added runtime·printf.)
This CL recognizes the unexported type runtime.hex as an integer
that should be printed in hexadecimal instead of decimal.
It's a little kludgy, but it's restricted to package runtime.
Other packages can define type hex with no effect at all.
Now we can translate that original printf as the more compact:
print("[signal ", hex(g->sig), " code=", hex(g->sigcode0),
" addr=", hex(g->sigcode1), " pc=", hex(g->sigpc), "]\n")
LGTM=r, iant
R=r, iant
CC=golang-codereviews
https://golang.org/cl/133220043
ErrorContext now has all the information it needs from the Node,
rather than depending on the template that contains it. This makes
it easier for html/template to generate correct locations in its
error messages.
Updated html/template to use this ability where it is easy, which is
not everywhere, but more work can probably push it through.
Fixes#8577.
LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/130620043
Remove C version of GC.
Convert freeOSMemory to Go.
Restore g0 check in GC.
Remove unknownGCPercent check in GC,
it's initialized explicitly now.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews, khr
https://golang.org/cl/139910043
Breaks on Plan 9, apparently.
The other systems must not run sprintf during all.bash.
I'd write a test but it's all going away.
TBR=r
CC=0intro, golang-codereviews
https://golang.org/cl/133260044
I've started with just one function with 8 arguments,
but stdcall is called from nosplit functions
and 8 args overflow nosplit area.
LGTM=aram, alex.brainman
R=golang-codereviews, aram, alex.brainman, dave
CC=golang-codereviews, iant, khr, rsc
https://golang.org/cl/135090043
PNG filters are applied to get better compression ratio.
It does not make sense to apply them if we are not going
to compress.
LGTM=nigeltao
R=nigeltao
CC=golang-codereviews
https://golang.org/cl/137830043
Also fix a bunch of bugs:
1. Accesses to last_gc must be atomic (it's int64).
2. last_gc still can be 0 during first checks in sysmon, check for 0.
3. forcegc.g can be unitialized when sysmon accesses it:
forcegc.g is initialized by main goroutine (forcegc.g = newproc1(...)),
and main goroutine is unsynchronized with both sysmon and forcegc goroutine.
Initialize forcegc.g in the forcegc goroutine itself instead.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rsc
https://golang.org/cl/136770043
Shrinks the text segment size by about 1.5% for the "go", "gofmt",
and "camlistored" commands on linux/amd64.
Before:
$ size go gofmt camlistored
text data bss dec hex filename
6506842 136996 105784 6749622 66fdb6 go
2376046 85232 90984 2552262 26f1c6 gofmt
17051050 190256 130320 17371626 10911ea camlistored
After:
$ size go gofmt camlistored
text data bss dec hex filename
6403034 136996 105784 6645814 656836 go
2331118 85232 90984 2507334 264246 gofmt
16842586 190256 130320 17163162 105e39a camlistored
Fixes#8604.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/137790043
This is a reapplication of CL 93520045 (changeset 5012df7fac58)
since that was lost during the move to an internal package.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/134020043
In revision 05c3fee13eb3, openbsd/386's tfork implementation was
accidentally changed in one instruction from using the "params"
parameter to using the "psize" parameter.
While here, OpenBSD's __tfork system call returns a pid_t which is an
int32 on all OpenBSD architectures, so change runtime.tfork's return
type from int64 to int32 and update the assembly implementations
accordingly.
LGTM=iant
R=rsc, iant
CC=golang-codereviews, jsing
https://golang.org/cl/133190043
Package addition.
PositionFor permits access to both, positions
adjusted by //line comments (like the Position
accessors), and unadjusted "raw" positions
unaffected by //line comments.
Raw positions are required for correct formatting
of source code via go/printer which until now had
to manually fix adjusted positions.
Fixes#7702.
LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/135110044
This is getting a little annoying, but once the runtime structs are
being defined in Go, these will go away. So it's only a temporary cost.
TBR=bradfitz
CC=golang-codereviews
https://golang.org/cl/135940043
'range hash' makes a copy of the hash array in the stack, creating
a very large stack frame. It's just the right amount that it
uses most but not all of the total stack size. If you have a lot
of environment variables, like the builders, then this is too
much and the g0 stack runs out of space.
TBR=bradfitz
CC=golang-codereviews
https://golang.org/cl/132350043
In order to support different compression levels, make the
encoder type public, and add an Encoder method to it.
Fixes#8499.
LGTM=nigeltao
R=nigeltao, ruiu
CC=golang-codereviews
https://golang.org/cl/129190043
I changed all the NACL_SYSJMP to NACL_SYSCALL in
an earlier CL, but I missed the fact that NACL_SYSCALL
will push another return PC on the stack, so that the
arguments will no longer be in the right place.
Since we have to make our own call, we also have to
copy the arguments. Do that.
Fixes nacl/386 build.
TBR=minux
CC=golang-codereviews
https://golang.org/cl/135050044
This will allow structs containing Efaces in C to be
manipulated as structs containing real interfaces in Go.
The eface struct is still defined for use by Go code.
LGTM=iant
R=golang-codereviews, iant
CC=dvyukov, golang-codereviews, khr, r
https://golang.org/cl/133980044
Mutex is consistent with package sync, and when in the
unexported Go form it avoids having a conflcit between
the type (now mutex) and the function (lock).
LGTM=iant
R=golang-codereviews, iant
CC=dvyukov, golang-codereviews, r
https://golang.org/cl/133140043
This adds the minimal support for AArch64/arm64 relocations
needed to get cgo to work (when an isomorphic patch is applied
to gccgo) and a test.
This change uses the "AAarch64" name for the architecture rather
than the more widely accepted "arm64" because that's the name that
the relevant docs from ARM such as
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.pdf
all use.
Fixes#8533.
LGTM=iant
R=golang-codereviews, aram, gobot, iant, minux
CC=golang-codereviews
https://golang.org/cl/132000043
NaCl requires the addition of a 32-byte "halt sled" at the end
of the text segment. This means that segtext.len is actually
32 bytes shorter than reality. The computation of the file offset
of the end of the data segment did not take this 32 bytes into
account, so if len and len+32 rounded up (by 64k) to different
values, the symbol table overwrote the last page of the data
segment.
The last page of the data segment is usually the C .string
symbols, which contain the strings used in error prints
by the runtime. So when this happens, your program
probably crashes, and then when it does, you get binary
garbage instead of all the usual prints.
The chance of hitting this with a randomly sized text segment
is 32 in 65536, or 1 in 2048.
If you add or remove ANY code while trying to debug this
problem, you're overwhelmingly likely to bump the text
segment one way or the other and make the bug disappear.
Correct all the computations to use segdata.fileoff+segdata.filelen
instead of trying to rederive segdata.fileoff.
This fixes the failure during the nacl/amd64p32 build.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/135050043
The NaCl "system calls" were assumed to have a compatible
return convention with the C compiler, and we were using
tail jumps to those functions. Don't do that anymore.
Correct mistake introduced in newstackcall duringconversion
from (SP) to (FP) notation. (Actually this fix, in asm_amd64p32.s,
slipped into the C compiler change, but update the name to
match what go vet wants.)
Correct computation of caller stack pointer in morestack:
on amd64p32, the saved PC is the size of a uintreg, not uintptr.
This may not matter, since it's been like this for a while,
but uintreg is the correct one. (And on non-NaCl they are the same.)
This will allow the NaCl build to get much farther.
It will probably still not work completely.
There's a bug in 6l that needs fixing too.
TBR=minux
CC=golang-codereviews
https://golang.org/cl/134990043
runtime._sfloat2 now returns the lr value on the stack, not R0.
Credit to Russ Cox for the fix.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/133120045
uintptr or uint64 in the runtime C were turning into uint in the Go,
bool was turning into uint8, and so on. Fix that.
Also delete Go wrappers for C functions.
The C functions can be called directly now
(but still eventually need to be converted to Go).
LGTM=bradfitz, minux, iant
R=golang-codereviews, bradfitz, iant, minux
CC=golang-codereviews, khr, r
https://golang.org/cl/138740043
nanotime1 is not a Go function and must not store its result at 0(FP).
That overwrites some data owned by the caller.
TBR=aram
CC=golang-codereviews
https://golang.org/cl/138730043
Windows needs the return result in AX, but runtime.sighandler
no longer stores it in AX. Load it back during the assembly trampoline.
TBR=brainman
CC=golang-codereviews
https://golang.org/cl/133980043
The Go calling convention uses more stack space than C.
On 64-bit systems we've been right up against the limit
(128 bytes, so only 16 words) and doing awful things to
our source code to work around it. Instead of continuing
to do awful things, raise the limit to 160 bytes.
I am prepared to raise the limit to 192 bytes if necessary,
but I think this will be enough.
Should fix current link-time stack overflow errors on
- nacl/arm
- netbsd/amd64
- openbsd/amd64
- solaris/amd64
- windows/amd64
TBR=r
CC=golang-codereviews, iant
https://golang.org/cl/131450043
To date, the C compilers and Go compilers differed only in how
values were returned from functions. This made it difficult to call
Go from C or C from Go if return values were involved. It also made
assembly called from Go and assembly called from C different.
This CL changes the C compiler to use the Go conventions, passing
results on the stack, after the arguments.
[Exception: this does not apply to C ... functions, because you can't
know where on the stack the arguments end.]
By doing this, the CL makes it possible to rewrite C functions into Go
one at a time, without worrying about which languages call that
function or which languages it calls.
This CL also updates all the assembly files in package runtime to use
the new conventions. Argument references of the form 40(SP) have
been rewritten to the form name+10(FP) instead, and there are now
Go func prototypes for every assembly function called from C or Go.
This means that 'go vet runtime' checks effectively every assembly
function, and go vet's output was used to automate the bulk of the
conversion.
Some functions, like seek and nsec on Plan 9, needed to be rewritten.
Many assembly routines called from C were reading arguments
incorrectly, using MOVL instead of MOVQ or vice versa, especially on
the less used systems like openbsd.
These were found by go vet and have been corrected too.
If we're lucky, this may reduce flakiness on those systems.
Tested on:
darwin/386
darwin/amd64
linux/arm
linux/386
linux/amd64
If this breaks another system, the bug is almost certainly in the
sys_$GOOS_$GOARCH.s file, since the rest of the CL is tested
by the combination of the above systems.
LGTM=dvyukov, iant
R=golang-codereviews, 0intro, dave, alex.brainman, dvyukov, iant
CC=golang-codereviews, josharian, r
https://golang.org/cl/135830043
Every change to g->atomicstatus is now done atomically so that we can
ensure that all gs pass through a gc safepoint on demand. This allows
the GC to move from one phase to the next safely. In some phases the
stack will be scanned. This CL only deals with the infrastructure that
allows g->atomicstatus to go from one state to another. Future CLs
will deal with scanning and monitoring what phase the GC is in.
The major change was to moving to using a Gscan bit to indicate that
the status is in a scan state. The only bug fix was in oldstack where
I wasn't moving to a Gcopystack state in order to block scanning until
the new stack was in place. The proc.go file is waiting for an atomic
load instruction.
LGTM=rsc
R=golang-codereviews, dvyukov, josharian, rsc
CC=golang-codereviews, khr
https://golang.org/cl/132960044
Update #8527
Fixes two warnings:
src/cmd/gc/mparith3.c:255:10: runtime error: shift exponent 52 is too large for 32-bit type 'int'
src/cmd/gc/mparith3.c:254:14: runtime error: shift exponent 52 is too large for 32-bit type 'int'
LGTM=rsc
R=r, dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/134940044
Also make genzabbrs.go more self-contained.
Also run it (on Linux; does that matter?) to update the table.
LGTM=rsc
R=rsc, alex.brainman
CC=golang-codereviews
https://golang.org/cl/128350044
I noticed that 5g doesn't flush the float64 result back to the stack, hence the change in the function signature. I'm wondering if I should also change the signature for the other two functions.
LGTM=rsc
R=minux, josharian, rsc
CC=golang-codereviews
https://golang.org/cl/132990044
There are both many callers and many implementations of these
interfaces, so make the contract explicit. Callers generally
assume this, and at least the standard library and other
implementations obey this, but it's never stated explicitly,
making it somewhat risky to assume.
LGTM=gri, rsc
R=golang-codereviews, gri
CC=golang-codereviews, r, rsc
https://golang.org/cl/132150043
Also: use 0x644 file permission if a new file
is created (should not happen anymore, though).
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/126610044
Simplify the invocation (and speed it up substantially) in preparation
for move to go generate.
LGTM=bradfitz, mpvl
R=mpvl, bradfitz, josharian
CC=golang-codereviews
https://golang.org/cl/135790043
Once and for all.
Broken in cl/108640043.
I've messed it before. To test scavenger-related changes
one needs to alter the constants during final testing.
And then it's very easy to submit with the altered constants.
LGTM=rsc
R=golang-codereviews
CC=golang-codereviews, rsc
https://golang.org/cl/136720044
Deleted in cl/123700044.
I am not sure whether I need to restore it,
or delete rest of the uses...
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/129580043
Before, a slice with cap=0 or a string with len=0 might have its
base pointer pointing beyond the actual slice/string data into
the next block. The collector had to ignore slices and strings with
cap=0 in order to avoid misinterpreting the base pointer.
Now, a slice with cap=0 or a string with len=0 still has a base
pointer pointing into the actual slice/string data, no matter what.
The collector can now always scan the pointer, which means
strings and slices are no longer special.
Fixes#8404.
LGTM=khr, josharian
R=josharian, khr, dvyukov
CC=golang-codereviews
https://golang.org/cl/112570044
Fixes test failure in build, probably a good idea anyway.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/131210043
It was respected by unmarshal, but not marshal, so they didn't
round-trip.
Fixes#8582
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/132960043
A whole thread is too much for background scavenger that sleeps all the time anyway.
We already have sysmon thread that can do this work.
Also remove g->isbackground and simplify enter/exitsyscall.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews, khr, rlh
https://golang.org/cl/108640043
Normally, an expression of the form x.f or *y can be reordered
with function calls and communications.
Select is stricter than normal: each channel expression is evaluated
in source order. If you have case <-x.f and case <-foo(), then if the
evaluation of x.f causes a panic, foo must not have been called.
(This is in contrast to an expression like x.f + foo().)
Enforce this stricter ordering.
Fixes#8336.
LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews, r
https://golang.org/cl/126570043
Reduce duration of critical section,
make pcbuf local to function.
LGTM=rsc
R=golang-codereviews
CC=golang-codereviews, rsc
https://golang.org/cl/102600043
Update #8525
Some temporary variables that were fully registerized nevertheless had stack space allocated for them because the Addrs were still marked as having associated nodes.
Distribution of stack space reserved for temporary variables while running make.bash (6g):
BEFORE
40.89% 7026 allocauto: 0 to 0
7.83% 1346 allocauto: 0 to 24
7.22% 1241 allocauto: 0 to 8
6.30% 1082 allocauto: 0 to 16
4.96% 853 allocauto: 0 to 56
4.59% 789 allocauto: 0 to 32
2.97% 510 allocauto: 0 to 40
2.32% 399 allocauto: 0 to 48
2.10% 360 allocauto: 0 to 64
1.91% 328 allocauto: 0 to 72
AFTER
48.49% 8332 allocauto: 0 to 0
9.52% 1635 allocauto: 0 to 16
5.28% 908 allocauto: 0 to 48
4.80% 824 allocauto: 0 to 32
4.73% 812 allocauto: 0 to 8
3.38% 581 allocauto: 0 to 24
2.35% 404 allocauto: 0 to 40
2.32% 399 allocauto: 0 to 64
1.65% 284 allocauto: 0 to 56
1.34% 230 allocauto: 0 to 72
LGTM=rsc
R=rsc
CC=dave, dvyukov, golang-codereviews, minux
https://golang.org/cl/126160043
cmd/6g has been doing this for a long time.
Arrays are still problematic on 5g because the addressing
for t[0] where local var t has type [3]uintptr takes the address of t.
That's issue 8125.
Fixes#8123.
LGTM=josharian
R=josharian, dave
CC=golang-codereviews
https://golang.org/cl/102890046
CL 130240043 did this but broke ARM, because
it made newErrorCString start allocating, so we rolled
it back in CL 133810043.
CL 133820043 removed that allocation.
Try again.
Fixes#8405.
LGTM=bradfitz, dave
R=golang-codereviews, bradfitz
CC=dave, golang-codereviews, r
https://golang.org/cl/133830043
Now 'go vet runtime' only shows:
malloc.go:200: possible misuse of unsafe.Pointer
malloc.go:214: possible misuse of unsafe.Pointer
malloc.go:250: possible misuse of unsafe.Pointer
stubs.go:167: possible misuse of unsafe.Pointer
Those are all unavoidable.
LGTM=josharian
R=golang-codereviews, dvyukov, josharian
CC=dave, golang-codereviews
https://golang.org/cl/135730043
It now serves as an example for go generate as well as for yacc.
NOTE: Depends on go generate, which is not yet checked in.
This is a proof of concept of the approach.
That is https://golang.org/cl/125580044.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/125620044
This is based on the crash dump provided by Alan
and on mental experiments:
sweep 0 74
fatal error: gc: unswept span
runtime stack:
runtime.throw(0x9df60d)
markroot(0xc208002000, 0x3)
runtime.parfordo(0xc208002000)
runtime.gchelper()
I think that when we moved all stacks into heap,
we introduced a bunch of bad data races. This was later
worsened by parallel stack shrinking.
Observation 1: exitsyscall can allocate a stack from heap at any time (including during STW).
Observation 2: parallel stack shrinking can (surprisingly) grow heap during marking.
Consider that we steadily grow stacks of a number of goroutines from 8K to 16K.
And during GC they all can be shrunk back to 8K. Shrinking will allocate lots of 8K
stacks, and we do not necessary have that many in heap at this moment. So shrinking
can grow heap as well.
Consequence: any access to mheap.allspans in GC (and otherwise) must take heap lock.
This is not true in several places.
Fix this by protecting accesses to mheap.allspans and introducing allspans cache for marking,
similar to what we use for sweeping.
LGTM=rsc
R=golang-codereviews, rsc
CC=adonovan, golang-codereviews, khr, rlh
https://golang.org/cl/126510043
The low-level implementation of divide on ARM assumes that
it can panic with an error created by newErrorCString without
allocating. If we make interface data words require pointer values,
the current definition would require an allocation when stored
in an interface. Changing the definition to use unsafe.Pointer
instead of uintptr avoids the allocation. This change is okay
because the field really is a pointer (to a C string in rodata).
Update #8405.
This should make CL 133830043 safe to try again.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=dave, golang-codereviews, r
https://golang.org/cl/133820043
This change broke divmod.go on all arm platforms.
««« original CL description
cmd/gc: change interface representation: only pointers in data word
Note that there are various cleanups that can be made if we keep
this change, but I do not want to start making changes that
depend on this one until the 1.4 cycle closes.
Fixes#8405.
LGTM=r
R=golang-codereviews, adg, r, bradfitz
CC=golang-codereviews, iant
https://golang.org/cl/130240043
»»»
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/133810043
This is a very dumb translation to keep the code as close to the original C as possible.
LGTM=rsc
R=khr, minux, rsc, josharian
CC=golang-codereviews
https://golang.org/cl/126490043
Note that there are various cleanups that can be made if we keep
this change, but I do not want to start making changes that
depend on this one until the 1.4 cycle closes.
Fixes#8405.
LGTM=r
R=golang-codereviews, adg, r, bradfitz
CC=golang-codereviews, iant
https://golang.org/cl/130240043
This makes newproc invisible to the GC. This is a pretty simple change since parts of newproc already depends on being run on the M stack.
LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews, khr
https://golang.org/cl/129520043
The current code is correct, but vet does not understand it:
asm_amd64.s:963: [amd64] invalid MOVL of ret+0(FP); int64 is 8-byte value
asm_amd64.s:964: [amd64] invalid offset ret+4(FP); expected ret+0(FP)
LGTM=minux
R=golang-codereviews, minux
CC=golang-codereviews
https://golang.org/cl/125200044
Fix issue by always appending newline after user input, before
the closing curly bracket. The adjust func is modified to remove
this new newline.
Add test case (it fails before CL, passes after).
Fixes#8411.
LGTM=gri
R=golang-codereviews, bradfitz, josharian, gri
CC=golang-codereviews
https://golang.org/cl/124700043
Fixes#8503.
Thanks to no.smile.face for the original report.
LGTM=bradfitz, r, ruiu
R=bradfitz, ruiu, r
CC=golang-codereviews
https://golang.org/cl/132730043
1) Interpret a comment of the form
//gofmt <flags>
in test files to drive the respective
gofmt command. Eliminates the need to
enumerate all test files in the test
harness.
2) Added -update flag to make it easier
to update test cases.
LGTM=josharian
R=golang-codereviews, josharian
CC=golang-codereviews
https://golang.org/cl/130440043
Update #8527
Fixes, src/cmd/6l/../ld/pcln.c:93:18: runtime error: left shift of negative value -2
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/127440043
This files were added accidentally and are
not required for running the tests (they
are produced by failing tests for easier
debugging).
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/131030044
Fixes compilation of runtime on Solaris where the inner struct
was not called "_4_".
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/129460043
Australian timezones abbreviation for standard and daylight saving time were recently
changed from EST for both to AEST and AEDT in the icann tz database (see changelog
on www.iana.org/time-zones).
A test in the time package was written to check that the ParseInLocation function
understand that Feb EST and Aug EST are different time zones, even though they are
both called EST. This is no longer the case, and the Date function now returns
AEST or AEDT for australian tz on every Linux system with an up to date tz database
(and this makes the test fail).
Since I wasn't able to find another country that 1) uses daylight saving and 2) has
the same abbreviation for both on tzdata, I changed the test to make sure that
ParseInLocation does not get confused when it parses, in different locations, two
dates with the same abbreviation (this was suggested in the mailing list).
Fixes#8547.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/130920043
Cleanup before converting to Go.
Fortunately nobody using it, because it is incorrect:
monotonic runtime time instead of claimed real time.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rsc
https://golang.org/cl/129480043
These are required for chans, semaphores, timers, etc.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/123640043
UndefinedBehaviorSanitizer claims it is UB in C:
src/cmd/gc/racewalk.c:422:37: runtime error: member access within null pointer of type 'Node' (aka 'struct Node')
src/cmd/gc/racewalk.c:423:37: runtime error: member access within null pointer of type 'Node' (aka 'struct Node')
LGTM=rsc
R=dave, rsc
CC=golang-codereviews
https://golang.org/cl/125570043
Init GC later as it needs to read GOGC env var.
Fixes#8562.
LGTM=daniel.morsing, rsc
R=golang-codereviews, daniel.morsing, rsc
CC=golang-codereviews, khr, rlh
https://golang.org/cl/130990043
Calling ReadMemStats which does stoptheworld on m0 holding locks
was not a good idea.
Stoptheworld holding locks is a recipe for deadlocks (added check for this).
Stoptheworld on g0 may or may not work (added check for this as well).
As far as I understand scavenger will print incorrect numbers now,
as stack usage is not subtracted from heap. But it's better than deadlocking.
LGTM=khr
R=golang-codereviews, rsc, khr
CC=golang-codereviews, rlh
https://golang.org/cl/124670043
zsyscall_windows_386.go and zsyscall_windows_amd64.go contain same bytes
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/124640043
The existing lock needed to be held longer. If a timeout occured
while writing (but after the guarded timeout check), the writes
would clobber a future connection's buffer.
Also remove a harmless warning by making Write also set the
flag that headers were sent (implicitly), so we don't try to
write headers later (a no-op + warning) on timeout after we've
started writing.
Fixes#8414Fixes#8209
LGTM=ruiu, adg
R=adg, ruiu
CC=golang-codereviews
https://golang.org/cl/123610043
Half the code in the garbage collector accesses the bitmap
as an array of bytes instead of as an array of uintptrs.
This is tricky to do correctly in a portable fashion,
it breaks on big-endian systems.
Make the bitmap a byte array.
Simplifies markallocated, scanblock and span sweep along the way,
as we don't need to recalculate bitmap position for each word.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/125250043
We allocate scannable memory w/o type only in few places in runtime.
All these cases are not-performance critical (e.g. G or finq args buffer),
and in long term they all need to go away.
It's not worth it to have special code for this case in mallocgc.
So use special fake "notype" type for such allocations.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/127450044
Currently goroutines in onM can't be copied/shrunk
(including the very goroutine that triggers GC).
Special case onM to allow copying.
LGTM=daniel.morsing, khr
R=golang-codereviews, daniel.morsing, khr, rsc
CC=golang-codereviews, rlh
https://golang.org/cl/124550043
Newly allocated memory is subtracted from inuse, while it was never added to inuse.
Span leftovers are subtracted from both inuse and idle,
while they were never added.
Fixes#8544.
Fixes#8430.
LGTM=khr, cookieo9
R=golang-codereviews, khr, cookieo9
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/130200044
We need to change the interface value representation for
concurrent garbage collection, so that there is no ambiguity
about whether the data word holds a pointer or scalar.
This CL does NOT make any representation changes.
Instead, it removes representation assumptions from
various pieces of code throughout the tree.
The isdirectiface function in cmd/gc/subr.c is now
the only place that decides that policy.
The policy propagates out from there in the reflect
metadata, as a new flag in the internal kind value.
A follow-up CL will change the representation by
changing the isdirectiface function. If that CL causes
problems, it will be easy to roll back.
Update #8405.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/129090043
The change to pc-relative addressing will make this illegal.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/129890043
Update #8527
Fixes, cmd/6g/reg.c:847:24: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
LGTM=minux, rsc
R=minux, rsc
CC=dvyukov, golang-codereviews
https://golang.org/cl/129290043
type T byte
func (T) String() string { return "X" }
fmt.Sprintf("%s", []T{97, 98, 99, 100}) == "abcd"
fmt.Sprintf("%x", []T{97, 98, 99, 100}) == "61626364"
fmt.Sprintf("%v", []T{97, 98, 99, 100}) == "[X X X X]"
This change makes the last case print correctly.
Before, it would have been "[97 98 99 100]".
Fixes#8360.
LGTM=r
R=r, dan.kortschak
CC=golang-codereviews
https://golang.org/cl/129330043
Improve performance of move-to-front by using cache-friendly
copies instead of doubly-linked list. Simplify so that the
underlying slice is the object. Remove the n=0 special case,
which was actually slower with the copy approach.
benchmark old ns/op new ns/op delta
BenchmarkDecodeDigits 26429714 23859699 -9.72%
BenchmarkDecodeTwain 76684510 67591946 -11.86%
benchmark old MB/s new MB/s speedup
BenchmarkDecodeDigits 1.63 1.81 1.11x
BenchmarkDecodeTwain 1.63 1.85 1.13x
Updates #6754.
LGTM=adg, agl, josharian
R=adg, agl, josharian
CC=golang-codereviews
https://golang.org/cl/131840043
Currently we do the following dance after sweeping a span:
1. lock mcentral
2. remove the span from a list
3. unlock mcentral
4. unmark span
5. lock mheap
6. insert the span into heap
7. unlock mheap
8. lock mcentral
9. observe empty list
10. unlock mcentral
11. lock mheap
12. grab the span
13. unlock mheap
14. mark span
15. lock mcentral
16. insert the span into empty list
17. unlock mcentral
This change short-circuits this sequence to nothing,
that is, we just cache and use the span after sweeping.
This gives us functionality similar (even better) to tcmalloc's transfer cache.
benchmark old ns/op new ns/op delta
BenchmarkMalloc8 22.2 19.5 -12.16%
BenchmarkMalloc16 31.0 26.6 -14.19%
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/119550043
Mallocgc must be atomic wrt GC, but for performance reasons
don't acquirem/releasem on fast path. The code does not have
split stack checks, so it can't be preempted by GC.
Functions like roundup/add are inlined. And onM/racemalloc are nosplit.
Also add debug code that checks these assumptions.
benchmark old ns/op new ns/op delta
BenchmarkMalloc8 20.5 17.2 -16.10%
BenchmarkMalloc16 29.5 27.0 -8.47%
BenchmarkMallocTypeInfo8 31.5 27.6 -12.38%
BenchmarkMallocTypeInfo16 34.7 30.9 -10.95%
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/123100043
Fixes#8480.
This CL reapplies CL 114420043. This attempt doesn't blow up when encountering hidden symbols.
LGTM=minux
R=minux
CC=golang-codereviews
https://golang.org/cl/128310043
When building golang, the environment variable GOROOT_FINAL can be set
to indicate a different installation location from the build
location. This works fine, except that the goc2c build step embeds
line numbers in the resulting c source files that refer to the build
location, no the install location.
This would not be a big deal, except that in turn the linker uses the
location of runtime/string.goc to embed the gdb script in the
resulting binary and as a net result, the debugger now complains that
the script is outside its load path (it has the install location
configured).
See https://code.google.com/p/go/issues/detail?id=8524 for the full
description.
Fixes#8524.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/128230046
bv.data is an array of uint32s but the code was using
offsets computed for an array of bytes.
Add a test for stack GC info.
Fixes#8531.
LGTM=rsc
R=golang-codereviews
CC=golang-codereviews, khr, rsc
https://golang.org/cl/124450043
E.g., here's the new "go build" output:
$ go build misc/cgo/errors/issue8442.go
# command-line-arguments
could not determine kind of name for C.issue8442foo
gcc errors for preamble:
misc/cgo/errors/issue8442.go:11:19: error: unknown type name 'UNDEF'
Fixes#8442.
LGTM=iant
R=iant, alex.brainman
CC=golang-codereviews
https://golang.org/cl/129160043
In cgo, now that recursive calls to typeConv.Type() always work,
we can more robustly calculate the array sizes based on the size
of our element type.
Also, in debug/dwarf, the decision to call zeroType is made
based on a type's usage within a particular struct, but dwarf.Type
values are cached in typeCache, so the modification might affect
uses of the type in other structs. Current compilers don't appear
to share DWARF type entries for "[]foo" and "[0]foo", but they also
don't consistently share type entries in other cases. Arguably
modifying the types is an improvement in some cases, but varying
translated types according to compiler whims seems like a bad idea.
Lastly, also in debug/dwarf, zeroType only needs to rewrite the
top-level dimension, and only if the rest of the array size is
non-zero.
Fixes#8428.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/127980043
Restore https://golang.org/cl/41040043 after GC rewrite.
Original description:
On the plus side, we don't need to change the bits on malloc and free.
On the downside, we need to mark objects in the free lists during GC.
But the free lists are small at GC time, so it should be a net win.
benchmark old ns/op new ns/op delta
BenchmarkMalloc8 21.9 20.4 -6.85%
BenchmarkMalloc16 31.1 29.6 -4.82%
LGTM=khr
R=khr
CC=golang-codereviews, rlh, rsc
https://golang.org/cl/122280043
Basically this cleanup replaces all the usage usages of strcmp() == 0,
found by the following command line:
$ grep -R strcmp cmd/dist | grep "0"
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/123330043
The file is used by assembly code to define symbols like NOSPLIT.
Having it hidden inside the cmd directory makes it hard to access
outside the standard repository.
Solution: As with a couple of other files used by cgo, copy the
file into the pkg directory and add a -I argument to the assembler
to access it. Thus one can write just
#include "textflag.h"
in .s files.
The names in runtime are not updated because in the boot sequence the
file has not been copied yet when runtime is built. All other .s files
in the repository are updated.
Changes to doc/asm.html, src/cmd/dist/build.c, and src/cmd/go/build.go
are hand-made. The rest are just the renaming done by a global
substitution. (Yay sam).
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/128050043
This allows implementing address-of-global
as a pc-relative address instead of as a
32-bit integer constant.
LGTM=rminnich, iant
R=golang-codereviews, rminnich, iant
CC=golang-codereviews
https://golang.org/cl/128070045
This allows changing the addressing mode for constant
global addresses to use pc-relative addressing.
LGTM=rminnich, iant
R=golang-codereviews, rminnich, iant
CC=golang-codereviews
https://golang.org/cl/129830043
Add a clause to the doc comment for the package and a
paragraph in the compatibility document explaining the
situation.
LGTM=bradfitz, adg, rsc
R=golang-codereviews, adg, bradfitz, minux, rsc
CC=golang-codereviews
https://golang.org/cl/129820043
codeblk and datblk were truncating their
arguments to int32. Don't do that.
LGTM=dvyukov, rminnich
R=iant, dvyukov, rminnich
CC=golang-codereviews
https://golang.org/cl/126050043
See golang.org/s/go14customimport for design.
Added case to deps_test to allow go/build to import regexp.
Not a new dependency, because go/build already imports go/doc
which imports regexp.
Fixes#7453.
LGTM=r
R=r, josharian
CC=golang-codereviews
https://golang.org/cl/124940043
It's unclear why we do this broken double-checked locking.
The mutex is not held for the whole duration of CPU profiling.
Fixes#8365.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/116290043
This function does not have a declaration/prototype in a.h, and it is used only
in buf.c, so it is local to it and thus can be marked as private by adding
'static' to it.
LGTM=iant
R=rsc, iant
CC=golang-codereviews
https://golang.org/cl/122300043
misc/cgo/testgodefs was added by revision d1cf884a594f, but not
add to run.bash.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/129760044
This makes GCC behavior (and cgo build failures) deterministic.
Fixes#8487.
Ran this shell command on linux/amd64 (Ubuntu 12.04) before and
after this change:
for x in `seq 100`; do
go tool cgo -debug-gcc=true issue8441.go 2>&1 | md5sum
done | sort | uniq -c
Before:
67 2cdcb8c7c4e290f7d9009abc581b83dd -
10 9a55390df94f7cec6d810f3e20590789 -
10 acfad22140d43d9b9517bbc5dfc3c0df -
13 c337f8fee2304b3a8e3158a4362d8698 -
After:
100 785c316cbcbcd50896695050e2fa23c1 -
LGTM=minux, iant
R=golang-codereviews, bradfitz, minux, iant
CC=golang-codereviews
https://golang.org/cl/126990043
Credit to Rémy for finding and writing test case.
Fixes#8325.
LGTM=r
R=golang-codereviews, r
CC=dave, golang-codereviews, iant, remyoudompheng
https://golang.org/cl/124950043
The >>1 shift needs to happen before converting to int32, otherwise
large values will decode with an incorrect sign bit.
The <<31 shift can happen before or after, but before is consistent
with liblink and the go12symtab doc.
Bug demo at http://play.golang.org/p/jLrhPUakIu
LGTM=rsc
R=golang-codereviews, minux, rsc
CC=golang-codereviews
https://golang.org/cl/119630043
When formatting time zone offsets with seconds using the stdISO8601Colon
and stdNumColon layouts, the colon was missing between the hour and minute
parts.
Fixes#8497.
LGTM=r
R=golang-codereviews, iant, gobot, r
CC=golang-codereviews
https://golang.org/cl/126840043
On OS X 10.10 Yosemite, if you have a directory that can be returned
in a single getdirentries64 call (for example, a directory with one file),
and you read from the directory at EOF twice, you get EOF both times:
fd = open("dir")
getdirentries64(fd) returns data
getdirentries64(fd) returns 0 (EOF)
getdirentries64(fd) returns 0 (EOF)
But if you remove the file in the middle between the two calls, the
second call returns an error instead.
fd = open("dir")
getdirentries64(fd) returns data
getdirentries64(fd) returns 0 (EOF)
remove("dir/file")
getdirentries64(fd) returns ENOENT/EINVAL
Whether you get ENOENT or EINVAL depends on exactly what was
in the directory. It is deterministic, just data-dependent.
This only happens in small directories. A directory containing more data
than fits in a 4k getdirentries64 call will return EOF correctly.
(It's not clear if the criteria is that the directory be split across multiple
getdirentries64 calls or that it be split across multiple file system blocks.)
We could change package os to avoid the second read at EOF,
and maybe we should, but that's a bit involved.
For now, treat the EINVAL/ENOENT as EOF.
With this CL, all.bash passes on my MacBook Air running
OS X 10.10 (14A299l) and Xcode 6 beta 5 (6A279r).
I tried filing an issue with Apple using "Feedback Assistant", but it was
unable to send the report and lost it.
C program reproducing the issue, also at http://swtch.com/~rsc/readdirbug.c:
#include <stdio.h>
#include <dirent.h>
#include <unistd.h>
#include <sys/stat.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
static void test(int);
int
main(void)
{
int fd, n;
DIR *dir;
struct dirent *dp;
struct stat st;
char buf[10000];
long basep;
int saw;
if(stat("/tmp/readdirbug", &st) >= 0) {
fprintf(stderr, "please rm -r /tmp/readdirbug and run again\n");
exit(1);
}
fprintf(stderr, "mkdir /tmp/readdirbug\n");
if(mkdir("/tmp/readdirbug", 0777) < 0) {
perror("mkdir /tmp/readdirbug");
exit(1);
}
fprintf(stderr, "create /tmp/readdirbug/file1\n");
if((fd = creat("/tmp/readdirbug/file1", 0666)) < 0) {
perror("create /tmp/readdirbug/file1");
exit(1);
}
close(fd);
test(0);
test(1);
fprintf(stderr, "ok - everything worked\n");
}
static void
test(int doremove)
{
DIR *dir;
struct dirent *dp;
int numeof;
fprintf(stderr, "\n");
fprintf(stderr, "opendir /tmp/readdirbug\n");
dir = opendir("/tmp/readdirbug");
if(dir == 0) {
perror("open /tmp/readdirbug");
exit(1);
}
numeof = 0;
for(;;) {
errno = 0;
dp = readdir(dir);
if(dp != 0) {
fprintf(stderr, "readdir: found %s\n", dp->d_name);
continue;
}
if(errno != 0) {
perror("readdir");
exit(1);
}
fprintf(stderr, "readdir: EOF\n");
if(++numeof == 3)
break;
if(doremove) {
fprintf(stderr, "rm /tmp/readdirbug/file1\n");
if(remove("/tmp/readdirbug/file1") < 0) {
perror("remove");
exit(1);
}
}
}
fprintf(stderr, "closedir\n");
closedir(dir);
}
Fixes#8423.
LGTM=bradfitz, r
R=golang-codereviews, bradfitz, dsymonds, dave, r
CC=golang-codereviews, iant
https://golang.org/cl/119530044
Current version of Getwd calls Stat that
calls Getwd therefore infinite recursion.
LGTM=minux
R=golang-codereviews, minux
CC=golang-codereviews
https://golang.org/cl/119600043
The ast and printer don't care, and go/types can provide
a better error message.
This change requires an update to the tests for go/types
(go.tools repo). CL forthcoming.
Fixes#8493.
LGTM=adonovan
R=rsc, adonovan
CC=golang-codereviews
https://golang.org/cl/123010044
Some systems, like Ubuntu, pass --build-id when linking. The
effect is to put a note in the output file. This is not
useful when generating an object file with the -r option, as
it eventually causes multiple build ID notes in the final
executable, all but one of which are for tiny portions of the
file and are therefore useless.
Disable that by passing an explicit --build-id=none when
linking with -r on systems that might do this.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/119460043
There are lots of internal synchronization in gob,
so it makes sense to have parallel benchmarks.
Also add a benchmark with slices and interfaces.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/115960043
This lets us have non-main packages like cmd/internal or cmd/nm/internal/whatever.
The src/pkg migration (see golang.org/s/go14mainrepo) will allow this
as a natural side effect. The explicit change here just allows use of the
effect a little sooner.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/117630043
