1
0
mirror of https://github.com/golang/go synced 2024-11-08 11:46:31 -07:00
Commit Graph

10939 Commits

Author SHA1 Message Date
Heschi Kreinick
3ba818c894 cmd/compile: distinguish args and return values in DWARF
Set DW_AT_variable_parameter on DW_TAG_formal_parameters that are
actually return values. variable_parameter is supposed to indicate inout
parameters, but Go doesn't really have those, and DWARF doesn't have
explicit support for multiple return values. This seems to be the best
compromise, especially since the implementation of the two is very
similar -- both are stack slots.

Fixes #21100

Change-Id: Icebabc92b7b397e0aa00a7237478cce84ad1a670
Reviewed-on: https://go-review.googlesource.com/71670
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-10-18 18:46:04 +00:00
Dhananjay Nakrani
4c8e8fc301 cmd/compile: fix segfault in race instrumentation
Fixes #13265.

Change-Id: I792eb4ee26bef8a56e279e23f9802cb39019e0d0
Reviewed-on: https://go-review.googlesource.com/34929
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-18 16:46:18 +00:00
Russ Cox
b614ed4ca4 cmd/go: clean up x.exe properly in TestImportMain
More generally I'm concerned about these tests using
$GOROOT/src/cmd/go as scratch space, especially
combined wtih tg.parallel() - it's easy to believe some other
test might inadvertently also try to write x.exe about the
same time. This CL only solves the "didn't clean up x.exe"
problem and leaves for another day the "probably shouldn't
write to cmd/go at all" problem.

Fixes #22266.

Change-Id: I651534d70e2d360138e0373fb4a316081872550b
Reviewed-on: https://go-review.googlesource.com/71410
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-18 00:21:04 +00:00
Jay Conrod
94e3a30452 cmd/cover: don't try to attach directives to synthetic decls
Fixed an error that occurred in atomic mode. cover adds a global
variable declaration that forces sync/atomic to be used. fixDirectives
was confused by this declaration since it has an invalid
position. These declarations are now skipped.

Fixes #22309

Change-Id: I84f5fec13ef847fca35ad49f7704fb93b60503e0
Reviewed-on: https://go-review.googlesource.com/71351
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-17 22:14:28 +00:00
griesemer
58cf881c1c cmd/compile/internal/parser: removed TODO (cleanup)
When an opening "{" of a block is missing and after advancing we
find a closing "}", it's likely better to assume the end of the
block. Fixed and removed TODO.

Change-Id: I20c9b4ecca798933a7cd4cbf21185bd4ca04f5f7
Reviewed-on: https://go-review.googlesource.com/71291
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-17 17:00:15 +00:00
griesemer
c37090f00f cmd/compile/internal/parser: use same logic for stmtList as for other lists (cleanup)
Change-Id: I2c2571b33603f0fd0ba5a79400da7b845d246b8c
Reviewed-on: https://go-review.googlesource.com/71290
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-17 16:59:59 +00:00
Ben Shi
47193dcc0c cmd/internal/obj/arm: better solution of .S/.P/.U/.W suffix check
Current suffix check is based on instruction, which is not very
accurate. For example, "MOVW.S R1, R2" is valid, but
"MOVW.S $0xaaaaaaaa, R1" and "MOVW.P CPSR, R9" are not.

This patch fixes the above kinds of issues by checking suffix
based on []optab. And also more test cases are added.

fixes #20509

Change-Id: Ibad91be72c78eefa719412a83b4d44370d2202a8
Reviewed-on: https://go-review.googlesource.com/70910
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-17 15:18:12 +00:00
Ian Lance Taylor
378de1ae43 runtime: unify 386 entry point code
Unify the 386 entry point code as much as possible.

The main function could not be unified because on Windows 386 it is
called _main. Putting main in asm_386.s caused multiple definition
errors when using the external linker.

Add the _lib entry point to various operating systems.  A future CL
will enable c-archive/c-shared mode for those targets.

Fix _rt0_386_windows_lib_go--it was passing arguments as though it
were amd64.

Change-Id: Ic73f1c95cdbcbea87f633f4a29bbc218a5db4f58
Reviewed-on: https://go-review.googlesource.com/70530
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2017-10-17 04:03:16 +00:00
griesemer
4fe43f8146 cmd/compile/internal/parser: removed TODO (cleanup)
- checking for the correct closing token leads to slightly better
  behavior for some randomly bogus programs
- removed `switch` in favor of an `if` statement

Follow-up on https://go-review.googlesource.com/c/go/+/71250.

