summaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
...
* runtime: make sysMemStats' methods nosplitMichael Anthony Knyszek2020-10-261-0/+6
| | | | | | | | | | | | | | | | | | sysMemStats are updated early on in runtime initialization, so triggering a stack growth would be bad. Mark them nosplit. Thank you so much to cherryyz@google.com for finding this fix! Fixes #42218. Change-Id: Ic62db76e6a4f829355d7eaabed1727c51adfbd0f Reviewed-on: https://go-review.googlesource.com/c/go/+/265157 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org>
* cmd/internal/obj/{arm,s390x}: make return jump print nicerCherry Zhang2020-10-262-2/+6
| | | | | | | | | | | | | | | | | | | | | When a function with non-zero frame size makes a return jump (RET target), it assembles to, conceptually, MOV (SP), LR ADD $framesize, SP JMP target We did not clear some fields in the first instruction's Prog.To, causing it printed like (on ARM) MOVW.P 4(R13), (R14)(R14)(REG) Clear the fields to make it print nicer. Change-Id: I180901aeea41f1ff287d7c6034a6d69005927744 Reviewed-on: https://go-review.googlesource.com/c/go/+/264343 Trust: Cherry Zhang <cherryyz@google.com> Reviewed-by: Joel Sing <joel@sing.id.au>
* runtime,runtime/metrics: add heap goal and GC cycle metricsMichael Anthony Knyszek2020-10-264-8/+86
| | | | | | | | | | | | | | | This change adds three new metrics: the heap goal, GC cycle count, and forced GC count. These metrics are identical to their MemStats counterparts. For #37112. Change-Id: I5a5e8dd550c0d646e5dcdbdf38274895e27cdd88 Reviewed-on: https://go-review.googlesource.com/c/go/+/247044 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime,runtime/metrics: add heap object count metricMichael Anthony Knyszek2020-10-264-2/+26
| | | | | | | | | | | For #37112. Change-Id: Idd3dd5c84215ddd1ab05c2e76e848aa0a4d40fb0 Reviewed-on: https://go-review.googlesource.com/c/go/+/247043 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: add readMetrics latency benchmarkMichael Anthony Knyszek2020-10-262-5/+50
| | | | | | | | | | | | | | | | This change adds a new benchmark to the runtime tests for measuring the latency of the new metrics implementation, based on the ReadMemStats latency benchmark. readMetrics will have more metrics added to it in the future, and this benchmark will serve as a way to measure the cost of adding additional metrics. Change-Id: Ib05e3ed4afa49a70863fc0c418eab35b72263e24 Reviewed-on: https://go-review.googlesource.com/c/go/+/247042 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime,runtime/metrics: add memory metricsMichael Anthony Knyszek2020-10-269-6/+781
| | | | | | | | | | | | | | | | | | This change adds support for a variety of runtime memory metrics and contains the base implementation of Read for the runtime/metrics package, which lives in the runtime. It also adds testing infrastructure for the metrics package, and a bunch of format and documentation tests. For #37112. Change-Id: I16a2c4781eeeb2de0abcb045c15105f1210e2d8a Reviewed-on: https://go-review.googlesource.com/c/go/+/247041 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Trust: Michael Knyszek <mknyszek@google.com>
* runtime: move malloc stats into consistentHeapStatsMichael Anthony Knyszek2020-10-266-109/+90
| | | | | | | | | | | | | | | | | | | This change moves the mcache-local malloc stats into the consistentHeapStats structure so the malloc stats can be managed consistently with the memory stats. The one exception here is tinyAllocs for which moving that into the global stats would incur several atomic writes on the fast path. Microbenchmarks for just one CPU core have shown a 50% loss in throughput. Since tiny allocation counnt isn't exposed anyway and is always blindly added to both allocs and frees, let that stay inconsistent and flush the tiny allocation count every so often. Change-Id: I2a4b75f209c0e659b9c0db081a3287bf227c10ca Reviewed-on: https://go-review.googlesource.com/c/go/+/247039 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: replace some memstats with consistent statsMichael Anthony Knyszek2020-10-262-29/+62
| | | | | | | | | | | | | | This change replaces stacks_inuse, gcWorkBufInUse and gcProgPtrScalarBitsInUse with their corresponding consistent stats. It also adds checks to make sure the rest of the sharded stats line up with existing stats in updatememstats. Change-Id: I17d0bd181aedb5c55e09c8dff18cef5b2a3a14e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/247038 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: add consistent heap statisticsMichael Anthony Knyszek2020-10-264-3/+230
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This change adds a global set of heap statistics which are similar to existing memory statistics. The purpose of these new statistics is to be able to read them and get a consistent result without stopping the world. The goal is to eventually replace as many of the existing memstats statistics with the sharded ones as possible. The consistent memory statistics use a tailor-made synchronization mechanism to allow writers (allocators) to proceed with minimal synchronization by using a sequence counter and a global generation counter to determine which set of statistics to update. Readers increment the global generation counter to effectively grab a snapshot of the statistics, and then iterate over all Ps using the sequence counter to ensure that they may safely read the snapshotted statistics. To keep statistics fresh, the reader also has a responsibility to merge sets of statistics. These consistent statistics are computed, but otherwise unused for now. Upcoming changes will integrate them with the rest of the codebase and will begin to phase out existing statistics. Change-Id: I637a11f2439e2049d7dccb8650c5d82500733ca5 Reviewed-on: https://go-review.googlesource.com/c/go/+/247037 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime/metrics: add package interfaceMichael Anthony Knyszek2020-10-266-0/+232
| | | | | | | | | | | | | | This change creates the runtime/metrics package and adds the initial interface as laid out in the design document. For #37112. Change-Id: I202dcee08ab008dd63bf96f7a4162f5b5f813637 Reviewed-on: https://go-review.googlesource.com/c/go/+/247040 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: add helper for getting an mcache in allocation contextsMichael Anthony Knyszek2020-10-262-23/+25
| | | | | | | | | | | | | | This change adds a function getMCache which returns the current P's mcache if it's available, and otherwise tries to get mcache0 if we're bootstrapping. This function will come in handy as we need to replicate this behavior in multiple places in future changes. Change-Id: I536073d6f6dc6c6390269e613ead9f8bcb6e7f98 Reviewed-on: https://go-review.googlesource.com/c/go/+/246976 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: remove memstats.heap_allocMichael Anthony Knyszek2020-10-262-9/+6
| | | | | | | | | | | | | memstats.heap_alloc is 100% a duplicate and unnecessary copy of memstats.alloc which exists because MemStats used to be populated from memstats via a memmove. Change-Id: I995489f61be39786e573b8494a8ab6d4ea8bed9c Reviewed-on: https://go-review.googlesource.com/c/go/+/246975 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: remove memstats.heap_idleMichael Anthony Knyszek2020-10-263-6/+18
| | | | | | | | | | | | | | | | | | | | | This statistic is updated in many places but for MemStats may be computed from existing statistics. Specifically by definition heap_idle = heap_sys - heap_inuse since heap_sys is all memory allocated from the OS for use in the heap minus memory used for non-heap purposes. heap_idle is almost the same (since it explicitly includes memory that *could* be used for non-heap purposes) but also doesn't include memory that's actually used to hold heap objects. Although it has some utility as a sanity check, it complicates accounting and we want fewer, orthogonal statistics for upcoming metrics changes, so just drop it. Change-Id: I40af54a38e335f43249f6e218f35088bfd4380d1 Reviewed-on: https://go-review.googlesource.com/c/go/+/246974 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: break down memstats.gc_sysMichael Anthony Knyszek2020-10-267-27/+39
| | | | | | | | | | | | | | | This change breaks apart gc_sys into three distinct pieces. Two of those pieces are pieces which come from heap_sys since they're allocated from the page heap. The rest comes from memory mapped from e.g. persistentalloc which better fits the purpose of a sysMemStat. Also, rename gc_sys to gcMiscSys. Change-Id: I098789170052511e7b31edbcdc9a53e5c24573f7 Reviewed-on: https://go-review.googlesource.com/c/go/+/246973 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: copy in MemStats fields explicitlyMichael Anthony Knyszek2020-10-261-23/+47
| | | | | | | | | | | | | | Currently MemStats is populated via an unsafe memmove from memstats, but this places unnecessary structural restrictions on memstats, is annoying to reason about, and tightly couples the two. Instead, just populate the fields of MemStats explicitly. Change-Id: I96f6a64326b1a91d4084e7b30169a4bbe6a331f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/246972 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: delineate which memstats are system stats with a typeMichael Anthony Knyszek2020-10-2617-130/+109
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This change modifies the type of several mstats fields to be a new type: sysMemStat. This type has the same structure as the fields used to have. The purpose of this change is to make it very clear which stats may be used in various functions for accounting (usually the platform-specific sys* functions, but there are others). Currently there's an implicit understanding that the *uint64 value passed to these functions is some kind of statistic whose value is atomically managed. This understanding isn't inherently problematic, but we're about to change how some stats (which currently use mSysStatInc and mSysStatDec) work, so we want to make it very clear what the various requirements are around "sysStat". This change also removes mSysStatInc and mSysStatDec in favor of a method on sysMemStat. Note that those two functions were originally written the way they were because atomic 64-bit adds required a valid G on ARM, but this hasn't been the case for a very long time (since golang.org/cl/14204, but even before then it wasn't clear if mutexes required a valid G anymore). Today we implement 64-bit adds on ARM with a spinlock table. Change-Id: I4e9b37cf14afc2ae20cf736e874eb0064af086d7 Reviewed-on: https://go-review.googlesource.com/c/go/+/246971 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: make the span allocation purpose more explicitMichael Anthony Knyszek2020-10-264-30/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This change modifies mheap's span allocation API to have each caller declare a purpose, defined as a new enum called spanAllocType. The purpose behind this change is two-fold: 1. Tight control over who gets to allocate heap memory is, generally speaking, a good thing. Every codepath that allocates heap memory places additional implicit restrictions on the allocator. A notable example of a restriction is work bufs coming from heap memory: write barriers are not allowed in allocation paths because then we could have a situation where the allocator calls into the allocator. 2. Memory statistic updating is explicit. Instead of passing an opaque pointer for statistic updating, which places restrictions on how that statistic may be updated, we use the spanAllocType to determine which statistic to update and how. We also take this opportunity to group all the statistic updating code together, which should make the accounting code a little easier to follow. Change-Id: Ic0b0898959ba2a776f67122f0e36c9d7d60e3085 Reviewed-on: https://go-review.googlesource.com/c/go/+/246970 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: rename mcache fields to match Go styleMichael Anthony Knyszek2020-10-267-71/+71
| | | | | | | | | | | | | | This change renames a bunch of malloc statistics stored in the mcache that are all named with the "local_" prefix. It also renames largeAlloc to allocLarge to prevent a naming conflict, and next_sample because it would be the last mcache field with the old C naming style. Change-Id: I29695cb83b397a435ede7e9ad5c3c9be72767ea3 Reviewed-on: https://go-review.googlesource.com/c/go/+/246969 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: flush local_scan directly and more oftenMichael Anthony Knyszek2020-10-264-75/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | Now that local_scan is the last mcache-based statistic that is flushed by purgecachedstats, and heap_scan and gcController.revise may be interacted with concurrently, we don't need to flush heap_scan at arbitrary locations where the heap is locked, and we don't need purgecachedstats and cachestats anymore. Instead, we can flush local_scan at the same time we update heap_live in refill, so the two updates may share the same revise call. Clean up unused functions, remove code that would cause the heap to get locked in the allocSpan when it didn't need to (other than to flush local_scan), and flush local_scan explicitly in a few important places. Notably we need to flush local_scan whenever we flush the other stats, but it doesn't need to be donated anywhere, so have releaseAll do the flushing. Also, we need to flush local_scan before we set heap_scan at the end of a GC, which was previously handled by cachestats. Just do so explicitly -- it's not much code and it becomes a lot more clear why we need to do so. Change-Id: I35ac081784df7744d515479896a41d530653692d Reviewed-on: https://go-review.googlesource.com/c/go/+/246968 Run-TryBot: Michael Knyszek <mknyszek@google.com> Trust: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: don't flush local_tinyallocsMichael Anthony Knyszek2020-10-264-11/+14
| | | | | | | | | | | | | This change makes local_tinyallocs work like the rest of the malloc stats and doesn't flush local_tinyallocs, instead making that the source-of-truth. Change-Id: I3e6cb5f1b3d086e432ce7d456895511a48e3617a Reviewed-on: https://go-review.googlesource.com/c/go/+/246967 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: remove mcentral.nmalloc and add mcache.local_nsmallallocMichael Anthony Knyszek2020-10-263-52/+41
| | | | | | | | | | | | | | | | | | | | | | This change removes mcentral.nmalloc and adds mcache.local_nsmallalloc which fulfills the same role but may be accessed non-atomically. It also moves responsibility for updating heap_live and local_nsmallalloc into mcache functions. As a result of this change, mcache is now the sole source-of-truth for malloc stats. It is also solely responsible for updating heap_live and performing the various operations required as a result of updating heap_live. The overall improvement here is in code organization: previously malloc stats were fairly scattered, and now they have one single home, and nearly all the required manipulations exist in a single file. Change-Id: I7e93fa297c1debf17e3f2a0d68aeed28a9c6af00 Reviewed-on: https://go-review.googlesource.com/c/go/+/246966 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: make nlargealloc and largealloc mcache fieldsMichael Anthony Knyszek2020-10-264-54/+55
| | | | | | | | | | | | | | | | | | | | | | | This change makes nlargealloc and largealloc into mcache fields just like nlargefree and largefree. These local fields become the new source-of-truth. This change also moves the accounting for these fields out of allocSpan (which is an inappropriate place for it -- this accounting generally happens much closer to the point of allocation) and into largeAlloc. This move is partially possible now that we can call gcController.revise at that point. Furthermore, this change moves largeAlloc into mcache.go and makes it a method of mcache. While there's a little bit of a mismatch here because largeAlloc barely interacts with the mcache, it helps solidify the mcache as the first allocation layer and provides a clear place to aggregate and manage statistics. Change-Id: I37b5e648710733bb4c04430b71e96700e438587a Reviewed-on: https://go-review.googlesource.com/c/go/+/246965 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: make distributed/local malloc stats the source-of-truthMichael Anthony Knyszek2020-10-265-38/+69
| | | | | | | | | | | | | | | | | | | | This change makes it so that various local malloc stats (excluding heap_scan and local_tinyallocs) are no longer written first to mheap fields but are instead accessed directly from each mcache. This change is part of a move toward having stats be distributed, and cleaning up some old code related to the stats. Note that because there's no central source-of-truth, when an mcache dies, it must donate its stats to another mcache. It's always safe to donate to the mcache for the 0th P, so do that. Change-Id: I2556093dbc27357cb9621c9b97671f3c00aa1173 Reviewed-on: https://go-review.googlesource.com/c/go/+/246964 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: access the assist ratio atomicallyMichael Anthony Knyszek2020-10-263-15/+56
| | | | | | | | | | | | | | | | | | | This change makes it so that the GC assist ratio (the pair of gcControllerState fields assistBytesPerWork and assistWorkPerByte) is updated atomically. Note that the pair of fields are not updated together atomically, but that's OK. The code here was already racy for some time and in practice the assist ratio moves very slowly. The purpose of this change is so that we can document gcController.revise to be safe for concurrent use, which will be useful in further changes. Change-Id: Ie25d630207c88e4f85f2b8953f6a0051ebf1b4ea Reviewed-on: https://go-review.googlesource.com/c/go/+/246963 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: make next_gc atomically accessedMichael Anthony Knyszek2020-10-264-11/+19
| | | | | | | | | | | | | | | | | | | next_gc is mostly updated only during a STW, but may occasionally be updated by calls to e.g. debug.SetGCPercent. In this case the update is supposed to be protected by the heap lock, but in reality it's accessed by gcController.revise which may be called without the heap lock held (despite its documentation, which will be updated in a later change). Change the synchronization policy on next_gc so that it's atomically accessed when the world is not stopped to aid in making revise safe for concurrent use. Change-Id: I79657a72f91563f3241aaeda66e8a7757d399529 Reviewed-on: https://go-review.googlesource.com/c/go/+/246962 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: load gcControllerState.scanWork atomically in reviseMichael Anthony Knyszek2020-10-261-2/+3
| | | | | | | | | | | | | | | | | | | | gcControllerState.scanWork's docs state that it must be accessed atomically during a GC cycle, but gcControllerState.revise does not do this (even when called with the heap lock held). This change makes it so that gcControllerState.revise accesses scanWork atomically and explicitly. Note that we don't update gcControllerState.revise's erroneous doc comment here because this change isn't about revise's guarantees, just about heap_scan. The comment is updated in a later change. Change-Id: Iafc3ad214e517190bfd8a219896d23da19f7659d Reviewed-on: https://go-review.googlesource.com/c/go/+/246961 Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: define and enforce synchronization on heap_scanMichael Anthony Knyszek2020-10-263-5/+8
| | | | | | | | | | | | | | | | | | | Currently heap_scan is mostly protected by the heap lock, but gcControllerState.revise sometimes accesses it without a lock. In an effort to make gcControllerState.revise callable from more contexts (and have its synchronization guarantees actually respected), make heap_scan atomically read from and written to, unless the world is stopped. Note that we don't update gcControllerState.revise's erroneous doc comment here because this change isn't about revise's guarantees, just about heap_scan. The comment is updated in a later change. Change-Id: Iddbbeb954767c704c2bd1d221f36e6c4fc9948a6 Reviewed-on: https://go-review.googlesource.com/c/go/+/246960 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* cmd/go/internal/fsys: rewrite non-idiomatic if statementsRuss Cox2020-10-261-10/+14
| | | | | | | | | | | | https://golang.org/doc/effective_go.html#if Change-Id: I4d868e05c7827638f45b3b06d8762f5a298d56f7 Reviewed-on: https://go-review.googlesource.com/c/go/+/264537 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
* runtime: fix sub-uintptr-sized Windows callback argumentsAustin Clements2020-10-262-39/+136
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The Windows callback support accepts Go functions with arguments that are uintptr-sized or smaller. However, it doesn't implement smaller arguments correctly. It assumes the Windows arguments layout is equivalent to the Go argument layout. This is often true, but because Windows C ABIs pad arguments to word size, while Go packs arguments, the layout is different if there are multiple sub-word-size arguments in a row. For example, a function with two uint16 arguments will have a two-word C argument frame, but only a 4 byte Go argument frame. There are also subtleties surrounding floating-point register arguments that it doesn't handle correctly. To fix this, when constructing a callback, we examine the Go function's signature to construct a mapping between the C argument frame and the Go argument frame. When the callback is invoked, we use this mapping to build the Go argument frame and copy the result back. This adds several test cases to TestStdcallAndCDeclCallbacks that exercise more complex function signatures. These all fail with the current code, but work with this CL. In addition to fixing these callback types, this is also a step toward the Go register ABI (#40724), which is going to make the ABI translation more complex. Change-Id: I19fb1681b659d9fd528ffd5e88912bebb95da052 Reviewed-on: https://go-review.googlesource.com/c/go/+/263271 Trust: Austin Clements <austin@google.com> Trust: Alex Brainman <alex.brainman@gmail.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
* runtime: tidy Windows callback testAustin Clements2020-10-261-134/+90
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This simplifies the systematic test of Windows callbacks with different signatures and prepares it for expanded coverage of function signatures. It now returns a result from the Go function and threads it back through C. This simplifies things, but also previously the code could have succeeded by simply not calling the callbacks at all (though other tests would have caught that). It bundles together the C function description and the Go function it's intended to call. Now the test source generation and the test running both loop over a single slice of test functions. Since the C function and Go function are now bundled, it generates the C function by reflectively inspecting the signature of the Go function. For the moment, we keep the same test suite, which is entirely functions with "uintptr" arguments, but we'll expand this shortly. It now use sub-tests. This way tests automatically get useful diagnostic labels in failures and the tests don't have to catch panics on their own. It eliminates the DLL function argument. I honestly couldn't figure out what the point of this was, and it added what appeared to be an unnecessary loop level to the tests. Change-Id: I120dfd4785057cc2c392bd2c821302f276bd128e Reviewed-on: https://go-review.googlesource.com/c/go/+/263270 Trust: Austin Clements <austin@google.com> Trust: Alex Brainman <alex.brainman@gmail.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
* runtime: tidy compileCallbackAustin Clements2020-10-262-41/+53
| | | | | | | | | | | | This makes a few minor cleanups and simplifications to compileCallback. Change-Id: Ibebf4b5ed66fb68bba7c84129c127cd4d8a691fe Reviewed-on: https://go-review.googlesource.com/c/go/+/263269 Trust: Austin Clements <austin@google.com> Trust: Alex Brainman <alex.brainman@gmail.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
* runtime: tidy cgocallbackAustin Clements2020-10-264-27/+19
| | | | | | | | | | | | | | | | | | | | On amd64 and 386, we have a very roundabout way of remembering that we need to dropm on return that currently involves saving a zero to needm's argument slot and later bringing it back. Just store the zero. This also makes amd64 and 386 more consistent with cgocallback on all other platforms: rather than saving the old M to the G stack, they now save it to a named slot on the G0 stack. The needm function no longer needs a dummy argument to get the SP, so we drop that. Change-Id: I7e84bb4a5ff9552de70dcf41d8accf02310535e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/263268 Trust: Austin Clements <austin@google.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
* runtime,cmd/cgo: simplify C -> Go call pathAustin Clements2020-10-2632-695/+470
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This redesigns the way calls work from C to exported Go functions. It removes several steps from the call path, makes cmd/cgo no longer sensitive to the Go calling convention, and eliminates the use of reflectcall from cgo. In order to avoid generating a large amount of FFI glue between the C and Go ABIs, the cgo tool has long depended on generating a C function that marshals the arguments into a struct, and then the actual ABI switch happens in functions with fixed signatures that simply take a pointer to this struct. In a way, this CL simply pushes this idea further. Currently, the cgo tool generates this argument struct in the exact layout of the Go stack frame and depends on reflectcall to unpack it into the appropriate Go call (even though it's actually reflectcall'ing a function generated by cgo). In this CL, we decouple this struct from the Go stack layout. Instead, cgo generates a Go function that takes the struct, unpacks it, and calls the exported function. Since this generated function has a generic signature (like the rest of the call path), we don't need reflectcall and can instead depend on the Go compiler itself to implement the call to the exported Go function. One complication is that syscall.NewCallback on Windows, which converts a Go function into a C function pointer, depends on cgocallback's current dynamic calling approach since the signatures of the callbacks aren't known statically. For this specific case, we continue to depend on reflectcall. Really, the current approach makes some overly simplistic assumptions about translating the C ABI to the Go ABI. Now we're at least in a much better position to do a proper ABI translation. For comparison, the current cgo call path looks like: GoF (generated C function) -> crosscall2 (in cgo/asm_*.s) -> _cgoexp_GoF (generated Go function) -> cgocallback (in asm_*.s) -> cgocallback_gofunc (in asm_*.s) -> cgocallbackg (in cgocall.go) -> cgocallbackg1 (in cgocall.go) -> reflectcall (in asm_*.s) -> _cgoexpwrap_GoF (generated Go function) -> p.GoF Now the call path looks like: GoF (generated C function) -> crosscall2 (in cgo/asm_*.s) -> cgocallback (in asm_*.s) -> cgocallbackg (in cgocall.go) -> cgocallbackg1 (in cgocall.go) -> _cgoexp_GoF (generated Go function) -> p.GoF Notably: 1. We combine _cgoexp_GoF and _cgoexpwrap_GoF and move the combined operation to the end of the sequence. This combined function also handles reflectcall's previous role. 2. We combined cgocallback and cgocallback_gofunc since the only purpose of having both was to convert a raw PC into a Go function value. We instead construct the Go function value in cgocallbackg1. 3. cgocallbackg1 no longer reaches backwards through the stack to get the arguments to cgocallback_gofunc. Instead, we just pass the arguments down. 4. Currently, we need an explicit msanwrite to mark the results struct as written because reflectcall doesn't do this. Now, the results are written by regular Go assignments, so the Go compiler generates the necessary MSAN annotations. This also means we no longer need to track the size of the arguments frame. Updates #40724, since now we don't need to teach cgo about the register ABI or change how it uses reflectcall. Change-Id: I7840489a2597962aeb670e0c1798a16a7359c94f Reviewed-on: https://go-review.googlesource.com/c/go/+/258938 Trust: Austin Clements <austin@google.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
* cmd/link: preserve alignment for stackmap symbolsCherry Zhang2020-10-261-2/+6
| | | | | | | | | | | | | | | | | Stackmap symbols are content-addressable, so it may be dedup'd with another symbol with same content. We want stackmap symbols 4-byte aligned. But if it dedup's with another symbol with larger alignment, preserve that alignment. Fixes #42071. Change-Id: I1616dd2b0c175b2aac8f68782a5c7a62053c0b57 Reviewed-on: https://go-review.googlesource.com/c/go/+/264897 Trust: Cherry Zhang <cherryyz@google.com> Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Joel Sing <joel@sing.id.au> Reviewed-by: Than McIntosh <thanm@google.com>
* net: prefer /etc/hosts over DNS when no /etc/nsswitch.conf is presentNatanael Copa2020-10-252-8/+10
| | | | | | | | | | | | | | | | | | | | | | Do not mimic glibc behavior if /etc/nsswitch.conf is missing. This will will likely be missing on musl libc systems and glibc systems will likely always have it, resulting in localhost lookup being done over DNS rather than from /etc/hosts. Do what makes most sense rather than making any assumption about the libc. Fixes #35305 Change-Id: I20bd7e24131bba8eaa39a20c8950fe552364784d GitHub-Last-Rev: 119409839d37c8c7268f5f6db19c1789d9d96074 GitHub-Pull-Request: golang/go#39685 Reviewed-on: https://go-review.googlesource.com/c/go/+/238629 Run-TryBot: Dan Peterson <dpiddy@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dan Peterson <dpiddy@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Trust: Emmanuel Odeke <emmanuel@orijtech.com>
* path/filepath: allow EvalSymlinks to work on UNC share roots on WindowsKevin Parsons2020-10-252-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes #42079 Previously, EvalSymlinks returned an error when called with the root of a UNC share (e.g. \\server\share). This was due to Windows's FindFirstFile function not supporting a share root path. To resolve this, now return early from toNorm in the case where the path after the volume name is empty. Skipping the later path component resolution shouldn't have any negative impact in this case, as if the path is empty, there aren't any path components to resolve anyways. The test case uses the localhost admin share (c$), as it should be present in most situations. This allows testing without setting up an external file share. However, this fix applies to all UNC share root paths. Change-Id: I05035bd86be93662d7bea34fab4b75fc8e918206 GitHub-Last-Rev: bd3db2cda65aae1cdf8d94b03bc7197dff68dc44 GitHub-Pull-Request: golang/go#42096 Reviewed-on: https://go-review.googlesource.com/c/go/+/263917 Trust: Alex Brainman <alex.brainman@gmail.com> Trust: Giovanni Bajo <rasky@develer.com> Run-TryBot: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
* log/syslog: set local to true if network is any of "unix", or "unixgram"imxyb2020-10-252-10/+28
| | | | | | | | | | | | | | Fixes #41960 Change-Id: I0e0f0e11610dd2658a8f6b7e345a4aae2c19c85d GitHub-Last-Rev: 8cac718e4854773ca411116043b4b832e0468f09 GitHub-Pull-Request: golang/go#42135 Reviewed-on: https://go-review.googlesource.com/c/go/+/264297 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Emmanuel Odeke <emm.odeke@gmail.com>
* cmd/dist: document why test fails on incomplete portsTobias Klauser2020-10-241-0/+3
| | | | | | | | | | | It might not be obvious from reading the code why we consider the test as failed on incomplete ports even though it passed. Add a comment documenting this behavior, as suggested by Dmitri in CL 155839. Change-Id: I3eb7db27d01d63db277172381e5fa51577dad941 Reviewed-on: https://go-review.googlesource.com/c/go/+/264682 Trust: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
* lib/time, time/tzdata: update tz data to 2020dTobias Klauser2020-10-241-6809/+6810
| | | | | | | | | | | | | | | See http://mm.icann.org/pipermail/tz-announce/2020-October/000060.html and http://mm.icann.org/pipermail/tz-announce/2020-October/000062.html for a description of the changes. Updates #22487 Change-Id: I4b2717d2d642284889345a0125eb6c614575c0ea Reviewed-on: https://go-review.googlesource.com/c/go/+/264681 Trust: Tobias Klauser <tobias.klauser@gmail.com> Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
* cmd/dist: add openbsd/mips64 as incomplete portJoel Sing2020-10-241-1/+3
| | | | | | | | | Update #40995 Change-Id: Id497f7688b00658b50feb7338157e0411b861910 Reviewed-on: https://go-review.googlesource.com/c/go/+/250578 Trust: Joel Sing <joel@sing.id.au> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
* cmd/compile: enforce strongly typed rules for ARM (read)Constantin Konstantinidis2020-10-242-23/+23
| | | | | | | | | | | | | | | Add type casting to offset. L246-L247 L1473-L1475 toolstash-check successful. Change-Id: I816c7556609ca6dd67bff8007c2d006cab89ee2b Reviewed-on: https://go-review.googlesource.com/c/go/+/257639 Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Trust: Alberto Donizetti <alb.donizetti@gmail.com>
* cmd/internal/obj/riscv: support additional register to register movesJoel Sing2020-10-242-26/+59
| | | | | | | | | | | | | | Add support for signed and unsigned register to register moves of various sizes. This makes it easier to handle zero and sign extension and will allow for further changes that improve the compiler optimisations for riscv64. While here, change the existing register to register moves from obj.Prog rewriting to instruction generation. Change-Id: Id21911019b76922367a134da13c3449a84a1fb08 Reviewed-on: https://go-review.googlesource.com/c/go/+/264657 Trust: Joel Sing <joel@sing.id.au> Reviewed-by: Cherry Zhang <cherryyz@google.com>
* debug/dwarf: add support for DWARFv5 to (*Data).RangesAlessandro Arzilli2020-10-245-59/+317
| | | | | | | | | | | | | | | | Updates the (*Data).Ranges method to work with DWARFv5 which uses the new debug_rnglists section instead of debug_ranges. This does not include supporting DW_FORM_rnglistx. General support for DWARFv5 was added by CL 175138. Change-Id: I01f919a865616a3ff12f5bf649c2c9abf89fcf52 Reviewed-on: https://go-review.googlesource.com/c/go/+/236657 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Trust: Emmanuel Odeke <emm.odeke@gmail.com>
* runtime: dump the status of lockedg on errorTiwei Bie2020-10-241-2/+2
| | | | | | | | | | | | | | | The dumpgstatus() will dump current g's status anyway. When lockedg's status is bad, it's more helpful to dump lockedg's status as well than dumping current g's status twice. Change-Id: If5248cb94b9cdcbf4ceea07562237e1d6ee28489 GitHub-Last-Rev: da814c51ff42f56fb28582f088f4d72b500061fe GitHub-Pull-Request: golang/go#40248 Reviewed-on: https://go-review.googlesource.com/c/go/+/243097 Reviewed-by: Keith Randall <khr@golang.org> Trust: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Go Bot <gobot@golang.org>
* syscall: disable TestAllThreadsSyscall on linux-ppc64Andrew G. Morgan2020-10-241-0/+6
| | | | | | | | | | | | | | | | | | For some reason, currently unknown, this test case fails exclusively on the linux-ppc64 platform. Until such time as it can be made to work, we'll disable this test case on that platform. The same issue causes TestSetuidEtc to fail too, so disable that on this platform. Updates #42178 Change-Id: Idd3f6c2ee9f2fba2eb8ce4de69de7f316858bb15 Reviewed-on: https://go-review.googlesource.com/c/go/+/264719 Trust: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
* net/http: fix typo in TestTransportReadToEndReusesConnKeiichi Hirobe2020-10-241-1/+1
| | | | | | | | | | | | The test sets a Content-Type where it looks like it wants a Content-Length. The test passes because the Content-Length header is automatically added anyway, but fix the typo and set Content-Length as intended. Change-Id: Ic2af778f82c3e9d58e164892f6ac6ef5745f884f Reviewed-on: https://go-review.googlesource.com/c/go/+/246977 Reviewed-by: Damien Neil <dneil@google.com> Trust: Alberto Donizetti <alb.donizetti@gmail.com> Trust: Damien Neil <dneil@google.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Go Bot <gobot@golang.org>
* crypto/x509: deprecate legacy PEM encryptionFilippo Valsorda2020-10-241-11/+24
| | | | | | | | | | | | | | | | | It's unfortunate that we don't implement PKCS#8 encryption (#8860) so we can't recommend an alternative but PEM encryption is so broken that it's worth deprecating outright. Fixes #41949 Fixes #32777 Change-Id: Ieb46444662adec108d0de3550b693a50545c2344 Reviewed-on: https://go-review.googlesource.com/c/go/+/264159 Trust: Filippo Valsorda <filippo@golang.org> Trust: Roland Shoemaker <roland@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
* crypto/tls: add no-shared to openssl build instructionsThom Wiggers2020-10-241-1/+1
| | | | | | | | | | | | | | | | This prevents the custom-built version of openssl prefering the system libraries over the ones compiled with the specified (weak crypto) options necessary to generate the updates. This difference can lead to confusing failures when updating the tests. Fixes #31809 Change-Id: I2dd257f3121d6c6c62c6aeba52e1c74046b3c584 GitHub-Last-Rev: 6d4eeafadf0b4671b7e17c6810f1a66a9fda7d3c GitHub-Pull-Request: golang/go#41630 Reviewed-on: https://go-review.googlesource.com/c/go/+/257517 Trust: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
* runtime: add tests for addrRanges.addMichael Anthony Knyszek2020-10-232-2/+163
| | | | | | | | | | Change-Id: I249deb482df74068b0538e9d773b9a87bc5a6df3 Reviewed-on: https://go-review.googlesource.com/c/go/+/242681 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: throw on zero-sized range passed to addrRanges.addMichael Anthony Knyszek2020-10-231-1/+7
| | | | | | | | | | | | | | | | | | addrRanges represents a set of addresses. Currently, passing in a zero-sized range will cause that range to be added to the list, even though it doesn't represent any address (addrRanges.contains will still always return false, and findSucc will give surprising results). We could ignore this input, but it's almost always a bug for the calling code to pass in a zero-sized range, so just throw. Change-Id: I8ed09e15b79a3a33e2d0cf5ed55f9e497388e7a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/242817 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Austin Clements <austin@google.com>