The method Method expects index to be an index of exported fields,
but, before this change, the index used by MethodByName could
take into account unexported fields if those happened sort
before the exported one.
Fixes#21177
Change-Id: I90bb64a47b23e2e43fdd2b8a1e0a2c9a8a63ded2
Reviewed-on: https://go-review.googlesource.com/51810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
pkgPath always received the empty string. Worse yet, it panicked if it
received anything else. This has been the case ever since newName was
introduced in early 2016.
Change-Id: I5f164305bd30c34455ef35e776c7616f303b37e4
Reviewed-on: https://go-review.googlesource.com/54331
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
We don't use it any more, remove it.
Change-Id: I76ce1a4c2e7048fdd13a37d3718b5abf39ed9d26
Reviewed-on: https://go-review.googlesource.com/44474
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Just use fun[0]==0 to indicate a bad itab.
Change-Id: I28ecb2d2d857090c1ecc40b1d1866ac24a844848
Reviewed-on: https://go-review.googlesource.com/44473
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Keep itabs in a growable hash table.
Use a simple open-addressable hash table, quadratic probing, power
of two sized.
Synchronization gets a bit more tricky. The common read path now
has two atomic reads, one to get the table pointer and one to read
the entry out of the table.
I set the max load factor to 75%, kind of arbitrarily. There's a
space-speed tradeoff here, and I'm not sure where we should land.
Because we use open addressing the itab.link field is no longer needed.
I'll remove it in a separate CL.
Fixes#20505
Change-Id: Ifb3d9a337512d6cf968c1fceb1eeaf89559afebf
Reviewed-on: https://go-review.googlesource.com/44472
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
When StructOf is used with an anonymous field that has methods, and
that anonymous field is not the first field, the methods we generate
are incorrect because they do not offset to the field as required.
If we encounter that case, panic rather than doing the wrong thing.
Fixes#20824
Updates #15924
Change-Id: I3b0901ddbc6d58af5f7e84660b5e3085a431035d
Reviewed-on: https://go-review.googlesource.com/47035
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Following the spec clarification in CL 40393, copy that text
to reflect docs to state that the initial capacity of MakeMapWithSize
is a hint/approximate.
Fixes#19903
Change-Id: I6b3315b8183cafaa61fbb2839a4e42b76fd71544
Reviewed-on: https://go-review.googlesource.com/46270
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
According to the language spec, a struct field name should
be an identifier.
identifier = letter { letter | unicode_digit } .
letter = unicode_letter | "_" .
Implements a function 'isValidFieldName(fieldName string) bool'.
To check if the field name is a valid identifier or not.
It will panic if the field name is invalid.
It uses the non-exported function implementation 'isLetter'
from the package 'scanner', used to parse an identifier.
Fixes#20600.
Change-Id: I1db7db1ad88cab5dbea6565be15cc7461cc56c44
Reviewed-on: https://go-review.googlesource.com/45590
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
v is not a pointer receiver, and v.typ isn't used in the lines below.
The assignment is dead. Remove it.
Keep the comment, as it refers to the whole case block and not just the
removed line.
Change-Id: Icb2d20c287d9a41bf620ebe5cdec764cd84178a7
Reviewed-on: https://go-review.googlesource.com/43134
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We do a division by the elem type size to check if the array size would
be too large for the virtual address space. This is a silly check if the
size is 0, but the problem is that it means a division by zero and a
panic.
Since arrays of empty structs are valid in a regular program, make them
also work in reflect.
Use a separate, explicit test with struct{}{} to make sure the test for
a zero-sized type is not confused with the rest.
Fixes#20313.
Change-Id: I47b8b87e6541631280b79227bdea6a0f6035c9e0
Reviewed-on: https://go-review.googlesource.com/43131
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When constructing a new type for an array type in ArrayOf, we don't
reset tflag to 0. All the other methods in the package, such as SliceOf,
do this already. This results in the new array type having weird issues
when being printed, such as having tflagExtraStar set when it shouldn't.
That flag removes the first char to get rid of '*', but when used
incorrectly in this case it eats the '[' character leading to broken
strings like "3]int".
This was fixed in 56752eb2 for issue #16722, but ArrayOf was missed.
Also make the XM test struct have a non-zero size as that leads to a
division by zero panic in ArrayOf.
Fixes#20311.
Change-Id: I18f1027fdbe9f71767201e7424269c3ceeb23eb5
Reviewed-on: https://go-review.googlesource.com/43130
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL adds a simple explanation about what means the ptrdata field of
the reflect.rtype type.
Also document that rtype needs to be kept in sync with the runtime._type
type that rtype mirrors.
Change-Id: Icd9663a2e4bb94d922a2417cfe4537861d2ccc97
Reviewed-on: https://go-review.googlesource.com/40917
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Providing size hint when creating a map allows avoiding re-allocating
underlying data structure if we know how many elements are going to
be inserted. This can be used for example during decoding maps in
gob.
Fixes#19599
Change-Id: I108035fec29391215d2261a73eaed1310b46bab1
Reviewed-on: https://go-review.googlesource.com/38335
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
reflect.callReflect heap-allocates a stack frame and then constructs
pointers to the arguments and result areas of that frame. However, if
there are no results, the results pointer will point past the end of
the frame allocation. If there are also no arguments, the arguments
pointer will also point past the end of the frame allocation. If the
GC observes either these pointers, it may panic.
Fix this by not constructing these pointers if these areas of the
frame are empty.
This adds a test of calling no-argument/no-result methods via reflect,
since nothing in std did this before. However, it's quite difficult to
demonstrate the actual failure because it depends on both exact
allocation patterns and on GC scanning the goroutine's stack while
inside one of the typedmemmovepartial calls.
I also audited other uses of typedmemmovepartial and
memclrNoHeapPointers in reflect, since these are the most susceptible
to this. These appear to be the only two cases that can construct
out-of-bounds arguments to these functions.
Fixes#19724.
Change-Id: I4b83c596b5625dc4ad0567b1e281bad4faef972b
Reviewed-on: https://go-review.googlesource.com/38736
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The chanrecv funcs don't use it at all. The chansend ones do, but the
element type is now part of the hchan struct, which is already a
parameter.
hchan can be nil in chansend when sending to a nil channel, so when
instrumenting we must copy to the stack to be able to read the channel
type.
name old time/op new time/op delta
ChanUncontended 6.42µs ± 1% 6.22µs ± 0% -3.06% (p=0.000 n=19+18)
Initially found by github.com/mvdan/unparam.
Fixes#19591.
Change-Id: I3a5e8a0082e8445cc3f0074695e3593fd9c88412
Reviewed-on: https://go-review.googlesource.com/38351
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The gcdata field only records ptrdata entries, not size entries.
Also fix an obsolete comment: the enforced limit on pointer maps is
now 2048 bytes, not 16 bytes.
I wasn't able to contruct a test case for this. It would require
building a type whose size is greater than 64 bytes but less than 128
bytes, with at least one pointer in first 64 bytes but no pointers
after the first 64 bytes, such that the linker arranges for the one
byte gcbits value to be immediately followed by a non-zero byte.
Change-Id: I9118d3e4ec6f07fd18b72f621c1e5f4fdfe5f80b
Reviewed-on: https://go-review.googlesource.com/37142
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Otherwise, calling PtrTo on the result will fail.
Fixes#19003
Change-Id: I8d7d1981a5d0417d5aee52740469d71e90734963
Reviewed-on: https://go-review.googlesource.com/36731
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Make the documentation more explicit that it is not safe to directly
compare Value. Get straight to the point on how to do it correctly.
Updates #18871
Change-Id: I2aa3253f779636b2f72a1aae8c9bb45d3c32c902
Reviewed-on: https://go-review.googlesource.com/36018
Reviewed-by: Keith Randall <khr@golang.org>
For #18130.
f8b4123613 [dev.typealias] spec: use term 'embedded field' rather than 'anonymous field'
9ecc3ee252 [dev.typealias] cmd/compile: avoid false positive cycles from type aliases
49b7af8a30 [dev.typealias] reflect: add test for type aliases
9bbb07ddec [dev.typealias] cmd/compile, reflect: fix struct field names for embedded byte, rune
43c7094386 [dev.typealias] reflect: fix StructOf use of StructField to match StructField docs
9657e0b077 [dev.typealias] cmd/doc: update for type alias
de2e5459ae [dev.typealias] cmd/compile: declare methods after resolving receiver type
9259f3073a [dev.typealias] test: match gccgo error messages on alias2.go
5d92916770 [dev.typealias] cmd/compile: change Func.Shortname to *Sym
a7c884efc1 [dev.typealias] go/internal/gccgoimporter: support for type aliases
5802cfd900 [dev.typealias] cmd/compile: export/import test cases for type aliases
d7cabd40dd [dev.typealias] go/types: clarified doc string
cc2dcce3d7 [dev.typealias] cmd/compile: a few better comments related to alias types
5c160b28ba [dev.typealias] cmd/compile: improved error message for cyles involving type aliases
b2386dffa1 [dev.typealias] cmd/compile: type-check type alias declarations
ac8421f9a5 [dev.typealias] cmd/compile: various minor cleanups
f011e0c6c3 [dev.typealias] cmd/compile, go/types, go/importer: various alias related fixes
49de5f0351 [dev.typealias] cmd/compile, go/importer: define export format and implement importing of type aliases
5ceec42dc0 [dev.typealias] go/types: export TypeName.IsAlias so clients can use it
aa1f0681bc [dev.typealias] go/types: improved Object printing
c80748e389 [dev.typealias] go/types: remove some more vestiges of prior alias implementation
80d8b69e95 [dev.typealias] go/types: implement type aliases
a917097b5e [dev.typealias] go/build: add go1.9 build tag
3e11940437 [dev.typealias] cmd/compile: recognize type aliases but complain for now (not yet supported)
e0a05c274a [dev.typealias] cmd/gofmt: added test cases for alias type declarations
2e5116bd99 [dev.typealias] go/ast, go/parser, go/printer, go/types: initial type alias support
Change-Id: Ia65f2e011fd7195f18e1dce67d4d49b80a261203
For #18130.
Change-Id: Idd77cb391178c185227cfd779c70fec16351f825
Reviewed-on: https://go-review.googlesource.com/35733
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Will also fix type aliases.
Fixes#17766.
For #18130.
Change-Id: I9e1584d47128782152e06abd0a30ef423d5c30d2
Reviewed-on: https://go-review.googlesource.com/35732
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The runtime internal structField interprets name=="" as meaning anonymous,
but the exported reflect.StructField has always set Name, even for anonymous
fields, and also set Anonymous=true.
The initial implementation of StructOf confused the internal and public
meanings of the StructField, expecting the runtime representation of
anonymous fields instead of the exported reflect API representation.
It also did not document this fact, so that users had no way to know how
to create an anonymous field.
This CL changes StructOf to use the previously documented interpretation
of reflect.StructField instead of an undocumented one.
The implementation of StructOf also, in some cases, allowed creating
structs with unexported fields (if you knew how to ask) but set the
PkgPath incorrectly on those fields. Rather than try to fix that, this CL
changes StructOf to reject attempts to create unexported fields.
(I think that may be the right design choice, not just a temporary limitation.
In any event, it's not the topic for today's work.)
For #17766.
Fixes#18780.
Change-Id: I585a4e324dc5a90551f49d21ae04d2de9ea04b6c
Reviewed-on: https://go-review.googlesource.com/35731
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When traceback sees reflect.makeFuncStub (or reflect.methodValueCall)
on the stack, it expects to be able to get the *reflect.makeFuncImpl
(or *reflect.methodValue) for that call from the first outgoing
argument slot of makeFuncStub/methodValueCall.
However, currently this object isn't necessarily kept live across
makeFuncStub. This means it may get garbage collected while in a
reflect call and reused for something else. If we then try to
traceback, the runtime will see a corrupted makeFuncImpl object and
panic. This was not a problem in previous releases because we always
kept arguments live across the whole function. This became a problem
when we stopped doing this.
Fix this by using reflect.KeepAlive to keep the
makeFuncImpl/methodValue live across all of callReflect/callMethod,
which in turn keeps it live as long as makeFuncStub/methodValueCall
are on the stack.
Fixes#18635.
Change-Id: I91853efcf17912390fddedfb0230648391c33936
Reviewed-on: https://go-review.googlesource.com/35151
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The runtime no longer hard-codes the offset of
reflect.methodValue.stack, so remove these obsolete comments. Also,
reflect.methodValue and runtime.reflectMethodValue must also agree
with reflect.makeFuncImpl, so update the comments on all three to
mention this.
This was pointed out by Minux on CL 31138.
Change-Id: Ic5ed1beffb65db76aca2977958da35de902e8e58
Reviewed-on: https://go-review.googlesource.com/34590
Reviewed-by: Keith Randall <khr@golang.org>
This CL adds a simple example for StructOf.
The example shows how StructOf can be used in a JSON roundtrip.
Change-Id: I9ff1ea9cb8c0cf297c5fae74e68b89931076adfd
Reviewed-on: https://go-review.googlesource.com/33953
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Sigh, forgot to run `git mail`.
Change-Id: Idc49be2bb20d6f0e392cb472a63267ffee2ca22c
Reviewed-on: https://go-review.googlesource.com/33476
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Introduce R_WEAKADDROFF, a "weak" variation of the R_ADDROFF relocation
that will only reference the type described if it is in some other way
reachable.
Use this for the ptrToThis field in reflect type information where it
is safe to do so (that is, types that don't need to be included for
interface satisfaction, and types that won't cause the compiler to
recursively generate an endless series of ptr-to-ptr-to-ptr-to...
types).
Also fix a small bug in reflect, where StructOf was not clearing the
ptrToThis field of new types.
Fixes#17931
Change-Id: I4d3b53cb9c916c97b3b16e367794eee142247281
Reviewed-on: https://go-review.googlesource.com/33427
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It's possible for the pkgPath of a field to be different than that of
the struct type as a whole. In that case, store the field's pkgPath in
the name field. Use the field's pkgPath when setting PkgPath and when
checking for type identity.
Fixes#17952.
Change-Id: Iebaf92f0054b11427c8f6e4158c3bebcfff06f45
Reviewed-on: https://go-review.googlesource.com/33333
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
An unexported field of a struct is not visible outside of the package
that defines it, so the package path is implicitly part of the
definition of any struct with an unexported field.
Change-Id: I17c6aac822bd0c24188ab8ba1cc406d6b5d82771
Reviewed-on: https://go-review.googlesource.com/32820
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Since barrier-less memclr is only safe in very narrow circumstances,
this commit renames memclr to avoid accidentally calling memclr on
typed memory. This can cause subtle, non-deterministic bugs, so it's
worth some effort to prevent. In the near term, this will also prevent
bugs creeping in from any concurrent CLs that add calls to memclr; if
this happens, whichever patch hits master second will fail to compile.
This also adds the other new memclr variants to the compiler's
builtin.go to minimize the churn on that binary blob. We'll use these
in future commits.
Updates #17503.
Change-Id: I00eead049f5bd35ca107ea525966831f3d1ed9ca
Reviewed-on: https://go-review.googlesource.com/31369
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The hybrid barrier requires distinguishing typed and untyped memory
even when zeroing because the *current* contents of the memory matters
even when overwriting.
This commit introduces runtime.typedmemclr and runtime.memclrHasPointers
as a typed memory clearing functions parallel to runtime.typedmemmove.
Currently these simply call memclr, but with the hybrid barrier we'll
need to shade any pointers we're overwriting. These will provide us
with the necessary hooks to do so.
Updates #17503.
Change-Id: I74478619f8907825898092aaa204d6e4690f27e6
Reviewed-on: https://go-review.googlesource.com/31366
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently reflectcall has a subtle dance with write barriers where the
assembly code copies the result values from the stack to the in-heap
argument frame without write barriers and then calls into the runtime
after the fact to invoke the necessary write barriers.
For the hybrid barrier (and for ROC), we need to switch to a
*pre*-write write barrier, which is very difficult to do with the
current setup. We could tie ourselves in knots of subtle reasoning
about why it's okay in this particular case to have a post-write write
barrier, but this commit instead takes a different approach. Rather
than making things more complex, this simplifies reflection calls so
that the argument copy is done in Go using normal bulk write barriers.
The one difficulty with this approach is that calling into Go requires
putting arguments on the stack, but the call* functions "donate" their
entire stack frame to the called function. We can get away with this
now because the copy avoids using the stack and has copied the results
out before we clobber the stack frame to call into the write barrier.
The solution in this CL is to call another function, passing arguments
in registers instead of on the stack, and let that other function
reserve more stack space and setup the arguments for the runtime.
This approach seemed to work out the best. I also tried making the
call* functions reserve 32 extra bytes of frame for the write barrier
arguments and adjust SP up by 32 bytes around the call. However, even
with the necessary changes to the assembler to correct the spdelta
table, the runtime was still having trouble with the frame layout (and
the changes to the assembler caused many other things that do strange
things with the SP to fail to assemble). The approach I took doesn't
require any funny business with the SP.
Updates #17503.
Change-Id: Ie2bb0084b24d6cff38b5afb218b9e0534ad2119e
Reviewed-on: https://go-review.googlesource.com/31655
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This was supposed to be in CL 31354
but was dropped due to a Git usage error.
For #16573.
Change-Id: I3d99087c8efc8cbc016c55e8365d0005f79d1b2f
Reviewed-on: https://go-review.googlesource.com/31461
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