Change-Id: I47f6c47b43baf790907f55ed97a947661687a9db
Reviewed-on: https://go-review.googlesource.com/71252
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-17 03:24:52 +00:00
griesemer
0b2cb89196 cmd/compile/internal/syntax: better recovery after missing closing parentheses
Fine-tune skipping of tokens after missing closing parentheses in lists.

Fixes #22164.

Change-Id: I575d86e21048cd40340a2c08399e8b0deec337cf
Reviewed-on: https://go-review.googlesource.com/71250
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-17 01:04:56 +00:00
Jay Conrod
38a3c2cfe9 cmd/cover: preserve compiler directives in floating comments
Previously, cover printed directives (//go: comments) near the top of
the file unless they were in doc comments. However, directives
frequently apply to specific definitions, and they are not written in
doc comments to prevent godoc from printing them. Moving all
directives to the top of the file affected semantics of tests.

With this change, directives are kept together with the following
top-level declarations. Only directives that occur after all top-level
declarations are moved.

Fixes #22022

Change-Id: Ic5c61c4d3969996e4ed5abccba0989163789254c
Reviewed-on: https://go-review.googlesource.com/69630
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
2017-10-16 23:38:38 +00:00
Daniel Martí
bb45bc27b5 cmd/compile: make more use of value switches
Use them to replace if/else chains with at least three comparisons,
where the code becomes clearly simpler.

Passes toolstash -cmp on std cmd.

Change-Id: Ic98aa3905944ddcab5aef5f9d9ba376853263d94
Reviewed-on: https://go-review.googlesource.com/70934
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 19:59:24 +00:00
Cherry Zhang
e0111bb0f4 cmd/compile: remove needwritebarrier from the frontend
The write barrier insertion has moved to the SSA backend's
writebarrier pass. There is still needwritebarrier function
left in the frontend. This function is used in two places:

- fncall, which is called in ascompatet, which is called in
  walking OAS2FUNC. For OAS2FUNC, in order pass we've already
  created temporaries, and there is no write barrier for the
  assignments of these temporaries.

- updateHasCall, which updates the HasCall flag of a node. the
  HasCall flag is then used in
  - fncall, mentioned above.
  - ascompatet. As mentioned above, this is an assignment to
    a temporary, no write barrier.
  - reorder1, which is always called with a list produced by
    ascompatte, which is a list of assignments to stack, which
    have no write barrier.
  - vmatch1, which is called in oaslit with r.Op as OSTRUCTLIT,
    OARRAYLIT, OSLICELIT, or OMAPLIT. There is no write barrier
    in those literals.

Therefore, the needwritebarrier function is unnecessary. This
CL removes it.

Passes "toolstash -cmp" on std cmd.

Updates #17583.

Change-Id: I4b87ba8363d6583e4282a9e607a9ec8ce3ab124a
Reviewed-on: https://go-review.googlesource.com/43640
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 18:42:18 +00:00
Cherry Zhang
290de1f880 cmd/asm: reject STREX with same source and destination register on ARM
On ARM, STREX does not permit the same register used as both the
source and the destination. Reject the bad instruction.

The assembler also accepted special cases
	STREX R0, (R1)	as STREX R0, (R1), R0
	STREX (R1), R0	as STREX R0, (R1), R0
both are illegal. Remove this special case as well.

For STREXD, check that the destination is not source, and not
source+1. Also check that the source register is even numbered,
as required by the architecture's manual.

Fixes #22268.

Change-Id: I6bfde86ae692d8f1d35bd0bd7aac0f8a11ce8e22
Reviewed-on: https://go-review.googlesource.com/71190
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 18:30:56 +00:00
Matthew Dempsky
fc5841af9e cmd/compile: remove unnecessary Xoffset assignment
In golang.org/cl/61130, I removed the need for setting Xoffset on
OXCASE Nodes, but missed this assignment.

Passes toolstash-check.

Change-Id: I90ab05add14981b89ee18e73e1cdf2f13e9f9934
Reviewed-on: https://go-review.googlesource.com/66934
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
2017-10-16 18:15:37 +00:00
griesemer
645c661a54 cmd/compile/internal/syntax: factor out list parsing
Instead of repeating the same list parsing pattern for parenthesized
of braced comma or semicolon-separated lists, introduce a single list
parsing function that can be parametrized and which takes a closure
to parse list elements.

This ensures the same error handling and recovery logic is used across
all lists and simplifies the code.

No semantic change.

Change-Id: Ia738d354d6c2e0c3d84a5f1c7269a6eb95685edc
Reviewed-on: https://go-review.googlesource.com/70492
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 17:20:25 +00:00
griesemer
f8f0d6c4de cmd/compile/internal/syntax: match argument and parameter parsing (cleanup)
No semantic change. Move functionality not related to argument
out of the argument parsing function, and thus match parameter
parsing. Also, use a better function name.

Change-Id: Ic550875251d64e6fe1ebf91c11d33a9e4aec9fdd
Reviewed-on: https://go-review.googlesource.com/70491
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 17:18:32 +00:00
griesemer
4b7325c7e3 cmd/compile/internal/syntax: cleanups around parser tracing
These changes affect the parser only when the internal trace
constant is set.

- factored our printing code used for tracing
- streamlined advance function and added trace output

The parser's trace output now more clearly prints what tokens
are skipped and which is the next token in case of an error.

Example trace:

    4: . . . . . . . . . . call (
    4: . . . . . . . . . . . expr (
    4: . . . . . . . . . . . . unaryExpr (
    4: . . . . . . . . . . . . . pexpr (
    4: . . . . . . . . . . . . . . operand name (
    4: . . . . . . . . . . . . . . )
    4: . . . . . . . . . . . . . . call (
    4: . . . . . . . . . . . . . . )
    4: . . . . . . . . . . . . . )
    4: . . . . . . . . . . . . )
    4: . . . . . . . . . . . )
    4: . . . . . . . . . . . syntax error: expecting comma or )
    4: . . . . . . . . . . . skip ;
    6: . . . . . . . . . . . skip name
    6: . . . . . . . . . . . skip :=
    6: . . . . . . . . . . . skip literal
    6: . . . . . . . . . . . skip ;
    7: . . . . . . . . . . . skip }
    7: . . . . . . . . . . . skip ;
    9: . . . . . . . . . . . skip func
    9: . . . . . . . . . . . skip name
    9: . . . . . . . . . . . skip (
    9: . . . . . . . . . . . next )
    9: . . . . . . . . . . )

