This will produce better errors when earlier versions of
Go compile code using //go:embed. (The import will cause
a compilation error but then the go command will add to
the output that the Go toolchain in use looks too old
and maybe that's the problem.)
This CL also adds a test for disallowing embed of a var inside a func.
It's a bit too difficult to rebase down into that CL.
The build system configuration check is delayed in order to
make it possible to use errorcheck for these tests.
Change-Id: I12ece4ff2d8d53380b63f54866e8f3497657d54c
Reviewed-on: https://go-review.googlesource.com/c/go/+/282718
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>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If there are already errors emitted, don't run the Asmb2 pass
and just exit. At the point of Asmb2 relocations are already
resolved and errors should have been reported, if any. Asmb2 is
unlikely to emit additional useful users errors. Instead, the
invalid input may cause inconsistencies and crash the linker, or
it may emit some internal errors which are more confusing than
helpful. Exit on error before Asmb2.
Fixes#43748.
Change-Id: Icf6e27f2eef5b6259e921ec0e64bebad5dd805f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/284576
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
As described in the FS documentation.
This prevents http.FS and other clients from panicking when the
go:embed directive is missing.
For #43682
Related #43698
Change-Id: Iecf26d229a099e55d24670c3119cd6c6d17ecc6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/283852
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This switches openbsd/amd64 to thread creation via pthreads, rather than doing
direct system calls.
Update #36435
Change-Id: I1105d5c392aa3e4c445d99c8cb80b927712e3529
Reviewed-on: https://go-review.googlesource.com/c/go/+/250180
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Rather than inline lists of GOOS values, factor out the code that checks
if a runtime makes system calls via libcall.
Change-Id: Ib19d7e63a2b4b8314f1841c0ff26e1b3a16b4b22
Reviewed-on: https://go-review.googlesource.com/c/go/+/259239
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Rather than repeat long lists of GOOS values, factor out the code that checks
if a runtime starts on a system allocated stack. Note that this adds aix to
one case, which appears to have been previously missed.
Change-Id: I5cecb0bb47dd79cde8d723e5a42ba541e43cbfff
Reviewed-on: https://go-review.googlesource.com/c/go/+/250179
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
These symbols are implementation details and don't correspond to Go
source symbols, so directly create them as linker symbols and get rid
of their pseudo packages.
Passes toolstash -cmp w/ -gcflags=all=-abiwrap.
Change-Id: I2e97374c21f3e909f6d350f15e7a5ed3574cadf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/284372
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>
Currently there's a lot of logic within package types for creating
Linksyms. This CL pulls it out into base, where it can be more easily
reused by other compiler code that shouldn't need to depend on package
types.
Package base probably isn't the best place for this, but it's
convenient because it's a package that types already depends on. It's
also where the Ctxt object lives, which these functions depend upon.
Passes toolstash -cmp w/ -gcflags=all=-abiwrap.
Change-Id: I50d8b7e4596955205036969eab24d7dab053b363
Reviewed-on: https://go-review.googlesource.com/c/go/+/284231
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Now that TailCallStmt carries an *ir.Name instead of a *types.Sym,
callTargetLSym can be similarly updated to take the target function as
an *ir.Name.
This inches us closer towards being able to move Linksym and other
properties from *types.Sym to *ir.Name, where they belong.
Passes toolstash -cmp w/ -gcflags=all=-abiwrap.
Change-Id: I091da290751970eba8ed0438f66d6cca88b665a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/284228
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Calls to lock may need to use global members of mOS that also need to be
cleaned up before the thread exits. Before this commit, these resources
would leak. Moving them to be cleaned up in unminit, however, would race
with gstack on unix. So this creates a new helper, mdestroy, to release
resources that must be destroyed only after locks are no longer
required. We also move highResTimer lifetime to the same semantics,
since it doesn't help to constantly acquire and release the timer object
during dropm.
Updates #43720.
Change-Id: Ib3f598f3fda1b2bbcb608099616fa4f85bc1c289
Reviewed-on: https://go-review.googlesource.com/c/go/+/284137
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
By calling NewConsoleFile on syscall.Stdin, we wind up closing it when
the function returns, which causes errors when all the tests are run in
a loop. To fix this, we instead create a duplicate handle of stdin.
Fixes#43720.
Change-Id: Ie6426e6306c7e1e39601794f4ff48bbf2fe67502
Reviewed-on: https://go-review.googlesource.com/c/go/+/284140
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Calculating and emitting stack objects are essentially part of
liveness analysis, so move the code from ssagen to liveness. Allows
unexporting liveness.ShouldTrack.
Passes toolstash -cmp.
Change-Id: I88b5b2e75b8dfb46b8b03a2fa09a9236865cbf3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/284413
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Currently, typecheck leaves arguments to OPANIC as their original
type. This CL changes it to insert implicit OCONVIFACE operations to
convert arguments to `interface{}` like how any other function call
would be handled.
No immediate benefits, other than getting to remove a tiny bit of
special-case logic in order.go's handling of OPANICs. Instead, the
generic code path for handling OCONVIFACE is used, if necessary.
Longer term, this should be marginally helpful for #43753, as it
reduces the number of cases where we need values to be addressable for
runtime calls.
However, this does require adding some hacks to appease existing
tests:
1. We need yet another kludge in inline budgeting, to ensure that
reflect.flag.mustBe stays inlinable for cmd/compile/internal/test's
TestIntendedInlining.
2. Since the OCONVIFACE expressions are now being introduced during
typecheck, they're now visible to escape analysis. So expressions like
"panic(1)" are now seen as "panic(interface{}(1))", and escape
analysis warns that the "interface{}(1)" escapes to the heap. These
have always escaped to heap, just now we're accurately reporting about
it.
(Also, unfortunately fmt.go hides implicit conversions by default in
diagnostics messages, so instead of reporting "interface{}(1) escapes
to heap", it actually reports "1 escapes to heap", which is
confusing. However, this confusing messaging also isn't new.)
Change-Id: Icedf60e1d2e464e219441b8d1233a313770272af
Reviewed-on: https://go-review.googlesource.com/c/go/+/284412
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>
Those functions only use (*ir.Name).Linksym(), so just change them to
get an *obj.LSym directly. This helps get rid of un-necessary
validations that their callers have already done.
Passes toolstash -cmp.
For #43737.
Change-Id: Ifd6c2525e472f8e790940bc167665f9d74dd1bc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/284121
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>
ir.Pkgs.Itablink isn't used anymore. (I don't recall what it was ever
used for.)
ir.Pkgs.Race and ir.Pkgs.Msan are only needed in exactly only place,
so just create them on demand there, the same way that we create
"main" on demand.
Change-Id: I3474bb949f71cd40c7a462b9f4a369adeacde0d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/284230
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 CL updates walkConvInterface to use LinksymOffsetExpr for
referencing runtime.staticuint64s and runtime.zerobase.
Passes toolstash -cmp (surprisingly).
Change-Id: Iad7e30371f89c8a5e176b5ddbc53faf57012ba0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/284229
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 CL splits out ORETJMP as a new TailCallStmt node, separate from
the other BranchStmt nodes. In doing so, this allows us to change it
from identifying a function by *types.Sym to identifying one by
directly pointing to the *ir.Func.
While here, also rename the operation to OTAILCALL.
Passes toolstash -cmp.
Change-Id: I273e6ea5d92bf3005ae02fb59b3240a190a6cf1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/284227
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 284223 tightened down the allowed expressions in mayCall, but
evidently a little too tight. The linux-amd64-noopt builder does in
fact see expressions with non-empty Init lists in arguments list.
Since I believe these can only appear on the RHS of LogicalExpr
expressions, this CL relaxes that one case.
Change-Id: I1e6bbd0449778c40ed2610b3e1ef6a825a84ada7
Reviewed-on: https://go-review.googlesource.com/c/go/+/284226
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>
Because NameOffsetExpr is always used with global variables, and SSA
backend only needs (*Name).Linksym() to generate value for them.
Passes toolstash -cmp.
Updates #43737
Change-Id: I17209e21383edb766070c0accd1fa4660659caef
Reviewed-on: https://go-review.googlesource.com/c/go/+/284119
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>
It is always used with global variables, so we can skip analyze it, the
same as what we are doing for ONAME/PEXTERN nodes.
While at it, add a Fatalf check to ensure NewNameOffsetExpr is only
called for global variables.
For #43737
Change-Id: Iac444ed8d583baba5042bea096531301843b1e8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/284118
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 simplifies the previous one a little bit further, by combining
reordering stack-temporary initialization and getting rid of an
unneeded temporary variable. (Does not pass toolstash -cmp.)
Change-Id: I17799dfe368484f33a8ddd0ab4f68647d6262147
Reviewed-on: https://go-review.googlesource.com/c/go/+/284225
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 CL adds a few new helper functions for constructing and
initializing temporary variables during walk.
Passes toolstash -cmp.
Change-Id: I54965d992cd8dfef7cb7dc92a17c88372e52a0d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/284224
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>
After CL 284220, we now only need to detect expressions that contain
function calls in the arguments list of further function calls. So we
can simplify Node.HasCall/fncall/etc a lot.
Instead of incrementally tracking whether an expression contains
function calls all throughout walk, simply check once at the point of
using an expression as a function call argument. Since any expression
checked here will itself become a function call argument, it won't be
checked again because we'll short circuit at the enclosing function
call.
Also, restructure the recursive walk code to use mayCall, and trim
down the list of acceptable expressions. It should be okay to be
stricter, since we'll now only see function call arguments and after
they've already been walked.
It's possible I was overly aggressive removing Ops here. But if so,
we'll get an ICE, and it'll be easy to re-add them. I think this is
better than the alternative of accidentally allowing expressions
through that risk silently clobbering the stack.
Passes toolstash -cmp.
Change-Id: I585ef35dcccd9f4018e4bf2c3f9ccb1514a826f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/284223
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>
Currently, to ensure OAS2FUNC results are assigned in the correct
order, they're always assigned to temporary variables. However, these
temporary variables are typed based on the destination type, which may
require an interface conversion. This means walk may have to then
introduce a second set of temporaries to ensure result parameters are
all copied out of the results area, before it emits calls to runtime
conversion functions.
That's just silly. Instead, this CL changes order to allocate the
result temporaries with the same type as the function returns in the
first place, and then assign them one at a time to their destinations,
with conversions as needed.
While here, also fix an order-of-evaluation issue with has-ok
assignments that I almost added to multi-value function call
assignments, and add tests for each.
Change-Id: I9f4e962425fe3c5e3305adbbfeae2c7f253ec365
Reviewed-on: https://go-review.googlesource.com/c/go/+/284220
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>
Went in a semi-automated way through the clearest renames of functions,
and updated comments and error messages where it made sense.
Change-Id: Ied8e152b562b705da7f52f715991a77dab60da35
Reviewed-on: https://go-review.googlesource.com/c/go/+/284216
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
After CL 283233, SSA can now handle new(typ) without the frontend to
generate the type address, so we can remove ONEWOBJ in favor of ONEW
only.
This is also not save for toolstash, the same reason with CL 284115.
Change-Id: Ie03ea36b3b6f95fc7ce080376c6f7afc402d51a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/284117
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>
CL 283233 added reflectType method to ssagen.state, which we can use to
setup type address in the SSA backend in favor of the frontend. However,
this will change the order of symbols generation, so not safe for toolstash.
Change-Id: Ib6932ec42a9d28c3fd7a1c055596e75494c29843
Reviewed-on: https://go-review.googlesource.com/c/go/+/284115
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>
These functions rely on DuplicateHandle succeeding, but they don't check
the return value, which might be masking subtle bugs that cause other
problems down the line.
Updates #43720.
Change-Id: I77f0e6645affa534777ffc173144a52e4afa5f81
Reviewed-on: https://go-review.googlesource.com/c/go/+/284135
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Tests should avoid writing to GOROOT when possible. Such writes
would fail if GOROOT is non-writeable, and it can interfere with
other tests that don't expect GOROOT to change during test execution.
Updates #28387.
Change-Id: I7d72614f218df3375540f5c2f9c9f8c11034f602
Reviewed-on: https://go-review.googlesource.com/c/go/+/284293
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The current implementation requires saying "string" or "[]byte"
and disallows aliases, defined types, and even "[]uint8".
This was not 100% intended and mostly just fell out of when
the checks were being done in the implementation (too early,
before typechecking).
After discussion on #43217 (forked into #43602),
the consensus was to allow all string and byte slice types,
same as we do for string conversions in the language itself.
This CL does that.
It's more code than you'd expect because the decision has
to be delayed until after typechecking.
But it also more closely aligns with the version that's
already on dev.regabi.
Fixes#43602.
Change-Id: Iba919cfadfbd5d7116f2bf47e2512fb1d5c36731
Reviewed-on: https://go-review.googlesource.com/c/go/+/282715
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Allowing embedding into []byte inside a func creates an
unfortunate problem: either all calls start with the same
underlying data and can see each other's changes to the
underlying data (surprising and racy!) or all calls start
by making their own copy of the underlying data
(surprising and expensive!).
After discussion on #43216, the consensus was to remove
support for all vars embedded inside functions.
Fixes#43216.
Change-Id: I01e62b5f0dcd9e8566c6d2286218e97803f54704
Reviewed-on: https://go-review.googlesource.com/c/go/+/282714
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The docs were never updated for the change to the placement
of the DO NOT EDIT line.
Also, the description of the DO NOT EDIT line interrupted the
description of the //go:generate line, which made for some
confusing references in the text that followed. Move it lower.
Fixes#41196.
Change-Id: I6af2a199fa98d45f5ccac7cdf7e9e54257699e61
Reviewed-on: https://go-review.googlesource.com/c/go/+/283633
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This fixes an inconsistency where the type for nil in code such as
var x unsafe.Pointer = nil
and in conversions of the form
T(nil)
(where T is a pointer, function, slice, map, channel, interface, or
unsafe.Pointer) was reported as (converted to) the respective type.
For all other operations that accept a nil value, we don't do this
conversion for nil.
(We never change the type of the untyped nil value, in contrast to
other untyped values where we give the values context-specific types.)
It may still be useful to change this behavior and - consistently -
report a converted nil type like we do for any other type, but for
now this CL simply fixes the existing inconsistency.
Added tests and fixed existing test harness.
Updates #13061.
Change-Id: Ia82832845c096e3cbc4a239ba3d6c8b9a9d274c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/284052
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
There's on need to expose this to the frozen syscall package, and it
also doesn't need to be unsafe. So we move it into internal/syscall and
have the generator make a safer function signature.
Fixes#43704.
Change-Id: Iccae69dc273a0aa97ee6846eb537f1dc1412f2de
Reviewed-on: https://go-review.googlesource.com/c/go/+/283992
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Previously, testKillProcess needlessly invoked processKiller in a
separate goroutine and failed to wait for that goroutine to complete,
causing the calls to t.Fatalf in that goroutine to potentially occur
after the test function had already returned.
Fixes#43722
Change-Id: I5d03cb24af51bb73f0ff96419dac57ec39776967
Reviewed-on: https://go-review.googlesource.com/c/go/+/284153
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
WriteType isn't safe for direct concurrent use, and users should
instead use TypeLinksym or another higher-level API provided by
reflectdata. After the previous CL, there are no remaining uses of
WriteType elsewhere in the compiler, so unexport it to keep it that
way.
For #43701.
[git-generate]
cd src/cmd/compile/internal/reflectdata
rf '
mv WriteType writeType
'
Change-Id: I294a78be570a47feb38a1ad4eaae7723653d5991
Reviewed-on: https://go-review.googlesource.com/c/go/+/284077
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>
The code for allocating linksyms and recording that we need runtime
type descriptors is now concurrent-safe, so move it to where those
symbols are actually needed to reduce complexity and risk of failing
to generate all needed symbols in advance.
For #43701.
Change-Id: I759d2508213ac9a4e0b504b51a75fa10dfa37a8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/284076
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>
We decide during escape analysis whether to pass closure variables by
value or reference. One of the factors that's considered is whether a
variable has had its address taken.
However, this analysis is based only on the user-written source code,
whereas order+walk may introduce rewrites that take the address of a
variable (e.g., passing a uint16 key by reference to the size-generic
map runtime builtins).
Typically this would be harmless, albeit suboptimal. But in #43701 it
manifested as needing a stack object for a function where we didn't
realize we needed one up front when we generate symbols.
Probably we should just generate symbols on demand, now that those
routines are all concurrent-safe, but this is a first fix.
Thanks to Alberto Donizetti for reporting the issue, and Cuong Manh Le
for initial investigation.
Fixes#43701.
Change-Id: I16d87e9150723dcb16de7b43f2a8f3cd807a9437
Reviewed-on: https://go-review.googlesource.com/c/go/+/284075
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>
The feature being tested is insensitive to the OS anyway.
Change-Id: Ieac9bfaafc6a54c00017afcc0b87bd8bbe80af7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/284032
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
fmt.go:dumpNodeHeader uses reflection to call all "func() bool"-typed
methods on Nodes during printing, but the OnStack method that I added
in CL 283233 isn't meant to be called on non-variables.
dumpNodeHeader does already guard against panics, as happen in some
other accessors, but not against Fatalf, as I was using in OnStack. So
simply change OnStack to use panic too.
Thanks to drchase@ for the report.
Change-Id: I0cfac84a96292193401a32fc5e7fd3c48773e008
Reviewed-on: https://go-review.googlesource.com/c/go/+/284074
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
On Apple Silicon Mac, the C compiler has an annoying default
target selection, depending on the ancestor processes'
architecture. In particular, if the shell or IDE is x86, when
running "go build" even with a native ARM64 Go toolchain, the C
compiler defaults to x86, causing build failures. We pass "-arch"
flag explicitly to avoid this situation.
Fixes#43692.
Fixes#43476.
Updates golang/vscode-go#1087.
Change-Id: I80b6a116a114e11e273c6886e377a1cc969fa3f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/283812
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The cgo header has an unnecessary space in the exported function
definition on non-windows goos.
This was introduced in go1.16 so it would be good to fix it before
release.
Example:
// Current behavior, notice there is an unecessary space
// between extern and void
extern void Foo();
// With this CL
extern void Foo();
Change-Id: Ic2c21f8d806fe35a7be7183dbfe35ac605b6e4f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/283892
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Katie Hockman <katie@golang.org>
For #29062Fixes#43263
Change-Id: I160197c94cc4f936967cc22c82cec01663a14fe6
Reviewed-on: https://go-review.googlesource.com/c/go/+/283873
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This partially addresses the issue below: In many (all) cases we want to
handle invalid ... use in the parser as a syntax error; but this ensures
that we get a decent error if we get here anyway.
Updates #43680.
Change-Id: I93af43a5f5741d8bc76e7a13c0db75e6edf43111
Reviewed-on: https://go-review.googlesource.com/c/go/+/283475
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
