Replaced ../gc/reflect.c with
cmd/compile/internal/gc/reflect.go.

Fixes #20525

Change-Id: Ibafd36ea446ace5c677df27873a4bbf716a0a9bc
Reviewed-on: https://go-review.googlesource.com/44376Reviewed-by: Ian Lance Taylor <iant@golang.org>

Updates golang_org/x/net/lif to rev fcc8ed8 for:
- lif: rename internal types (http://golang.org/cl/43070)

Change-Id: I0aad9a8b504d673b29d3c26b3717ffa8512e0cdf
Reviewed-on: https://go-review.googlesource.com/44392Reviewed-by: Ian Lance Taylor <iant@golang.org>

key32 is called between entersyscallblock and exitsyscall
stack split may occur if disable inlining and the G is preempted

Fix the problem by describing key32 as nosplit function

Fixes #20510

Change-Id: I1f0787995936f34ef0052cf79fde036f1b338865
Reviewed-on: https://go-review.googlesource.com/44390Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

cmd/compile/internal/ld/decodesym.go is now
cmd/link/internal/ld/decodesym.go

Change-Id: I16ec5c89aa3507e70676c2b50d70f1fde533a085
Reviewed-on: https://go-review.googlesource.com/44373Reviewed-by: Ian Lance Taylor <iant@golang.org>

Vet returns with a nonzero exit for all possible messages in the
buildtag check. However for this file:

    //+buildlinux

    package main

vet returns a zero exit status:

    $ go vet main.go
    demo.go:1: possible malformed +build comment
    $ echo $?
    0

This CL sets the exit status to non zero for the remaining messages in
the buildtag check.

Change-Id: Ia2c35ebc3ec5ac311d2a0295b5b9fdd997a85726
Reviewed-on: https://go-review.googlesource.com/44371Reviewed-by: Rob Pike <r@golang.org>

In preparation for CL 41770, upgrade .debug_info to DWARF4, and emit
DW_AT_frame_base on subprograms. This should make no semantic
difference.

Also fix a long-standing bug/inconsistency in puttattr: it didn't
add the addend to ref_addrs. Previously this didn't matter because it
was only used for types, but now it's used for section offsets into
symbols that have multiple entries.

RELNOTE=yes

Change-Id: Ib10654ac92edfa29c5167c44133648151d70cf76
Reviewed-on: https://go-review.googlesource.com/44210Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>

Instead of allocating a new reflect.Value object on every loop we zero it.

DecodeComplex128Slice-8  13.1µs ± 7%  13.2µs ± 8%     ~     (p=0.347 n=18+19)
DecodeFloat64Slice-8     8.13µs ± 5%  8.00µs ± 3%     ~     (p=0.168 n=20+19)
DecodeInt32Slice-8       8.27µs ± 5%  8.08µs ± 5%   -2.27%  (p=0.001 n=19+18)
DecodeStringSlice-8      17.9µs ±12%  17.8µs ±11%     ~     (p=0.989 n=20+19)
DecodeInterfaceSlice-8    163µs ±10%   159µs ± 4%     ~     (p=0.057 n=19+19)
DecodeMap-8               220µs ± 2%   183µs ± 1%  -17.07%  (p=0.000 n=19+18)

Updates #19525

Change-Id: I27f8edd4761787f6b9928d34cefa08a34a6e25b2
Reviewed-on: https://go-review.googlesource.com/39203Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

This updates the bundled copy of x/net/http2 to x/net git rev
3470a06c1357df53 for:

http2: fix typo in comment
https://golang.org/cl/44271

http2: fix nil dereference after Read completes with an error
https://golang.org/cl/44330

Fixes #20501

Change-Id: Ie6a5fb092f77044f504f146721dae86299e04011
Reviewed-on: https://go-review.googlesource.com/44331
Run-TryBot: Tom Bergan <tombergan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

See: https://pkware.cachefly.net/webdocs/APPNOTE/APPNOTE-6.3.0.TXT

Document says:
> If general purpose bit 11 is set, the filename and comment must support The
> Unicode Standard, Version 4.1.0 or greater using the character encoding form
> defined by the UTF-8 storage specification.

Since Go encode the filename to UTF-8, general purpose bit 11 should be set.

Change-Id: Ica4af02b4dc695e9a5c015ae360e70171efb6ee3
Reviewed-on: https://go-review.googlesource.com/39570Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

This avoids false-positive TSAN reports when using the C sigaction
function to read handlers registered by the Go runtime.

(Unfortunately, I can't seem to coax the runtime into reproducing the
failure in a small unit-test.)

Change-Id: I744279a163708e24b1fbe296ca691935c394b5f3
Reviewed-on: https://go-review.googlesource.com/44270
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

Currently debug/dwarf assumes all paths in line tables will be
UNIX-style paths, which obviously isn't the case for binaries built on
Windows. However, we can't simply switch from the path package to the
filepath package because we don't know that we're running on the same
host type that built the binary and we want this to work even if we're
not. This is essentially the approach taken by GDB, which treats paths
in accordance with the system GDB itself is compiled for. In fact, we
can't even guess the compilation system from the type of the binary
because it may have been cross-compiled.

We fix this by heuristically determining whether paths are UNIX-style
or DOS-style by looking for a drive letter or UNC path. If we see a
DOS-style path, we use appropriate logic for determining whether the
path is absolute and for joining two paths. This is helped by the fact
that we should basically always be starting with an absolute path.
However, it could mistake a relative UNIX-style path that begins with
a directory like "C:" for an absolute DOS-style path. There doesn't
seem to be any way around this.

Fixes #19784.

Change-Id: Ie13b546d2f1dcd8b02e668583a627b571b281588
Reviewed-on: https://go-review.googlesource.com/44017
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>

Change-Id: I3b3ae4ecad0894781a3019326c7262cb9790ad4d
Reviewed-on: https://go-review.googlesource.com/44250Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

It is expected to test assembly code for ARMv5, ARMv6 and ARMv7
in cmd/asm/internal/asm/endtoend_test.go. But actually the loop
in "func TestARMEndToEnd(t *testing.T)" runs three times all
for ARMv5.

This patch fixes that bug and adds a new armv6.s which is only tested
with GOARM=6.

fixes #20465

Change-Id: I5dbf00809a47ace2c195335e2c9bdd768479aada
Reviewed-on: https://go-review.googlesource.com/43930Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

"ADDF F0, R1, F2" is silently accepted by the arm assembler and
assembled to the same binary code of "ADDF F0, F1, F2". So does
"CMPF F0, R1".

"ABSF F0, F1, F2" is also silently accepted and assembled to a
different instruction.

This patch reports those illegal forms and adds test cases.

fix #20464

Change-Id: I88b80dc29de24c6266ac7bf7bce1578c5adbc68c
Reviewed-on: https://go-review.googlesource.com/43931
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

Currently, the heap arena allocator allocates monotonically increasing
addresses. This is fine on 64-bit where we stake out a giant block of
the address space for ourselves and start at the beginning of it, but
on 32-bit the arena starts at address 0 but we start allocating from
wherever the OS feels like giving us memory. We can generally hint the
OS to start us at a low address, but this doesn't always work.

As a result, on 32-bit, if the OS gives us an arena block that's lower
than the current block we're allocating from, we simply say "thanks
but no thanks", return the whole (256MB!) block of memory, and then
take a fallback path that mmaps just the amount of memory we need
(which may be as little as 8K).

We have to do this because mheap_.arena_used is *both* the highest
used address in the arena and the next address we allocate from.

Fix all of this by separating the second role of arena_used out into a
new field called arena_alloc. This lets us accept any arena block the
OS gives us. This also slightly changes the invariants around
arena_end. Previously, we ensured arena_used <= arena_end, but this
was related to arena_used's second role, so the new invariant is
arena_alloc <= arena_end. As a result, we no longer necessarily update
arena_end when we're updating arena_used.

Fixes #20259 properly. (Unlike the original fix, this one should not
be cherry-picked to Go 1.8.)

