1
0
mirror of https://github.com/golang/go synced 2024-11-17 21:24:55 -07:00
Commit Graph

44493 Commits

Author SHA1 Message Date
Matthew Dempsky
7388956b76 cmd/cgo: fix mangling of enum and union types
Consider this test package:

    package p

    // enum E { E0 };
    // union U { long x; };
    // void f(enum E e, union U* up) {}
    import "C"

    func f() {
    	C.f(C.enum_E(C.E0), (*C.union_U)(nil))
    }

In Go 1.14, cgo translated this to (omitting irrelevant details):

    type _Ctype_union_U [8]byte

    func f() {
    	_Cfunc_f(uint32(_Ciconst_E0), (*[8]byte)(nil))
    }

    func _Cfunc_f(p0 uint32, p1 *[8]byte) (r1 _Ctype_void) { ... }

Notably, _Ctype_union_U was declared as a defined type, but uses were
being rewritten into uses of the underlying type, which matched how
_Cfunc_f was declared.

After CL 230037, cgo started consistently rewriting "C.foo" type
expressions as "_Ctype_foo", which caused it to start emitting:

    type _Ctype_enum_E uint32
    type _Ctype_union_U [8]byte

    func f() {
    	_Cfunc_f(_Ctype_enum_E(_Ciconst_E0), (*_Ctype_union_U)(nil))
    }

    // _Cfunc_f unchanged

Of course, this fails to type-check because _Ctype_enum_E and
_Ctype_union_U are defined types.

This CL changes cgo to emit:

    type _Ctype_enum_E = uint32
    type _Ctype_union_U = [8]byte

    // f unchanged since CL 230037
    // _Cfunc_f still unchanged

It would probably be better to fix this in (*typeConv).loadType so
that cgo generated code uses the _Ctype_foo aliases too. But as it
wouldn't have any effect on actual compilation, it's not worth the
risk of touching it at this point in the release cycle.

Updates #39537.
Fixes #40494.

Change-Id: I88269660b40aeda80a9a9433777601a781b48ac0
Reviewed-on: https://go-review.googlesource.com/c/go/+/246057
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-07-31 16:35:33 +00:00
Michael Anthony Knyszek
b56791cdea runtime: validate candidate searchAddr in pageAlloc.find
Currently pageAlloc.find attempts to find a better estimate for the
first free page in the heap, even if the space its looking for isn't
necessarily going to be the first free page in the heap (e.g. if npages
>= 2). However, in doing so it has the potential to return a searchAddr
candidate that doesn't actually correspond to mapped memory, but this
candidate might still be adopted. As a result, pageAlloc.alloc's fast
path may look at unmapped summary memory and segfault. This case is rare
on most operating systems since the heap is kept fairly contiguous, so
the chance that the candidate searchAddr discovered is unmapped is
fairly low. Even so, this is totally possible and outside the user's
control when it happens (in fact, it's likely to happen consistently for
a given user on a given system).

Fix this problem by ensuring that our candidate always points to mapped
memory. We do this by looking at mheap's arenas structure first. If it
turns out our candidate doesn't correspond to mapped memory, then we
look at inUse to round up the searchAddr to the next mapped address.

While we're here, clean up some documentation related to searchAddr.

Fixes #40191.

Change-Id: I759efec78987e4a8fde466ae45aabbaa3d9d4214
Reviewed-on: https://go-review.googlesource.com/c/go/+/242680
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-31 14:35:39 +00:00
Jeremy Faller
128f4e51f2 [dev.link] ensure package path is set when TEXT symbols are created
We're reworking pclntab generation in the linker, and with that we're
moving FuncID generation in to the compiler. Determining the FuncID is
done by a lookup on the package.function name; therefore, we need the
package whenever we make the TEXT symbols.

Change-Id: I805445ffbf2f895f06ce3a91fb09126d012bf86e
Reviewed-on: https://go-review.googlesource.com/c/go/+/245318
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-07-31 13:55:19 +00:00
Jeremy Faller
6ac9914383 [dev.link] create runtime.funcnametab
Move the function names out of runtime.pclntab_old, creating
runtime.funcnametab.  There is an unfortunate artifact in this change in
that calculating the funcID still requires loading the name. Future work
will likely pull this out and put it into the object file Funcs.