For #22164.

Change-Id: I4a233696b1f989ee3287472172afaf92cf424565
Reviewed-on: https://go-review.googlesource.com/70490
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 17:18:08 +00:00
Michael Hudson-Doyle
302f0d1646 cmd/link: replace SCONTAINER with an attribute bit
This is much easier than replacing SSUB so split it out from my other CL.

Change-Id: If01e4005da5355895404456320a2156bde4ec09a
Reviewed-on: https://go-review.googlesource.com/71050
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 06:40:37 +00:00
Michael Hudson-Doyle
1341104ae2 cmd/link: replace SHIDDEN bit in SymKind with a bit of Attribute
This is https://go-review.googlesource.com/42025 but with some more fixes --
hidden symbols implicitly passed "Type == 0 || Type == SXREF" checks. (This
sort of thing is part of why I wanted to make this change)

Change-Id: I2273ee98570fd7f2dd8a799c692a2083c014235e
Reviewed-on: https://go-review.googlesource.com/42330
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 06:40:13 +00:00
Daniel Martí
270a789c52 cmd/compile: simplify some declarations
Reduce the scope of some. Also remove vars that were simply the index or
the value in a range statement. While at it, remove a var that was
exactly the length of a slice.

Also replaced 'bad' with a more clear 'errored' of type bool, and
renamed a single-char name with a comment to a name that is
self-explanatory.

And removed a few unnecessary Index calls within loops.

Passes toolstash -cmp on std cmd.

Change-Id: I26eee5f04e8f7e5418e43e25dca34f89cca5c80a
Reviewed-on: https://go-review.googlesource.com/70930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-15 19:24:40 +00:00
Matthew Dempsky
f3d4ff7ddc cmd/compile: omit ICE diagnostics after normal error messages
After we detect errors, the AST is in a precarious state and more
likely to trip useless ICE failures. Instead let the user fix any
existing errors and see if the ICE persists.  This makes Fatalf more
consistent with how panics are handled by hidePanic.

While here, also fix detection for release versions: release version
strings begin with "go" ("go1.8", "go1.9.1", etc), not "release".

Fixes #22252.

Change-Id: I1c400af62fb49dd979b96e1bf0fb295a81c8b336
Reviewed-on: https://go-review.googlesource.com/70850
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-14 01:00:31 +00:00
Cherry Zhang
e01eac371a cmd/compile: mark LoweredGetCallerPC rematerializeable
The caller's PC is always available in the frame. We can just
load it when needed, no need to spill.

