This reverts CL 351751.
Reason for revert: new test is failing on many builders.
Change-Id: I066211c9f25607ca9eb5299aedea2ecc5069e34f
Reviewed-on: https://go-review.googlesource.com/c/go/+/360757
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Correctly track if the index expression is addressable.
Rewrote code slightly.
Fixes#49275.
Change-Id: Ic54edd0213a091173ff5403ab0e3e1f1fca0e361
Reviewed-on: https://go-review.googlesource.com/c/go/+/360603
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For constraint type inference failures where the type parameter doesn't
match the constraint, print the type parameter rather than its type name
object. This provides more flexibility for improving the error message
down the road.
Change-Id: I188871d6f26a16cd96e59770966a1ec65607b128
Reviewed-on: https://go-review.googlesource.com/c/go/+/360514
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When converting a constant to a type parameter, the result is never
constant (type parameters are not constant types), but we still need
to verfy that the constant is representable by each specific type in
the type set of the type parameter.
Fixes#49247.
Change-Id: I460983c7070b33baadce25dd23210e10930cfb2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/360396
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Use gp.m.curg instead of the gp when recording cpu profiler stack
traces. This ensures profiler labels are captured when systemstack or similar
is executing on behalf of the current goroutine.
After this there are still rare cases of samples containing the labelHog
function, so more work might be needed. This patch should fix ~99% of the
problem.
Fixes#48577.
Change-Id: I27132110e3d09721ec3b3ef417122bc70d8f3279
Reviewed-on: https://go-review.googlesource.com/c/go/+/351751
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
TestExec and TestExecHelper check for a workaround of a particular OS
bug on darwin that's triggered more often via asynchronous preemption.
As part of this, the test sets up 100 CPU-bound goroutines, and sets
GOMAXPROCS to 50, sleeping for a little bit before calling Exec. Thus
far, this is fine because the scheduler runs the Execing goroutine in a
timely manner. However, CL 309869 will reduce the minimum heap size,
causing a GC to happen during the test.
On a 16 CPU machine, with GOMAXPROCS at 50, and 100 CPU-bound
goroutines, both the OS scheduler and the Go scheduler are severly
oversaturated. As a result, the test often (not always, but often) runs
out for the full lifetime of those 100 goroutines, which run for about 1
second.
The minimum heap size reduction is not necessary to trigger this; an
additional call to runtime.GC in the helper is also sufficient to
trigger this delay.
The delay on its own isn't great, since it adds a whole second to
all.bash on its own. However, it also seems correlated with other
subprocess tests in the syscall package, namely TestPassFD and
TestFcntlFlock. These tests fail in a fairly superficial way: the file
descriptor for the temporary directories they make gets clobbered, is
closed, or becomes stale.
Change-Id: I213dd5e38967d19a8b317e6d4c5024b57f9e3fed
Reviewed-on: https://go-review.googlesource.com/c/go/+/360574
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add asan tests to check the use of Go with -asan option.
Currenly, the address sanitizer in Go only checks for error
memory access to heap objects.
TODO: Enable check for error memory access to global objects.
Updates #44853.
Change-Id: I83579f229f117b5684a369fc8f365f4dea140648
Reviewed-on: https://go-review.googlesource.com/c/go/+/298615
Trust: fannie zhang <Fannie.Zhang@arm.com>
Run-TryBot: fannie zhang <Fannie.Zhang@arm.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Go exchanges siginfo and sigevent structures with the kernel. They
contain unions, but Go's use is limited to the first few fields. Pad out
the rest so the size Go sees is the same as what the Linux kernel sees.
This is a follow-up to CL 342052 which added the sigevent struct without
padding, and to CL 353136 which added the padding but with an assertion
that confused several type-checkers. It updates the siginfo struct as
well so there are no bad examples in the defs_linux_*.go files.
Reviewed-on: https://go-review.googlesource.com/c/go/+/353136
Change-Id: I9610632ff0ec43eba91f560536f5441fa907b36f
Reviewed-on: https://go-review.googlesource.com/c/go/+/360094
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Add explicit address sanitizer instrumentation to the runtime and
syscall packages. The compiler does not instrument the runtime
package. It does instrument the syscall package, but we need to add
a couple of cases that it can't see.
Refer to the implementation of the asan malloc runtime library,
this patch also allocates extra memory as the redzone, around the
returned memory region, and marks the redzone as unaddressable to
detect the overflows or underflows.
Updates #44853.
Change-Id: I2753d1cc1296935a66bf521e31ce91e35fcdf798
Reviewed-on: https://go-review.googlesource.com/c/go/+/298614
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: fannie zhang <Fannie.Zhang@arm.com>
This is one step towards optimizing WriteMsgUDPAddrPort.
Further steps remain, namely to avoid the syscall.Sockaddr interface,
as we do for UDPConn.WriteToUDP and UDPConn.ReadFromUDP.
A previous change optimized ReadMsgUDPAddrPort by having
ReadMsgUDP call ReadMsgUDPAddrPort rather than the other way around.
This change does not implement WriteMsgUDP in terms of WriteMsgUDPAddrPort,
because a few layers deep, on posix platforms only
(in ipToSockaddrInet4 and ipToSockaddrInet6),
is special handling of zero-length IP addresses and IPv4zero.
It treats IP(nil) as equivalent to 0.0.0.0 or ::,
and 0.0.0.0 as equivalent to :: in an IPv6 context.
Based on the comments, I suspect that this treatment was intended
for the Listen* API, not the Write* API, but it affects both,
and I am nervous about changing the behavior for Write*.
The netip package doesn't have a way to represent a "zero-length IP address"
as distinct from an invalid IP address (which is a good thing),
so to implement WriteMsgUDP using WriteMsgUDPAddrPort,
we would have to duplicate this special handling at the start of WriteMsgUDP.
But this handling depends on whether the UDPConn is an IPv4 or an IPv6 conn,
which is also platform-specific information.
As a result, every attempt I made to implement WriteMsgUDP using
WriteMsgUDPAddrPort while preserving behavior ended up
being considerably worse than copy/paste/modify.
This does mean that WriteMsgUDP and WriteMsgUDPAddrPort will have
different behavior in these cases.
name old time/op new time/op delta
ReadWriteMsgUDPAddrPort-8 5.29µs ± 6% 5.02µs ± 7% -5.14% (p=0.000 n=13+15)
name old alloc/op new alloc/op delta
ReadWriteMsgUDPAddrPort-8 128B ± 0% 64B ± 0% -50.00% (p=0.000 n=15+15)
name old allocs/op new allocs/op delta
ReadWriteMsgUDPAddrPort-8 4.00 ± 0% 2.00 ± 0% -50.00% (p=0.000 n=15+15)
Change-Id: Ia78eb49734f4301d7772dfdbb5a87e4d303a9f7a
Reviewed-on: https://go-review.googlesource.com/c/go/+/360597
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead of implementing ReadMsgUDPAddrPort in terms of ReadMsgUDP,
do it the other way around. This keeps the code minimal while
still avoiding allocs.
We could also rearrange ReadMsgUDP to be mid-stack inlined to avoid
allocating the *UDPAddr, but anyone who's trying to eliminate
allocs should use ReadMsgUDPAddrPort instead anyway,
because ReadMsgUDP will always allocate at least once (the IP slice).
name old time/op new time/op delta
ReadWriteMsgUDPAddrPort-8 5.26µs ± 3% 5.29µs ± 6% ~ (p=0.429 n=12+13)
name old alloc/op new alloc/op delta
ReadWriteMsgUDPAddrPort-8 176B ± 0% 128B ± 0% -27.27% (p=0.000 n=15+15)
name old allocs/op new allocs/op delta
ReadWriteMsgUDPAddrPort-8 5.00 ± 0% 4.00 ± 0% -20.00% (p=0.000 n=15+15)
Change-Id: I15228cb4ec4f13f2f390407b6c62c44c228e7201
Reviewed-on: https://go-review.googlesource.com/c/go/+/360596
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In case of reference to method call of an imported fully-instantiated
type, nameNode.Func will be nil causes checkFetchBody panic. To fix
this, make sure checkFetchBody is only called when Func is not nil.
Fixes#49246
Change-Id: I32e9208385a86d4600d8ebf6f5efd8fca571ea16
Reviewed-on: https://go-review.googlesource.com/c/go/+/360056
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Previously, when resolving references of form
(https://golang.org/?hello).ResolveReference(?)
we only used URL.RawQuery to determine whether or not a query part is
defined. Go 1.7 introduced URL.ForceQuery as a flag for the situation
where a query part is provided but empty. But we did not use it in
ResolveReference. This leads to the erroneous output
https://golang.org/?hello
when the correct output should be
https://golang.org/?
This commit rectifies that error.
Fixes#46033
Change-Id: I05bc0b48bf2bbf13b4ddc0dd10599ea613dc2188
Reviewed-on: https://go-review.googlesource.com/c/go/+/317930
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Damien Neil <dneil@google.com>
When type inference cannot infer type arguments it prints the list of
inferred type arguments (often empty) at the end of the error message.
This was meant as debugging support only. Removed for now.
Eventually we should provide a better error message.
Fixes#49272.
Change-Id: I68d43a6ace91081009cab0f2fbad7bfbddf5e76b
Reviewed-on: https://go-review.googlesource.com/c/go/+/360554
Trust: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
This benchmark simulates object starvation in order to force Ps to steal
objects from other Ps. Extracted from CL 314229.
Change-Id: Iee31df355ba04d80fbd91c4414e397a375e6d6d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/360256
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Trust: Cherry Mui <cherryyz@google.com>
If a identical input is already present in the corpus, don't re-add it.
This may happen when the same input produces a different coverage map,
causing the coordinator to think it has found a new input.
This fixes a race between reading/writing cached inputs.
Fixes#48721
Change-Id: I4807602f433c2b99396d25ceaa58b827796b3555
Reviewed-on: https://go-review.googlesource.com/c/go/+/359755
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
Fixes#49213
Change-Id: I2bfc151b74b0d14efbd00e5d28584f4180126c5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/359656
Trust: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
This is a port of CL 359656 from go/types to types2.
For #49213.
Change-Id: Ib98f9a344c1397af92e061cafd519ea374fd60bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/360294
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This is a port of CL 357814 from go/types to types2 with minor
adjustments due to small differences in error handling code.
Change-Id: I72ecc4532e8349f569cabb38006f3d8ff517bf30
Reviewed-on: https://go-review.googlesource.com/c/go/+/360276
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When checking assignability, a value of a named type (incl. a type parameter)
can never be assigned to a variable of a differently named type. Specifically,
if the types are two differently named type parameters, then values of one are
never assignable to variables of the other.
This CL clarifies the assignabiliy rules and simplifies the implementation.
Fixes#49242.
Change-Id: Id72a2c9bed5cdb726855e7a707137db1009e7953
Reviewed-on: https://go-review.googlesource.com/c/go/+/360274
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Add a comment how strings of length 0 are treated and
that they always will result in the return of a string
equal to the constant string "".
The previous implementation would return a string header that uses
runtime.zerobase as the backing array pointer while the string constant
"" has 0 as pointer value.
Using 0 has the backing array pointer is also the behaviour of
string([]byte(input)) which makes the new behaviour a better drop in
replacement.
Change-Id: Ic5460e9494b6708edbdfa4361e878d50db54ba10
Reviewed-on: https://go-review.googlesource.com/c/go/+/360255
Trust: Martin Möhrmann <martin@golang.org>
Run-TryBot: Martin Möhrmann <martin@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Updated multiple tests in test/codegen/arithmetic.go to verify
on ppc64/ppc64le as well
Change-Id: I79ca9f87017ea31147a4ba16f5d42ba0fcae64e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/358546
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This code is only generated when linking cgo internally with fixed
position code. This feature of the internal linker is only supported
on ppc64le/linux targets. This moves ppc64le/linux a little closer
to supporting PIE when internal linking.
This is more similar to the implementation suggested in the power
architecture elfv2 supplement, and works with both PIE and static
code.
Change-Id: I0b64e1c22b9e07b5151378d2ab19ee0e50405fc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/357332
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This is heavily based on CL 341336 by Joe Tsai and CL 351452 by
Jay Conrod.
T.Run and T.Name use a map[string]int64 to hold the next suffix to use
when duplicate names are passed to T.Run. This map necessarily retains
one entry per unique name. However, it's a waste of memory to retain
one entry per duplicate name: when we encounter the Nth duplicate, we
know that names 00 through N-1 have been used just by looking at N.
We do still need to store (and check for collisions againsts) explicit
names provided by the caller. For example, if the user passes in "a",
then "a#01", then "a" again, we cannot deduplicate the second "a" to
"a#01" — we need to instead skip ahead to "a#02". We can do so by
checking the count of "a", then generating a proposed deduplicated
name, then double-checking that proposed name against only the
explicit names so far.
This somewhat reduces memory usage for tests that spawn large numbers
of duplicate subtests, but doesn't solve the problem of memory growth
for fuzzing — we still have to track all of the explicit,
user-provided subtest names, and in a long-running fuzz test that set
alone may be unbounded.
This fixes memory growth for the example described in
https://golang.org/issue/44517#issuecomment-897104060,
but not the one in
https://golang.org/issue/44517#issuecomment-933825661.
For #44517
Change-Id: Ia159ecfcf44561ba67508d3af6377c27856df31d
Reviewed-on: https://go-review.googlesource.com/c/go/+/354749
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Use a pointer reciever to avoid copying the hiter struct when
checking if it is intialized.
Found through profiling that showed reflect map iteration spending
a good amount of time in duffcopy.
This change will also help other MapIter methods checking hiter struct
initialization like Value() and Key().
name old time/op new time/op delta
MapIterNext-12 97.9ns ± 4% 83.8ns ± 2% -14.37% (p=0.000 n=10+10)
Change-Id: I73ab964fa28061ee7e6d5c663a85048bd2e0274e
Reviewed-on: https://go-review.googlesource.com/c/go/+/360254
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Trust: Martin Möhrmann <martin@golang.org>
Simplify the parsing of array or slice constraint types added in CL
359134, following the port in CL 360135.
Change-Id: Ia86d4b0149a222423d3b19623dd39d4aeb23857d
Reviewed-on: https://go-review.googlesource.com/c/go/+/360115
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>
The intention is for BenchmarkFooBar functions to map 1:1 to drawFooBar
functions. Recent draw.go changes have added more drawFooBar functions
and have further modified the mapping, as fallback drawFooBar functions
aren't invoked as often as they used to.
This commit restores the 1:1 mapping and reorganizes the BenchmarkFooBar
functions in the same order as the matching drawFooBar functions appear.
Also modify a TestDraw test case from vgradGreen(255) = {0, 136, 0, 255}
to vgradGreen(90) = {0, 48, 0, 90}. Doing so matches the existing "The
source pixel is {0, 48, 0, 90}" comment but also makes for a more
interesting test case, as the source pixel is no longer fully opaque.
Fully opaque is already covered by the vgradGray() test case on the next
line.
Also fix a "variable source" comment copy-pasto when the source image is
actually uniform, not variable.
Also add a func DrawMask type switch comment about interface types.
Change-Id: I828e71f2ee8ec617f523c8aafb118fb7ba166876
Reviewed-on: https://go-review.googlesource.com/c/go/+/358974
Trust: Nigel Tao <nigeltao@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is a port of the idea used in CL 359134 from go/parser to syntax,
with adjustments due to the slightly different structure of the two
parsers, and some refactoring to simplify the logic.
Fixes#49175.
Change-Id: Ib4955bde708f2b08345f35523e6094c03ab3076c
Reviewed-on: https://go-review.googlesource.com/c/go/+/360135
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
There are multiple things preventing the -memprofilerate flag from
working right now:
- CmdFlags.MemProfileRate has type int64, which is currently not
supported by the compiler's reflection-based registerFlags.
Unfortunately, rather than letting you know this, registerFlags
simply ignores this field.
- Nothing consumes CmdFlags.MemProfileRate anyway. startProfile
instead uses a package-local memprofilerate variable that is never
set to anything.
Fix this by making CmdFlags.MemProfileRate an int (that's what
runtime.MemProfileRate is anyway) and using it in startProfile. While
we're here, prevent similar flag parsing bugs in the future by making
registerFlags panic if it encounters a flag field of unsupported type.
Change-Id: Ib9a1fcd8f4c5e9d7175a4fabc375f31e79774f9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/359955
Trust: Austin Clements <austin@google.com>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Currently, the compiler will emit any const that doesn't fit in an
int64 to go_asm.h like
#define const_stackPreempt constant.intVal{val:(*big.Int)(0xc000c06c40)}
This happens because dumpasmhdr formats the constant.Value using the
verb "%#v". Since constant.Value doesn't implement the GoString()
method, this just prints the Go-syntax representation of the value.
This happens to work for small integer constants, which go/constant
represents directly as an int64, but not for integer constants that
don't fit in an int64, which go/constant represents as a big.Int.
Make these constants usable by changing the formatting verb to "%v",
which will call the String() method, giving a reasonable result in all
cases.
Change-Id: I365eeb88c8acfc43ff377cc873432269bde3f541
Reviewed-on: https://go-review.googlesource.com/c/go/+/359954
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This abstracts the clunky and not constant time math/big elliptic.Curve
compatibility layer away from the pure fiat-backed group logic.
Change-Id: I3b7a7495034d0c569b21c442ae36958763b8b2d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/320074
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
Use types2.Structure() to get single underlying type of typeparams, to
handle some unusual cases where a type param is constrained to a single
underlying struct or map type.
Fixes#48538
Change-Id: I289fb7b31d489f7586f2b04aeb1df74e15a9f965
Reviewed-on: https://go-review.googlesource.com/c/go/+/359335
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Fix two defer bugs related to adding/removing open defer entries.
The bugs relate to the way that we add and remove open defer entries
from the defer chain. At the point of a panic, when we want to start
processing defer entries in order during the panic process, we need to
add entries to the defer chain for stack frames with open defers, since
the normal fast-defer code does not add these entries. We do this by
calling addOneOpenDeferFrame() at the beginning of each time around the
defer loop in gopanic(). Those defer entries get sorted with other open
and non-open-coded defer frames.
However, the tricky part is that we also need to remove defer entries if
they end not being needed because of a recover (which means we are back
to executing the defer code inline at function exits). But we need
to deal with multiple panics and in-process defers on the stack, so we
can't just remove all open-coded defers from the the defer chain during
a recover.
The fix (and new invariant) is that we should not add any open-coded
defers to the defer chain that are higher up the stack than an open-coded
defer that is in progress. We know that open-coded defer will still be
run until completed, and when it is completed, then a more outer frame
will be added (if there is one). This fits with existing code in gopanic
that only removes open-coded defer entries up to any defer in progress.
These bugs were because of the previous inconsistency between adding and
removing open defer entries, which meant that stale defer entries could
be left on the list, in these unusual cases with both recursive
panics plus multiple independent (non-nested) cases of panic & recover.
The test for #48898 was difficult to add to defer_test.go (while keeping
the failure mode), so I added as a go/test/fixedbug test instead.
Fixes#43920
Updates #43941Fixes#48898
Change-Id: I593b77033e08c33094315abf8089fbc4cab07376
Reviewed-on: https://go-review.googlesource.com/c/go/+/356011
Trust: Dan Scales <danscales@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
This is a clean port of CL 357915 to go/types.
Change-Id: Idf5850a8bdcf3596c561c97bcd60539945139743
Reviewed-on: https://go-review.googlesource.com/c/go/+/359877
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 is a port of CL 357779 to go/types. A test error message was
repositioned on the sliced operand.
Change-Id: Ie775c128f70d9cb08a2eba54b8bc082134ec3200
Reviewed-on: https://go-review.googlesource.com/c/go/+/359876
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 is a port of CL 357778 to go/types, adjusted to include error codes
and to use the different range statement syntax in go/ast.
Change-Id: Id537c195cd33a8b422a366269ca8730c2a5bccf1
Reviewed-on: https://go-review.googlesource.com/c/go/+/359875
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
This is a straightforward port of CL 357776 to go/types.
Change-Id: I64220840a01f57cd7955f7d956b9aa8227473b46
Reviewed-on: https://go-review.googlesource.com/c/go/+/359874
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>