ls -l cmd/compile (darwin):
  before: 18524016
  after:  18519952

The difference in size can be attributed to alignment in pclntab_old.

Change-Id: Ibcbb230d4632178f8fcd0667165f5335786381f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/243223
Reviewed-by: Austin Clements <austin@google.com>
2020-07-31 13:55:07 +00:00
Jeremy Faller
3067a8dc02 [dev.link] cmd/link: use pclntabState and eliminate globals
Non functional change.

As runtime.pclntab breaks up, it'll be easier if we can just pass around
the pclntab state. Also, eliminate the globals in pclntab.

Change-Id: I2a5849e8f5f422a336a881e53a261e3997d11c44
Reviewed-on: https://go-review.googlesource.com/c/go/+/242599
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-07-31 13:54:51 +00:00
Cherry Zhang
365059e1d1 [dev.link] cmd/link: add back SUNDEFEXT case
The SUNDEFEXT case was lost during the refactoring. Add it back.

Fix ppc64le build.

Change-Id: I14594ee2c3e0a794c93839247fb3e6206c2e657a
Reviewed-on: https://go-review.googlesource.com/c/go/+/245919
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-30 20:03:52 +00:00
Jeremy Faller
ba9c639470 [dev.link] cmd/link: add runtime.pcheader
As of July 2020, a fair amount of the new linker's live memory, and
runtime is spent generating pclntab. In an effort to streamline that
code, this change starts breaking up the generation of runtime.pclntab
into smaller chunks that can run later in a link. These changes are
described in an (as yet not widely distributed) document that lays out
an improved format. Largely the work consists of breaking up
runtime.pclntab into smaller pieces, stopping much of the data
rewriting, and getting runtime.pclntab into a form where we can reason
about its size and look to shrink it. This change is the first part of
that work -- just pulling out the header, and demonstrating where a
majority of that work will be.

Change-Id: I65618d0d0c780f7e5977c9df4abdbd1696fedfcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/241598
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
2020-07-30 19:36:06 +00:00
Katie Hockman
10374e2435 testing: fix quotation marks
Change-Id: I4b816e26718ef5521afba2b200a6333373b09c58
Reviewed-on: https://go-review.googlesource.com/c/go/+/245136
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-07-30 19:08:42 +00:00
Cherry Zhang
b3e3c339ff [dev.link] cmd/internal/obj: trim trailing zeros for content hashing
The symbol's data in the object file (sym.P) may already not
contain trailing zeros (e,g, for [10]int{1}), but sometimes it
does (e.g. for [10]int{1,0}). The linker can already handle this
case. We just always trim the trailing zeros for content hashing,
so it can deduplicate [10]int{1} and [10]int{1,0}.

Note: in theory we could just trim the zeros in the symbol data
as well. But currently the linker depends on reading symbol data
for certain symbols (e.g. type symbol decoding), and trimming
will complicates things in the linker.

Change-Id: I9e90e41e6ac808b36855b0713a85e61c33bf093a
Reviewed-on: https://go-review.googlesource.com/c/go/+/245717
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-30 18:37:08 +00:00
Cholerae Hu
7f86080476 cmd/compile: don't addLocalInductiveFacts if there is no direct edge from if block to phi block
Currently in addLocalInductiveFacts, we only check whether
direct edge from if block to phi block exists. If not, the
following logic will treat the phi block as the first successor,
which is wrong.

This patch makes prove pass more conservative, so we disable
some cases in test/prove.go. We will do some optimization in
the following CL and enable these cases then.

Fixes #40367.

Change-Id: I27cf0248f3a82312a6f7dabe11c79a1a34cf5412
Reviewed-on: https://go-review.googlesource.com/c/go/+/244579
Reviewed-by: Zach Jones <zachj1@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-30 17:23:11 +00:00
Cherry Zhang
4a92371291 [dev.link] cmd/link: remove "2", another round
Rename Reloc2 to Reloc, At2 to At, Aux2 to Aux.

Change-Id: Ic98d83c080e8cd80fbe1837c8f0aa134033508ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/245578
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-30 16:37:23 +00:00
Cherry Zhang
80b287fd28 [dev.link] cmd/link: remove loader.Reloc
We have Reloc and Reloc2. Reloc2 is the better approach and most
code uses Reloc2. There are still uses of Reloc. This CL migrates
them to Reloc2, and removes Reloc.