Change-Id: I9c0a525903e574bb4eec9fe53cbeb8c64321166a
Reviewed-on: https://go-review.googlesource.com/70710
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-10-14 00:53:20 +00:00
David Crawshaw
c996d07fee cmd/link: use the correct module data on ppc64le
Fixes #22250

Change-Id: I0e39d10ff6f0785cd22b0105de2d839e569db4b7
Reviewed-on: https://go-review.googlesource.com/70810
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-13 22:46:25 +00:00
David Crawshaw
350b74bc4b cmd/link: zero symtab fields correctly
CL 69370 introduced a hasmain field to moduledata after the
modulehashes slice. However that code was relying on the zeroing
code after it to cover modulehashes if len(Shlibs) == 0. The
hasmain field gets in the way of that. So clear modulehashes
explicitly in that case.

Found when looking at #22250. Not sure if it's related.

Change-Id: I81050cb4554cd49e9f245d261ef422f97d026df4
Reviewed-on: https://go-review.googlesource.com/70730
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 17:48:54 +00:00
Martin Möhrmann
743117a85e cmd/compile: simplify slice/array range loops for some element sizes
In range loops over slices and arrays besides a variable to track the
index an extra variable containing the address of the current element
is used. To compute a pointer to the next element the elements size is
added to the address.

On 386 and amd64 an element of size 1, 2, 4 or 8 bytes can by copied
from an array using a MOV instruction with suitable addressing mode
that uses the start address of the array, the index of the element and
element size as scaling factor. Thereby, for arrays and slices with
suitable element size we can avoid keeping and incrementing an extra
variable to compute the next elements address.

Shrinks cmd/go by 4 kilobytes.

AMD64:
name                   old time/op    new time/op    delta
BinaryTree17              2.66s ± 7%     2.54s ± 0%  -4.53%  (p=0.000 n=10+8)
Fannkuch11                3.02s ± 1%     3.02s ± 1%    ~     (p=0.579 n=10+10)
FmtFprintfEmpty          45.6ns ± 1%    42.2ns ± 1%  -7.46%  (p=0.000 n=10+10)
FmtFprintfString         69.8ns ± 1%    70.4ns ± 1%  +0.84%  (p=0.041 n=10+10)
FmtFprintfInt            80.1ns ± 1%    79.0ns ± 1%  -1.35%  (p=0.000 n=10+10)
FmtFprintfIntInt          127ns ± 1%     125ns ± 1%  -1.00%  (p=0.007 n=10+9)
FmtFprintfPrefixedInt     158ns ± 2%     152ns ± 1%  -4.11%  (p=0.000 n=10+10)
FmtFprintfFloat           218ns ± 1%     214ns ± 1%  -1.61%  (p=0.000 n=10+10)
FmtManyArgs               508ns ± 1%     504ns ± 1%  -0.93%  (p=0.001 n=9+10)
GobDecode                6.76ms ± 1%    6.78ms ± 1%    ~     (p=0.353 n=10+10)
GobEncode                5.84ms ± 1%    5.77ms ± 1%  -1.31%  (p=0.000 n=10+9)
Gzip                      223ms ± 1%     218ms ± 1%  -2.39%  (p=0.000 n=10+10)
Gunzip                   40.3ms ± 1%    40.4ms ± 3%    ~     (p=0.796 n=10+10)
HTTPClientServer         73.5µs ± 0%    73.3µs ± 0%  -0.28%  (p=0.000 n=10+9)
JSONEncode               12.7ms ± 1%    12.6ms ± 8%    ~     (p=0.173 n=8+10)
JSONDecode               57.5ms ± 1%    56.1ms ± 2%  -2.40%  (p=0.000 n=10+10)
Mandelbrot200            3.80ms ± 1%    3.86ms ± 6%    ~     (p=0.579 n=10+10)
GoParse                  3.25ms ± 1%    3.23ms ± 1%    ~     (p=0.052 n=10+10)
RegexpMatchEasy0_32      74.4ns ± 1%    76.9ns ± 1%  +3.39%  (p=0.000 n=10+10)
RegexpMatchEasy0_1K       243ns ± 2%     248ns ± 1%  +1.86%  (p=0.000 n=10+8)
RegexpMatchEasy1_32      71.0ns ± 2%    72.8ns ± 1%  +2.55%  (p=0.000 n=10+10)
RegexpMatchEasy1_1K       370ns ± 1%     383ns ± 0%  +3.39%  (p=0.000 n=10+9)
RegexpMatchMedium_32      107ns ± 0%     113ns ± 1%  +5.33%  (p=0.000 n=6+10)
RegexpMatchMedium_1K     35.0µs ± 1%    36.0µs ± 1%  +3.13%  (p=0.000 n=10+10)
RegexpMatchHard_32       1.65µs ± 1%    1.69µs ± 1%  +2.23%  (p=0.000 n=10+9)
RegexpMatchHard_1K       49.8µs ± 1%    50.6µs ± 1%  +1.59%  (p=0.000 n=10+10)
Revcomp                   398ms ± 1%     396ms ± 1%  -0.51%  (p=0.043 n=10+10)
Template                 63.4ms ± 1%    60.8ms ± 0%  -4.11%  (p=0.000 n=10+9)
TimeParse                 318ns ± 1%     322ns ± 1%  +1.10%  (p=0.005 n=10+10)
TimeFormat                323ns ± 1%     336ns ± 1%  +4.15%  (p=0.000 n=10+10)

