Similar to the previous CL to suppress escape analysis diagnostics for
method wrappers, suppress liveness analysis diagnostics too. It's
hardly useful to know that all of a wrapper method's arguments are
live at entry.
Change-Id: I0d1e44552c6334ee3b454adc107430232abcb56a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330749
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This is code in progress to generate the two main other types of entries
in dictionaries:
- all types in the instantiated function derived from the type
arguments (which are currently concrete, but will eventually be
gcshapes)
- pointers (i.e. mainly the unique name) to all needed sub-dictionaries
In order to generate these entries, we now generate cached information
gfInfo about generic functions/methods that can be used for creating the
instantiated dictionaries. We use the type substituter to compute the
right type args for instantiated sub-dictionaries.
If infoPrintMode is changed to true, the code prints out all the
information gathered about generic functions, and also the entries in
all the dictionaries that are instantiated. The debug mode also prints
out the locations where we need main dictionaries in non-instantiated
functions.
Other changes:
- Moved the dictionary generation back to stencil.go from reflect.go,
since we need to do extra analysis for the new dictionary entries. In
the process, made getInstantiation generate both the function
instantiation and the associated dictionary.
- Put in small change for now in reflect.go, so that we don't try
generate separate dictionaries for Value[T].get and the
auto-generated (*Value[T]).get. The auto-generated wrapper shouldn't really
need a dictionary.
- Detected, but not handling yet, a new case which needs
dictionaries - closures that have function params or captured
variables whose types are derived from type arguments.
- Added new tests in dictionaryCapture for use of method
value/expressions in generic functions and for mutually recursive
generic functions.
Change-Id: If0cbde8805a9f673a23f5ec798769c85c9c5359b
Reviewed-on: https://go-review.googlesource.com/c/go/+/327311
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This CL changes escape analysis to skip reporting diagnostics (at
least for parameter tagging) for generated wrappers.
We're inconsistent about when/where wrappers are generated, which made
errorcheck tests of escape analysis unnecessarily brittle to changes
in wrapper generation. This CL addresses this making errorcheck tests
only care about tagging of the actual functions themselves, not the
wrappers too.
Change-Id: Ia1a0b9dabee4d4162b05647f871db03b032c945a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330689
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
CL 329571 fold the checking has type params logic, but did not realize
that the instance in typIdx can be folded, too.
Change-Id: I4682af3779535af6a6e843972cada12ba1bae6ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/330389
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The previous code for walking the syntax AST to find declarations
needed to know whether a declaration appeared within block scope, but
syntax.Crawl (née syntax.Walk) made that somewhat awkward.
This CL simplifies it a little, taking advantage of syntax.Walk's
support for keeping per-subtree state.
Change-Id: I03c7da8c44bec40f88e983852dc6bbab7e6ac13c
Reviewed-on: https://go-review.googlesource.com/c/go/+/330549
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
- Fix handling of method expressions with embedded fields. Fix an
incorrect lookup for method expressions, which have only the
top-level type (and don't have DOT operations for the embedded
fields). Add the embedded field dot operations into the closure.
- Don't need a dictionary and so don't build a closure if the last
embedded field reached in a method expression is an interface value.
- Fix methodWrapper() to use the computed 'dot' node in the
generic-only part of the code.
- For a method expression, don't create a generic wrapper if the last
embedded field reached before the method lookup is an interface.
Copied cmd/compile/internal/types2/testdata/fixedbugs/issue44688.go2 to
test/typeparam/issue44688.go, made it fully runnable (rather than just
for compilation), and added a bunch more tests.
Change-Id: I90c1aa569e1c7272e986c9d2ae683e553c3a38a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/329550
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
This CL adds go/ast's Visitor, Walk, and Inspect functions to package
syntax. Having functions with the same API and semantics as their
go/ast counterparts reduces the mental load of context switching
between go/ast and syntax.
It also renames the existing Walk function into Crawl, and marks it as
a deprecated wrapper around Inspect. (I named it "Crawl" because it's
less functional than "Walk"... get it??)
There aren't that many callers to Crawl, so we can probably remove it
in the future. But it doesn't seem pressing, and I'm more concerned
about the risk of forgetting to invert a bool condition somewhere.
Change-Id: Ib2fb275873a1d1a730249c9cb584864cb6ec370e
Reviewed-on: https://go-review.googlesource.com/c/go/+/330429
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When wrapping a go/defer statement like:
go f(g(), "x", 42)
we were wrapping it like:
_0, _1, _2, _3 := f, g(), "x", 42
go func() { _0(_1, _2, _3) }()
This is simple and general (and often necessary), but suboptimal in
some cases, such as this. Instead of evaluating the constant arguments
at the go/defer statement, and storing them into the closure context,
we can just keep them in the wrapped call expression.
This CL changes the code to instead generate (assuming f is a declared
function, not a function-typed variable):
_0 := g()
go func() { f(_0, "x", 42) }()
Change-Id: I2bdd4951e7ee93363e1656ecf9b5bd69a121c38a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330332
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>
This CL extends escape analysis to analyze function calls using method
expressions the same as it would a normal method call. That is, it now
analyzes "T.M(recv, args...)" the same as "recv.M(args...)".
This is useful because it means the frontend can eventually stop
supporting both function calls and method calls. We can simply desugar
method calls into function calls, like we already do in the backend to
simplify SSA construction.
Change-Id: I9cd5ec0d534cbcd9860f0014c86e4ae416920c26
Reviewed-on: https://go-review.googlesource.com/c/go/+/330331
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>
This CL moves two bits of related code from order.go to escape
analysis:
1. The recognition of "unsafe uintptr" arguments passed to
syscall-like functions.
2. The wrapping of go/defer function calls in parameter-free function
literals.
As with previous CLs, it would be nice to push this logic even further
forward, but for now escape analysis seems most pragmatic.
A couple side benefits:
1. It allows getting rid of the uintptrEscapesHack kludge.
2. When inserting wrappers, we can move some expressions into the
wrapper and escape analyze them better. For example, the test
expectation changes are all due to slice literals in go/defer calls
where the slice is now constructed at the call site, and can now be
stack allocated.
Change-Id: I73679bcad7fa8d61d2fc52d4cea0dc5ff0de8c0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/330330
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>
Currently ORECOVER is a single operation that both (1) calculates
the (logical) caller frame pointer and (2) calls runtime.gorecover.
This is normally fine, but it's inconvenient for regabi, which wants
to wrap "defer recover()" into "defer func() { recover() }" and
needs (1) and (2) to happen at different times.
The current solution is to apply walkRecover early to split it into
the two steps, but calling it during order is a minor layering
violation. It works well today because the order and walk phases are
closely related anyway and walkRecover is relatively simple, but it
won't work for go/defer wrapping earlier into the frontend.
This CL adds a new, lower-level ORECOVERFP primitive, which represents
just part (2); and OGETCALLER{PC,SP} primitives, which provide a way
to compute (1) in the frontend too.
OGETCALLERPC isn't needed/used today, but it seems worth including for
completeness. Maybe it will be useful at some point for intrinsifying
runtime.getcaller{pc,sp}, like we already do for runtime.getg.
Change-Id: Iaa8ae51e09306c45c147b6759a5b7c24dcc317ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/330192
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>
This CLs adds new frontend ops that will be used in the next CL. Split
out separately so generated code is less distracting in the main CL.
Change-Id: I66125e0ec2217bfa05f7b0ea0bc99ada13f563f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/330191
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The current go/defer wrapping code goes to some length to clear
ClosureCalled when a function call will end up not being called
directly, and so it will need to use the context register.
But we already have a flag to indicate we need to use the context
register: Needctxt. The real issue here is just that buildssa was
using fn.ClosureCalled instead of fn.Needctxt.
Change-Id: Ic9f5f23b66eb467fc61fa84eacb45d46c54133d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/330329
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This CL is a prep refactoring for an upcoming CL to move go/defer
wrapping into escape analysis. That CL is unfortunately unavoidably
complex and subtle, so this CL takes care of some more mundane
refactoring details.
Change-Id: Ifbefe1d522a8d57066646be09536437f42e7082c
Reviewed-on: https://go-review.googlesource.com/c/go/+/330251
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>
This flag is only needed to prevent the directClosureCall optimization
in walkCall, when called for walkGoDefer. But walkGoDefer don't need
to call walkCall: at this point in the compile, the call expression
isn't a real call anymore.
Instead, we just need to walkExpr on the function expression.
Change-Id: I8a5176cfe1bff53700cbd21ed1b479ebd9a839ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/330271
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
order already takes care of wrapping all go/defer function calls, so
there's no need for walk to duplicate that logic: it's never going to
be used.
Change-Id: I54e545404e52ab8f9d60151d1bd2aff4b9bd8b72
Reviewed-on: https://go-review.googlesource.com/c/go/+/330270
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This CL makes OCHECKNIL typecheckable. Simplifies IR construction code
slightly, and gives one convenient place to check for misuse.
Change-Id: I280b8e47eddcac12947a41d6f911b25bc12a66bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/330194
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
CaptureName currently does a few things: checks if a variable needs to
be captured at all; checks if the variable has already been captured;
and creates and saves a new variable. This full suite of functionality
is useful for noder and irgen, but unified IR and other backend code
only has a need for the last feature.
This CL refactors CaptureName a little bit and extracts out
NewClosureVar as a function usable for callers that don't need the
extra features of CaptureName.
Change-Id: I8a67c6375e44babe53344bf78e335535c57f9607
Reviewed-on: https://go-review.googlesource.com/c/go/+/330193
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
CL 325369 improved this logic in types2. Port this improvement back to
go/types.
Change-Id: I5f859cbffd88bb3db09a81c2389269f7bd0869f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/330069
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Remove logic related to guarding against allowing type parameters from
cmd/gofmt. At this point, it was only restricting tests.
Change-Id: Idd198389aaa422636d61af547a37be49f3be6c97
Reviewed-on: https://go-review.googlesource.com/c/go/+/329931
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This CL reorganizes the code from package escape into multiple files,
so the relationships between bits of code are hopefully easier to
follow. Besides moving code around and adding necessary
copyright/import declarations, no code is touched at all.
Change-Id: Iddd396c3a140f4eb1a7a6266d92a4098118b575b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329989
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>
This CL removes the special escape analysis tags added to support
//go:uintptrescapes and calls to external functions. Instead, these
are kept as function pragmas.
This CL by itself isn't very interesting, but I expect will help with
subsequent cleanups I have planned here.
Change-Id: Ifb960289a27e0a6295ce2d2f5ec233cac590522b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329969
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Originally, overloading -d=inlfuncswithclosures=0 to enable quirks
mode was convenient because toolstash -cmp doesn't provide a way to
pass different gcflags to the installed vs stashed toolchains. Prior
to unified IR being merged, the stashed toolchain wouldn't know about
or accept any unified-specific flags.
However, this concern is no longer applicable since unified IR has
been merged, and the TestUnifiedCompare test can easily specify
different flag sets for the baseline and experiment build configs.
This CL adds a new -d=unifiedquirks flag to enable quirks mode, so
that it's possible to test unified IR with -d=inlfuncswithclosures=0
without also affecting a bunch of other compilation details.
Change-Id: Id1932f332822622aa8617278e82ec6d1a53b1b46
Reviewed-on: https://go-review.googlesource.com/c/go/+/329733
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This patch reinstates a fix for PowerPC with regard to making VDSO calls
while receiving a signal, and subsequently crashing. The crash happens
because certain VDSO calls can modify the r30 register, which is where g
is stored. This change was reverted for PowerPC because r30 is supposed
to be a non-volatile register. This is true, but that only makes a
guarantee across function calls, but not "within" a function call. This
patch was seemingly fine before because the Linux kernel still had hand
rolled assembly VDSO function calls, however with a recent change to C
function calls it seems the compiler used can generate instructions
which temporarily clobber r30. This means that when we receive a signal
during one of these calls the value of r30 will not be the g as the
runtime expects, causing a segfault.
You can see from this assembly dump how the register is clobbered during
the call:
(the following is from a 5.13rc2 kernel)
```
Dump of assembler code for function __cvdso_clock_gettime_data:
0x00007ffff7ff0700 <+0>: cmplwi r4,15
0x00007ffff7ff0704 <+4>: bgt 0x7ffff7ff07f0 <__cvdso_clock_gettime_data+240>
0x00007ffff7ff0708 <+8>: li r9,1
0x00007ffff7ff070c <+12>: slw r9,r9,r4
0x00007ffff7ff0710 <+16>: andi. r10,r9,2179
0x00007ffff7ff0714 <+20>: beq 0x7ffff7ff0810 <__cvdso_clock_gettime_data+272>
0x00007ffff7ff0718 <+24>: rldicr r10,r4,4,59
0x00007ffff7ff071c <+28>: lis r9,32767
0x00007ffff7ff0720 <+32>: std r30,-16(r1)
0x00007ffff7ff0724 <+36>: std r31,-8(r1)
0x00007ffff7ff0728 <+40>: add r6,r3,r10
0x00007ffff7ff072c <+44>: ori r4,r9,65535
0x00007ffff7ff0730 <+48>: lwz r8,0(r3)
0x00007ffff7ff0734 <+52>: andi. r9,r8,1
0x00007ffff7ff0738 <+56>: bne 0x7ffff7ff07d0 <__cvdso_clock_gettime_data+208>
0x00007ffff7ff073c <+60>: lwsync
0x00007ffff7ff0740 <+64>: mftb r30 <---- RIGHT HERE
=> 0x00007ffff7ff0744 <+68>: ld r12,40(r6)
```
What I believe is happening is that the kernel changed the PowerPC VDSO
calls to use standard C calls instead of using hand rolled assembly. The
hand rolled assembly calls never touched r30, so this change was safe to
roll back. That does not seem to be the case anymore as on the 5.13rc2
kernel the compiler *is* generating assembly which modifies r30, making
this change again unsafe and causing a crash when the program receives a
signal during these calls (which will happen often due to async
preempt). This change happened here:
https://lwn.net/ml/linux-kernel/235e5571959cfa89ced081d7e838ed5ff38447d2.1601365870.git.christophe.leroy@csgroup.eu/.
I realize this was reverted due to unexplained hangs in PowerPC
builders, but I think we should reinstate this change and investigate
those issues separately:
f4ca3c1e0aFixes#46803
Change-Id: Ib18d7bbfc80a1a9cb558f0098878d41081324b52
GitHub-Last-Rev: c3002bcfca
GitHub-Pull-Request: golang/go#46767
Reviewed-on: https://go-review.googlesource.com/c/go/+/328110
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Apparently, TestMorestack is still flaky on darwin/arm64 builder
after CL 307730. Let it spend more time in copying the stack.
With this CL, on my Apple M1 machine it passes reliably in short
mode for 1000 runs, and reliably gets 250+ samples in the 5-second
interval in long mode.
May fix#46755.
Change-Id: I07b36c1cf63ad35f7820e1f8e837e29376a37b2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/329869
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
CL 280634 remove Sym.Importdef, so ipkg in importsym is not used
anymore. So we can remove it from importsym and all other import*
functions, which just call importsym internally.
Change-Id: I15b9d11c4445dbe40982f7ff2a33a2116705e790
Reviewed-on: https://go-review.googlesource.com/c/go/+/329573
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
So making it less verbose and clearer to the reader what that check means.
Change-Id: I41587aab399e63600356c5cecec64978048bed36
Reviewed-on: https://go-review.googlesource.com/c/go/+/329571
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We've observed some occasional os-arch specific timeouts
in signal.TestSignalTrace(). While the main purpose of a
short timeout is to ensure the passing tests complete
quickly, the unexpected failure path can tolerate waiting
longer (the test is not intended to test how slow or
overloaded the OS is at the time it is run).
Fixes#46736
Change-Id: Ib392fc6ce485a919612784ca88ed76c30f4898e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/329502
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
The new RGBA64At method is equivalent to the existing At method (and the
new SetRGBA64 method is equivalent to the existing Set method in the
image/draw package), but they can avoid allocations from converting
concrete color types to the color.Color interface type.
Also update api/go1.17.txt and doc/go1.17.html
Fixes#44808
Change-Id: I8671f3144512b1200fa373840ed6729a5d61bc35
Reviewed-on: https://go-review.googlesource.com/c/go/+/311129
Trust: Nigel Tao <nigeltao@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This CL adds a longtest test to make sure -d=unified=1 produces output
identical to -d=unified=0.
Change-Id: I2c5d38f67dbc8fecd8332a91ba7cae22225b090c
Reviewed-on: https://go-review.googlesource.com/c/go/+/329429
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
I wrote code that relied on this API, but I misunderstood the original
description of the "more" result. As a consequence, my code always
stopped one frame early.
This CL expands the documentation to be more explicit and specifically
call out my confusion (i.e., that the "more" result indicates whether
the *next* Next call will return a valid Frame, and not whether this
call did).
Change-Id: If135f8f8c05425073d45377c4179e4f79e6bd6ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/329389
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Otherwise, in c-archive or c-shared mode, there is the chance of
getting a SIGPROF just after the signal handler is removed but before
profiling is disabled, in which case the program will die.
Fixes#46498
Change-Id: I5492beef45fec9fb9a7f58724356d6aedaf799ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/329290
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Change-Id: I876933370a6bcb6586eda9d8fc28a081bf31b1cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/328511
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
While at it, also rename "useUnifiedIR" to "unified", to be consistent
with "-d=unified" and "GOEXPERIMENT=unified".
Change-Id: I48ffdb4b36368343893b74f174608f5f59278249
Reviewed-on: https://go-review.googlesource.com/c/go/+/328989
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL extends the unified export data format's existing sync
mechanism to save writer stacks, controlled by the -d=syncframes debug
flag. This allows readers to provide more details when reporting
desync errors, which should simplify development of the data format
and the various reader/writer implementations.
For example, CL 328051 updated reader and writer, but missed making a
similar change to the linker (fix in CL 328054). Re-reviewing the CL
in isolation after the failure, it was not immediately obvious what
was going wrong. But the pair of stack traces below identifies exactly
what happened: it should have updated linker.relocFuncExt to write out
the new sync marker too.
```
data sync error: package "internal/abi", section 6, index 4, offset 536
found UseReloc, written at:
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/encoder.go:221: (*encoder).reloc +0x44
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:214: (*linker).relocFuncExt +0x580
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:233: (*linker).relocTypeExt +0x234
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:161: (*linker).relocObj +0x2198
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:64: (*linker).relocIdx +0x196
expected ImplicitTypes, reading at:
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:796: (*reader).implicitTypes +0x36
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:810: (*reader).addBody +0x81
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:727: (*reader).funcExt +0x542
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:651: (*reader).method +0x324
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:557: (*pkgReader).objIdx +0x2704
```
Change-Id: I911193edd2a965f81b7459f15fb613a773584685
Reviewed-on: https://go-review.googlesource.com/c/go/+/328909
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: Robert Griesemer <gri@golang.org>
CL 328336 introduced two new packages that the runtime and other
low-level packages depend on. Include them as targets to copy in this
test with other such packages.
Fixes the dev.typeparams longtest builders.
Change-Id: Ia886f0264962a68acd06ebca002eef8515f06487
Reviewed-on: https://go-review.googlesource.com/c/go/+/329251
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is port of CL 328053 for types2 to go/type.
The change is identical, but for some tweaks to the error positions in
tests.
Updates #46749
Change-Id: I8d34c5b1669e59e4ec7d91f81dcf655b2bfd89a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/328869
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Left a couple of tests with old notation so that we keep testing it
while the notation is still supported.
Change-Id: Ia6a3e7911af87eaccc7b06189c10f79789575a98
Reviewed-on: https://go-review.googlesource.com/c/go/+/328256
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Unified includes a check to make sure that types2 memory has been
garbage collected, but it relies on precise finalization, which we
provide (for dynamically allocated objects, at least) but isn't
guaranteed by the Go spec. In particular, Go 1.4 doesn't provide this.
The check is strictly unnecessary and only exists to make sure we
don't regress and start holding onto types2 memory accidentally. So
just disable the check during bootstrap builds.
Change-Id: Ie54fe53b2edba02c0b0b1e5ae39d81be8a0ace8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/329269
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>
runtime/pprof has a more complete list of platforms with broken
profiling than I used in cmd/pprof in https://golang.org/cl/325809.
Duplicate that list in cmd/pprof and clean it up a bit in runtime/pprof
for easier reference.
Change-Id: I8f2580aac223de9b73cfff4355f49916f7b76493
Reviewed-on: https://go-review.googlesource.com/c/go/+/329149
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Currently, callMethod has some ABI translation code that is not agnostic
of endianness. This change rectifies that by adding a method to
internal/abi.RegArgs for safely returning an offset into a register slot
that's endianness-dependent.
No tests for this because it's just best-effort. There's no actual way
to test this because we don't support a register ABI on any big endian
architectures yet.
Change-Id: Ic68d9ee1bfdea0fc2992d467d749e2b083e92de3
Reviewed-on: https://go-review.googlesource.com/c/go/+/328348
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Change-Id: Ia0da5e5d1e8d20327690cb53c9df067401f3428c
Reviewed-on: https://go-review.googlesource.com/c/go/+/328812
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change-Id: I522263eb5112b78639340b83d92e80a13f738bd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/328811
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>