Change-Id: Id5f6a6019e1e044add682d05e70ebb1548ec58d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/245577
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-30 16:37:04 +00:00
Cherry Zhang
317c1ca9f2 [dev.link] cmd/link: refactor ExtReloc data structures
We used to generate all external relocations in memory, then emit
the relocation records at a later pass. The data structures were
chosen so that it takes as little memory as possible. Now we just
stream out external relocations, and ExtReloc is just a local
variable. Change the data structure to avoid repeated read of
some fields. Also get rid of ExtRelocView, as it is no longer
necessary.

Change-Id: I40209bbe4387af231b29788125c3b4ebb0ff4a33
Reviewed-on: https://go-review.googlesource.com/c/go/+/245479
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-30 16:36:25 +00:00
Cherry Zhang
880f43c87f [dev.link] cmd/link: move arch-specific extreloc to common code
Change-Id: Ia833818862b277ac83266919f39e5c25faac895e
Reviewed-on: https://go-review.googlesource.com/c/go/+/245478
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-30 16:36:09 +00:00
Michael Munday
54e75e8f9d crypto/ed25519: remove s390x KDSA implementation
This reverts CL 202578 and CL 230677 which added an optimization
to use KDSA when available on s390x.

Inconsistencies have been found between the two implementations
in their handling of certain edge cases. Since the Go 1.15 release
is extremely soon it seems prudent to remove this optimization
for now and revisit it in a future release.

Fixes #40475.

Change-Id: Ifb2ed9b9e573784df57383671f1c29d8abae90d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/245497
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ruixin(Peter) Bao <ruixin.bao@ibm.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2020-07-30 16:00:05 +00:00
Michael Anthony Knyszek
6b4dcf19fa runtime: hold sched.lock over globrunqputbatch in runqputbatch
globrunqputbatch should never be called without sched.lock held.
runqputbatch's documentation even says it may acquire sched.lock in
order to call it.

Fixes #40457.

Change-Id: I5421b64f1da3a6087dfebbef7203db0c95d213a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/245377
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-07-30 15:46:39 +00:00
Cherry Zhang
2369e01234 [dev.link] cmd/link: fix accidental escape in extreloc
We accidentally passed the address of a local to a function
pointer, where we should pass the address of a global.

Linking cmd/compile with external linking:

Asmb2_GC       32.5ms ± 5%    21.6ms ± 3%  -33.57%  (p=0.016 n=5+4)
Asmb2_GC       29.2MB ± 0%     6.4MB ± 0%  -78.20%  (p=0.008 n=5+5)
Asmb2_GC        1.43M ± 0%     0.00M ± 4%  -99.98%  (p=0.008 n=5+5)

Change-Id: I4754189bcc20f824627d95858ba35285d53c614d
Reviewed-on: https://go-review.googlesource.com/c/go/+/245337
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-28 19:47:05 +00:00
Michael Pratt
85afa2eb19 runtime: ensure startm new M is consistently visible to checkdead
If no M is available, startm first grabs an idle P, then drops
sched.lock and calls newm to start a new M to run than P.

Unfortunately, that leaves a window in which a G (e.g., returning from a
syscall) may find no idle P, add to the global runq, and then in stopm
discover that there are no running M's, a condition that should be
impossible with runnable G's.

To avoid this condition, we pre-allocate the new M ID in startm before
dropping sched.lock. This ensures that checkdead will see the M as
running, and since that new M must eventually run the scheduler, it will
handle any pending work as necessary.

Outside of startm, most other calls to newm/allocm don't have a P at
all. The only exception is startTheWorldWithSema, which always has an M
if there is 1 P (i.e., the currently running M), and if there is >1 P
the findrunnable spinning dance ensures the problem never occurs.

This has been tested with strategically placed sleeps in the runtime to
help induce the correct race ordering, but the timing on this is too
narrow for a test that can be checked in.

Fixes #40368

Change-Id: If5e0293a430cc85154b7ed55bc6dadf9b340abe2
Reviewed-on: https://go-review.googlesource.com/c/go/+/245018
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2020-07-28 16:59:04 +00:00
Keith Randall
c4fed25553 cmd/compile: add floating point load+op operations to addressing modes pass
They were missed as part of the refactoring to use a separate
addressing modes pass.

Fixes #40426

