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
