1. 05 Aug, 2015 6 commits
  2. 04 Aug, 2015 18 commits
    • Robert Griesemer's avatar
      reflect: fix doc string · 3cfc34a5
      Robert Griesemer authored
      Fixes #12017.
      
      Change-Id: I3dfcf9d0b62cae02eca1973383f0aad286a6ef4d
      Reviewed-on: https://go-review.googlesource.com/13136Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      3cfc34a5
    • Austin Clements's avatar
      runtime: fix typos in comments · be39a429
      Austin Clements authored
      Change-Id: I66f7937b22bb6e05c3f2f0f2a057151020ad9699
      Reviewed-on: https://go-review.googlesource.com/13049Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      be39a429
    • Austin Clements's avatar
      runtime: fix assist utilization computation · e3870aa6
      Austin Clements authored
      When commit 510fd135 enabled assists during the scan phase, it failed
      to also update the code in the GC controller that computed the assist
      CPU utilization and adjusted the trigger based on it. Fix that code so
      it uses the start of the scan phase as the wall-clock time when
      assists were enabled rather than the start of the mark phase.
      
      Change-Id: I05013734b4448c3e2c730dc7b0b5ee28c86ed8cf
      Reviewed-on: https://go-review.googlesource.com/13048Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      e3870aa6
    • Austin Clements's avatar
      runtime: revise assist ratio aggressively · 1fb01a88
      Austin Clements authored
      At the start of a GC cycle, the garbage collector computes the assist
      ratio based on the total scannable heap size. This was intended to be
      conservative; after all, this assumes the entire heap may be reachable
      and hence needs to be scanned. But it only assumes that the *current*
      entire heap may be reachable. It fails to account for heap allocated
      during the GC cycle. If the trigger ratio is very low (near zero), and
      most of the heap is reachable when GC starts (which is likely if the
      trigger ratio is near zero), then it's possible for the mutator to
      create new, reachable heap fast enough that the assists won't keep up
      based on the assist ratio computed at the beginning of the cycle. As a
      result, the heap can grow beyond the heap goal (by hundreds of megs in
      stress tests like in issue #11911).
      
      We already have some vestigial logic for dealing with situations like
      this; it just doesn't run often enough. Currently, every 10 ms during
      the GC cycle, the GC revises the assist ratio. This was put in before
      we switched to a conservative assist ratio (when we really were using
      estimates of scannable heap), and it turns out to be exactly what we
      need now. However, every 10 ms is far too infrequent for a rapidly
      allocating mutator.
      
      This commit reuses this logic, but replaces the 10 ms timer with
      revising the assist ratio every time the heap is locked, which
      coincides precisely with when the statistics used to compute the
      assist ratio are updated.
      
      Fixes #11911.
      
      Change-Id: I377b231ab064946228378fa10422a46d1b50f4c5
      Reviewed-on: https://go-review.googlesource.com/13047Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      1fb01a88
    • Austin Clements's avatar
      runtime: when gcpacertrace > 0, print information about assist ratio · f9dc3382
      Austin Clements authored
      This was useful in debugging the mutator assist behavior for #11911,
      and it fits with the other gcpacertrace output.
      
      Change-Id: I1e25590bb4098223a160de796578bd11086309c7
      Reviewed-on: https://go-review.googlesource.com/13046Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      f9dc3382
    • Austin Clements's avatar
      runtime: make sweep proportional to spans bytes allocated · fc9ca85f
      Austin Clements authored
      Proportional concurrent sweep is currently based on a ratio of spans
      to be swept per bytes of object allocation. However, proportional
      sweeping is performed during span allocation, not object allocation,
      in order to minimize contention and overhead. Since objects are
      allocated from spans after those spans are allocated, the system tends
      to operate in debt, which means when the next GC cycle starts, there
      is often sweep debt remaining, so GC has to finish the sweep, which
      delays the start of the cycle and delays enabling mutator assists.
      
      For example, it's quite likely that many Ps will simultaneously refill
      their span caches immediately after a GC cycle (because GC flushes the
      span caches), but at this point, there has been very little object
      allocation since the end of GC, so very little sweeping is done. The
      Ps then allocate objects from these cached spans, which drives up the
      bytes of object allocation, but since these allocations are coming
      from cached spans, nothing considers whether more sweeping has to
      happen. If the sweep ratio is high enough (which can happen if the
      next GC trigger is very close to the retained heap size), this can
      easily represent a sweep debt of thousands of pages.
      
      Fix this by making proportional sweep proportional to the number of
      bytes of spans allocated, rather than the number of bytes of objects
      allocated. Prior to allocating a span, both the small object path and
      the large object path ensure credit for allocating that span, so the
      system operates in the black, rather than in the red.
      
      Combined with the previous commit, this should eliminate all sweeping
      from GC start up. On the stress test in issue #11911, this reduces the
      time spent sweeping during GC (and delaying start up) by several
      orders of magnitude:
      
                      mean    99%ile     max
          pre fix      1 ms    11 ms   144 ms
          post fix   270 ns   735 ns   916 ns
      
      Updates #11911.
      
      Change-Id: I89223712883954c9d6ec2a7a51ecb97172097df3
      Reviewed-on: https://go-review.googlesource.com/13044Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      fc9ca85f
    • Austin Clements's avatar
      runtime: always give concurrent sweep some heap distance · e30c6d64
      Austin Clements authored
      Currently it's possible for the next_gc heap size trigger computed for
      the next GC cycle to be less than the current allocated heap size.
      This means the next cycle will start immediately, which means there's
      no time to perform the concurrent sweep between GC cycles. This places
      responsibility for finishing the sweep on GC itself, which delays GC
      start-up and hence delays mutator assist.
      
      Fix this by ensuring that next_gc is always at least a little higher
      than the allocated heap size, so we won't trigger the next cycle
      instantly.
      
      Updates #11911.
      
      Change-Id: I74f0b887bf187518d5fedffc7989817cbcf30592
      Reviewed-on: https://go-review.googlesource.com/13043Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      e30c6d64
    • Austin Clements's avatar
      runtime: assist the GC during GC startup and shutdown · fb5230af
      Austin Clements authored
      Currently there are two sensitive periods during which a mutator can
      allocate past the heap goal but mutator assists can't be enabled: 1)
      at the beginning of GC between when the heap first passes the heap
      trigger and sweep termination and 2) at the end of GC between mark
      termination and when the background GC goroutine parks. During these
      periods there's no back-pressure or safety net, so a rapidly
      allocating mutator can allocate past the heap goal. This is
      exacerbated if there are many goroutines because the GC coordinator is
      scheduled as any other goroutine, so if it gets preempted during one
      of these periods, it may stay preempted for a long period (10s or 100s
      of milliseconds).
      
      Normally the mutator does scan work to create back-pressure against
      allocation, but there is no scan work during these periods. Hence, as
      a fall back, if a mutator would assist but can't yet, simply yield the
      CPU. This delays the mutator somewhat, but more importantly gives more
      CPU time to the GC coordinator for it to complete the transition.
      
      This is obviously a workaround. Issue #11970 suggests a far better but
      far more invasive way to fix this.
      
      Updates #11911. (This very nearly fixes the issue, but about once
      every 15 minutes I get a GC cycle where the assists are enabled but
      don't do enough work.)
      
      Change-Id: I9768b79e3778abd3e06d306596c3bd77f65bf3f1
      Reviewed-on: https://go-review.googlesource.com/13026Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      fb5230af
    • Austin Clements's avatar
      runtime: recheck GC trigger before actually starting GC · 88e945fd
      Austin Clements authored
      Currently allocation checks the GC trigger speculatively during
      allocation and then triggers the GC without rechecking. As a result,
      it's possible for G 1 and G 2 to detect the trigger simultaneously,
      both enter startGC, G 1 actually starts GC while G 2 gets preempted
      until after the whole GC cycle, then G 2 immediately starts another GC
      cycle even though the heap is now well under the trigger.
      
      Fix this by re-checking the GC trigger non-speculatively just before
      actually kicking off a new GC cycle.
      
      This contributes to #11911 because when this happens, we definitely
      don't finish the background sweep before starting the next GC cycle,
      which can significantly delay the start of concurrent scan.
      
      Change-Id: I560ab79ba5684ba435084410a9765d28f5745976
      Reviewed-on: https://go-review.googlesource.com/13025Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      Reviewed-by: 's avatarRick Hudson <rlh@golang.org>
      88e945fd
    • Ian Lance Taylor's avatar
      doc: link to design doc for GOMAXPROCS change in go1.5.html · d5f5e658
      Ian Lance Taylor authored
      Change-Id: Ifac10621fece766f3a0e8551e98d1f8d7072852f
      Reviewed-on: https://go-review.googlesource.com/13068Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      Reviewed-by: 's avatarRob Pike <r@golang.org>
      d5f5e658
    • Ian Lance Taylor's avatar
      cmd/go: fix documentation for exported functions · c2ef8e75
      Ian Lance Taylor authored
      I accidentally submitted https://golang.org/cl/13080 too early.
      
      Update #11955.
      
      Change-Id: I1a5a6860bb46bc4bc6fd278f8a867d2dd9e411e1
      Reviewed-on: https://go-review.googlesource.com/13096Reviewed-by: 's avatarAndrew Gerrand <adg@golang.org>
      c2ef8e75
    • Vincent Batts's avatar
      archive/tar: don't treat multiple file system links as a tar hardlink · a1d093d9
      Vincent Batts authored
      Do not assume that if stat shows multiple links that we should mark the
      file as a hardlink in the tar format.  If the hardlink link was not
      referenced, this caused a link to "/".  On an overlay file system, all
      files have multiple links.
      
      The caller must keep the inode references and set TypeLink, Size = 0,
      and LinkName themselves.
      
      Change-Id: I873b8a235bc8f8fbb271db74ee54232da36ca013
      Reviewed-on: https://go-review.googlesource.com/13045Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      a1d093d9
    • Ian Lance Taylor's avatar
      cmd/go: document that functions are exported by cgo · bc5a6ce6
      Ian Lance Taylor authored
      The buildmode docs mention exported functions, but don't say anything
      about how to export them.  Mention the cgo tool to make this somewhat
      clearer.
      
      Fixes #11955.
      
      Change-Id: Ie5420445daa87f5aceec6ad743465d5d32d0a786
      Reviewed-on: https://go-review.googlesource.com/13080Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      bc5a6ce6
    • Rob Pike's avatar
      go/types: remove the renaming import of go/constant · ecead89b
      Rob Pike authored
      For niceness, when go/exact was moved from x/tools, it
      was renamed go/constant.
      
      For simplicity, when go/types was moved from x/tools, its
      imports of (now) go/constant were done with a rename:
      
          import exact "go/constant"
      
      This kept the code just as it was before and avoided the issue
      of what to call the internal constant called, um, constant.
      
      But not all was hidden, as the text of some fields of structs and
      the like leaked the old name, so things like "exact.Value" appeared
      in type definitions and function signatures in the documentation.
      This is unacceptable.
      
      Fix the documentation issue by fixing the code. Rename the constant
      constant constant_, and remove the renaming import.
      
      This should go into 1.5. It's mostly a mechanical change, is
      internal to the package, and fixes the documentation. It contains
      no semantic changes except to fix a benchmark that was broken
      in the original transition.
      
      Fixes #11949.
      
      Change-Id: Ieb94b6558535b504180b1378f19e8f5a96f92d3c
      Reviewed-on: https://go-review.googlesource.com/13051Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      ecead89b
    • Caleb Spare's avatar
      doc: link to the release cycle from contribute.html · 2bd52370
      Caleb Spare authored
      Change-Id: Ia5d41b66006682084fcbfac3da020946ea3dd116
      Reviewed-on: https://go-review.googlesource.com/13093Reviewed-by: 's avatarAndrew Gerrand <adg@golang.org>
      2bd52370
    • Caleb Spare's avatar
      cmd/go: re-run mkalldocs.sh after testflag change · 8ac16b9d
      Caleb Spare authored
      Change-Id: Ia21501df23a91c065d9f2acc6f043019a1419b22
      Reviewed-on: https://go-review.googlesource.com/13092Reviewed-by: 's avatarAndrew Gerrand <adg@golang.org>
      8ac16b9d
    • Andy Maloney's avatar
      doc: Mention contributor agreement immediately after Gerrit · c53de859
      Andy Maloney authored
      I walked through the steps for a contribution but ended up
      with an error when doing "git mail" because I didn't have a
      signed agreement.
      
      Added a section to check for or create one through Gerrit right
      after the user has created the account and logged in.
      
      Moved some info from copyright section to the new section.
      
      Change-Id: I79bbd3e18fc3a742fa59a242085da14be9e19ba0
      Reviewed-on: https://go-review.googlesource.com/13062Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Reviewed-by: 's avatarAndrew Gerrand <adg@golang.org>
      c53de859
    • Caleb Spare's avatar
      cmd/go: documented default value of the -timeout testflag · a65fa205
      Caleb Spare authored
      Change-Id: I4dc75065038a9cfd06f61c0deca1c86c70713d3a
      Reviewed-on: https://go-review.googlesource.com/13091Reviewed-by: 's avatarAndrew Gerrand <adg@golang.org>
      a65fa205
  3. 03 Aug, 2015 7 commits
    • Russ Cox's avatar
      cmd/go: clean up installHeader action · b3bf38e7
      Russ Cox authored
      This was confusing when I was trying to fix go build -o.
      Perhaps due to that fix, this can now be simplified from
      three functions to one.
      
      Change-Id: I878a6d243b14132a631e7c62a3bb6d101bc243ea
      Reviewed-on: https://go-review.googlesource.com/13027Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      b3bf38e7
    • Russ Cox's avatar
      cmd/go: document and fix 'go build -o' semantics · 961f456a
      Russ Cox authored
      Quoting the new docs:
      
      «
      If the arguments to build are a list of .go files, build treats
      them as a list of source files specifying a single package.
      
      When compiling a single main package, build writes
      the resulting executable to an output file named after
      the first source file ('go build ed.go rx.go' writes 'ed' or 'ed.exe')
      or the source code directory ('go build unix/sam' writes 'sam' or 'sam.exe').
      The '.exe' suffix is added when writing a Windows executable.
      
      When compiling multiple packages or a single non-main package,
      build compiles the packages but discards the resulting object,
      serving only as a check that the packages can be built.
      
      The -o flag, only allowed when compiling a single package,
      forces build to write the resulting executable or object
      to the named output file, instead of the default behavior described
      in the last two paragraphs.
      »
      
      There is a change in behavior here, namely that 'go build -o x.a x.go'
      where x.go is not a command (not package main) did not write any
      output files (back to at least Go 1.2) but now writes x.a.
      This seems more reasonable than trying to explain that -o is
      sometimes silently ignored.
      
      Otherwise the behavior is unchanged.
      
      The lines being deleted in goFilesPackage look like they are
      setting up 'go build x.o' to write 'x.a', but they were overridden
      by the p.target = "" in runBuild. Again back to at least Go 1.2,
      'go build x.go' for a non-main package has never produced
      output. It seems better to keep it that way than to change it,
      both for historical consistency and for consistency with
      'go build strings' and 'go build std'.
      
      All of this behavior is now tested.
      
      Fixes #10865.
      
      Change-Id: Iccdf21f366fbc8b5ae600a1e50dfe7fc3bff8b1c
      Reviewed-on: https://go-review.googlesource.com/13024Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Reviewed-by: 's avatarDave Day <djd@golang.org>
      961f456a
    • Brad Fitzpatrick's avatar
      net/http: deflake TestZeroLengthPostAndResponse · e3c26b2b
      Brad Fitzpatrick authored
      It was failing with multiple goroutines a few out of every thousand
      runs (with errRequestCanceled) because it was using the same
      *http.Request for all 5 RoundTrips, but the RoundTrips' goroutines
      (notably the readLoop method) were all still running, sharing that
      same pointer. Because the response has no body (which is what
      TestZeroLengthPostAndResponse tests), the readLoop was marking the
      connection as reusable early (before the caller read until the body's
      EOF), but the Transport code was clearing the Request's cancelation
      func *AFTER* the caller had already received it from RoundTrip. This
      let the test continue looping and do the next request with the same
      pointer, fetch a connection, and then between getConn and roundTrip
      have an invariant violated: the Request's cancelation func was nil,
      tripping this check:
      
              if !pc.t.replaceReqCanceler(req.Request, pc.cancelRequest) {
                      pc.t.putIdleConn(pc)
                      return nil, errRequestCanceled
              }
      
      The solution is to clear the request cancelation func in the readLoop
      goroutine in the no-body case before it's returned to the caller.
      
      This now passes reliably:
      
      $ go test -race -run=TestZeroLengthPostAndResponse -count=3000
      
      I think we've only seen this recently because we now randomize scheduling
      of goroutines in race mode (https://golang.org/cl/11795). This race
      has existed for a long time but the window was hard to hit.
      
      Change-Id: Idb91c582919f85aef5b9e5ef23706f1ba9126e9a
      Reviewed-on: https://go-review.googlesource.com/13070
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      e3c26b2b
    • Brad Fitzpatrick's avatar
      net/http: fix server/transport data race when sharing the request body · 7aa4e29d
      Brad Fitzpatrick authored
      Introduced in https://go-review.googlesource.com/12865 (git rev c2db5f4c).
      
      This fix doesn't add any new lock acquistions: it just moves the
      existing one taken by the unreadDataSize method and moves it out
      wider.
      
      It became flaky at rev c2db5f4c, but now reliably passes again:
      $ go test -v -race -run=TestTransportAndServerSharedBodyRace -count=100 net/http
      
      Fixes #11985
      
      Change-Id: I6956d62839fd7c37e2f7441b1d425793f4a0db30
      Reviewed-on: https://go-review.googlesource.com/12909
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      7aa4e29d
    • Mikio Hara's avatar
      runtime: skip TestCgoCallbackGC on dragonfly · 5e15e28e
      Mikio Hara authored
      Updates #11990.
      
      Change-Id: I6c58923a1b5a3805acfb6e333e3c9e87f4edf4ba
      Reviewed-on: https://go-review.googlesource.com/13050Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      5e15e28e
    • Rob Pike's avatar
      doc: document new linker -X syntax in go1.5.html · 99912272
      Rob Pike authored
      Fixes #11973.
      
      Change-Id: Icffa3213246663982b7cc795982e0923e272f405
      Reviewed-on: https://go-review.googlesource.com/12919Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      99912272
    • Andrew Gerrand's avatar
      doc: link to proposal process from contribution guidelines · 3c10bdd1
      Andrew Gerrand authored
      Change-Id: I992cb1afeef498353d529238e508fa438d6c069c
      Reviewed-on: https://go-review.googlesource.com/12912Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      Reviewed-by: 's avatarRob Pike <r@golang.org>
      3c10bdd1
  4. 02 Aug, 2015 3 commits
  5. 31 Jul, 2015 6 commits
    • Robert Griesemer's avatar
      spec: fixed various example code snippets · 02d74485
      Robert Griesemer authored
      Per suggestions by Peter Olsen (https://github.com/pto).
      
      Fixes #11964.
      
      Change-Id: Iae261ac14f75abf848f5601f59d7fe6e95b6805a
      Reviewed-on: https://go-review.googlesource.com/13006Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      02d74485
    • Ian Lance Taylor's avatar
      cmd/objdump: don't run TestDisasmExtld if cgo is not enabled · 384789e8
      Ian Lance Taylor authored
      The test uses external linking mode, which is probably not available
      if cgo does not work.
      
      Fixes #11969.
      
      Change-Id: Id1c2828cd2540391e16b422bf51674ba6ff084b0
      Reviewed-on: https://go-review.googlesource.com/13005Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      384789e8
    • Dave Cheney's avatar
      os: add explicit tests for fchown(2) and lchown(2) on unix platforms · 296529b7
      Dave Cheney authored
      Fixes #11919
      
      Issue #11918 suggested that os.File.Chown and os.Lchown were under tested.
      
      Change-Id: Ib41f7cb2d2fe0066d2ccb4d1bdabe1795efe80fc
      Reviewed-on: https://go-review.googlesource.com/12834Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      296529b7
    • Russ Cox's avatar
      cmd/go: fix go get x/... matching internal directories · 226b28c8
      Russ Cox authored
      Fixes #11960.
      
      Change-Id: I9361a9f17f4eaf8e4f54b4ba380fd50a4b9cf003
      Reviewed-on: https://go-review.googlesource.com/13023Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      226b28c8
    • Russ Cox's avatar
      cmd/go: fix disallow of p/vendor/x during vendor experiment · 45971c60
      Russ Cox authored
      The percolation of errors upward in the load process could
      drop errors, meaning that a build tree could, depending on the
      processing order, import the same directory as both "p/vendor/x"
      and as "x". That's not supposed to be allowed. But then, worse,
      the build would generate two jobs for building that directory,
      which would use the same work space and overwrite each other's files,
      leading to very strange failures.
      
      Two fixes:
      
      1. Fix the propagation of errors upward (prefer errors over success).
      2. Check explicitly for duplicated packages before starting a build.
      
      New test for #1.
      Since #2 can't happen, tested #2 by hand after reverting fix for #1.
      
      Fixes #11913.
      
      Change-Id: I6d2fc65f93b8fb5f3b263ace8d5f68d803a2ae5c
      Reviewed-on: https://go-review.googlesource.com/13022Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      45971c60
    • Russ Cox's avatar
      cmd/compile, runtime: fix placement of map bucket overflow pointer on nacl · c5dff728
      Russ Cox authored
      On most systems, a pointer is the worst case alignment, so adding
      a pointer field at the end of a struct guarantees there will be no
      padding added after that field (to satisfy overall struct alignment
      due to some more-aligned field also present).
      
      In the runtime, the map implementation needs a quick way to
      get to the overflow pointer, which is last in the bucket struct,
      so it uses size - sizeof(pointer) as the offset.
      
      NaCl/amd64p32 is the exception, as always.
      The worst case alignment is 64 bits but pointers are 32 bits.
      There's a long history that is not worth going into, but when
      we moved the overflow pointer to the end of the struct,
      we didn't get the padding computation right.
      The compiler computed the regular struct size and then
      on amd64p32 added another 32-bit field.
      And the runtime assumed it could step back two 32-bit fields
      (one 64-bit register size) to get to the overflow pointer.
      But in fact if the struct needed 64-bit alignment, the computation
      of the regular struct size would have added a 32-bit pad already,
      and then the code unconditionally added a second 32-bit pad.
      This placed the overflow pointer three words from the end, not two.
      The last two were padding, and since the runtime was consistent
      about using the second-to-last word as the overflow pointer,
      no harm done in the sense of overwriting useful memory.
      But writing the overflow pointer to a non-pointer word of memory
      means that the GC can't see the overflow blocks, so it will
      collect them prematurely. Then bad things happen.
      
      Correct all this in a few steps:
      
      1. Add an explicit check at the end of the bucket layout in the
      compiler that the overflow field is last in the struct, never
      followed by padding.
      
      2. When padding is needed on nacl (not always, just when needed),
      insert it before the overflow pointer, to preserve the "last in the struct"
      property.
      
      3. Let the compiler have the final word on the width of the struct,
      by inserting an explicit padding field instead of overwriting the
      results of the width computation it does.
      
      4. For the same reason (tell the truth to the compiler), set the type
      of the overflow field when we're trying to pretend its not a pointer
      (in this case the runtime maintains a list of the overflow blocks
      elsewhere).
      
      5. Make the runtime use "last in the struct" as its location algorithm.
      
      This fixes TestTraceStress on nacl/amd64p32.
      The 'bad map state' and 'invalid free list' failures no longer occur.
      
      Fixes #11838.
      
      Change-Id: If918887f8f252d988db0a35159944d2b36512f92
      Reviewed-on: https://go-review.googlesource.com/12971Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      Reviewed-by: 's avatarAustin Clements <austin@google.com>
      c5dff728