This is reasonably low risk. I verified several key properties of the
32-bit code path with both 4K and 64K physical pages using a symbolic
model and the change does not materially affect 64-bit (arena_used ==
arena_alloc on 64-bit). The only oddity is that we no longer call
setArenaUsed with racemap == false to indicate that we're creating a
hole in the address space, but this only happened in a 32-bit-only
code path, and the race detector require 64-bit, so this never
mattered anyway.

Change-Id: Ib1334007933e615166bac4159bf357ae06ec6a25
Reviewed-on: https://go-review.googlesource.com/44010
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

CL 43779/commit 6a6c792e
broke the builds at tip, and that CL doesn't account for
cases where Redirect is directly invoked with a full URL
that itself has a query string.

Updates #17841

Change-Id: Idb0486bae8625e1f9e033ca4cfcd87de95bc835c
Reviewed-on: https://go-review.googlesource.com/44100Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Update golang.org/x/net/nettest to revision 7dcfb8076726a3fdd9353b6b8a1f1b6be6811bd6.

Change-Id: Ib6505423910d34142d7b1bcb6792a5017df4da47
Reviewed-on: https://go-review.googlesource.com/44131Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

We weren't setting r0 to 0, as required by our generated code.

Before this patch, the misc/cgo/testcarchive tests failed on ppc64le.
After this patch, they work, so enable them.

