When a function triggers a signal (like a segfault which translates to
a nil pointer exception) during execution, a sigpanic handler is just
below it on the stack. The function itself did not stop at a
safepoint, so we have to figure out what safepoint we should use to
scan its stack frame.
Previously we used the site of the most recent defer to get the live
variables at the signal site. That answer is not quite correct, as
explained in #27518. Instead, use the site of a deferreturn call.
It has all the right variables marked as live (no args, all the return
values, except those that escape to the heap, in which case the
corresponding PAUTOHEAP variables will be live instead).
This CL requires stack objects, so that all the local variables
and args referenced by the deferred closures keep the right variables alive.
Fixes#27518
Change-Id: Id45d8a8666759986c203181090b962e2981e48ca
Reviewed-on: https://go-review.googlesource.com/c/134637
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The previous CL introduced stack objects. This CL removes the old
ambiguously live liveness analysis. After this CL we're relying
on stack objects exclusively.
Update a bunch of liveness tests to reflect the new world.
Fixes#22350
Change-Id: I739b26e015882231011ce6bc1a7f426049e59f31
Reviewed-on: https://go-review.googlesource.com/c/134156
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Rework how the compiler+runtime handles stack-allocated variables
whose address is taken.
Direct references to such variables work as before. References through
pointers, however, use a new mechanism. The new mechanism is more
precise than the old "ambiguously live" mechanism. It computes liveness
at runtime based on the actual references among objects on the stack.
Each function records all of its address-taken objects in a FUNCDATA.
These are called "stack objects". The runtime then uses that
information while scanning a stack to find all of the stack objects on
a stack. It then does a mark phase on the stack objects, using all the
pointers found on the stack (and ancillary structures, like defer
records) as the root set. Only stack objects which are found to be
live during this mark phase will be scanned and thus retain any heap
objects they point to.
A subsequent CL will remove all the "ambiguously live" logic from
the compiler, so that the stack object tracing will be required.
For this CL, the stack tracing is all redundant with the current
ambiguously live logic.
Update #22350
Change-Id: Ide19f1f71a5b6ec8c4d54f8f66f0e9a98344772f
Reviewed-on: https://go-review.googlesource.com/c/134155
Reviewed-by: Austin Clements <austin@google.com>
This CL removes all unused code from bimport.go and bexport.go.
In the interest of keeping this CL strictly delete-only and easier to
review, the task of consolidating the vestigial code elsewhere is left
to future CLs.
Change-Id: Ib757cc27e3fe814cbf534776d026e4d4cddfc6db
Reviewed-on: https://go-review.googlesource.com/c/139338
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The new indexed package export format appears stable, and no reports
of needing to revert back to binary package export.
This CL disables the binary package export format by mechanically
replacing 'flagiexport' with 'true', and then superficial code
cleanups to keep the resulting code idiomatic. The resulting dead code
is removed in a followup CL.
Change-Id: Ic30d85f78778a31d279a56b9ab14e80836d50135
Reviewed-on: https://go-review.googlesource.com/c/139337
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Don't worry, this patch just remove trailing whitespace from
assembly files, and does not touch any logical changes.
Change-Id: Ia724ac0b1abf8bc1e41454bdc79289ef317c165d
Reviewed-on: https://go-review.googlesource.com/c/113595
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In some optimization rules the type of generated OffPtr was
incorrectly set to the type of the pointee, instead of the
pointer. When the OffPtr value is spilled, this may generate
a spill of the wrong type, e.g. a floating point spill of an
integer (pointer) value. On Wasm, this leads to invalid
bytecode.
Fixes#27961.
Change-Id: I5d464847eb900ed90794105c0013a1a7330756cc
Reviewed-on: https://go-review.googlesource.com/c/139257
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Richard Musiol <neelance@gmail.com>
p = path.Dir(p) converges to either "." or "/". The current
implementation of modload.QueryPackage() has a loop that
terminates only on ".", not "/". This leads to the go command
hanging in an infinite loop if the user manages to supply
a file path starting with "/" as package path.
An example of the issue is if the user (incorrectly) attempts
to use an absolute directory path in an import statement within
a module (import "/home/bob/myproj") and then runs go list.
Fixes#27558
Change-Id: Iaa6a4f7b05eba30609373636e50224ae2e7d6158
GitHub-Last-Rev: 3a70d3a427
GitHub-Pull-Request: golang/go#27976
Reviewed-on: https://go-review.googlesource.com/c/139098
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This commit adds AIX operating system to cmd/dist package for ppc64
architecture.
The stack guard is increased because of syscalls made inside the runtime
which need a larger stack.
Disable cmd/vet/all tests until aix/ppc64 is fully available.
Change-Id: I7e3caf86724249ae564a152d90c1cbd4de288814
Reviewed-on: https://go-review.googlesource.com/c/138715
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In the arm assembler, "AMOVW" never falls into optab
case 13, so the check "if p.As == AMOVW" is useless.
Change-Id: Iec241d5b4cffb358a1477f470619dc9a6287884a
Reviewed-on: https://go-review.googlesource.com/c/138575
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
dump/fdump is a reflection-based data structure dumper slightly
customized for the compiler's Node data structure. It dumps the
transitivle closure of Node (and other) data structures using a
recursive descent depth first traversal and permits filtering
options (recursion depth limitation, filtering of struct fields).
I have been using it to diagnose compiler bugs and found it more
useful than the existing node printing code in some cases because
field filtering reduces the output to the interesting parts.
No impact on rest of compiler if functions are not called (which
they only should during a debugging session).
Change-Id: I79d7227f10dd78dbd4bbcdf204db236102fc97a7
Reviewed-on: https://go-review.googlesource.com/136397
Reviewed-by: Alan Donovan <adonovan@google.com>
During a call to a reflect-generated function or method (via
makeFuncStub or methodValueCall), when should we scan the return
values?
When we're starting a reflect call, the space on the stack for the
return values is not initialized yet, as it contains whatever junk was
on the stack of the caller at the time. The return space must not be
scanned during a GC.
When we're finishing a reflect call, the return values are
initialized, and must be scanned during a GC to make sure that any
pointers in the return values are found and their referents retained.
When the GC stack walk comes across a reflect call in progress on the
stack, it needs to know whether to scan the results or not. It doesn't
know the progress of the reflect call, so it can't decide by
itself. The reflect package needs to tell it.
This CL adds another slot in the frame of makeFuncStub and
methodValueCall so we can put a boolean in there which tells the
runtime whether to scan the results or not.
This CL also adds the args length to reflectMethodValue so the
runtime can restrict its scanning to only the args section (not the
results) if the reflect package says the results aren't ready yet.
Do a delicate dance in the reflect package to set the "results are
valid" bit. We need to make sure we set the bit only after we've
copied the results back to the stack. But we must set the bit before
we drop reflect's copy of the results. Otherwise, we might have a
state where (temporarily) no one has a live copy of the results.
That's the state we were observing in issue #27695 before this CL.
The bitmap used by the runtime currently contains only the args.
(Actually, it contains all the bits, but the size is set so we use
only the args portion.) This is safe for early in a reflect call, but
unsafe late in a reflect call. The test issue27695.go demonstrates
this unsafety. We change the bitmap to always include both args
and results, and decide at runtime which portion to use.
issue27695.go only has a test for method calls. Function calls were ok
because there wasn't a safepoint between when reflect dropped its copy
of the return values and when the caller is resumed. This may change
when we introduce safepoints everywhere.
This truncate-to-only-the-args was part of CL 9888 (in 2015). That
part of the CL fixed the problem demonstrated in issue27695b.go but
introduced the problem demonstrated in issue27695.go.
TODO, in another CL: simplify FuncLayout and its test. stack return
value is now identical to frametype.ptrdata + frametype.gcdata.
Fixes#27695
Change-Id: I2d49b34e34a82c6328b34f02610587a291b25c5f
Reviewed-on: https://go-review.googlesource.com/137440
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
A simple grep over the codebase for "the the" which is often
missed by humans.
Change-Id: Ie4b4f07abfc24c73dcd51c8ef1edf4f73514a21c
Reviewed-on: https://go-review.googlesource.com/138335
Reviewed-by: Dave Cheney <dave@cheney.net>
The DIEs for global variables were all assigned to the first emitted
compile unit in debug_info, regardless of what it was. Move them
instead to their respective compile units.
Change-Id: If794fa0ba4702f5b959c6e8c16119b16e7ecf6d8
Reviewed-on: https://go-review.googlesource.com/137235
Reviewed-by: Than McIntosh <thanm@google.com>
Show a more specifc error message in the form of "%d variables but %v
returns %d values" if an assignment mismatch occurs with a function
or method call on the right.
Fixes#27595
Change-Id: Ibc97d070662b08f150ac22d686059cf224e012ab
Reviewed-on: https://go-review.googlesource.com/135575
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
I omitted vendor directories and anything necessary for bootstrapping.
(Tested by bootstrapping with Go 1.4)
Updates #27864
Change-Id: I7d9b68d0372d3a34dee22966cca323513ece7e8a
Reviewed-on: https://go-review.googlesource.com/137856
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change updates the expected output of the gdb debugging session
in the TestNexting internal/ssa test, aligning it with the changes
introduced in CL 134555.
Fixes#27863
Change-Id: I29e747930c7668b429e8936ad230c4d6aa24fdac
Reviewed-on: https://go-review.googlesource.com/137455
Reviewed-by: Than McIntosh <thanm@google.com>
Also in TestRelativeGOBINFail change to the test directory, to avoid
picking up whatever files are in the current directory.
Change-Id: Icac576dafa016555a9f27d026d0e965dc5cdfea0
Reviewed-on: https://go-review.googlesource.com/137337
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Document what the fields of regalloc mean.
Hopefully will help people understand how the register allocator works.
Change-Id: Ic322ed2019cc839b812740afe8cd2cf0b61da046
Reviewed-on: https://go-review.googlesource.com/137016
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
&^ and << have equal precedence. Add some parentheses to make sure
we shift before we andnot.
Fixes#27829
Change-Id: Iba8576201f0f7c52bf9795aaa75d15d8f9a76811
Reviewed-on: https://go-review.googlesource.com/136899
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Node.copy used to make a shallow copy of a node. Often, this is not
correct: If a node n's Orig field pointed to itself, the copy's Orig
field has to be adjusted to point to the copy. Otherwise, if n is
modified later, the copy's Orig appears modified as well (because it
points to n).
This was fixed for one specific case with
https://go-review.googlesource.com/c/go/+/136395 (issue #26855).
This change instead addresses copy in general:
In two cases we don't want the Orig adjustment as it causes escape
analysis output to fail (not match the existing error messages).
rawcopy is used in those cases.
In several cases Orig is set to the copy immediately after making
a copy; a new function sepcopy is used there.
Updates #26855.
Fixes#27765.
Change-Id: Idaadeb5c4b9a027daabd46a2361348f7a93f2b00
Reviewed-on: https://go-review.googlesource.com/136540
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The ADDconstmodify has similar logic with other constmodify like
instructions. This CL optimize them to share code via fallthrough.
And the size of pkg/linux_386/cmd/compile/internal/x86.a decreases
about 0.3KB.
Change-Id: Ibdf06228afde875e8fe8e30851b50ca2be513dd9
Reviewed-on: https://go-review.googlesource.com/136398
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Found while tracking down #26855.
Change-Id: Ice137fe390820ba351e1c7439b6a9a1b3bdc966b
Reviewed-on: https://go-review.googlesource.com/136396
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
See the change and comment in typecheck.go for a detailed explanation.
Fixes#26855.
Change-Id: I7867f948490fc0873b1bd849048cda6acbc36e76
Reviewed-on: https://go-review.googlesource.com/136395
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Use String and GoString methods instead of the xconf names
for the numeric conversion routines.
Also, fixed a couple of comments in fmt.go.
Change-Id: I1b8acdd95dbff3fc30273070fbb1ac4860031a3c
Reviewed-on: https://go-review.googlesource.com/136197
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The fconv flag arguments were 0, FmtSharp, and FmtSharp|FmtSign.
The 0 value was used for binary representation only, which was
readily available via Mpflt.String. Otherwise, FmtSharp was always
passed. FmtSign was used to print the '+' sign in case of a positive
number and only needed for complex number formatting. Instead
implemented cconv and handled it there.
Change-Id: I1f77282f995be9cfda05efb71a0e027836a9da26
Reviewed-on: https://go-review.googlesource.com/136195
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Some variants of Mercurial respond differently to “permission denied” errors
than to “file not found”, and we set HOME to point to an absolute path that may
produce the former instead of the latter.
To discourage Mercurial from trying HOME, give it an explicit (empty)
configuration in the working directory instead.
Change-Id: I82ae99a6892bba7fc3d41b77209ca181d24315e2
Reviewed-on: https://go-review.googlesource.com/136135
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Clean mod cache should print remove commands and not run them when with set "-n" option.
Fixes#27458.
Change-Id: I97242cb40c062b347784cdb61653c84a3a7eab44
GitHub-Last-Rev: 5a6f10cad8
GitHub-Pull-Request: golang/go#27710
Reviewed-on: https://go-review.googlesource.com/135695
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Since there are no nested loops and/or switches,
loop label can be removed and "bare continue" can be used.
Change-Id: Id642a0859299e4470af544d59884fec51dbb31ee
Reviewed-on: https://go-review.googlesource.com/135837
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Teach samesafeexpr to handle arithmetic unary and binary ops.
It makes map lookup optimization possible in
m[k+1] = append(m[k+1], ...)
m[-k] = append(m[-k], ...)
... etc
Does not cover "+" for strings (concatenation).
Change-Id: Ibbb16ac3faf176958da344be1471b06d7cf33a6c
Reviewed-on: https://go-review.googlesource.com/135795
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
AMD64's ADDQconstmodify/ADDLconstmodify have similar logic with
other constmodify like operators, but seperated case statements.
This CL simplify them with a fallthrough.
Change-Id: Ia73ffeaddc5080182f68c06c9d9b48fe32a14e38
Reviewed-on: https://go-review.googlesource.com/135855
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Updates #26148
Change-Id: I8f68b2c926c7b11dc86c9664ed7ff2d2f78b64b4
Reviewed-on: https://go-review.googlesource.com/128715
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Filippo Valsorda
Lynn Boger
Keith Randall
Matthew Dempsky
Zhou Peng
Cherry Zhang
Anton Gyllenberg
Clément Chigot
Ben Shi
Carlos Eduardo Seo
Robert Griesemer
Keith Randall
Ian Lance Taylor
Ian Davis
Alberto Donizetti
Alessandro Arzilli
David Heuschmann
Brad Fitzpatrick
Brian Kessler
Iskander Sharipov
Bryan C. Mills
Tim Xu
Iskander Sharipov
Jordan Rhee