Change-Id: Ie0418b2fac4ba1ffe720644ac918f6d728d5e420
Reviewed-on: https://go-review.googlesource.com/c/go/+/244859
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-07-27 18:24:32 +00:00
Cherry Zhang
19a932ceb8 cmd/link: don't mark shared library symbols reachable unconditionally
During the transitioning period, we mark symbols from Go shared
libraries reachable unconditionally. That might be useful when
there was still a large portion of the linker using sym.Symbols,
and only reachable symbols were converted to sym.Symbols. Marking
them reachable brings them to the dynamic symbol table, even if
they are not needed, increased the binary size unexpectedly.

That time has passed. Now we largely operate on loader symbols,
and it is not needed to mark them reachable anymore.

Fixes #40416.

Change-Id: I1e2bdb93a960ba7dc96575fabe15af93d8e95329
Reviewed-on: https://go-review.googlesource.com/c/go/+/244839
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-27 16:04:55 +00:00
Ian Lance Taylor
8696ae82c9 syscall: use correct file descriptor in dup2 fallback path
This fixes a mistake in CL 220422. This changes code that is only
executed on Linux kernel versions earlier than 2.6.27.

Change-Id: I01280184f4d7b75e06387c38f1891e8f0a81f793
Reviewed-on: https://go-review.googlesource.com/c/go/+/244630
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
2020-07-25 20:13:27 +00:00
Ian Lance Taylor
9591515f51 runtime, sync: add copyright headers to new files
For #38029

Change-Id: I71de2b66c1de617d32c46d4f2c1866f9ff1756ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/244631
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-25 03:26:17 +00:00
Cherry Zhang
4b3cfcee58 [dev.link] cmd/link: remove non-streaming external relocation code
Now we support streaming external relocations everywhere.

Change-Id: I8d107c8239fe979bd5410e6a7f3fe471ac3e8b35
Reviewed-on: https://go-review.googlesource.com/c/go/+/244764
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-24 19:08:19 +00:00
Cherry Zhang
4c544ddaea [dev.link] all: merge branch 'master' into dev.link
Clean merge.

Change-Id: Ied3a0a9d098ab8dd28a38fdfb36c1f17b004fef7
2020-07-24 13:18:51 -04:00
Filippo Valsorda
074f2d800f doc/go1.15: surface the crypto/x509 CommonName deprecation note
Updates #39568
Updates #37419
Updates #24151

Change-Id: I44c940e09e26a039076396bbfecb2b1574197cf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/243221
Reviewed-by: Kevin Burke <kev@inburke.com>
2020-07-24 17:09:32 +00:00
Austin Clements
78c20c81aa doc/go1.15: announce GO386=387 deprecation
For #40255.
Updates #37419.

Change-Id: If9210c855cc2eea079e7e469463d4203888748f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/243137
Reviewed-by: Andrew Bonventre <andybons@golang.org>
2020-07-23 18:07:28 +00:00
Cherry Zhang
9d22325681 [dev.link] cmd/link: stream external relocations on PPC64
Both ELF and Xcoff.

Now we support streaming on all platforms. Later CLs will clean
up the old code.

Change-Id: Ieeef7844a3e229429983a8bc108d7f3fabf618e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/244358
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-23 16:44:27 +00:00
Cherry Zhang
e4a3e57f47 [dev.link] cmd/link: stream external relocations on ARM and on Windows
Do them in the same CL so ARM's archreloc doesn't need to support
both streaming and non-streaming.

TODO: we haven't switched to using mmap to emit external
relocations on Windows.

Change-Id: Ica2ee89c03fc74839efd6b9e26c80585fcdce45c
Reviewed-on: https://go-review.googlesource.com/c/go/+/244357
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-23 16:38:56 +00:00
Cherry Zhang
71e2133304 [dev.link] cmd/link: stream external relocations on S390X
All the bits are there. Just need to enable it.

Change-Id: I12ee25317c6385838493dadc900fb57c3b49a416
Reviewed-on: https://go-review.googlesource.com/c/go/+/244277
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-23 16:38:43 +00:00
Cherry Zhang
ed6b8af509 [dev.link] cmd/link: stream external relocations on MIPS (32/64)
Change-Id: I47fbeb3a49754395dceff51af371638fd43350ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/244097
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-23 16:38:36 +00:00
Cherry Zhang
ea708dc94c [dev.link] cmd/link: avoid reading symbol Data in archreloc on ARM64
The previous CL changed it to drop the Data when writing out a
symbol. Don't read the data.