Change-Id: I53b16746961da9f7c34f59030a1e40953c9c1e05
Reviewed-on: https://go-review.googlesource.com/44093
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Add a missing newline.  Don't panic on an unexpected error type.

Change-Id: I82a4b12c498fbfdf4972737329631c0c02540005
Reviewed-on: https://go-review.googlesource.com/44092
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

The logic performs a series of shifts, which are useless given
that they are followed by an assignment that overrides the
value of the previous computation.

I suspect (but cannot prove) that this is leftover logic from an
original approach that attempted to store both the Huffman code
and the length within the same variable instead of using two
different variables as it currently does now.

Fixes #17949

Change-Id: Ibf6c807c6cef3b28bfdaf2b68d9bc13503ac21b2
Reviewed-on: https://go-review.googlesource.com/44091Reviewed-by: Ian Lance Taylor <iant@golang.org>

Test that we really do move spills down to the dominator of
all the uses.

Also add a test where go1.8 would have moved the spill out of
the loop into two exit points, but go1.9 doesn't move the spill.
This is a case where the 1.9 spill moving code does not subsume
the 1.8 spill moving code.

Maybe we resurrect moving-spills-out-of-loops CL to fix this one.
(I suspect it wouldn't be worth the effort, but would be happy
to hear evidence otherwise.)

Update #20472

Change-Id: I7dbf8d65e7f4d675d14e5ecf502887cebda35d2a
Reviewed-on: https://go-review.googlesource.com/44038
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>

Adds golang.org/x/net/nettest at revision 9773060888fba93b172cedcd70127db1ab739bd1.
This allows us to test net.Conn implementations for compliance.

Updates #18170

Change-Id: I8d3d3430b0a1abc83513180a677c39ee39303f5a
Reviewed-on: https://go-review.googlesource.com/37404Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Change-Id: I5d55c3b1011dd10552d8e740fb65886306d91b5c
Reviewed-on: https://go-review.googlesource.com/44035Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Fixes #19935

Change-Id: I28e4cd6924f1b965a429a6a5ebfc4c0f952e8ae6
Reviewed-on: https://go-review.googlesource.com/40390Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

When writing the 'Connection: close' header based on response Close
attribute we also check if it is already in the headers scheduled
to be written and skip if necessary.

Fixes #19499

Change-Id: I92357344a37ae385454ec8006114fa4cfa585810
Reviewed-on: https://go-review.googlesource.com/38076
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Despite the previously known behavior of Request.WithContext
shallow copying a request, usage of the request inside server.ServeHTTP
mutates the request's URL. This CL implements deep copying of the URL.

