If we want to break up package gc at all, we will need to move
the compiler IR it defines into a separate package that can be
imported by packages that gc itself imports. This CL does that.
It also removes the TINT8 etc aliases so that all code is clear
about which package things are coming from.
This CL is automatically generated by the script below.
See the comments in the script for details about the changes.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
# These names were never fully qualified
# when the types package was added.
# Do it now, to avoid confusion about where they live.
inline -rm \
Txxx \
TINT8 \
TUINT8 \
TINT16 \
TUINT16 \
TINT32 \
TUINT32 \
TINT64 \
TUINT64 \
TINT \
TUINT \
TUINTPTR \
TCOMPLEX64 \
TCOMPLEX128 \
TFLOAT32 \
TFLOAT64 \
TBOOL \
TPTR \
TFUNC \
TSLICE \
TARRAY \
TSTRUCT \
TCHAN \
TMAP \
TINTER \
TFORW \
TANY \
TSTRING \
TUNSAFEPTR \
TIDEAL \
TNIL \
TBLANK \
TFUNCARGS \
TCHANARGS \
NTYPE \
BADWIDTH
# esc.go and escape.go do not need to be split.
# Append esc.go onto the end of escape.go.
mv esc.go escape.go
# Pull out the type format installation from func Main,
# so it can be carried into package ir.
mv Main:/Sconv.=/-0,/TypeLinkSym/-1 InstallTypeFormats
# Names that need to be exported for use by code left in gc.
mv Isconst IsConst
mv asNode AsNode
mv asNodes AsNodes
mv asTypesNode AsTypesNode
mv basicnames BasicTypeNames
mv builtinpkg BuiltinPkg
mv consttype ConstType
mv dumplist DumpList
mv fdumplist FDumpList
mv fmtMode FmtMode
mv goopnames OpNames
mv inspect Inspect
mv inspectList InspectList
mv localpkg LocalPkg
mv nblank BlankNode
mv numImport NumImport
mv opprec OpPrec
mv origSym OrigSym
mv stmtwithinit StmtWithInit
mv dump DumpAny
mv fdump FDumpAny
mv nod Nod
mv nodl NodAt
mv newname NewName
mv newnamel NewNameAt
mv assertRepresents AssertValidTypeForConst
mv represents ValidTypeForConst
mv nodlit NewLiteral
# Types and fields that need to be exported for use by gc.
mv nowritebarrierrecCallSym SymAndPos
mv SymAndPos.lineno SymAndPos.Pos
mv SymAndPos.target SymAndPos.Sym
mv Func.lsym Func.LSym
mv Func.setWBPos Func.SetWBPos
mv Func.numReturns Func.NumReturns
mv Func.numDefers Func.NumDefers
mv Func.nwbrCalls Func.NWBRCalls
# initLSym is an algorithm left behind in gc,
# not an operation on Func itself.
mv Func.initLSym initLSym
mv nodeQueue NodeQueue
mv NodeQueue.empty NodeQueue.Empty
mv NodeQueue.popLeft NodeQueue.PopLeft
mv NodeQueue.pushRight NodeQueue.PushRight
# Many methods on Node are actually algorithms that
# would apply to any node implementation.
# Those become plain functions.
mv Node.funcname FuncName
mv Node.isBlank IsBlank
mv Node.isGoConst isGoConst
mv Node.isNil IsNil
mv Node.isParamHeapCopy isParamHeapCopy
mv Node.isParamStackCopy isParamStackCopy
mv Node.isSimpleName isSimpleName
mv Node.mayBeShared MayBeShared
mv Node.pkgFuncName PkgFuncName
mv Node.backingArrayPtrLen backingArrayPtrLen
mv Node.isterminating isTermNode
mv Node.labeledControl labeledControl
mv Nodes.isterminating isTermNodes
mv Nodes.sigerr fmtSignature
mv Node.MethodName methodExprName
mv Node.MethodFunc methodExprFunc
mv Node.IsMethod IsMethod
# Every node will need to implement RawCopy;
# Copy and SepCopy algorithms will use it.
mv Node.rawcopy Node.RawCopy
mv Node.copy Copy
mv Node.sepcopy SepCopy
# Extract Node.Format method body into func FmtNode,
# but leave method wrapper behind.
mv Node.Format:0,$ FmtNode
# Formatting helpers that will apply to all node implementations.
mv Node.Line Line
mv Node.exprfmt exprFmt
mv Node.jconv jconvFmt
mv Node.modeString modeString
mv Node.nconv nconvFmt
mv Node.nodedump nodeDumpFmt
mv Node.nodefmt nodeFmt
mv Node.stmtfmt stmtFmt
# Constant support needed for code moving to ir.
mv okforconst OKForConst
mv vconv FmtConst
mv int64Val Int64Val
mv float64Val Float64Val
mv Node.ValueInterface ConstValue
# Organize code into files.
mv LocalPkg BuiltinPkg ir.go
mv NumImport InstallTypeFormats Line fmt.go
mv syntax.go Nod NodAt NewNameAt Class Pxxx PragmaFlag Nointerface SymAndPos \
AsNode AsTypesNode BlankNode OrigSym \
Node.SliceBounds Node.SetSliceBounds Op.IsSlice3 \
IsConst Node.Int64Val Node.CanInt64 Node.Uint64Val Node.BoolVal Node.StringVal \
Node.RawCopy SepCopy Copy \
IsNil IsBlank IsMethod \
Node.Typ Node.StorageClass node.go
mv ConstType ConstValue Int64Val Float64Val AssertValidTypeForConst ValidTypeForConst NewLiteral idealType OKForConst val.go
# Move files to new ir package.
mv bitset.go class_string.go dump.go fmt.go \
ir.go node.go op_string.go val.go \
sizeof_test.go cmd/compile/internal/ir
'
: # fix mkbuiltin.go to generate the changes made to builtin.go during rf
sed -i '' '
s/\[T/[types.T/g
s/\*Node/*ir.Node/g
/internal\/types/c \
fmt.Fprintln(&b, `import (`) \
fmt.Fprintln(&b, ` "cmd/compile/internal/ir"`) \
fmt.Fprintln(&b, ` "cmd/compile/internal/types"`) \
fmt.Fprintln(&b, `)`)
' mkbuiltin.go
gofmt -w mkbuiltin.go
: # update cmd/dist to add internal/ir
cd ../../../dist
sed -i '' '/compile.internal.gc/a\
"cmd/compile/internal/ir",
' buildtool.go
gofmt -w buildtool.go
: # update cmd/compile TestFormats
cd ../..
go install std cmd
cd cmd/compile
go test -u || go test # first one updates but fails; second passes
Change-Id: I5f7caf6b20629b51970279e81231a3574d5b51db
Reviewed-on: https://go-review.googlesource.com/c/go/+/273008
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Move Flag, Debug, Ctxt, Exit, and error messages to
new package cmd/compile/internal/base.
These are the core functionality that everything in gc uses
and which otherwise prevent splitting any other code
out of gc into different packages.
A minor milestone: the compiler source code
no longer contains the string "yy".
[git-generate]
cd src/cmd/compile/internal/gc
rf '
mv atExit AtExit
mv Ctxt atExitFuncs AtExit Exit base.go
mv lineno Pos
mv linestr FmtPos
mv flusherrors FlushErrors
mv yyerror Errorf
mv yyerrorl ErrorfAt
mv yyerrorv ErrorfVers
mv noder.yyerrorpos noder.errorAt
mv Warnl WarnfAt
mv errorexit ErrorExit
mv base.go debug.go flag.go print.go cmd/compile/internal/base
'
: # update comments
sed -i '' 's/yyerrorl/ErrorfAt/g; s/yyerror/Errorf/g' *.go
: # bootstrap.go is not built by default so invisible to rf
sed -i '' 's/Fatalf/base.Fatalf/' bootstrap.go
goimports -w bootstrap.go
: # update cmd/dist to add internal/base
cd ../../../dist
sed -i '' '/internal.amd64/a\
"cmd/compile/internal/base",
' buildtool.go
gofmt -w buildtool.go
Change-Id: I59903c7084222d6eaee38823fd222159ba24a31a
Reviewed-on: https://go-review.googlesource.com/c/go/+/272250
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For the upcoming rewrite to access methods, a few direct accesses
are problematic for the automated tool, most notably direct copies
or use of Node structs as opposed to pointers.
Fix these manually.
Passes toolstash -cmp.
Change-Id: I8bdbb33216737c09e1edda284d5c414422d86284
Reviewed-on: https://go-review.googlesource.com/c/go/+/273006
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
A method expression today is an ONAME that has none of the
invariants or properties of other ONAMEs and is always a special case
(hence the Node.IsMethodExpression method).
Remove the special cases by making a separate Op.
Passes toolstash -cmp.
Change-Id: I7667693c9155d5486a6924dbf75ebb59891c4afc
Reviewed-on: https://go-review.googlesource.com/c/go/+/272867
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The original meaning of type Func was "extra fields factored out
of a few cases of type Node having to do with functions",
but those specific cases didn't necessarily have any relation.
A typical declared function is represented by an ODCLFUNC Node
at its declaration and an ONAME node at its uses, and both those
have a .Func field, but they are *different* Funcs.
Similarly, a closure is represented both by an OCLOSURE Node for
the value itself and an ODCLFUNC Node for the underlying function
implementing the closure. Those too have *different* Funcs,
and the Func.Closure field in one points to the other and vice versa.
This has led to no end of confusion over the years.
This CL elevates type Func to be the canonical identifier for
a given Go function.
This looks like a trivial CL but in fact is the result of a lot of
scaffolding and rewriting, discarded once the result was achieved, to
separate out the three different kinds of Func nodes into three
separate fields, limited in use to each specific Node type, to
understand which Func fields are used by which Node types and what the
possible overlaps are. There were a few overlaps, most notably around
closures, which led to more fields being added to type Func to keep
them separate even though there is now a single Func instead of two
different ones for each function.
A future CL can and should change Curfn to be a *Func instead of
a *Node, finally eliminating the confusion about whether Curfn
is an ODCLFUNC node (as it is most of the time) or an ONAME node
(as it is when type-checking an inlined function body).
Although sizeof_test.go makes it look like Func is growing by two
words, there are now half as many Funcs in a running compilation,
so the memory footprint has actually been reduced substantially.
Change-Id: I598bd96c95728093dc769a835d48f2154a406a61
Reviewed-on: https://go-review.googlesource.com/c/go/+/272253
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Properly speaking, "nil" is a zero value, not a constant. So
go/constant does not have a representation for it. To allow replacing
Val with constant.Value, we split out ONIL separately from OLITERAL so
we can get rid of CTNIL.
Passes toolstash-check.
Change-Id: I4c8e60cae3b3c91bbac43b3b0cf2a4ade028d6cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/272650
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
The next CL will introduce a package ir to hold the IR definitions.
This CL adjusts a few names and makes a few other minor changes
to make the next CL - an automated one - smoother.
Change-Id: Ie787a34732efd5b3d171bf0c1220b6dd91994ce3
Reviewed-on: https://go-review.googlesource.com/c/go/+/272251
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We want to introduce a package cmd/compile/internal/base,
and these will shadow it at points where it is needed.
Change-Id: Ic936733fba1ccba8c2ca1fdedbd4d2989df4bbf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/272249
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Automated factoring produced by rf script below to replace uses of
Func.Nname with Field.Nname or Node.MethodName as appropriate.
Some dead assignments to Func.Nname are left behind; these will be
removed in a subequent remove-only CL.
Passes toolstash-check.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
ex \
import "cmd/compile/internal/types"; \
var f *types.Field; \
var n *types.Node; \
f.Type.Nname() -> f.Nname; \
f.Type.SetNname(n) -> f.Nname = n; \
f.Type.FuncType().Nname -> f.Nname
ex \
var n *Node; \
asNode(n.Type.Nname()) -> n.MethodName(); \
asNode(n.Type.FuncType().Nname) -> n.MethodName(); \
asNode(callpartMethod(n).Type.Nname()) -> n.MethodName()
'
Change-Id: Iaae054324dfe7da6f5d8b8d57a1e05b58cc5968c
Reviewed-on: https://go-review.googlesource.com/c/go/+/272389
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
gc debug flags are currently stored in a 256-long array, that is then
addressed using the ASCII numeric value of the flag itself (a quirk
inherited from the old C compiler). It is also a little wasteful,
since we only define 16 flags, and the other 240 array elements are
always empty.
This change makes Debug a struct, which also provides static checking
that we're not referencing flags that does not exist.
Change-Id: I2f0dfef2529325514b3398cf78635543cdf48fe0
Reviewed-on: https://go-review.googlesource.com/c/go/+/263539
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Escape analysis is currently very naive about identifying calls to
known functions: it only recognizes direct calls to a declared
function, or direct calls to a closure.
This CL adds a new "staticValue" helper function that can trace back
through local variables that were initialized and never reassigned
based on a similar optimization already used by inlining. (And to be
used by inlining in a followup CL.)
Updates #41474.
Change-Id: I8204fd3b1e150ab77a27f583985cf099a8572b2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/256458
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
This change renames mustHeapAlloc to heapAllocReason, and changes it
to return the reason why the argument must escape, so we don't have to
re-deduce it in its callers just to print the escape reason. It also
embeds isSmallMakeSlice body in heapAllocReason, since the former was
only used by the latter, and deletes isSmallMakeSlice.
An outdated TODO to remove smallintconst, which the TODO claimed was
only used in one place, was also removed, since grepping shows we
currently call smallintconst in 11 different places.
Change-Id: I0bd11bf29b92c4126f5bb455877ff73217d5a155
Reviewed-on: https://go-review.googlesource.com/c/go/+/258678
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When explaining why the slice from a make() call escapes for the -m -m
message, we print "non-const size" if any one of Isconst(n.Left) and
Isconst(n.Right) return false; but for OMAKESLICE nodes with no cap,
n.Right is nil, so Isconst(n.Right, CTINT) will be always false.
Only call Isconst on n.Right if it's not nil.
Fixes#41635
Change-Id: I8729801a9b234b68ae40adad64d66fa7653adf09
Reviewed-on: https://go-review.googlesource.com/c/go/+/257641
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Update #40954
Change-Id: Ifaab7349631ccb12fc892882bbdf7f0ebf3d845f
Reviewed-on: https://go-review.googlesource.com/c/go/+/251158
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
Right now we just prevent such types from being on the heap. This CL
makes it so they cannot appear on the stack either. The distinction
between heap and stack is pretty vague at the language level (e.g. it
is affected by -N), and we don't need the flexibility anyway.
Once go:notinheap types cannot be in either place, we don't need to
consider pointers to such types to be pointers, at least according to
the garbage collector and stack copying. (This is the big win of this
CL, in my opinion.)
The distinction between HasPointers and HasHeapPointer no longer
exists. There is only HasPointers.
This CL is cleanup before possible use of go:notinheap to fix#40954.
Update #13386
Change-Id: Ibd895aadf001c0385078a6d4809c3f374991231a
Reviewed-on: https://go-review.googlesource.com/c/go/+/249917
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
More ergonomic that way. Also change Haspointers to HasPointers
while we are here.
Change-Id: I45bedc294c1a8c2bd01dc14bd04615ae77555375
Reviewed-on: https://go-review.googlesource.com/c/go/+/249959
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL uses fixVariadicCall before escape analyzing function calls.
This has a number of benefits, though also some minor obstacles:
Most notably, it allows us to remove ODDDARG along with the logic
involved in setting it up, manipulating EscHoles, and later copying
its escape analysis flags to the actual slice argument. Instead, we
uniformly handle all variadic calls the same way. (E.g., issue31573.go
is updated because now f() and f(nil...) are handled identically.)
It also allows us to simplify handling of builtins and generic
function calls. Previously handling of calls was hairy enough to
require multiple dispatches on n.Op, whereas now the logic is uniform
enough that we can easily handle it with a single dispatch.
The downside is handling //go:uintptrescapes is now somewhat clumsy.
(It used to be clumsy, but it still is, too.) The proper fix here is
probably to stop using escape analysis tags for //go:uintptrescapes
and unsafe-uintptr, and have an earlier pass responsible for them.
Finally, note that while we now call fixVariadicCall in Escape, we
still have to call it in Order, because we don't (yet) run Escape on
all compiler-generated functions. In particular, the generated "init"
function for initializing package-level variables can contain calls to
variadic functions and isn't escape analyzed.
Passes toolstash-check -race.
Change-Id: I4cdb92a393ac487910aeee58a5cb8c1500eef881
Reviewed-on: https://go-review.googlesource.com/c/go/+/229759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Previously for a method value "x.M", we always flowed x directly to
the heap, which led to the receiver argument generally needing to be
heap allocated.
This CL changes it to flow x to the closure and M's receiver
parameter. This allows receiver arguments to be stack allocated as
long as (1) the closure never escapes, *and* (2) method doesn't leak
its receiver parameter.
Within the standard library, this allows a handful of objects to be
stack allocated instead. Listed here are diagnostics that were
previously emitted by "go build -gcflags=-m std cmd" that are no
longer emitted:
archive/tar/writer.go:118:6: moved to heap: f
archive/tar/writer.go:208:6: moved to heap: f
archive/tar/writer.go:248:6: moved to heap: f
cmd/compile/internal/gc/initorder.go:252:2: moved to heap: d
cmd/compile/internal/gc/initorder.go:75:2: moved to heap: s
cmd/go/internal/generate/generate.go:206:7: &Generator literal escapes to heap
cmd/internal/obj/arm64/asm7.go:910:2: moved to heap: c
cmd/internal/obj/mips/asm0.go:415:2: moved to heap: c
cmd/internal/obj/pcln.go:294:22: new(pcinlineState) escapes to heap
cmd/internal/obj/s390x/asmz.go:459:2: moved to heap: c
crypto/tls/handshake_server.go:56:2: moved to heap: hs
Thanks to Cuong Manh Le for help coming up with this solution.
Fixes#27557.
Change-Id: I8c85d671d07fb9b53e11d2dd05949a34dbbd7e17
Reviewed-on: https://go-review.googlesource.com/c/go/+/228263
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL refactors tagHole to handle all three call situations (unknown
function; known function in same analysis batch; known function in
previous analysis batch). This will make it somewhat easier to reuse
in a followup CL.
Passes toolstash-check.
Change-Id: I764d047a333dfc593d721a881361683e94b485df
Reviewed-on: https://go-review.googlesource.com/c/go/+/229059
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For 1.15.
From the test:
{"range":{"start":{"line":7,"character":13},"end":{...},"severity":3,"code":"leaks","source":"go compiler","message":"parameter z leaks to ~r2 with derefs=0","relatedInformation":[
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: flow: y = z:"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from y = \u003cN\u003e (assign-pair)"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: flow: ~r1 = y:"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":11},"end":{...}},"message":"inlineLoc"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from y.b (dot of pointer)"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":11},"end":{...}},"message":"inlineLoc"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from \u0026y.b (address-of)"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":9},"end":...}},"message":"inlineLoc"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from ~r1 = \u003cN\u003e (assign-pair)"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":3},"end":...}},"message":"escflow: flow: ~r2 = ~r1:"},
{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":3},"end":...}},"message":"escflow: from return (*int)(~r1) (return)"}]}
Change-Id: Idf02438801f63e487c35a928cf5a0b6d3cc48674
Reviewed-on: https://go-review.googlesource.com/c/go/+/206658
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This CL detects infinite loops due to negative dereference cycles
during escape analysis, and terminates the loop gracefully. We still
fail to print a complete explanation of the escape path, but esc.go
didn't print *any* explanation for these test cases, so the release
blocking issue here is simply that we don't infinite loop.
Updates #35518.
Change-Id: I39beed036e5a685706248852f1fa619af3b7abbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/206619
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL also restores analysis details for (1) expressions that are
directly heap allocated because of being too large for the stack or
non-constant in size, and (2) for assignments that we short circuit
because we flow their address to another escaping object.
No change to normal compilation behavior. Only adds additional Printfs
guarded by -m=2.
Updates #31489.
Change-Id: I43682195d389398d75ced2054e29d9907bb966e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/205917
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a rough attempt at restoring -m=2 escape analysis diagnostics
on par with those that were available with esc.go. It's meant to be
simple and non-invasive.
For example, given this random example from bytes/reader.go:
138 func (r *Reader) WriteTo(w io.Writer) (n int64, err error) {
...
143 b := r.s[r.i:]
144 m, err := w.Write(b)
esc.go used to report:
bytes/reader.go:138:7: leaking param content: r
bytes/reader.go:138:7: from r.s (dot of pointer) at bytes/reader.go:143:8
bytes/reader.go:138:7: from b (assigned) at bytes/reader.go:143:4
bytes/reader.go:138:7: from w.Write(b) (parameter to indirect call) at bytes/reader.go:144:19
With this CL, escape.go now reports:
bytes/reader.go:138:7: parameter r leaks to {heap} with derefs=1:
bytes/reader.go:138:7: flow: b = *r:
bytes/reader.go:138:7: from r.s (dot of pointer) at bytes/reader.go:143:8
bytes/reader.go:138:7: from r.s[r.i:] (slice) at bytes/reader.go:143:10
bytes/reader.go:138:7: from b := r.s[r.i:] (assign) at bytes/reader.go:143:4
bytes/reader.go:138:7: flow: {heap} = b:
bytes/reader.go:138:7: from w.Write(b) (call parameter) at bytes/reader.go:144:19
Updates #31489.
Change-Id: I0c2b943a0f9ce6345bfff61e1c635172a9290cbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/196959
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
Generate inline code at defer time to save the args of defer calls to unique
(autotmp) stack slots, and generate inline code at exit time to check which defer
calls were made and make the associated function/method/interface calls. We
remember that a particular defer statement was reached by storing in the deferBits
variable (always stored on the stack). At exit time, we check the bits of the
deferBits variable to determine which defer function calls to make (in reverse
order). These low-cost defers are only used for functions where no defers
appear in loops. In addition, we don't do these low-cost defers if there are too
many defer statements or too many exits in a function (to limit code increase).
When a function uses open-coded defers, we produce extra
FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and
for each defer, the stack slots where the closure and associated args have been
stored. The funcdata also includes the location of the deferBits variable.
Therefore, for panics, we can use this funcdata to determine exactly which defers
are active, and call the appropriate functions/methods/closures with the correct
arguments for each active defer.
In order to unwind the stack correctly after a recover(), we need to add an extra
code segment to functions with open-coded defers that simply calls deferreturn()
and returns. This segment is not reachable by the normal function, but is returned
to by the runtime during recovery. We set the liveness information of this
deferreturn() to be the same as the liveness at the first function call during the
last defer exit code (so all return values and all stack slots needed by the defer
calls will be live).
I needed to increase the stackguard constant from 880 to 896, because of a small
amount of new code in deferreturn().
The -N flag disables open-coded defers. '-d defer' prints out the kind of defer
being used at each defer statement (heap-allocated, stack-allocated, or
open-coded).
Cost of defer statement [ go test -run NONE -bench BenchmarkDefer$ runtime ]
With normal (stack-allocated) defers only: 35.4 ns/op
With open-coded defers: 5.6 ns/op
Cost of function call alone (remove defer keyword): 4.4 ns/op
Text size increase (including funcdata) for go binary without/with open-coded defers: 0.09%
The average size increase (including funcdata) for only the functions that use
open-coded defers is 1.1%.
The cost of a panic followed by a recover got noticeably slower, since panic
processing now requires a scan of the stack for open-coded defer frames. This scan
is required, even if no frames are using open-coded defers:
Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ]
Without open-coded defers: 62.0 ns/op
With open-coded defers: 255 ns/op
A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers:
CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ]
Without open-coded defers: 443 ns/op
With open-coded defers: 347 ns/op
Updates #14939 (defer performance)
Updates #34481 (design doc)
Change-Id: I63b1a60d1ebf28126f55ee9fd7ecffe9cb23d1ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/202340
Reviewed-by: Austin Clements <austin@google.com>
Escaping all unsafe.Pointer conversions for -d=checkptr seems like it
might be a little too aggressive to enable for -race/-msan mode, since
at least some tests are written to expect unsafe.Pointer conversions
to not affect escape analysis.
So instead only enable that functionality behind -d=checkptr=2.
Updates #22218.
Updates #34959.
Change-Id: I2f0a774ea5961dabec29bc5b8ebe387a1b90d27b
Reviewed-on: https://go-review.googlesource.com/c/go/+/201840
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL tweaks escape analysis to treat unsafe.Pointer(ptr) as an
escaping operation when -d=checkptr is enabled. This allows better
detection of unsafe pointer arithmetic and conversions, because the
runtime checkptr instrumentation can currently only detect object
boundaries for heap objects, not stack objects.
Updates #22218.
Fixes#34959.
Change-Id: I856812cc23582fe4d0d401592583323e95919f28
Reviewed-on: https://go-review.googlesource.com/c/go/+/201781
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Generate inline code at defer time to save the args of defer calls to unique
(autotmp) stack slots, and generate inline code at exit time to check which defer
calls were made and make the associated function/method/interface calls. We
remember that a particular defer statement was reached by storing in the deferBits
variable (always stored on the stack). At exit time, we check the bits of the
deferBits variable to determine which defer function calls to make (in reverse
order). These low-cost defers are only used for functions where no defers
appear in loops. In addition, we don't do these low-cost defers if there are too
many defer statements or too many exits in a function (to limit code increase).
When a function uses open-coded defers, we produce extra
FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and
for each defer, the stack slots where the closure and associated args have been
stored. The funcdata also includes the location of the deferBits variable.
Therefore, for panics, we can use this funcdata to determine exactly which defers
are active, and call the appropriate functions/methods/closures with the correct
arguments for each active defer.
In order to unwind the stack correctly after a recover(), we need to add an extra
code segment to functions with open-coded defers that simply calls deferreturn()
and returns. This segment is not reachable by the normal function, but is returned
to by the runtime during recovery. We set the liveness information of this
deferreturn() to be the same as the liveness at the first function call during the
last defer exit code (so all return values and all stack slots needed by the defer
calls will be live).
I needed to increase the stackguard constant from 880 to 896, because of a small
amount of new code in deferreturn().
The -N flag disables open-coded defers. '-d defer' prints out the kind of defer
being used at each defer statement (heap-allocated, stack-allocated, or
open-coded).
Cost of defer statement [ go test -run NONE -bench BenchmarkDefer$ runtime ]
With normal (stack-allocated) defers only: 35.4 ns/op
With open-coded defers: 5.6 ns/op
Cost of function call alone (remove defer keyword): 4.4 ns/op
Text size increase (including funcdata) for go cmd without/with open-coded defers: 0.09%
The average size increase (including funcdata) for only the functions that use
open-coded defers is 1.1%.
The cost of a panic followed by a recover got noticeably slower, since panic
processing now requires a scan of the stack for open-coded defer frames. This scan
is required, even if no frames are using open-coded defers:
Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ]
Without open-coded defers: 62.0 ns/op
With open-coded defers: 255 ns/op
A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers:
CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ]
Without open-coded defers: 443 ns/op
With open-coded defers: 347 ns/op
Updates #14939 (defer performance)
Updates #34481 (design doc)
Change-Id: I51a389860b9676cfa1b84722f5fb84d3c4ee9e28
Reviewed-on: https://go-review.googlesource.com/c/go/+/190098
Reviewed-by: Austin Clements <austin@google.com>
The IsClosureVar, IsOutputParamHeapAddr, Assigned, Addrtaken,
InlFormal, and InlLocal flags are only interesting for ONAME nodes, so
it's better to set these flags on Name.flags instead of Node.flags.
Two caveats though:
1. Previously, we would set Assigned and Addrtaken on the entire
expression tree involved in an assignment or addressing operation.
However, the rest of the compiler only actually cares about knowing
whether the underlying ONAME (if any) was assigned/addressed.
2. This actually requires bumping Name.flags from bitset8 to bitset16,
whereas it doesn't allow shrinking Node.flags any. However, Name has
some trailing padding bytes, so expanding Name.flags doesn't cost any
memory.
Passes toolstash-check.
Change-Id: I7775d713566a38d5b9723360b1659b79391744c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/200898
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
n.Noescape() was overloaded for two uses: (1) to indicate a function
was annotated with //go:noescape, and (2) to indicate that certain
temporary allocations don't outlive the current statement.
The first use case is redundant with n.Func.Pragma&Noescape!=0, which
is the convention we use for checking other function-level pragmas.
The second use case is better served by renaming "Noescape" to
"Transient".
Passes toolstash-check.
Change-Id: I0f09d2d5767513894b7bf49da9cdabd04aa4a05e
Reviewed-on: https://go-review.googlesource.com/c/go/+/199822
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, escape analysis is able to record at most one dereference
when a parameter leaks to the heap; that is, at call sites, it can't
distinguish between any of these three functions:
func x1(p ****int) { sink = *p }
func x2(p ****int) { sink = **p }
func x3(p ****int) { sink = ***p }
Similarly, it's limited to recording parameter leaks to only the first
4 parameters, and only up to 6 dereferences.
All of these limitations are due to the awkward encoding scheme used
at the moment.
This CL replaces the encoding scheme with a simple [8]uint8 array,
which can handle up to the first 7 parameters, and up to 254
dereferences, which ought to be enough for anyone. And if not, it's
much more easily increased.
Shrinks export data size geometric mean for Kubernetes by 0.07%.
Fixes#33981.
Change-Id: I10a94b9accac9a0c91490e0d6d458316f5ca1e13
Reviewed-on: https://go-review.googlesource.com/c/go/+/197680
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This CL better abstracts away the parameter leak info that was
directly encoded into the uint16 value. Followup CL will rewrite the
implementation.
Passes toolstash-check.
Updates #33981.
Change-Id: I27f81d26f5dd2d85f5b0e5250ca529819a1f11c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/197679
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This CL changes cmd/compile to use Node.Right instead of
Node.Rlist for OAS2FUNC/OAS2RECV/OAS2MAPR/OAS2DOTTYPE nodes.
Fixes#32293
Change-Id: I4c9d9100be2d98d15e016797f934f64d385f5faa
Reviewed-on: https://go-review.googlesource.com/c/go/+/197817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL implements several optimizations for the escape analysis flow
graph:
1. Instead of recognizing heapLoc specially within Escape.outlives,
set heapLoc.escapes = true and recognize any location with escapes
set. This allows us to skip adding edges from the heap to escaped
variables in two cases:
1a. In newLoc, if the location is for a variable or allocation too
large to fit on the stack.
1b. During walkOne, if we discover that an object's address flows
somewhere that naturally outlives it.
2. When recording edges in Escape.flow, if x escapes and we're adding
an edge like "x = &y", we can simply mark that y escapes too.
3. During walkOne, if we reach a location that's marked as escaping,
we can skip visiting it again: we've either already walked from it, or
it's in queue to be walked from again.
On average, reduces the number of visited locations by 15%. Reduces
time spent in escape analysis for particularly hairy packages like
runtime and gc by about 8%. Reduces escape.go's TODO count by 22%.
Passes toolstash-check.
Change-Id: Iaf86a29d76044e4b4c8ab581b916ef5bb5df4437
Reviewed-on: https://go-review.googlesource.com/c/go/+/196811
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The old escape analysis code used to repeatedly walk the entire flow
graph until it reached a fixed point. With escape.go, I wanted to
avoid this if possible, so I structured the walking code with two
constraints:
1. Always walk from the heap location last.
2. If an object escapes, ensure it has flow edge to the heap location.
This works, but it precludes some graph construction
optimizations. E.g., if there's an assignment "heap = &x", then we can
immediately tell that 'x' escapes without needing to visit it during
the graph walk. Similarly, if there's a later assignment "x = &y", we
could immediately tell that 'y' escapes too. However, the natural way
to implement this optimization ends up violating the constraints
above.
Further, the constraints above don't guarantee that the 'transient'
flag is handled correctly. Today I think that's handled correctly
because of the order that locations happen to be constructed and
visited based on the AST, but I've felt uneasy about it for a little
while.
This CL changes walkAll to use a proper work queue (technically a work
stack) to track locations that need to be visited, and allows walkOne
to request that a location be re-visited.
Passes toolstash-check.
Change-Id: Iaa6f4d3fe4719c04d67009fb9a2a3e4930b3d7c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/196958
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit just adds a regress test for a few of the important corner
cases that I identified in #27557, which turn out to not be tested
anywhere.
While here, annotate a few of the existing test cases where we could
improve escape analysis.
Updates #27557.
Change-Id: Ie57792a538f7899bb17915485fabc86100f469a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/197137
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
OTYPESW and ORANGE were manually creating locations and flows around
them, which are relatively low-level graph construction primitives.
This CL changes them to use holes like the rest of the code.
Also, introduce "later" as an abstraction for assignment flows that
don't happen right away, and which need to prevent expressions from
being marked as "transient" (e.g., in ODEFER and ORANGE).
There's no behavior change here, but this does reduce the number of
newLoc call sites, which should help with restoring -m=2 diagnostics.
Passes toolstash-check.
Updates #31489.
Change-Id: Ic03d4488cb5162afe8b00b12432d203027e8d7d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/196619
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This information is redundant with the position information already
provided. Also, no other -m diagnostics print out function name.
While here, report parameter leak diagnostics against the parameter
declaration position rather than the function, and use Warnl for
"moved to heap" messages.
Test cases updated programmatically by removing the first word from
every "no match for" error emitted by run.go:
go run run.go |& \
sed -E -n 's/^(.*):(.*): no match for `([^ ]* (.*))` in:$/\1!\2!\3!\4/p' | \
while IFS='!' read -r fn line before after; do
before=$(echo "$before" | sed 's/[.[\*^$()+?{|]/\\&/g')
after=$(echo "$after" | sed -E 's/(\&|\\)/\\&/g')
fn=$(find . -name "${fn}" | head -1)
sed -i -E -e "${line}s/\"${before}\"/\"${after}\"/" "${fn}"
done
Passes toolstash-check.
Change-Id: I6e02486b1409e4a8dbb2b9b816d22095835426b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/195040
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This CL moves parameter tagging to before escape analysis is complete,
so we still have access to EscLocation. This will be useful once
EscLocation starts tracking higher-fidelity escape details.
Notably, this CL stops using n.Esc to record parameter escape analysis
details. Now escape analysis only ever sets n.Esc to EscNone or
EscHeap. (It still defaults to EscUnknown, and is set to EscNever in
some places though.)
Passes toolstash-check.
Updates #33981.
Change-Id: I50a91ea1e38c442092de6cd14e20b211f8f818c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/193178
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Use the following (suboptimal) script to obtain a list of possible
typos:
#!/usr/bin/env sh
set -x
git ls-files |\
grep -e '\.\(c\|cc\|go\)$' |\
xargs -n 1\
awk\
'/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
hunspell -d en_US -l |\
grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
sort -f |\
uniq -c |\
awk '$1 == 1 { print $2; }'
Then, go through the results manually and fix the most obvious typos in
the non-vendored code.
Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
Add block method to preserve loop depth when evaluating statements in a
block, so escape analysis can handle looping label more precisely.
Updates #22438
Change-Id: I39b306544a6c0ee3fcbebbe0d0ee735cb71773e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/193517
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In general, a conversion to interface type may require values to be
boxed, which in turn necessitates escape analysis to determine whether
the boxed representation can be stack allocated.
However, esc.go used to unconditionally print escape analysis
decisions about OCONVIFACE, even for conversions that don't require
boxing (e.g., pointers, channels, maps, functions).
For test compatibility with esc.go, escape.go similarly printed these
useless diagnostics. This CL removes the diagnostics, and updates test
expectations accordingly.
Change-Id: I97c57a4a08e44d265bba516c78426ff4f2bf1e12
Reviewed-on: https://go-review.googlesource.com/c/go/+/192697
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