Fix ARM64 build.

Change-Id: I121e9b0ebef123dbbc4ddffc02bf1a42788532f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/244038
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-23 16:38:27 +00:00
Ian Lance Taylor
71218dbc40 runtime: don't mlock on Ubuntu 5.4 systems
For #35777
For #37436
Fixes #40184

Change-Id: I68561497d9258e994d1c6c48d4fb41ac6130ee3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/244059
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2020-07-22 21:31:19 +00:00
Cherry Zhang
ee8541e5b8 [dev.link] cmd/link: remove OutData
OutData was used for a symbol to point to its data in the output
buffer, in order to apply relocations. Now we fold relocation
application to Asmb next to symbol data writing. We can just pass
the output data as a local variable.

Linking cmd/compile,

name         old time/op    new time/op    delta
Asmb_GC        19.0ms ±10%    16.6ms ± 9%  -12.50%  (p=0.032 n=5+5)

name         old alloc/op   new alloc/op   delta
Asmb_GC        3.78MB ± 0%    0.14MB ± 1%  -96.41%  (p=0.008 n=5+5)

name         old live-B     new live-B     delta
Asmb_GC         27.5M ± 0%     23.9M ± 0%  -13.24%  (p=0.008 n=5+5)

Change-Id: Id870a10dce2a0a7447a05029c6d0ab39b47d0a12
Reviewed-on: https://go-review.googlesource.com/c/go/+/244017
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-21 21:23:44 +00:00
Cherry Zhang
bf1816c7b7 [dev.link] cmd/link: stream external relocations on ARM64 and on Darwin
Support streaming external relocations on ARM64. Support
architecture-specific relocations.

Also support streaming external relocations on Darwin. Do it in
the same CL so ARM64's archreloc doesn't need to support both
streaming and non-streaming.

Change-Id: Ia7fee9957892f98c065022c69a51f47402f4d6e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/243644
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-21 21:15:02 +00:00
Cherry Zhang
936a2d6966 [dev.link] cmd/link: stream external relocations on 386 ELF
Change-Id: I17ff3ac82c8ac313f3a3c8e8129800ec9c05b991
Reviewed-on: https://go-review.googlesource.com/c/go/+/243643
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-21 21:14:28 +00:00
Cherry Zhang
0e3114871e [dev.link] cmd/link: fix hash collision check
For content-addressable symbols, we build its content hash based
on the symbol data and relocations. When the compiler builds the
symbol data, it may not always include the trailing zeros, e.g.
the data of [10]int64{1,2,3} is only the first 24 bytes.
Therefore, we may end up with symbols with the same contents
(thus same hash) but different sizes. This is not actually a hash
collision. In this case, we can deduplicate them and keep the one
with the larger size.

Change-Id: If6834542d7914cc00f917d7db151955e5aee6f30
Reviewed-on: https://go-review.googlesource.com/c/go/+/243718
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-21 21:14:19 +00:00
Alberto Donizetti
11f92e9dae cmd/compile: add test for fixed ICE on untyped conversion
The ICE reported as #33308 was fixed by a related CL; this change adds
a regression test with the crasher.

Fixes #33308

Change-Id: I3260075dbe3823b56b8825e6269e57a0fad185a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/243458
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-07-20 18:18:56 +00:00
Cherry Zhang
526d99a49a [dev.link] cmd/internal/obj: handle content-addressable symbols with relocations
For content-addressable symbols with relocations, we build a
content hash based on its content and relocations. Depending on
the category of the referenced symbol, we choose different hash
algorithms such that the hash is globally consistent.

For now, we only support content-addressable symbols with
relocations when the current package's import path is known, so
that the symbol names are fully expanded. Otherwise, if the
referenced symbol is a named symbol whose name is not fully
expanded, the hash won't be globally consistent, and can cause
erroneous collisions. This is fine for now, as the deduplication
is just an optimization, not a requirement for correctness (until
we get to type descriptors).

Change-Id: I639e4e03dd749b5d71f0a55c2525926575b1ac30
Reviewed-on: https://go-review.googlesource.com/c/go/+/243142
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-20 17:26:32 +00:00
Cherry Zhang
289c238a33 [dev.link] cmd/compile: make read-only static temps content-addressable
For now, we only do this for symbols without relocations.

