Variables declared with 'var' have no sym->def.
Fixes#7794.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/88360043
The new code is adapted from the Go 1.2 nosplit code,
but it does not have the bug reported in issue 7623:
g% go run nosplit.go
g% go1.2 run nosplit.go
BUG
rejected incorrectly:
main 0 call f; f 120
linker output:
# _/tmp/go-test-nosplit021064539
main.main: nosplit stack overflow
120 guaranteed after split check in main.main
112 on entry to main.f
-8 after main.f uses 120
g%
Fixes#6931.
Fixes#7623.
LGTM=iant
R=golang-codereviews, iant, ality
CC=golang-codereviews, r
https://golang.org/cl/88190043
Trying to make GODEBUG=gcdead=1 work with liveness
and in particular ambiguously live variables.
1. In the liveness computation, mark all ambiguously live
variables as live for the entire function, except the entry.
They are zeroed directly after entry, and we need them not
to be poisoned thereafter.
2. In the liveness computation, compute liveness (and deadness)
for all parameters, not just pointer-containing parameters.
Otherwise gcdead poisons untracked scalar parameters and results.
3. Fix liveness debugging print for -live=2 to use correct bitmaps.
(Was not updated for compaction during compaction CL.)
4. Correct varkill during map literal initialization.
Was killing the map itself instead of the inserted value temp.
5. Disable aggressive varkill cleanup for call arguments if
the call appears in a defer or go statement.
6. In the garbage collector, avoid bug scanning empty
strings. An empty string is two zeros. The multiword
code only looked at the first zero and then interpreted
the next two bits in the bitmap as an ordinary word bitmap.
For a string the bits are 11 00, so if a live string was zero
length with a 0 base pointer, the poisoning code treated
the length as an ordinary word with code 00, meaning it
needed poisoning, turning the string into a poison-length
string with base pointer 0. By the same logic I believe that
a live nil slice (bits 11 01 00) will have its cap poisoned.
Always scan full multiword struct.
7. In the runtime, treat both poison words (PoisonGC and
PoisonStack) as invalid pointers that warrant crashes.
Manual testing as follows:
- Create a script called gcdead on your PATH containing:
#!/bin/bash
GODEBUG=gcdead=1 GOGC=10 GOTRACEBACK=2 exec "$@"
- Now you can build a test and then run 'gcdead ./foo.test'.
- More importantly, you can run 'go test -short -exec gcdead std'
to run all the tests.
Fixes#7676.
While here, enable the precise scanning of slices, since that was
disabled due to bugs like these. That now works, both with and
without gcdead.
Fixes#7549.
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83410044
1. Use n->alloc, not n->left, to hold the allocated temp being
passed from orderstmt/orderexpr to walk.
2. Treat method values the same as closures.
3. Use killed temporary for composite literal passed to
non-escaping function argument.
4. Clean temporaries promptly in if and for statements.
5. Clean temporaries promptly in select statements.
As part of this, move all the temporary-generating logic
out of select.c into order.c, so that the temporaries can
be reclaimed.
With the new temporaries, can re-enable the 1-entry
select optimization. Fixes issue 7672.
While we're here, fix a 1-line bug in select processing
turned up by the new liveness test (but unrelated; select.c:72).
Fixes#7686.
6. Clean temporaries (but not particularly promptly) in switch
and range statements.
7. Clean temporary used during convT2E/convT2I.
8. Clean temporaries promptly during && and || expressions.
---
CL 81940043 reduced the number of ambiguously live temps
in the godoc binary from 860 to 711.
CL 83090046 reduced the number from 711 to 121.
This CL reduces the number from 121 to 23.
15 the 23 that remain are in fact ambiguously live.
The final 8 could be fixed but are not trivial and
not common enough to warrant work at this point
in the release cycle.
These numbers only count ambiguously live temps,
not ambiguously live user-declared variables.
There are 18 such variables in the godoc binary after this CL,
so a total of 41 ambiguously live temps or user-declared
variables.
The net effect is that zeroing anything on entry to a function
should now be a rare event, whereas earlier it was the
common case.
This is good enough for Go 1.3, and probably good
enough for future releases too.
Fixes#7345.
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83000048
1. In functions with heap-allocated result variables or with
defer statements, the return sequence requires more than
just a single RET instruction. There is an optimization that
arranges for all returns to jump to a single copy of the return
epilogue in this case. Unfortunately, that optimization is
fundamentally incompatible with PC-based liveness information:
it takes PCs at many different points in the function and makes
them all land at one PC, making the combined liveness information
at that target PC a mess. Disable this optimization, so that each
return site gets its own copy of the 'call deferreturn' and the
copying of result variables back from the heap.
This removes quite a few spurious 'ambiguously live' variables.
2. Let orderexpr allocate temporaries that are passed by address
to a function call and then die on return, so that we can arrange
an appropriate VARKILL.
2a. Do this for ... slices.
2b. Do this for closure structs.
2c. Do this for runtime.concatstring, which is the implementation
of large string additions. Change representation of OADDSTR to
an explicit list in typecheck to avoid reconstructing list in both
walk and order.
3. Let orderexpr allocate the temporary variable copies used for
range loops, so that they can be killed when the loop is over.
Similarly, let it allocate the temporary holding the map iterator.
CL 81940043 reduced the number of ambiguously live temps
in the godoc binary from 860 to 711.
This CL reduces the number to 121. Still more to do, but another
good checkpoint.
Update #7345
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83090046
The new channel and map runtime routines take pointers
to values, typically temporaries. Without help, the compiler
cannot tell when those temporaries stop being needed,
because it isn't sure what happened to the pointer.
Arrange to insert explicit VARKILL instructions for these
temporaries so that the liveness analysis can avoid seeing
them as "ambiguously live".
The change is made in order.c, which was already in charge of
introducing temporaries to preserve the order-of-evaluation
guarantees. Now its job has expanded to include introducing
temporaries as needed by runtime routines, and then also
inserting the VARKILL annotations for all these temporaries,
so that their lifetimes can be shortened.
In order to do its job for the map runtime routines, order.c arranges
that all map lookups or map assignments have the form:
x = m[k]
x, y = m[k]
m[k] = x
where x, y, and k are simple variables (often temporaries).
Likewise, receiving from a channel is now always:
x = <-c
In order to provide the map guarantee, order.c is responsible for
rewriting x op= y into x = x op y, so that m[k] += z becomes
t = m[k]
t2 = t + z
m[k] = t2
While here, fix a few bugs in order.c's traversal: it was failing to
walk into select and switch case bodies, so order of evaluation
guarantees were not preserved in those situations.
Added tests to test/reorder2.go.
Fixes#7671.
In gc/popt's temporary-merging optimization, allow merging
of temporaries with their address taken as long as the liveness
ranges do not intersect. (There is a good chance of that now
that we have VARKILL annotations to limit the liveness range.)
Explicitly killing temporaries cuts the number of ambiguously
live temporaries that must be zeroed in the godoc binary from
860 to 711, or -17%. There is more work to be done, but this
is a good checkpoint.
Update #7345
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/81940043
1. On entry to a function, only zero the ambiguously live stack variables.
Before, we were zeroing all stack variables containing pointers.
The zeroing is pretty inefficient right now (issue 7624), but there are also
too many stack variables detected as ambiguously live (issue 7345),
and that must be addressed before deciding how to improve the zeroing code.
(Changes in 5g/ggen.c, 6g/ggen.c, 8g/ggen.c, gc/pgen.c)
Fixes#7647.
2. Make the regopt word-based liveness analysis preserve the
whole-variable liveness property expected by the garbage collection
bitmap liveness analysis. That is, if the regopt liveness decides that
one word in a struct needs to be preserved, make sure it preserves
the entire struct. This is particularly important for multiword values
such as strings, slices, and interfaces, in which all the words need
to be present in order to understand the meaning.
(Changes in 5g/reg.c, 6g/reg.c, 8g/reg.c.)
Fixes#7591.
3. Make the regopt word-based liveness analysis treat a variable
as having its address taken - which makes it preserved across
all future calls - whenever n->addrtaken is set, for consistency
with the gc bitmap liveness analysis, even if there is no machine
instruction actually taking the address. In this case n->addrtaken
is incorrect (a nicer way to put it is overconservative), and ideally
there would be no such cases, but they can happen and the two
analyses need to agree.
(Changes in 5g/reg.c, 6g/reg.c, 8g/reg.c; test in bug484.go.)
Fixes crashes found by turning off "zero everything" in step 1.
4. Remove spurious VARDEF annotations. As the comment in
gc/pgen.c explains, the VARDEF must immediately precede
the initialization. It cannot be too early, and it cannot be too late.
In particular, if a function call sits between the VARDEF and the
actual machine instructions doing the initialization, the variable
will be treated as live during that function call even though it is
uninitialized, leading to problems.
(Changes in gc/gen.c; test in live.go.)
Fixes crashes found by turning off "zero everything" in step 1.
5. Do not treat loading the address of a wide value as a signal
that the value must be initialized. Instead depend on the existence
of a VARDEF or the first actual read/write of a word in the value.
If the load is in order to pass the address to a function that does
the actual initialization, treating the load as an implicit VARDEF
causes the same problems as described in step 4.
The alternative is to arrange to zero every such value before
passing it to the real initialization function, but this is a much
easier and more efficient change.
(Changes in gc/plive.c.)
Fixes crashes found by turning off "zero everything" in step 1.
6. Treat wide input parameters with their address taken as
initialized on entry to the function. Otherwise they look
"ambiguously live" and we will try to emit code to zero them.
(Changes in gc/plive.c.)
Fixes crashes found by turning off "zero everything" in step 1.
7. An array of length 0 has no pointers, even if the element type does.
Without this change, the zeroing code complains when asked to
clear a 0-length array.
(Changes in gc/reflect.c.)
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/80160044
Revision 3ae4607a43ff introduced CONVNOP layers
to fix type checking issues arising from comparisons.
The added complexity made 8g run out of registers
when compiling an equality function in go.net/ipv6.
A similar issue occurred in test/sizeof.go on
amd64p32 with 6g.
Fixes#7405.
LGTM=khr
R=rsc, dave, iant, khr
CC=golang-codereviews
https://golang.org/cl/78100044
A too large float constant is an error.
A too small float constant is rounded to zero.
Fixes#7419
Update #6902
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/76730046
The lowering to runtime calls introduces hidden pointers to the
arguments of select clauses. When implicit conversions were
involved it could end up with incompatible pointers. Since the
pointed-to types have the same representation, we can introduce a
forced conversion.
Fixes#6847.
LGTM=rsc
R=rsc, iant, khr
CC=golang-codereviews
https://golang.org/cl/72380043
The garbage collector uses type information to guide the
traversal of the heap. If it sees a field that should be a string,
it marks the object pointed at by the string data pointer as
visited but does not bother to look at the data, because
strings contain bytes, not pointers.
If you save s[len(s):] somewhere, though, the string data pointer
actually points just beyond the string data; if the string data
were exactly the size of an allocated block, the string data
pointer would actually point at the next block. It is incorrect
to mark that next block as visited and not bother to look at
the data, because the next block may be some other type
entirely.
The fix is to ignore strings with zero length during collection:
they are empty and can never become non-empty: the base
pointer will never be used again. The handling of slices already
does this (but using cap instead of len).
This was not a bug in Go 1.2, because until January all string
allocations included a trailing NUL byte not included in the
length, so s[len(s):] still pointed inside the string allocation
(at the NUL).
This bug was causing the crashes in test/run.go. Specifically,
the parsing of a regexp in package regexp/syntax allocated a
[]syntax.Inst with rounded size 1152 bytes. In fact it
allocated many such slices, because during the processing of
test/index2.go it creates thousands of regexps that are all
approximately the same complexity. That takes a long time, and
test/run works on other tests in other goroutines. One such
other test is chan/perm.go, which uses an 1152-byte source
file. test/run reads that file into a []byte and then calls
strings.Split(string(src), "\n"). The string(src) creates an
1152-byte string - and there's a very good chance of it
landing next to one of the many many regexp slices already
allocated - and then because the file ends in a \n,
strings.Split records the tail empty string as the final
element in the slice. A garbage collection happens at this
point, the collection finds that string before encountering
the []syntax.Inst data it now inadvertently points to, and the
[]syntax.Inst data is not scanned for the pointers that it
contains. Each syntax.Inst contains a []rune, those are
missed, and the backing rune arrays are freed for reuse. When
the regexp is later executed, the runes being searched for are
no longer runes at all, and there is no match, even on text
that should match.
On 64-bit machines the pointer in the []rune inside the
syntax.Inst is larger (along with a few other pointers),
pushing the []syntax.Inst backing array into a larger size
class, avoiding the collision with chan/perm.go's
inadvertently sized file.
I expect this was more prevalent on OS X than on Linux or
Windows because those managed to run faster or slower and
didn't overlap index2.go with chan/perm.go as often. On the
ARM systems, we only run one errorcheck test at a time, so
index2 and chan/perm would never overlap.
It is possible that this bug is the root cause of other crashes
as well. For now we only know it is the cause of the test/run crash.
Many thanks to Dmitriy for help debugging.
Fixes#7344.
Fixes#7455.
LGTM=r, dvyukov, dave, iant
R=golang-codereviews, dave, r, dvyukov, delpontej, iant
CC=golang-codereviews, khr
https://golang.org/cl/74250043
Some of the errorcheck tests have many many identical regexps.
Use a map to avoid storing the compiled form many many times
in memory. Change the filterRe to a simple string to avoid
the expense of those regexps as well.
Cuts the time for run.go on index2.go by almost 50x.
Noticed during debugging of issue 7344.
LGTM=bradfitz
R=bradfitz, josharian
CC=golang-codereviews
https://golang.org/cl/74380043
The byte that r is or'd into is already 0x7, so the failure to zero r only
impacts the generated machine code if the register is > 7.
Fixes#7044.
LGTM=dave, minux.ma, rsc
R=dave, minux.ma, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/73730043
The cached computed interface tables are indexed by the interface
types, not by the unnamed underlying interfaces
To preserve the invariants expected by interface comparison, an
itab generated for an interface type must not be used for a value
of a different interface type even if the representation is identical.
Fixes#7207.
LGTM=rsc
R=rsc, iant, khr
CC=golang-codereviews
https://golang.org/cl/69210044
See golang.org/s/go13nacl for design overview.
This CL is the mostly mechanical changes from rsc's Go 1.2 based NaCl branch, specifically 39cb35750369 to 500771b477cf from https://code.google.com/r/rsc-go13nacl. This CL does not include working NaCl support, there are probably two or three more large merges to come.
CL 15750044 is not included as it involves more invasive changes to the linker which will need to be merged separately.
The exact change lists included are
15050047: syscall: support for Native Client
15360044: syscall: unzip implementation for Native Client
15370044: syscall: Native Client SRPC implementation
15400047: cmd/dist, cmd/go, go/build, test: support for Native Client
15410048: runtime: support for Native Client
15410049: syscall: file descriptor table for Native Client
15410050: syscall: in-memory file system for Native Client
15440048: all: update +build lines for Native Client port
15540045: cmd/6g, cmd/8g, cmd/gc: support for Native Client
15570045: os: support for Native Client
15680044: crypto/..., hash/crc32, reflect, sync/atomic: support for amd64p32
15690044: net: support for Native Client
15690048: runtime: support for fake time like on Go Playground
15690051: build: disable various tests on Native Client
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/68150047
Revision c0e0467635ec (cmd/gc: return canonical Node* from temp)
exposed original nodes of temporaries, allowing callers to mutate
their types.
In walkcompare a temporary could be typed as ideal because of
this. Additionnally, assignment of a comparison result to
a custom boolean type was broken.
Fixes#7366.
LGTM=rsc
R=rsc, iant, khr
CC=golang-codereviews
https://golang.org/cl/66930044
The error message was previously off by one in all cases.
Fixes#7150.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/65850043
Update #6853
For an ephemeral binary - one created, run, and then deleted -
there is no need to write dwarf debug information, since the
binary will not be used with gdb. In this case, instruct the linker
not to spend time and disk space generating the debug information
by passing the -w flag to the linker.
Omitting dwarf information reduces the size of most binaries by 25%.
We may be more aggressive about this in the future.
LGTM=bradfitz, r
R=r, bradfitz
CC=golang-codereviews
https://golang.org/cl/65890043
Not recording the address being taken was causing
the liveness analysis not to preserve x in the absence
of direct references to x, which in turn was making the
net test fail with GOGC=0.
In addition to the test, this fixes a bug wherein
GOGC=0 go test -short net
crashed if liveness analysis was in use (like at tip, not like Go 1.2).
TBR=ken2
CC=golang-codereviews
https://golang.org/cl/64470043
The VARDEF placement must be before the initialization
but after any final use. If you have something like s = ... using s ...
the rhs must be evaluated, then the VARDEF, then the lhs
assigned.
There is a large comment in pgen.c on gvardef explaining
this in more detail.
This CL also includes Ian's suggestions from earlier CLs,
namely commenting the use of mode in link.h and fixing
the precedence of the ~r check in dcl.c.
This CL enables the check that if liveness analysis decides
a variable is live on entry to the function, that variable must
be a function parameter (not a result, and not a local variable).
If this check fails, it indicates a bug in the liveness analysis or
in the generated code being analyzed.
The race detector generates invalid code for append(x, y...).
The code declares a temporary t and then uses cap(t) before
initializing t. The new liveness check catches this bug and
stops the compiler from writing out the buggy code.
Consequently, this CL disables the race detector tests in
run.bash until the race detector bug can be fixed
(golang.org/issue/7334).
Except for the race detector bug, the liveness analysis check
does not detect any problems (this CL and the previous CLs
fixed all the detected problems).
The net test still fails with GOGC=0 but the rest of the tests
now pass or time out (because GOGC=0 is so slow).
TBR=iant
CC=golang-codereviews
https://golang.org/cl/64170043
The existing tests issue4463.go and issue4654.go had failures at
typechecking and did not test walking the AST.
Fixes#7272.
LGTM=khr
R=khr, rsc, iant
CC=golang-codereviews
https://golang.org/cl/60550044
When the liveness code doesn't know a function doesn't return
(but the generated code understands that), the liveness analysis
invents a control flow edge that is not really there, which can cause
variables to seem spuriously live. This is particularly bad when the
variables are uninitialized.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/63720043
The registerization code needs the function to end in a RET,
even if that RET is actually unreachable.
The liveness code needs to avoid such unreachable RETs.
It had a special case for final RET after JMP, but no case
for final RET after UNDEF. Instead of expanding the special
cases, let fixjmp - which already knows what is and is not
reachable definitively - mark the unreachable RET so that
the liveness code can identify it.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/63680043
A normal RET is treated as using the return values,
but a tail jump RET does not - it is jumping to the
function that is going to fill in the return values.
If a tail jump RET is recorded as using the return values,
since nothing initializes them they will be marked as
live on entry to the function, which is clearly wrong.
Found and tested by the new code in plive.c that looks
for variables that are incorrectly live on entry.
That code is disabled for now because there are other
cases remaining to be fixed. But once it is enabled,
test/live1.go becomes a real test of this CL.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/63570045
Any initialization of a variable by a block copy or block zeroing
or by multiple assignments (componentwise copying or zeroing
of a multiword variable) needs to emit a VARDEF. These cases were not.
Fixes#7205.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/63650044
Before, an unnamed return value turned into an ONAME node n with n->sym
named ~anon%d, and n->orig == n.
A blank-named return value turned into an ONAME node n with n->sym
named ~anon%d but n->orig == the original blank n. Code generation and
printing uses n->orig, so that this node formatted as _.
But some code does not use n->orig. In particular the liveness code does
not know about the n->orig convention and so mishandles blank identifiers.
It is possible to fix but seemed better to avoid the confusion entirely.
Now the first kind of node is named ~r%d and the second ~b%d; both have
n->orig == n, so that it doesn't matter whether code uses n or n->orig.
After this change the ->orig field is only used for other kinds of expressions,
not for ONAME nodes.
This requires distinguishing ~b from ~r names in a few places that care.
It fixes a liveness analysis bug without actually changing the liveness code.
TBR=ken2
CC=golang-codereviews
https://golang.org/cl/63630043
Make the loop nesting depth of &x depend on where x is declared,
not on where the &x appears. The latter is only a conservative
estimate of the former. Being more careful can avoid some
variables escaping, and it is easier to reason about.
It would have avoided issue 7313, although that was still a bug
worth fixing.
Not much effect in the tree: one variable in the whole tree
is saved from a heap allocation (something in x509 parsing).
LGTM=daniel.morsing
R=daniel.morsing
CC=golang-codereviews
https://golang.org/cl/62380043
Logically, the init statement is in the enclosing scopes loopdepth, not inside the for loop.
Fixes#7313.
LGTM=rsc
R=golang-codereviews, gobot, rsc
CC=golang-codereviews
https://golang.org/cl/62430043
Array values are comparable if values of the array element type
are comparable.
Fixes#6526.
LGTM=khr
R=rsc, bradfitz, khr
CC=golang-codereviews
https://golang.org/cl/58580043
This CL makes the bitmaps a little more precise about variables
that have their address taken but for which the address does not
escape to the heap, so that the variables are kept in the stack frame
rather than allocated on the heap.
The code before this CL handled these variables by treating every
return statement as using every such variable and depending on
liveness analysis to essentially treat the variable as live during the
entire function. That approach has false positives and (worse) false
negatives. That is, it's both sloppy and buggy:
func f(b1, b2 bool) { // x live here! (sloppy)
if b2 {
print(0) // x live here! (sloppy)
return
}
var z **int
x := new(int)
*x = 42
z = &x
print(**z) // x live here (conservative)
if b2 {
print(1) // x live here (conservative)
return
}
for {
print(**z) // x not live here (buggy)
}
}
The first two liveness annotations (marked sloppy) are clearly
wrong: x cannot be live if it has not yet been declared.
The last liveness annotation (marked buggy) is also wrong:
x is live here as *z, but because there is no return statement
reachable from this point in the code, the analysis treats x as dead.
This CL changes the liveness calculation to mark such variables
live exactly at points in the code reachable from the variable
declaration. This keeps the conservative decisions but fixes
the sloppy and buggy ones.
The CL also detects ambiguously live variables, those that are
being marked live but may not actually have been initialized,
such as in this example:
func f(b1 bool) {
var z **int
if b1 {
x := new(int)
*x = 42
z = &x
} else {
y := new(int)
*y = 54
z = &y
}
print(**z) // x, y live here (conservative)
}
Since the print statement is reachable from the declaration of x,
x must conservatively be marked live. The same goes for y.
Although both x and y are marked live at the print statement,
clearly only one of them has been initialized. They are both
"ambiguously live".
These ambiguously live variables cause problems for garbage
collection: the collector cannot ignore them but also cannot
depend on them to be initialized to valid pointer values.
Ambiguously live variables do not come up too often in real code,
but recent changes to the way map and interface runtime functions
are invoked has created a large number of ambiguously live
compiler-generated temporary variables. The next CL will adjust
the analysis to understand these temporaries better, to make
ambiguously live variables fairly rare.
Once ambiguously live variables are rare enough, another CL will
introduce code at the beginning of a function to zero those
slots on the stack. At that point the garbage collector and the
stack copying routines will be able to depend on the guarantee that
if a slot is marked as live in a liveness bitmap, it is initialized.
R=khr
CC=golang-codereviews, iant
https://golang.org/cl/51810043
For historical reasons, temp was returning a copy
of the created Node*, not the original Node*.
This meant that if analysis recorded information in the
returned node (for example, n->addrtaken = 1), the
analysis would not show up on the original Node*, the
one kept in fn->dcl and consulted during liveness
bitmap creation.
Correct this, and watch for it when setting addrtaken.
Fixes#7083.
R=khr, dave, minux.ma
CC=golang-codereviews
https://golang.org/cl/51010045
Nodes of goto statements were corrupted when written
to export data.
Fixes#7023.
R=rsc, dave, minux.ma
CC=golang-codereviews
https://golang.org/cl/46190043
Gccgo doesn't have the same equivalent of file name and
package as the gc compiler, so the error messages are
necessarily different.
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/40510048
fixedbugs/issue4510.dir/f2.go:7: error: 'fmt' defined as both imported name and global name
f1.go:7: note: 'fmt' imported here
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/41530044
const1.go:33: error: integer constant overflow
<similar errors omitted>
const1.go:64: error: division by zero
const1.go:67: error: floating point constant overflow
const1.go:68: error: floating point constant overflow
const1.go:69: error: floating point constant overflow
const1.go:70: error: division by zero
const1.go:71: error: expected integer type
const1.go:77: error: argument 1 has incompatible type (cannot use type int8 as type int)
const1.go:78: error: argument 1 has incompatible type (cannot use type int8 as type int)
const1.go:79: error: argument 1 has incompatible type (cannot use type uint8 as type int)
const1.go:81: error: argument 1 has incompatible type (cannot use type float32 as type int)
const1.go:82: error: argument 1 has incompatible type (cannot use type float64 as type int)
const1.go:83: error: floating point constant truncated to integer
const1.go:85: error: argument 1 has incompatible type (cannot use type float64 as type int)
const1.go:86: error: argument 1 has incompatible type (cannot use type string as type int)
const1.go:87: error: argument 1 has incompatible type (cannot use type bool as type int)
const1.go:90: error: const initializer cannot be nil
const1.go:91: error: expression is not constant
const1.go:92: error: expression is not constant
const1.go:93: error: invalid constant type
const1.go:94: error: invalid constant type
fixedbugs/bug462.go:17: error: unknown field 'os.File' in 'T'
fixedbugs/issue3705.go:9: error: cannot declare init - must be func
fixedbugs/issue4251.go:12: error: inverted slice range
fixedbugs/issue4251.go:16: error: inverted slice range
fixedbugs/issue4251.go:20: error: inverted slice range
fixedbugs/issue4405.go:11: error: invalid character 0x7 in identifier
fixedbugs/issue4405.go:12: error: invalid character 0x8 in identifier
fixedbugs/issue4405.go:13: error: invalid character 0xb in identifier
fixedbugs/issue4405.go:14: error: invalid character 0xc in identifier
fixedbugs/issue4429.go:15: error: expected pointer
fixedbugs/issue4517d.go:9: error: cannot import package as init
fixedbugs/issue4545.go:17: error: invalid context-determined non-integer type for left operand of shift
fixedbugs/issue4545.go:16: error: incompatible types in binary expression
fixedbugs/issue4610.go:15: error: expected ';' or '}' or newline
fixedbugs/issue4610.go:16: error: expected declaration
fixedbugs/issue4654.go:15: error: value computed is not used
<similar errors omitted>
fixedbugs/issue4776.go:9: error: program must start with package clause
fixedbugs/issue4776.go:9: error: expected ';' or newline after package clause
fixedbugs/issue4813.go:31: error: index must be integer
<similar errors omitted>
fixedbugs/issue4847.go:22: error: initialization expression for 'matchAny' depends upon itself
fixedbugs/issue5089.go:13: error: redefinition of 'bufio.Buffered': receiver name changed
fixedbugs/issue5089.go:11: note: previous definition of 'bufio.Buffered' was here
fixedbugs/issue5172.go:17: error: reference to undefined field or method 'bar'
fixedbugs/issue5172.go:18: error: reference to undefined field or method 'bar'
fixedbugs/issue5172.go:12: error: use of undefined type 'bar'
fixedbugs/issue5358.go:16: error: argument 2 has incompatible type
fixedbugs/issue5581.go:29: error: use of undefined type 'Blah'
funcdup.go:10: error: redefinition of 'i'
funcdup.go:10: note: previous definition of 'i' was here
<similar errors omitted>
funcdup2.go:10: error: redefinition of 'i'
funcdup2.go:10: note: previous definition of 'i' was here
<similar errors omitted>
slice3err.go:20: error: middle index required in 3-index slice
<similar errors omitted>
slice3err.go:20: error: final index required in 3-index slice
<similar errors omitted>
slice3err.go:21: error: final index required in 3-index slice
slice3err.go:46: error: invalid 3-index slice of string
<similar errors omitted>
slice3err.go:57: error: inverted slice range
<similar errors omitted>
slice3err.go:62: error: invalid slice index: capacity less than length
slice3err.go:64: error: invalid slice index: capacity less than start
slice3err.go:65: error: invalid slice index: capacity less than start
slice3err.go:66: error: invalid slice index: capacity less than start
slice3err.go:68: error: invalid slice index: capacity less than length
slice3err.go:70: error: invalid slice index: capacity less than start
slice3err.go:80: error: invalid slice index: capacity less than length
slice3err.go:82: error: invalid slice index: capacity less than start
slice3err.go:83: error: invalid slice index: capacity less than start
slice3err.go:84: error: invalid slice index: capacity less than start
slice3err.go:86: error: invalid slice index: capacity less than length
slice3err.go:88: error: invalid slice index: capacity less than start
slice3err.go:99: error: array index out of bounds
<similar errors omitted>
slice3err.go:106: error: invalid slice index: capacity less than length
slice3err.go:107: error: invalid slice index: capacity less than start
slice3err.go:118: error: invalid slice index: capacity less than length
slice3err.go:119: error: invalid slice index: capacity less than start
syntax/semi1.go:10: error: missing '{' after if clause
syntax/semi1.go:10: error: reference to undefined name 'x'
syntax/semi1.go:10: error: reference to undefined name 'y'
syntax/semi1.go:12: error: reference to undefined name 'z'
syntax/semi2.go:10: error: missing '{' after switch clause
syntax/semi2.go:10: error: reference to undefined name 'x'
syntax/semi3.go:10: error: missing '{' after for clause
syntax/semi3.go:10: error: reference to undefined name 'x'
syntax/semi3.go:10: error: reference to undefined name 'y'
syntax/semi3.go:10: error: reference to undefined name 'z'
syntax/semi3.go:12: error: reference to undefined name 'z'
syntax/semi4.go:11: error: missing '{' after for clause
syntax/semi4.go:10: error: reference to undefined name 'x'
syntax/semi4.go:12: error: reference to undefined name 'z'
typecheck.go:12: error: reference to undefined name 'b'
typecheck.go:17: error: reference to undefined name 'c'
typecheck.go:11: error: use of undefined type 'b'
typecheck.go:16: error: not enough arguments
typecheck.go:17: error: not enough arguments
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/41520044
There is no necessary relationship between the imports of the
packages a and b, and gccgo happens to import them in a
different order, leading to different output. This ordering
is not the purpose of the test in any case.
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/40400043
blank1.go:10:9: error: invalid package name _
blank1.go:17:2: error: cannot use _ as value
blank1.go:18:7: error: cannot use _ as value
blank1.go:20:8: error: invalid use of ‘_’
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/14088044
When a floating point constant is used as an array/slice
index, gccgo prints "error: index must be integer"; gc prints
"constant 2.1 truncated to integer".
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/14044044
The select2.go test assumed that the memory allocated between
its two samplings of runtime.ReadMemStats is strictly
increasing. To avoid failing the tests when this is not true,
a greater-than check is introduced before computing the
difference in allocated memory.
R=golang-dev, r, cshapiro
CC=golang-dev
https://golang.org/cl/13701046
This eliminates ~75% of the nil checks being emitted,
on all architectures. We can do better, but we need
a bit more general support from the compiler, and
I don't want to do that so close to Go 1.2.
What's here is simple but effective and safe.
A few small code generation cleanups were required
to make the analysis consistent on all systems about
which nil checks are omitted, at least in the test.
Fixes#6019.
R=ken2
CC=golang-dev
https://golang.org/cl/13334052
The implementation of division in the 5 toolchain is a bit too magical.
Hide the magic from the traceback routines.
Also add a test for the results of the software divide routine.
Fixes#5805.
R=golang-dev, minux.ma
CC=golang-dev
https://golang.org/cl/13239052
Bug #1:
Issue 5406 identified an interesting case:
defer iface.M()
may end up calling a wrapper that copies an indirect receiver
from the iface value and then calls the real M method. That's
two calls down, not just one, and so recover() == nil always
in the real M method, even during a panic.
[For the purposes of this entire discussion, a wrapper's
implementation is a function containing an ordinary call, not
the optimized tail call form that is somtimes possible. The
tail call does not create a second frame, so it is already
handled correctly.]
Fix this bug by introducing g->panicwrap, which counts the
number of bytes on current stack segment that are due to
wrapper calls that should not count against the recover
check. All wrapper functions must now adjust g->panicwrap up
on entry and back down on exit. This adds slightly to their
expense; on the x86 it is a single instruction at entry and
exit; on the ARM it is three. However, the alternative is to
make a call to recover depend on being able to walk the stack,
which I very much want to avoid. We have enough problems
walking the stack for garbage collection and profiling.
Also, if performance is critical in a specific case, it is already
faster to use a pointer receiver and avoid this kind of wrapper
entirely.
Bug #2:
The old code, which did not consider the possibility of two
calls, already contained a check to see if the call had split
its stack and so the panic-created segment was one behind the
current segment. In the wrapper case, both of the two calls
might split their stacks, so the panic-created segment can be
two behind the current segment.
Fix this by propagating the Stktop.panic flag forward during
stack splits instead of looking backward during recover.
Fixes#5406.
R=golang-dev, iant
CC=golang-dev
https://golang.org/cl/13367052
These tests were suggested in golang.org/issue/6080.
They were fixed as part of the new nil pointer checks
that I added a few weeks ago.
Recording the tests as part of marking the issue closed.
Fixes#6080.
R=golang-dev, r, bradfitz
CC=golang-dev
https://golang.org/cl/13255049
Types in function scope can have methods on them if they embed another type, but we didn't make the name unique, meaning that 2 identically named types in different functions would conflict with eachother.
Fixes#6269.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/13326045
The compiler computes initialization order by finding
a spanning tree between a package's global variables.
But it does so by walking both variables and functions
and stops detecting cycles between variables when they
mix with a cycle of mutually recursive functions.
Fixes#4847.
R=golang-dev, daniel.morsing, rsc
CC=golang-dev
https://golang.org/cl/9663047
syntax/*: update messages
sliceerr3.go: bizarre new error fixed by deleting a space.
I could have sworn I ran all.bash before submitting the CL that triggered these.
TBR=golang-dev@googlegroups.com
R=golang-dev
CC=golang-dev
https://golang.org/cl/12812044
See golang.org/s/go12nil.
This CL is about getting all the right checks inserted.
A followup CL will add an optimization pass to
remove redundant checks.
R=ken2
CC=golang-dev
https://golang.org/cl/12970043
Individual variables bigger than 10 MB are now
moved to the heap, as if they had escaped on
their own.
This avoids ridiculous stacks for programs that
do things like
x := [1<<30]byte{}
... use x ...
If 10 MB is too small, we can raise the limit.
Fixes#6077.
R=ken2
CC=golang-dev
https://golang.org/cl/12650045
The gc compiler only gives an error about an unused label if
it has not given any errors in an earlier pass. Remove all
unused labels in this test because they don't test anything
useful and they cause gccgo to give unexpected errors.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/12580044
The gc compiler only gives an error about fallthrough in a
type switch if it has not given any errors in an earlier pass.
Remove all functions in this test that use fallthrough in a
type switch because they don't test anything useful and they
cause gccgo to give unexpected errors.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/12614043
Backends do not exactly expect receiving binary operators with
constant operands or use workarounds to move them to
register/stack in order to handle them.
Fixes#5841.
R=golang-dev, daniel.morsing, rsc
CC=golang-dev
https://golang.org/cl/11107044
clearfat (used to zero initialize structures) will use AX for x86 block ops. If we write to AX while calculating the dest pointer, we will fill the structure with incorrect values.
Since 64-bit arithmetic uses AX to synthesize a 64-bit register, getting an adress by indexing with 64-bit ops can clobber the register.
Fixes#5820.
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/11383043
Deferred functions are not run by a call instruction. They are run by
the runtime editing registers to make the call start with a caller PC
returning to a
CALL deferreturn
instruction.
That instruction has always had the line number of the function's
closing brace, but that instruction's line number is irrelevant.
Stack traces show the line number of the instruction before the
return PC, because normally that's what started the call. Not so here.
The instruction before the CALL deferreturn could be almost anywhere
in the function; it's unrelated and its line number is incorrect to show.
Fix the line number by inserting a true hardware no-op with the right
line number before the returned-to CALL instruction. That is, the deferred
calls now appear to start with a caller PC returning to the second instruction
in this sequence:
NOP
CALL deferreturn
The traceback will show the line number of the NOP, which we've set
to be the line number of the function's closing brace.
The NOP here is not the usual pseudo-instruction, which would be
elided by the linker. Instead it is the real hardware instruction:
XCHG AX, AX on 386 and amd64, and AND.EQ R0, R0, R0 on ARM.
Fixes#5856.
R=ken2, ken
CC=golang-dev
https://golang.org/cl/11223043
Escape analysis needs the right curfn value on a dclfunc node, otherwise it will not analyze the function.
When generating method value wrappers, we forgot to set the curfn correctly.
Fixes#5753.
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/10383048
A struct with a single field was considered as equivalent to the
field type, which is incorrect is the field is blank.
Fields with padding could make the compiler think some
types are comparable when they are not.
Fixes#5698.
R=rsc, golang-dev, daniel.morsing, bradfitz, gri, r
CC=golang-dev
https://golang.org/cl/10271046
Design doc at golang.org/s/go12slice.
This is an experimental feature and may not be included in the release.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/10743046
fn can clearly hold a closure in memory.
argp/pc point into stack and so can hold
in memory a block that was previously
a large stack serment.
R=golang-dev, dave, rsc
CC=golang-dev
https://golang.org/cl/10784043
Exported inlined functions that perform a string conversion
using a non-exported named type may miss it in export data.
Fixes#5755.
R=rsc, golang-dev, ality, r
CC=golang-dev
https://golang.org/cl/10464043
Functions without bodies were excluded from the ordering logic,
because when I wrote the ordering logic there was no reason to
analyze them.
But then we added //go:noescape tags that need analysis, and we
didn't update the ordering logic.
So in the absence of good ordering, //go:noescape only worked
if it appeared before the use in the source code.
Fixes#5773.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/10570043
The existing compilers convert empty strings to empty
but non-nil byte and rune slices. The spec required
a nil byte and rune slice in those cases. That seems
an odd additional requirement. Adjust the spec to
match the reality.
Also, removed over-specification for conversions of
nil []byte and []rune: such nil slices already act
like empty slices and thus don't need extra language.
Added extra examples instead.
Fixes#5704.
R=rsc, r, iant
CC=golang-dev
https://golang.org/cl/10440045
This avoids problems with inlining in genwrappers, which
occurs after functions have been compiled. Compiling a
function may cause some unused local vars to be removed from
the list. Since a local var may be unused due to
optimization, it is possible that a removed local var winds up
beingused in the inlined version, in which case hilarity
ensues.
Fixes#5515.
R=golang-dev, khr, dave
CC=golang-dev
https://golang.org/cl/10210043
It was never tested and also breaks Windows.
run.go doesn't yet support the proper !windows,!plan9 syntax.
««« original CL description
test: do not run SIGCHLD test on Plan 9
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/10017045
»»»
R=golang-dev, dave
CC=golang-dev
https://golang.org/cl/10024044
It works on i386, but fails on amd64 and arm.
««« original CL description
runtime: prevent the GC from seeing the content of a frame in runfinq()
Fixes#5348.
R=golang-dev, dvyukov
CC=golang-dev
https://golang.org/cl/8954044
»»»
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8695051
They caused internal compiler errors and they're expensive enough that inlining them doesn't make sense.
Fixes#5259.
R=golang-dev, r, iant, remyoudompheng
CC=golang-dev
https://golang.org/cl/8636043
Don't measure wall time in map.go. Keep it portable
and only test NaN, but not time.
Move time tests to mapnan.go and only measure user CPU time,
not wall time. It builds on Darwin and Linux, the primary
platforms where people hack on the runtime & in particular
maps. The runtime is shared, though, so we don't need it to
run on all of the platforms.
Fixes flaky build failures like:
http://build.golang.org/log/ba67eceefdeaa1142cb6c990a62fa3ffd8fd73f8
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8479043
The offset of an embedded field s.X must be relative to s
and not to the implicit s.Field of which X is a direct field.
Moreover, no indirections may happen on the path.
Fixes#4909.
R=nigeltao, ality, daniel.morsing, iant, gri, r
CC=golang-dev
https://golang.org/cl/8287043
Reusing it when multiple comparisons occurred in the same
function call led to bad overwriting.
Fixes#5162.
R=golang-dev, daniel.morsing
CC=golang-dev
https://golang.org/cl/8174047
Usually, there is no esc info when inlining, but there will be when generating inlined wrapper functions.
If we don't use this information, we get invalid addresses on the stack.
Fixes#5056.
R=golang-dev, rsc
CC=golang-dev, remyoudompheng
https://golang.org/cl/7850045
It used to not mark parameters as escaping if only one of the
fields it points to leaks out of the function. This causes
problems when importing from another package.
Fixes#4964.
R=rsc, lvd, dvyukov, daniel.morsing
CC=golang-dev
https://golang.org/cl/7648045
Also rename the go parser test to GoParse so it doesn't grab the globally useful Parse name.
R=golang-dev, dave
CC=golang-dev
https://golang.org/cl/7732044
Composite literals using the &T{} form were incorrectly
exported, leading to weird errors at import time.
Fixes#4879.
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/7395054
The interpreter's os.Exit now triggers a special panic rather
than kill the test process. (It's semantically dubious, since
it will run deferred routines.) Interpret now returns its
exit code rather than calling os.Exit.
Also:
- disabled parts of a few $GOROOT/tests via os.Getenv("GOSSAINTERP").
- remove unnecessary 'slots' param to external functions; they
are never closures.
Most of the tests are disabled until go/types supports shifts.
They can be reenabled if you patch this workaround:
https://golang.org/cl/7312068
R=iant, bradfitz
CC=golang-dev, gri
https://golang.org/cl/7313062
The commands being run are 'go tool this' and 'go tool that',
and the go command will call Getwd during its init.
R=golang-dev, iant
CC=golang-dev
https://golang.org/cl/7336045
Previously merely printing an error would cause the golden
file comparison (in 'bash run') to fail, but that is no longer
the case with the new run.go driver.
R=iant
CC=golang-dev
https://golang.org/cl/7310087
Details:
- reorder.go: delete p8.
(Once expectation is changed per b/4627 it is identical to p1.)
- switch.go: added some more (degenerate) switches.
- range.go: improved error messages in a few cases.
- method.go: added tests of calls to promoted methods.
R=iant
CC=golang-dev
https://golang.org/cl/7306087
A new comment directive //go:noescape instructs the compiler
that the following external (no body) func declaration should be
treated as if none of its arguments escape to the heap.
Fixes#4099.
R=golang-dev, dave, minux.ma, daniel.morsing, remyoudompheng, adg, agl, iant
CC=golang-dev
https://golang.org/cl/7289048
If the analysis reached a node twice, then the analysis was cut off.
However, if the second arrival is at a lower depth (closer to escaping)
then it is important to repeat the traversal.
The repeating must be cut off at some point to avoid the occasional
infinite recursion. This CL cuts it off as soon as possible while still
passing all tests.
Fixes#4751.
R=ken2
CC=golang-dev, lvd
https://golang.org/cl/7303043
Was not re-walking the new AND node, so that its ullman
count was wrong, so that the code generator attempted to
store values in registers across the call.
Fixes#4752.
R=ken2
CC=golang-dev
https://golang.org/cl/7288054
For consistency with conversions that look like function calls,
conversions that don't look like function calls now allow an
optional trailing comma.
That is, int(x,) has always been syntactically valid.
Now []int(x,) is valid too.
Fixes#4162.
R=ken2
CC=golang-dev
https://golang.org/cl/7288045
Expressions involving nil, even if they can be evaluated
at compile time, do not count as Go constants and cannot
be used in const initializers.
Fixes#4673.
Fixes#4680.
R=ken2
CC=golang-dev
https://golang.org/cl/7278043
The test case of issue 4585 was not passing due to
miscalculation of memequal args, and the previous fix
does not handle padding at the end of a struct.
Handling of padding at end of structs also fixes the case
of [n]T where T is such a padded struct.
Fixes#4585.
(again)
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/7133059
runoutput styles tests generally consume a lot of memory. On arm platforms rotate?.go consume around 200mb each to compile, and as tests are sorted alphabetically, they all tend to run at once.
This change limits the number of runoutput jobs to 2 on arm platforms.
R=minux.ma, remyoudompheng, bradfitz, lucio.dere
CC=golang-dev
https://golang.org/cl/7099047
The linker split PKGDEF into (prefix, name, def) pairs,
and defines def to begin after a space following the identifier.
This is totally wrong for the following export data:
func "".FunctionName()
var SomethingCompletelyUnrelated int
The linker would parse
name=`"".FunctionName()\n\tvar`
def=`SomethingCompletelyUnrelated int`
since there is no space after FunctionName.
R=minux.ma, rsc
CC=golang-dev
https://golang.org/cl/7068051