Fixes #20068

Change-Id: I86857d7259e23ac624d196401bf12dde401c42af
Reviewed-on: https://go-review.googlesource.com/41308
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Apparently they exist in the wild. See:
https://github.com/golang/go/issues/18215#issuecomment-301182496
(Facebook / iOS)

Fixes #18215

Change-Id: I9ddad3896b5d784cb3f5b3ee9c6819081a4a2702
Reviewed-on: https://go-review.googlesource.com/44004
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

A for-range loop is simpler and also generally faster nowadays:

TrimASCII/1:1-4      69.2ns ± 1%  72.3ns ± 4%  +4.55%  (p=0.001 n=8+8)
TrimASCII/1:2-4       114ns ± 4%   104ns ± 3%  -8.71%  (p=0.000 n=9+8)
TrimASCII/1:4-4       112ns ± 1%   109ns ± 2%  -2.57%  (p=0.000 n=8+9)
TrimASCII/1:8-4       120ns ± 2%   118ns ± 4%    ~     (p=0.097 n=9+9)
TrimASCII/1:16-4      137ns ± 3%   132ns ± 3%  -3.82%  (p=0.001 n=9+9)
TrimASCII/16:1-4      129ns ± 1%   125ns ± 2%  -3.38%  (p=0.000 n=8+9)
TrimASCII/16:2-4      167ns ± 3%   159ns ± 1%  -4.99%  (p=0.000 n=9+8)
TrimASCII/16:4-4      165ns ± 2%   162ns ± 1%  -1.91%  (p=0.005 n=8+9)
TrimASCII/16:8-4      173ns ± 2%   170ns ± 1%  -1.29%  (p=0.018 n=9+9)
TrimASCII/16:16-4     188ns ± 2%   186ns ± 2%  -1.13%  (p=0.022 n=8+9)
TrimASCII/256:1-4    1.06µs ± 1%  0.98µs ± 2%  -7.64%  (p=0.000 n=8+9)
TrimASCII/256:2-4    1.08µs ± 1%  1.06µs ± 2%  -1.95%  (p=0.006 n=9+9)
TrimASCII/256:4-4    1.09µs ± 1%  1.07µs ± 3%    ~     (p=0.059 n=9+9)
TrimASCII/256:8-4    1.10µs ± 1%  1.07µs ± 2%  -2.63%  (p=0.000 n=9+8)
TrimASCII/256:16-4   1.10µs ± 1%  1.08µs ± 1%  -1.90%  (p=0.000 n=8+9)
TrimASCII/4096:1-4   15.8µs ± 1%  14.5µs ± 1%  -8.59%  (p=0.000 n=9+9)
TrimASCII/4096:2-4   15.6µs ± 1%  15.4µs ± 2%  -1.27%  (p=0.021 n=8+8)
TrimASCII/4096:4-4   15.6µs ± 1%  15.4µs ± 2%    ~     (p=0.094 n=9+9)
TrimASCII/4096:8-4   15.7µs ± 1%  15.8µs ± 6%    ~     (p=0.555 n=8+8)
TrimASCII/4096:16-4  15.7µs ± 2%  15.3µs ± 1%  -2.64%  (p=0.000 n=8+9)

Change-Id: I9b06689b67c0cf2c7ff446fc63a8c44cc5d6a246
Reviewed-on: https://go-review.googlesource.com/32891
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

This is one hurdle to building Go on Android; the runtime does
not build properly because *_linux.go files are excluded from
the "Building go_bootstrap" step when GOOS=android.

There are other hurdles; this is the first one.

Change-Id: I766e4bbf6ffc0d273888913f2516cf3e995a1786
Reviewed-on: https://go-review.googlesource.com/38308
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Avoid directly using the binary of the
gzipped encoded string in the handler.

Follow up of CL 37431.

Change-Id: Idcd04acb7940e67b7a35b2d6cb163d75b0e22e04
Reviewed-on: https://go-review.googlesource.com/44008
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