Updates: #15809.

Change-Id: I55915aaf6d26768e12247f8a8edf14e7630726d1
Reviewed-on: https://go-review.googlesource.com/38061
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2017-10-13 14:52:04 +00:00
Wei Xiao
531e6c06c4 cmd/asm: refine Go assembly for ARM64
Some ARM64-specific instructions (such as SIMD instructions) are not supported.
This patch adds support for the following:
1. Extended register, e.g.:
     ADD	Rm.<ext>[<<amount], Rn, Rd
     <ext> can have the following values:
       UXTB, UXTH, UXTW, UXTX, SXTB, SXTH, SXTW and SXTX
2. Arrangement for SIMD instructions, e.g.:
     VADDP	Vm.<T>, Vn.<T>, Vd.<T>
     <T> can have the following values:
       B8, B16, H4, H8, S2, S4 and D2
3. Width specifier and element index for SIMD instructions, e.g.:
     VMOV	Vn.<T>[index], Rd // MOV(to general register)
     <T> can have the following values:
       S and D
4. Register List, e.g.:
     VLD1	(Rn), [Vt1.<T>, Vt2.<T>, Vt3.<T>]
5. Register offset variant, e.g.:
     VLD1.P	(Rn)(Rm), [Vt1.<T>, Vt2.<T>] // Rm is the post-index register
6. Go assembly for ARM64 reference manual
     new added instructions are required to have according explanation items in
     the manual and items for existed instructions will be added incrementally