Mark static temps "local", as they are not referenced across DSO
boundaries. And deduplicating a local symbol and a non-local
symbol can be problematic.

Change-Id: I0a3dc4138aaeea7fd4f326998f32ab6305da8e4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/243141
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-20 17:26:05 +00:00
Brian Kessler
0951939fd9 doc/go1.15: add release notes for math/cmplx
Updates #37419

Change-Id: Id7c9aba518c826c1a6fccbbf82210072bd3346f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/242903
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-07-17 19:09:40 +00:00
Cherry Zhang
3dabaa44e8 [dev.link] all: merge branch 'master' into dev.link
Change-Id: I6545cb431e9e3efa02defca52af7eae502adb157
2020-07-17 14:56:36 -04:00
Dmitri Shuralyov
6f264801a7 go/printer: remove exported StdFormat flag
The StdFormat flag was added as part of CL 231461, where the primary aim
was to fix the bug #37476. It's expected that the existing printer modes
only adjust spacing but do not change any of the code text itself. A new
printing flag served as a way for cmd/gofmt and go/format to delegate
a part of formatting work to the printer—where it's more more convenient
and efficient to perform—while maintaining current low-level printing
behavior of go/printer unmodified.

We already have cmd/gofmt and the go/format API that implement standard
formatting of Go source code, so there isn't a need to expose StdFormat
flag to the world, as it can only cause confusion.

Consider that to format source in canonical gofmt style completely it
may require tasks A, B, C to be done. In one version of Go, the printer
may do both A and B, while cmd/gofmt and go/format will do the remaining
task C. In another version, the printer may take on doing just A, while
cmd/gofmt and go/format will perform B and C. This makes it hard to add
a gofmt-like mode to the printer without compromising on above fluidity.

This change prefers to shift back some complexity to the implementation
of the standard library, allowing us to avoid creating the new exported
printing flag just for the internal needs of gofmt and go/format today.

We may still want to re-think the API and consider if something better
should be added, but unfortunately there isn't time for Go 1.15. We are
not adding new APIs now, so we can defer this decision until Go 1.16 or
later, when there is more time.

For #37476.
For #37453.
For #39489.
For #37419.

Change-Id: I0bb07156dca852b043487099dcf05c5350b29e20
Reviewed-on: https://go-review.googlesource.com/c/go/+/240683
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2020-07-17 02:15:01 +00:00
Russ Cox
4469f5446a compress/flate: fix another deflate Reset inconsistency
While investigating #34121, fixed by CL 193605,
I discovered another case where Reset was not quite
resetting enough.

This specific case is not a problem in Reset itself but
rather that the Huffman bit writer in one code path
is using uninitialized memory left over from a previous
block, making the compression not choose the optimal
compression method.

Fixes #34121.

Change-Id: I29245b28214d924e382f91e2c56b4b8a9b7da13d
Reviewed-on: https://go-review.googlesource.com/c/go/+/243140
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-07-16 23:38:32 +00:00
Klaus Post
8d4330742c compress/flate: fix deflate Reset consistency
Modify the overflow detection logic to shuffle the contents
of the table to a lower offset to avoid leaking the effects
of a previous use of compress.Writer past Reset calls.

Fixes #34121

Change-Id: I9963eadfa5482881e7b7adbad4c2cae146b669ab
GitHub-Last-Rev: 8b35798cdd
GitHub-Pull-Request: golang/go#34128
Reviewed-on: https://go-review.googlesource.com/c/go/+/193605
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-16 20:54:37 +00:00
Cherry Zhang
88382a9f97 [dev.link] cmd/link: stream out external relocations on AMD64 ELF
Currently, when external linking, in relocsym (in asmb pass), we
convert Go relocations to an in-memory representation of external
relocations, and then in asmb2 pass we write them out to the
output file. This is not memory efficient.

This CL makes it not do the conversion but directly stream out
the external relocations based on Go relocations. Currently only
do this on AMD64 ELF systems.

This reduces memory usage, but makes the asmb2 pass a little
slower.

Linking cmd/compile with external linking:

name             old time/op    new time/op    delta
Asmb_GC            83.8ms ± 7%    70.4ms ± 4%  -16.03%  (p=0.008 n=5+5)
Asmb2_GC           95.6ms ± 4%   118.2ms ± 5%  +23.65%  (p=0.008 n=5+5)
TotalTime_GC        1.59s ± 2%     1.62s ± 1%     ~     (p=0.151 n=5+5)

name             old alloc/op   new alloc/op   delta
Asmb_GC            26.0MB ± 0%     4.1MB ± 0%  -84.15%  (p=0.008 n=5+5)
Asmb2_GC           8.19MB ± 0%    8.18MB ± 0%     ~     (p=0.222 n=5+5)

name             old live-B     new live-B     delta
Asmb_GC             49.2M ± 0%     27.4M ± 0%  -44.38%  (p=0.008 n=5+5)
Asmb2_GC            51.5M ± 0%     29.7M ± 0%  -42.33%  (p=0.008 n=5+5)

TODO: figure out what is slow. Possible improvements:
- Remove redundant work in relocsym.
- Maybe there is a better representation for external relocations
  now.
- Fine-grained parallelism in emitting external relocations.
- The old elfrelocsect only iterates over external relocations,
  now we iterate over all relocations. Is it too many?

Change-Id: Ib0a8ee8c88d65864c62b89a8d634614f7f2c813e
Reviewed-on: https://go-review.googlesource.com/c/go/+/242603
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-16 20:26:39 +00:00
Cherry Zhang
4f217d5aaa [dev.link] cmd/internal/goobj2, cmd/link: use short hash function for short symbols
For symbols of size 8 bytes or below, we can map them to 64-bit
hash values using the identity function. There is no need to use
longer and more expensive hash functions.

For them, we introduce another pseudo-package, PkgIdxHashed64. It
is like PkgIdxHashed except that the hash function is different.

Note that the hash value is not affected with trailing zeros,
e.g. "A" and "A\0\0\0" have the same hash value. This allows
deduplicating a few more symbols. When deduplicating them, we
need to keep the longer one.

Change-Id: Iad0c2e9e569b6a59ca6a121fb8c8f0c018c6da03
Reviewed-on: https://go-review.googlesource.com/c/go/+/242362
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-16 18:45:34 +00:00
Cherry Zhang
3c54069907 [dev.link] cmd/internal/obj: make integer/float constant symbols content-addressable
Fill in the data at compile time, and get rid of the preprocess
function in the linker.

We need to be careful with symbol alignment: data symbols are
generally naturally aligned, except for string symbols which are
not aligned. When deduplicating two symbols with same content but
different alignments, we need to keep the biggest alignment.

Change-Id: I4bd96adfdc5f704b5bf3a0e723457c9bfe16a684
Reviewed-on: https://go-review.googlesource.com/c/go/+/242081
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-16 18:44:36 +00:00
Cherry Zhang
17344d55d6 [dev.link] cmd/compile: make GC map symbols content-addressable
Change-Id: I20e5b580b3e0505473816fe7f277a74e13d33e64
Reviewed-on: https://go-review.googlesource.com/c/go/+/242080
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-16 18:44:15 +00:00
Cherry Zhang
27342e5bd9 [dev.link] cmd/internal/goobj2, cmd/link: add content addressable symbols
This CL introduces content-addressable symbols (a.k.a. hashed
symbols) to object files. Content-addressable symbols are
identified and referenced by their content hashes, instead of by
names.

In the object file, a new pseudo-package index PkgIdxHashed is
introduced, for content-addressable symbols, and a new block is
added to store their hashes. The hashes are used by the linker to
identify and deduplicate the symbols.

For now, we only support content-addressable symbols that are
always locally defined (i.e. no cross-package references).

As a proof of concept, make string constant symbols content-
addressable.

Change-Id: Iaf53efd74c0ffb54fa95f784628cc84e95844536
Reviewed-on: https://go-review.googlesource.com/c/go/+/242079
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-07-16 18:44:07 +00:00
Cherry Zhang
c5d7f2f1cb cmd/link: skip TestOldLink if the old linker does not exist
We don't ship the old linker in binary releases. Skip the test if
we cannot find the old linker.

Fixes #39509.

Change-Id: I1af5552bc56aff5314a384bcb5f3717b725d68e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/242604
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2020-07-15 00:59:51 +00:00