The existing Transport implementation does not detect gzip encoding
when the Content-Encoding header is not lower-case. This is not
compliant with RFC2616 section 3.5 "All content-coding values are
case-insensitive." and caused issues in the wild.

Fixes #19248

Change-Id: I1b49992832dc3c8ef700058596a27dd9909640a3
Reviewed-on: https://go-review.googlesource.com/37431Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Updates golang.org/x/net/http2/hpack to x/net git rev 4dbf598 for:

   http2/hpack: remove unused pair function from package
   https://golang.org/cl/43851

   http2/hpack: remove pair function and initialize directly
   https://golang.org/cl/43190

   http2/hpack: move initialization to a static table
   https://golang.org/cl/43090

Updates #6853

Change-Id: I933a094623143c7333a0da7867d5d2e41da05234
Reviewed-on: https://go-review.googlesource.com/44007
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

This updates the bundled copy of x/net/http2 to x/net git rev a8e8f92cd6 for:

    http2: remove extra goroutine stack from awaitGracefulShutdown
    https://golang.org/cl/43230

    http2: Discard DATA frames from the server after the response body is closed
    https://golang.org/cl/43810

Fixes #20302
Fixes #18471
Fixes #20448

Change-Id: I00972836deb2fe6049f631ee44901732a641b171
Reviewed-on: https://go-review.googlesource.com/44006Reviewed-by: Tom Bergan <tombergan@google.com>

This will be used to allow http2 servers to register a shutdown function
so that net/http.Server.Shutdown will work when the http2 server is
configured via a manual call to http2.ConfigureServer. Currently, Shutdown
only works when the http2 server is configured automatically by the
net/http package.

Updates #20302
Updates #18471

Change-Id: Ifc2b5f3126126a106b49ea4a7e999279852b9cc9
Reviewed-on: https://go-review.googlesource.com/44003
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Fixes #18438

Change-Id: I9599c1536d5e8bad7662b8ffa19e9b0746e27e60
Reviewed-on: https://go-review.googlesource.com/44000Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

- spaces are allowed anywhere but the last character of a boundary

Fixes #18768

Change-Id: I36b054462533ff6dfc060e37e7a58777ae4b66fe
Reviewed-on: https://go-review.googlesource.com/35507
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Commit 4dcba023 replaced select with pselect6 on linux/amd64 and
linux/arm, but it turns out the Android emulator uses linux/386. This
makes the equivalent change there, too.

Fixes #20409 more.

Change-Id: If542d6ade06309aab8758d5f5f6edec201ca7670
Reviewed-on: https://go-review.googlesource.com/44011
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Current code cannot handle string #define macros if those macros are
defined via other macros. This CL solve the issue.

Updates #18720

Change-Id: Ibed0773d10db3d545bb246b97e81c0d19e3af3d5
Reviewed-on: https://go-review.googlesource.com/41312Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Now:
$ GOARCH=arm GOARM=5 go install -x cmd/go
... followed by:
$ GOARCH=arm GOARM= go install -x cmd/go

... actually does work. Previously the second "go install" would reuse
the cached binaries from the GOARM=5 command and not rebuild.
(Or vice versa from GOARM= to GOARM=5)

And do the same for GO386.

Fixes #9737

Change-Id: I9630aab34d06465d5033e6743dfe6592c8247aa0
Reviewed-on: https://go-review.googlesource.com/43855
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

We only need to read the number of bytes required to store the value
"max - 1" to generate a random number in the range [0, max).

Before, there was an off-by-one error where an extra byte was read from
the io.Reader for inputs like "256" (right at the boundary for a byte).
There was a similar off-by-one error in the logic for clearing bits and
thus for any input that was a power of 2, there was a 50% chance the
read would continue to be retried as the mask failed to remove a bit.

Fixes #18165.

Change-Id: I548c1368990e23e365591e77980e9086fafb6518
Reviewed-on: https://go-review.googlesource.com/43891Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>