For more information about the refinement background, please refer to the
discussion (https://groups.google.com/forum/#!topic/golang-dev/rWgDxCrL4GU)

This patch only adds syntax and doesn't break any assembly that already exists.

Change-Id: I34e90b7faae032820593a0e417022c354a882008
Reviewed-on: https://go-review.googlesource.com/41654
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-13 13:41:19 +00:00
David Chase
e45e490296 cmd/compile: attempt to deflake debug_test.go
Excluded when -short because it still runs relatively long,
but deflaked.

Removed timeouts from normal path and ensured that they were
not needed and that reference files did not change.

Use "tbreak" instead of "break" with gdb to reduce chance
of multiple hits on main.main.  (Seems not enough, but a
move in the right direction).

By default, testing ignores repeated lines that occur when
nexting.  This appears to sometimes be timing-dependent and
is the observed source of flakiness in testing so far.
Note that these can also be signs of a bug in the generated
debugging output, but it is one of the less-confusing bugs
that can occur.

By default, testing with gdb uses compilation with
inlining disabled to prevent dependence on library code
(it's a bug that library code is seen while Nexting, but
the bug is current behavior).

Also by default exclude all source files outside /testdata
to prevent accidental dependence on library code.  Note that
this is currently only applicable to dlv because (for the
debugging information we produce) gdb does not indicate a
change in the source file for inlined code.

Added flags -i and -r to make gdb testing compile with
inlining and be sensitive to repeats in the next stream.
This is for developer-testing and so we can describe these
problems in bug reports.

Updates #22206.

Change-Id: I9a30ebbc65aa0153fe77b1858cf19743bdc985e4
Reviewed-on: https://go-review.googlesource.com/69930
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 03:25:23 +00:00
David Crawshaw
c58b98b2d6 cmd/link, runtime: put hasmain bit in moduledata
Currently we look to see if the main.main symbol address is in the
module data text range. This requires access to the main.main
symbol, which usually the runtime has, but does not when building
a plugin.

To avoid a dynamic relocation to main.main (which I haven't worked
out how to have the linker generate on darwin), stop using the
symbol. Instead record a boolean in the moduledata if the module
has the main function.

Fixes #22175

Change-Id: If313a118f17ab499d0a760bbc2519771ed654530
Reviewed-on: https://go-review.googlesource.com/69370
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 01:13:33 +00:00
David Crawshaw
d06815ba3f cmd/link: split PE loader into its own package
For #22095

Change-Id: I8f48fce571b69a7e8edf2ad7733ffdfd38676e63
Reviewed-on: https://go-review.googlesource.com/70310
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-12 21:35:43 +00:00
Matthew Dempsky
53bbddd527 cmd/compile: intrinsify runtime/internal/sys.Ctz{32,64} on ppc64
These functions are identical to math/bits.TrailingZeros{32,64}, which
are already intrinsified on ppc64.

Change-Id: If7ee57e7afe53154874f4b66bacdb6237806128a
Reviewed-on: https://go-review.googlesource.com/70350
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-12 20:01:16 +00:00
Austin Clements
97920373fa cmd/link: generate PC ranges for compilation unit DIEs
When we split separate packages into separate compilation units, we
lost PC range information because it was no longer contiguous. This
brings it back by constructing proper per-package PC range tables.

Change-Id: Id0ab5187e08ac5d13b3d3794977bfc857a56224f
Reviewed-on: https://go-review.googlesource.com/69974
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-12 18:56:26 +00:00
Austin Clements
d4dda76b5f cmd/link: one DWARF compilation unit per package
Currently, the linker generates one huge DWARF compilation unit for
the entire Go binary. This commit creates a separate compilation unit
and line table per Go package.

We temporarily lose compilation unit PC range information, since it's
now discontiguous, so harder to emit. We'll bring it back in the next
commit.

Beyond being "more traditional", this has various technical
advantages:

* It should speed up line table lookup, since that requires a
  sequential scan of the line table. With this change, a debugger can
  first locate the per-package line table and then scan only that line
  table.

* Once we emit compilation unit PC ranges again, this should also
  speed up various other debugger reverse PC lookups.

* It puts us in a good position to move more DWARF generation into the
  compiler, which could produce at least the CU header, per-function
  line table fragments, and per-function frame unwinding info that the
  linker could simply paste together.

* It will let us record a per-package compiler command-line flags
  (#22168).

Change-Id: Ibac642890984636b3ef1d4b37fe97f4453c2cc84
Reviewed-on: https://go-review.googlesource.com/69973
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-12 18:56:23 +00:00
Austin Clements
ce6e0b089f cmd/link: remove silly sym.R[:0] truncation
The DWARF code currently clears all section relocations every time it
creates a section. This is unnecessary and confusing, so don't do it.

This dates back to
https://codereview.appspot.com/7891044/diff/26001/src/cmd/ld/dwarf.c.
At the time, this was only done for one symbol and that symbol was
used solely for collecting relocations (which is why it made sense to
clear the relocations but not the actual data). Furthermore, DWARF
generation potentially required two passes, so it was important to
clear the state from the first pass. None of this is true now, but
this pattern had been cargo-culted all over the dwarf.go.

Change-Id: I87d4ff8ccd5c807796241559be46168ce3ccb49a
Reviewed-on: https://go-review.googlesource.com/70312
Run-TryBot: Austin Clements <austin@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-12 18:56:20 +00:00
Austin Clements
77c27c3102 cmd/link: eliminate .debug_aranges
The .debug_aranges section is an odd vestige of DWARF, since its
contents are easy and efficient for a debugger to reconstruct from the
attributes of the top-level compilation unit DIEs. Neither GCC nor
clang emit it by default these days. GDB and Delve ignore it entirely.
LLDB will use it if present, but is happy to construct the index from
the compilation unit attributes (and, indeed, a remarkable variety of
other ways if those aren't available either).

We're about to split up the compilation units by package, which means
they'll have discontiguous PC ranges, which is going to make
.debug_aranges harder to construct (and larger).

Rather than try to maintain this essentially unused code, let's
simplify things and remove it.

Change-Id: I8e0ccc033b583b5b8908cbb2c879b2f2d5f9a50b
Reviewed-on: https://go-review.googlesource.com/69972
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-12 18:56:18 +00:00
Heschi Kreinick
a0402b6bf8 cmd/link: suppress unnecessary DWARF relocs that confuse dsymutil
During Mach-O linking, dsymutil takes the DWARF from individual object
files and combines it into a debug archive. Because it's content-aware,
it doesn't need our help to do its job. Nonetheless, it does try to
honor relocations that are present in its input.

When dsymutil encounters a relocation, it uses the value of that
relocation as an index into the debug map to find its final location.
When it does that, it's assuming that the value is an address in the
object file. But DWARF references are section-relative. So when it
processes a relocation for a DWARF reference, it gets confused,
and if the value happens to match the address of a function or
data symbol, it will rewrite it incorrectly.

Since the relocations don't help, and can hurt, drop them when
externally linking a Mach-O binary.

Fixes #22068

Change-Id: I8ec36da626575d9f6c8d0e7a0b76eab8ba22d62c
Reviewed-on: https://go-review.googlesource.com/68330
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-10-12 18:51:54 +00:00
Russ Cox
6054f87f3a cmd/go: delete p.Internal.Target in favor of p.Target
The two are not meaningfully different, and it is confusing to have two.

Change-Id: Ie6a355ea4d79fb4bb79bf5124071a866038b19ba
Reviewed-on: https://go-review.googlesource.com/70211
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-10-12 13:56:15 +00:00
Matthew Dempsky
a509cae90d cmd/compile: record InlCost in export data
Previously, we were treating cross-package function calls as free for
inlining budgeting.

In theory, we should be able to recompute InlCost from the
exported/reimported function bodies. However, that process mutates the
structure of the Node AST enough that it doesn't preserve InlCost. To
avoid unexpected issues, just record and restore InlCost in the export
data.

Fixes #19261.

Change-Id: Iac2bc0d32d4f948b64524aca657051f9fc96d92d
Reviewed-on: https://go-review.googlesource.com/70151
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-11 23:20:44 +00:00
Daniel Martí
1fbeccb15a cmd/compile: deduplicate a few lines in swt.go
Noticed while reading some code that the two branches in this loop body
shared the last statements. Rewrite it in a way that they are not
duplicated.

Passes toolstash -cmp on std.

Change-Id: I3356ca9fa37c32eee496e221d7830bfc581dade1
Reviewed-on: https://go-review.googlesource.com/66470
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-11 23:03:47 +00:00
Hugues Bruant
4f70a2a699 cmd/compile: inline calls to local closures
Calls to a closure held in a local, non-escaping,
variable can be inlined, provided the closure body
can be inlined and the variable is never written to.

The current implementation has the following limitations:

 - closures with captured variables are not inlined because
   doing so naively triggers invariant violation in the SSA
   phase
 - re-assignment check is currently approximated by checking
   the Addrtaken property of the variable which should be safe
   but may miss optimization opportunities if the address is
   not used for a write before the invocation

Updates #15561

Change-Id: I508cad5d28f027bd7e933b1f793c14dcfef8b5a1
Reviewed-on: https://go-review.googlesource.com/65071
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
2017-10-11 22:32:36 +00:00
David Chase
edcf2d0cd6 cmd/compile: add line numbers to values & blocks in ssa.html
In order to improve the line numbering for debuggers,
it's necessary to trace lines through compilation.
This makes it (much) easier to follow.

The format of the last column of the ssa.html output was
also changed to reduce the spamminess of the file name,
which is usually the same and makes it far harder to read
instructions and line numbers, and to make it wider and also
able to break words when wrapping (long path names still
can push off the end otherwise; side-to-side scrolling was
tried but was more annoying than the occasional wrapped
line).

Sample output now, where [...] is elision for sake of making
the CL character-counter happy -- and the (##) line numbers
are rendered in italics and a smaller font (11 point) under
control of a CSS class "line-number".

genssa
      # /Users/drchase/[...]/ssa/testdata/hist.go
      00000 (35) TEXT	"".main(SB)
      00001 (35) FUNCDATA	$0, gclocals·7be4bb[...]1e8b(SB)
      00002 (35) FUNCDATA	$1, gclocals·9ab98a[...]4568(SB)
v920  00003 (36) LEAQ	""..autotmp_31-640(SP), DI
v858  00004 (36) XORPS	X0, X0
v6    00005 (36) LEAQ	-48(DI), DI
v6    00006 (36) DUFFZERO	$277
v576  00007 (36) LEAQ	""..autotmp_31-640(SP), AX
v10   00008 (36) TESTB	AX, (AX)
b1    00009 (36) JMP	10

and from an earlier phase:

b18: ← b17
v242 (47) = Copy <mem> v238
v243 (47) = VarKill <mem> {.autotmp_16} v242
v244 (48) = Addr <**bufio.Scanner> {scanner} v2
v245 (48) = Load <*bufio.Scanner> v244 v243
[...]
v279 (49) = Store <mem> {int64} v277 v276 v278
v280 (49) = Addr <*error> {.autotmp_18} v2
v281 (49) = Load <error> v280 v279
v282 (49) = Addr <*error> {err} v2
v283 (49) = VarDef <mem> {err} v279
v284 (49) = Store <mem> {error} v282 v281 v283
v285 (47) = VarKill <mem> {.autotmp_18} v284
v286 (47) = VarKill <mem> {.autotmp_17} v285
v287 (50) = Addr <*error> {err} v2
v288 (50) = Load <error> v287 v286
v289 (50) = NeqInter <bool> v288 v51
If v289 → b21 b22 (line 50)

Change-Id: I3f46310918f965761f59e6f03ea53067237c28a8
Reviewed-on: https://go-review.googlesource.com/69591
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-11 21:46:20 +00:00
David Crawshaw
ecfa7375e4 cmd/link: move ELF reader to its own package
Along the way, switch to using relocation constants from debug/elf.

For #22095

Change-Id: I1a64353619f95dde5aa39060c4b9d001af7dc1e4
Reviewed-on: https://go-review.googlesource.com/69013
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-11 20:53:39 +00:00
Russ Cox
d19ced040a cmd/dist: refresh deps.go after recent package io changes
Change-Id: Iaa960c85011289e047c64b53cf610838eb50332d
Reviewed-on: https://go-review.googlesource.com/70073
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-11 20:31:38 +00:00
Austin Clements
e29efbcbcb cmd/link: fix some unintentional symbol creation
There are two places in DWARF generation that create symbols when they
really just want to get the symbol if it exists. writeranges, in
particular, will create a DWARF range symbol for every single textp
symbol (though they won't get linked into any list, so they don't
affect the binary).

Fix these to use ROLookup instead of Lookup.

Change-Id: I401eadf22890e296bd08bccaa6ba2fd8fac800cd
Reviewed-on: https://go-review.googlesource.com/69971
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-11 20:06:41 +00:00
Russ Cox
7dcd33302a cmd/internal/buildid: add missing f.Close in ReadFile
On Windows, not closing f keeps us from being able to remove it.

Change-Id: Id4cb709b6ce0b30485b87364a9f0e6e71d2782bd
Reviewed-on: https://go-review.googlesource.com/70070
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-11 19:38:04 +00:00
Russ Cox
31896332b3 cmd/dist: reenable TestDeps
It looks like I forgot to reenable this test when I fixed #21522.
Update deps.go and reenable.

Change-Id: I68a45df09b418f48d93d2e7ab1d274e056c192e6
Reviewed-on: https://go-review.googlesource.com/70050
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-10-11 19:10:31 +00:00
Russ Cox
cdbc363cb2 cmd/go: record both build ID and content ID in archives and binaries
The content ID will be needed for content-based staleness
determination. It is defined as the SHA256 hash of the file
in which it appears, with occurrences of the build+content IDs
changed to zeros during the hashing operation.

Storing the content ID in the archives is a little tricky
but it means that later builds need not rehash the archives
each time they are referenced, so under the assumption
that each package is imported at least once after being
compiled, hashing at build time is a win. (Also the whole
file is more likely to be in cache at build time,
since we just wrote it.)

In my unscientific tests, the time for "go build -a std cmd"
rises from about 14.3s to 14.5s on my laptop, or under 2%.

Change-Id: Ia3d4dc657d003e8295631f73363868bd92ebf96a
Reviewed-on: https://go-review.googlesource.com/69054
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-10-11 19:03:26 +00:00
Austin Clements
85f93c889a cmd/compile: fix some plive comments
The liveness analysis no longer directly emits PCDATA. Fix stale
comments that say so.

Change-Id: Id26b112ddf4c13a12ebf766f64bf57c68fbfe3ef
Reviewed-on: https://go-review.googlesource.com/67691
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-11 18:29:36 +00:00
Keith Randall
e130dcf051 cmd/compile: abort earlier if stack frame too large
If the stack frame is too large, abort immediately.
We used to generate code first, then abort.
In issue 22200, generating code raised a panic
so we got an ICE instead of an error message.

Change the max frame size to 1GB (from 2GB).
Stack frames between 1.1GB and 2GB didn't used to work anyway,
the pcln table generation would have failed and generated an ICE.

Fixes #22200

Change-Id: I1d918ab27ba6ebf5c87ec65d1bccf973f8c8541e
Reviewed-on: https://go-review.googlesource.com/69810
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-11 18:24:13 +00:00
Keith Randall
624630b824 cmd/compile: fold constant comparisions into SETxxmem ops.
Fixes #22198

Change-Id: I5cb91c73069af8b16a2580d28756efd58c84b690
Reviewed-on: https://go-review.googlesource.com/69990
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-11 18:23:09 +00:00