1. 31 Mar, 2016 31 commits
    • Alexandru Moșoi's avatar
      cmd/compile: combine SHLQ into loads and stores · ec5083e4
      Alexandru Moșoi authored
      Very common, cuts about 70k from pkg/tools/linux_amd64/* binaries.
      
      Change-Id: Ied0c049e56e56a56810c781435d79027fbcaf274
      Reviewed-on: https://go-review.googlesource.com/21374Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
      ec5083e4
    • Christopher Nelson's avatar
      cmd/link: Replace fmt.Sprintf with filepath.Join · 8677cad1
      Christopher Nelson authored
      In a number of places the code was joining filepaths explicitly with
      "/", instead of using filepath.Join. This may cause problems on Windows
      (or other) platforms.
      
      This is in support of https://go-review.googlesource.com/#/c/18057
      
      Change-Id: Ieb1334f35ddb2e125be690afcdadff8d7b0ace10
      Reviewed-on: https://go-review.googlesource.com/21369Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      8677cad1
    • Joe Tsai's avatar
      compress/flate: make Reader.Read return io.EOF eagerly · c27efce6
      Joe Tsai authored
      Rather than checking the block final bit on the next invocation
      of nextBlock, we check it at the termination of the current block.
      This ensures that we return (n, io.EOF) instead of (0, io.EOF)
      more frequently for most streams.
      
      However, there are certain situations where an eager io.EOF is not done:
      1) We previously returned from Read because the write buffer of the internal
      dictionary was full, and it just so happens that there is no more data
      remaining in the stream.
      2) There exists a [non-final, empty, raw block] after all blocks that
      actually contain uncompressed data. We cannot return io.EOF eagerly here
      since it would break flushing semantics.
      
      Both situations happen infrequently, but it is still important to note that
      this change does *not* guarantee that flate will *always* return (n, io.EOF).
      
      Furthermore, this CL makes no changes to the pattern of ReadByte calls
      to the underlying io.ByteReader.
      
      Below is the motivation for this change, pulling the text from
      @bradfitz's CL/21290:
      
      net/http and other things work better when io.Reader implementations
      return (n, io.EOF) at the end, instead of (n, nil) followed by (0,
      io.EOF). Both are legal, but the standard library has been moving
      towards n+io.EOF.
      
      An investigation of net/http connection re-use in
      https://github.com/google/go-github/pull/317 revealed that with gzip
      compression + http/1.1 chunking, the net/http package was not
      automatically reusing the underlying TCP connections when the final
      EOF bytes were already read off the wire. The net/http package only
      reuses the connection if the underlying Readers (many of them nested
      in this case) all eagerly return io.EOF.
      
      Previous related CLs:
          https://golang.org/cl/76400046 - tls.Reader
          https://golang.org/cl/58240043 - http chunked reader
      
      In addition to net/http, this behavior also helps things like
      ioutil.ReadAll (see comments about performance improvements in
      https://codereview.appspot.com/49570044)
      
      Updates #14867
      Updates google/go-github#317
      
      Change-Id: I637c45552efb561d34b13ed918b73c660f668378
      Reviewed-on: https://go-review.googlesource.com/21302
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      c27efce6
    • Keith Randall's avatar
      cmd/compile: better job of naming compound types · 4a7aba77
      Keith Randall authored
      Compound AUTO types weren't named previously.  That was because live
      variable analysis (plive.go) doesn't handle spilling to compound types.
      It can't handle them because there is no valid place to put VARDEFs when
      regalloc is spilling compound types.
      
      compound types = multiword builtin types: complex, string, slice, and
      interface.
      
      Instead, we split named AUTOs into individual one-word variables.  For
      example, a string s gets split into a byte ptr s.ptr and an integer
      s.len.  Those two variables can be spilled to / restored from
      independently.  As a result, live variable analysis can handle them
      because they are one-word objects.
      
      This CL will change how AUTOs are described in DWARF information.
      Consider the code:
      
      func f(s string, i int) int {
          x := s[i:i+5]
          g()
          return lookup(x)
      }
      
      The old compiler would spill x to two consecutive slots on the stack,
      both named x (at offsets 0 and 8).  The new compiler spills the pointer
      of x to a slot named x.ptr.  It doesn't spill x.len at all, as it is a
      constant (5) and can be rematerialized for the call to lookup.
      
      So compound objects may not be spilled in their entirety, and even if
      they are they won't necessarily be contiguous.  Such is the price of
      optimization.
      
      Re-enable live variable analysis tests.  One test remains disabled, it
      fails because of #14904.
      
      Change-Id: I8ef2b5ab91e43a0d2136bfc231c05d100ec0b801
      Reviewed-on: https://go-review.googlesource.com/21233
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      4a7aba77
    • Brad Fitzpatrick's avatar
      net/http, net/http/httputil: rename lk to mu · e55896b9
      Brad Fitzpatrick authored
      The conventional name for a sync.Mutex is "mu".
      
      These "lk" names date back to a time before conventions.
      
      Change-Id: Iee57f9f4423d04269e1125b5d82455c453aac26f
      Reviewed-on: https://go-review.googlesource.com/21361
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      e55896b9
    • Keith Randall's avatar
      cmd/compile: don't put SP in index slot · 9f66636c
      Keith Randall authored
      For idx1 ops, SP can appear in the index slot.
      Swap SP into the base register slot so we can encode
      the instruction.
      
      Fixes #15053
      
      Change-Id: I19000cc9d6c86c7611743481e6e2cb78b1ef04eb
      Reviewed-on: https://go-review.googlesource.com/21384
      Run-TryBot: Keith Randall <khr@golang.org>
      Reviewed-by: default avatarAlexandru Moșoi <alexandru@mosoi.ro>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      9f66636c
    • Keith Randall's avatar
      cmd/compile: extend prove pass to handle constant comparisons · 47c9e139
      Keith Randall authored
      Find comparisons to constants and propagate that information
      down the dominator tree.  Use it to resolve other constant
      comparisons on the same variable.
      
      So if we know x >= 7, then a x > 4 condition must return true.
      
      This change allows us to use "_ = b[7]" hints to eliminate bounds checks.
      
      Fixes #14900
      
      Change-Id: Idbf230bd5b7da43de3ecb48706e21cf01bf812f7
      Reviewed-on: https://go-review.googlesource.com/21008Reviewed-by: default avatarAlexandru Moșoi <alexandru@mosoi.ro>
      47c9e139
    • Ilya Tocar's avatar
      hash/crc64: Add tests for ECMA polynomial · f5bd3556
      Ilya Tocar authored
      Currently we test crc64 only with ISO polynomial.
      
      Change-Id: Ibc5e202db3b960369cbbb18e31eb0fea07b54dba
      Reviewed-on: https://go-review.googlesource.com/21309
      Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      f5bd3556
    • Matthew Dempsky's avatar
      cmd/compile: remove Node.Nointerface field · 11d916b1
      Matthew Dempsky authored
      We already keep the entire pragma bitset in n.Func.Pragma, so there's
      no need to track Nointerface separately.
      
      Passes toolstash -cmp.
      
      Change-Id: Ic027ece477fcf63b0c1df128a08b89ef0f34fd58
      Reviewed-on: https://go-review.googlesource.com/21381
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      11d916b1
    • Matthew Dempsky's avatar
      cmd/compile: stop generating garbage when checking map key types · 7c4d53c2
      Matthew Dempsky authored
      Change-Id: Ib500ee92ae1a3d15f7c9f3f46d238b75184b4304
      Reviewed-on: https://go-review.googlesource.com/21382Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      7c4d53c2
    • Josh Bleecher Snyder's avatar
      cmd/compile: dump stack trace in Fatalf during development · 00289c29
      Josh Bleecher Snyder authored
      See discussion in #15029.
      
      Change-Id: I5cc8be5737ddb7c1f5e4a6cd92cf557af45e961d
      Reviewed-on: https://go-review.googlesource.com/21347Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      00289c29
    • David Crawshaw's avatar
      cmd/compile: include pkgPath on all struct types · 43a274fd
      David Crawshaw authored
      Fixes #15026.
      
      Change-Id: I61ed71152b99973270d79264d1e8f466f7343c02
      Reviewed-on: https://go-review.googlesource.com/21286
      Run-TryBot: David Crawshaw <crawshaw@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      43a274fd
    • Josh Bleecher Snyder's avatar
      cmd/compile: add sliceBound · e775b8df
      Josh Bleecher Snyder authored
      Add a constant for the magic -1 for slice bounds.
      Use it.
      Enforce more aggressively that bounds must be
      slice, ddd, or non-negative.
      Remove ad hoc check in plive.go.
      Check bounds before constructing an array type
      when typechecking.
      
      All changes are manual.
      
      Passes toolstash -cmp.
      
      Change-Id: I9fd9cc789d7d4b4eea3b30b24037a254d3788add
      Reviewed-on: https://go-review.googlesource.com/21348
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      e775b8df
    • Keith Randall's avatar
      cmd/compile: fix build · 4b95575b
      Keith Randall authored
      Pushed from an old client by mistake.  These are the
      missing changes.
      
      Change-Id: Ia8d61c5c0bde907369366ea9ea98711823342803
      Reviewed-on: https://go-review.googlesource.com/21349Reviewed-by: default avatarTodd Neal <todd@tneal.org>
      4b95575b
    • Keith Randall's avatar
      cmd/compile: Add more idx1 load/store instructions · af517da2
      Keith Randall authored
      Helpful for indexed loads and stores when the stride is not equal to
      the size being loaded/stored.
      
      Update #7927
      
      Change-Id: I8714dd4c7b18a96a611bf5647ee21f753d723945
      Reviewed-on: https://go-review.googlesource.com/21346
      Run-TryBot: Todd Neal <todd@tneal.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTodd Neal <todd@tneal.org>
      af517da2
    • Keith Randall's avatar
      cmd/compile: place combined loads at the location of the last byte load · b81f2f10
      Keith Randall authored
      We need to make sure all the bounds checks pass before issuing
      a load which combines several others.  We do this by issuing the
      combined load at the last load's block, where "last" = closest to
      the leaf of the dominator tree.
      
      Fixes #15002
      
      Change-Id: I7358116db1e039a072c12c0a73d861f3815d72af
      Reviewed-on: https://go-review.googlesource.com/21246Reviewed-by: default avatarTodd Neal <todd@tneal.org>
      b81f2f10
    • Josh Bleecher Snyder's avatar
      cmd/compile: encapsulate Type.Nname · b83618f9
      Josh Bleecher Snyder authored
      Generated by eg, manually fixed up.
      
      I’m not thrilled about having a setter,
      but given the variety of contexts in which this
      gets fiddled with, it is the cleanest
      available alternative.
      
      Change-Id: Ibdf23e638fe0bdabded014c9e59d557fab8c955f
      Reviewed-on: https://go-review.googlesource.com/21341Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      b83618f9
    • Brad Fitzpatrick's avatar
      net/http/httptest: clean up unnecessary goroutine · 139891e8
      Brad Fitzpatrick authored
      Finishes cleanup which was too late to do when discovered during the
      Go 1.6 cycle.
      
      Fixes #14291
      
      Change-Id: Idc69fadbba10baf246318a22b366709eff088a75
      Reviewed-on: https://go-review.googlesource.com/21360
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
      139891e8
    • Matthew Dempsky's avatar
      cmd/compile, runtime: fix pedantic int->string conversions · e6066711
      Matthew Dempsky authored
      Previously, cmd/compile rejected constant int->string conversions if
      the integer value did not fit into an "int" value. Also, runtime
      incorrectly truncated 64-bit values to 32-bit before checking if
      they're a valid Unicode code point. According to the Go spec, both of
      these cases should instead yield "\uFFFD".
      
      Fixes #15039.
      
      Change-Id: I3c8a3ad9a0780c0a8dc1911386a523800fec9764
      Reviewed-on: https://go-review.googlesource.com/21344Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      e6066711
    • Hiroshi Ioka's avatar
      net/mail: throw error when multiple addresses are given to ParseAddress · e7538df7
      Hiroshi Ioka authored
      Fixes #14610
      
      Change-Id: I3e57dd60b531c1495ea3bc37ef707a1e4e644baa
      Reviewed-on: https://go-review.googlesource.com/20180Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e7538df7
    • Brad Fitzpatrick's avatar
      crypto/x509: add SystemCertPool, refactor system cert pool loading · a62ae9f6
      Brad Fitzpatrick authored
      This exports the system cert pool.
      
      The system cert loading was refactored to let it be run multiple times
      (so callers get a copy, and can't mutate global state), and also to
      not discard errors.
      
      SystemCertPool returns an error on Windows. Maybe it's fixable later,
      but so far we haven't used it, since the system verifies TLS.
      
      Fixes #13335
      
      Change-Id: I3dfb4656a373f241bae8529076d24c5f532f113c
      Reviewed-on: https://go-review.googlesource.com/21293
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
      a62ae9f6
    • Matthew Dempsky's avatar
      cmd/link: remove -H elf flag · 71ab3c1c
      Matthew Dempsky authored
      We create appropriate ELF files automatically based on GOOS. There's
      no point in supporting -H elf flag, particularly since we need to emit
      different flavors of ELF depending on GOOS anyway.
      
      If that weren't reason enough, -H elf appears to be broken since at
      least Go 1.4. At least I wasn't able to find a way to make use of it.
      
      As best I can tell digging through commit history, -H elf is just an
      artifact leftover from Plan 9's 6l linker.
      
      Change-Id: I7393caaadbc60107bbd6bc99b976a4f4fe6b5451
      Reviewed-on: https://go-review.googlesource.com/21343
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      71ab3c1c
    • Brad Fitzpatrick's avatar
      test/fixedbugs: add test for divide by zero being optimized away · 758447cd
      Brad Fitzpatrick authored
      This only tests amd64 because it's currently broken on non-SSA
      backends.
      
      Fixes #8613
      
      Change-Id: I6bc501c81c395e533bb9c7335789750e0c6b7a8f
      Reviewed-on: https://go-review.googlesource.com/21325Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      758447cd
    • Brad Fitzpatrick's avatar
      net/http: allow Handlers to handle http2 upgrade PRI requests · a6557a05
      Brad Fitzpatrick authored
      The http2 spec defines a magic string which initates an http2 session:
      
          "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
      
      It was intentionally chosen to kinda look like an HTTP request, but
      just different enough to break things not ready for it. This change
      makes Go ready for it.
      
      Notably: Go now accepts the request header (the prefix "PRI *
      HTTP/2.0\r\n\r\n") as a valid request, even though it doesn't have a
      Host header. But we now mark it as "Connection: close" and teach the
      Server to never read a second request from the connection once that's
      seen. If the http.Handler wants to deal with the upgrade, it has to
      hijack the request, read out the "body", compare it against
      "SM\r\n\r\n", and then speak http2. One of the new tests demonstrates
      that hijacking.
      
      Fixes #14451
      Updates #14141 (h2c)
      
      Change-Id: Ib46142f31c55be7d00c56fa2624ec8a232e00c43
      Reviewed-on: https://go-review.googlesource.com/21327Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      a6557a05
    • Brad Fitzpatrick's avatar
      net/http: validate transmitted header fields · 0026cb78
      Brad Fitzpatrick authored
      This makes sure the net/http package never attempts to transmit a
      bogus header field key or value and instead fails fast with an error
      to the user, rather than relying on the server to maybe return an
      error.
      
      It's still possible to use x/net/http2.Transport directly to send
      bogus stuff. This change only stops h1 & h2 usage via the net/http
      package. A future change will update x/net/http2.
      
      This change also moves some code from request.go to lex.go, which in a
      separate future change should be moved so it can be shared with http2
      to reduce code bloat.
      
      Updates #14048
      
      Change-Id: I0a44ae1ab357fbfcbe037aa4b5d50669a87f2856
      Reviewed-on: https://go-review.googlesource.com/21326Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      0026cb78
    • Ian Lance Taylor's avatar
      cmd/pprof: use DWARF info to lookup unknown PC addresses · b4117995
      Ian Lance Taylor authored
      Test to follow in a separate CL that arranges for the runtime package to
      store non-Go addresses in a CPU profile.
      
      Change-Id: I33ce1d66b77340b1e62b54505fc9b1abcec108a9
      Reviewed-on: https://go-review.googlesource.com/21055Reviewed-by: default avatarAustin Clements <austin@google.com>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      b4117995
    • Keith Randall's avatar
      runtime: don't use REP;MOVSB if CPUID doesn't say it is fast · 4b209dbf
      Keith Randall authored
      Only use REP;MOVSB if:
       1) The CPUID flag says it is fast, and
       2) The pointers are unaligned
      Otherwise, use REP;MOVSQ.
      
      Update #14630
      
      Change-Id: I946b28b87880c08e5eed1ce2945016466c89db66
      Reviewed-on: https://go-review.googlesource.com/21300Reviewed-by: default avatarNigel Tao <nigeltao@golang.org>
      4b209dbf
    • Dave Cheney's avatar
      cmd/compile/internal/gc: avoid append when building Type fields · 1a9373bc
      Dave Cheney authored
      As a followup to CL 21296, avoid append operations when constructing the
      fields of a Type if the length is known beforehand
      
      This also includes some small scoping driveby cleanups, and a change to
      tointerface0 to avoid iterating over the field list twice.
      
      compilebench shows a very small reduction in allocations.
      
       name      old time/op    new time/op    delta
      Template     364ms ± 5%     363ms ± 4%    ~     (p=0.945 n=20+19)
      Unicode      182ms ±11%     185ms ±12%    ~     (p=0.445 n=20+20)
      GoTypes      1.14s ± 2%     1.14s ± 3%    ~     (p=0.221 n=20+20)
      Compiler     5.85s ± 2%     5.84s ± 2%    ~     (p=0.369 n=20+20)
      
      name      old alloc/op   new alloc/op   delta
      Template    56.7MB ± 0%    56.7MB ± 0%  -0.04%  (p=0.000 n=20+20)
      Unicode     38.3MB ± 0%    38.3MB ± 0%    ~     (p=0.728 n=20+19)
      GoTypes      180MB ± 0%     180MB ± 0%  -0.02%  (p=0.000 n=20+20)
      Compiler     812MB ± 0%     812MB ± 0%  -0.02%  (p=0.000 n=19+20)
      
      name      old allocs/op  new allocs/op  delta
      Template      482k ± 0%      480k ± 0%  -0.34%  (p=0.000 n=20+20)
      Unicode       377k ± 0%      377k ± 0%  -0.04%  (p=0.010 n=20+20)
      GoTypes      1.36M ± 0%     1.35M ± 0%  -0.24%  (p=0.000 n=20+20)
      Compiler     5.47M ± 0%     5.46M ± 0%  -0.11%  (p=0.000 n=20+18)
      
      Change-Id: Ibb4c40229fa3816acd8de98ba41d1571a2aabacf
      Reviewed-on: https://go-review.googlesource.com/21352Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Dave Cheney <dave@cheney.net>
      1a9373bc
    • Dave Cheney's avatar
      cmd/compile/internal/gc: don't let the argument to Fields.Set escape · ea5091fc
      Dave Cheney authored
      Apply Robert's optimisation from CL 21241 to Type.Fields. The results
      are less impressive, possibly because of the makeup of the test data.
      
      name      old time/op    new time/op    delta
      Template     365ms ± 5%     365ms ± 3%    ~     (p=0.888 n=20+16)
      Unicode      182ms ±10%     180ms ± 9%    ~     (p=0.883 n=20+20)
      GoTypes      1.14s ± 2%     1.13s ± 3%    ~     (p=0.096 n=20+20)
      Compiler     5.74s ± 1%     5.76s ± 2%    ~     (p=0.369 n=20+20)
      
      name      old alloc/op   new alloc/op   delta
      Template    56.8MB ± 0%    56.7MB ± 0%  -0.15%  (p=0.000 n=19+20)
      Unicode     38.3MB ± 0%    38.3MB ± 0%  -0.02%  (p=0.006 n=20+19)
      GoTypes      180MB ± 0%     180MB ± 0%  -0.13%  (p=0.000 n=20+20)
      Compiler     805MB ± 0%     804MB ± 0%  -0.05%  (p=0.000 n=20+20)
      
      name      old allocs/op  new allocs/op  delta
      Template      485k ± 0%      482k ± 0%  -0.54%  (p=0.000 n=19+20)
      Unicode       377k ± 0%      377k ± 0%  -0.05%  (p=0.005 n=20+20)
      GoTypes      1.37M ± 0%     1.36M ± 0%  -0.53%  (p=0.000 n=20+19)
      Compiler     5.42M ± 0%     5.41M ± 0%  -0.21%  (p=0.000 n=20+20)
      
      Change-Id: I6782659fadd605ce9931bf5c737c7058b96a29eb
      Reviewed-on: https://go-review.googlesource.com/21296Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      ea5091fc
    • Nigel Tao's avatar
      image/jpeg: reconstruct progressive images even if incomplete. · 225b223e
      Nigel Tao authored
      Fixes #14522.
      
      As I said on that issue:
      
      ----
      This is a progressive JPEG image. There are two dimensions of
      progressivity: spectral selection (variables zs and ze in scan.go,
      ranging in [0, 63]) and successive approximation (variables ah and al in
      scan.go, ranging in [0, 8), from LSB to MSB, although ah=0 implicitly
      means ah=8).
      
      For this particular image, there are three components, and the SOS
      markers contain this progression:
      
      zs, ze, ah, al:  0  0 0 0	components: 0, 1, 2
      zs, ze, ah, al:  1 63 0 0	components: 1
      zs, ze, ah, al:  1 63 0 0	components: 2
      zs, ze, ah, al:  1 63 0 2	components: 0
      zs, ze, ah, al:  1 10 2 1	components: 0
      zs, ze, ah, al: 11 63 2 1	components: 0
      zs, ze, ah, al:  1 10 1 0	components: 0
      
      The combination of all of these is complete (i.e. spectra 0 to 63 and
      bits 8 exclusive to 0) for components 1 and 2, but it is incomplete for
      component 0 (the luma component). In particular, there is no data for
      component 0, spectra 11 to 63 and bits 1 exclusive to 0.
      
      The image/jpeg code, as of Go 1.6, waits until both dimensions are
      complete before performing the de-quantization, IDCT and copy to an
      *image.YCbCr. This is the "if zigEnd != blockSize-1 || al != 0 { ...
      continue }" code and associated commentary in scan.go.
      
      Almost all progressive JPEG images end up complete in both dimensions
      for all components, but this particular image is incomplete for
      component 0, so the Go code never writes anything to the Y values of the
      resultant *image.YCbCr, which is why the broken output is so dark (but
      still looks recognizable in terms of red and blue hues).
      
      My reading of the ITU T.81 JPEG specification (Annex G) doesn't
      explicitly say that this is a valid image, but it also doesn't rule it
      out.
      
      In any case, the fix is, for progressive JPEG images, to always
      reconstruct the decoded blocks (by performing the de-quantization, IDCT
      and copy to an *image.YCbCr), regardless of whether or not they end up
      complete. Note that, in Go, the jpeg.Decode function does not return
      until the entire image is decoded, so we still only want to reconstruct
      each block once, not once per SOS (Start Of Scan) marker.
      ----
      
      A test image was also added, based on video-001.progressive.jpeg. When
      decoding that image, inserting a
      
      println("nComp, zs, ze, ah, al:", nComp, zigStart, zigEnd, ah, al)
      
      into decoder.processSOS in scan.go prints:
      
      nComp, zs, ze, ah, al: 3 0 0 0 1
      nComp, zs, ze, ah, al: 1 1 5 0 2
      nComp, zs, ze, ah, al: 1 1 63 0 1
      nComp, zs, ze, ah, al: 1 1 63 0 1
      nComp, zs, ze, ah, al: 1 6 63 0 2
      nComp, zs, ze, ah, al: 1 1 63 2 1
      nComp, zs, ze, ah, al: 3 0 0 1 0
      nComp, zs, ze, ah, al: 1 1 63 1 0
      nComp, zs, ze, ah, al: 1 1 63 1 0
      nComp, zs, ze, ah, al: 1 1 63 1 0
      
      In other words, video-001.progressive.jpeg contains 10 different scans.
      This little program below drops half of them (remembering to keep the
      "\xff\xd9" End of Image marker):
      
      ----
      package main
      
      import (
      	"bytes"
      	"io/ioutil"
      	"log"
      )
      
      func main() {
      	sos := []byte{0xff, 0xda}
      	eoi := []byte{0xff, 0xd9}
      
      	src, err := ioutil.ReadFile("video-001.progressive.jpeg")
      	if err != nil {
      		log.Fatal(err)
      	}
      	b := bytes.Split(src, sos)
      	println(len(b)) // Prints 11.
      	dst := bytes.Join(b[:5], sos)
      	dst = append(dst, eoi...)
      	if err := ioutil.WriteFile("video-001.progressive.truncated.jpeg", dst, 0666); err != nil {
      		log.Fatal(err)
      	}
      }
      ----
      
      The video-001.progressive.truncated.jpeg was converted to png via
      libjpeg and ImageMagick:
      
      djpeg -nosmooth video-001.progressive.truncated.jpeg > tmp.tga
      convert tmp.tga video-001.progressive.truncated.png
      rm tmp.tga
      
      Change-Id: I72b20cd4fb6746d36d8d4d587f891fb3bc641f84
      Reviewed-on: https://go-review.googlesource.com/21062Reviewed-by: default avatarRob Pike <r@golang.org>
      225b223e
    • Dave Cheney's avatar
      cmd/compile/internal/gc: don't iterate over field list twice · 03731283
      Dave Cheney authored
      In tostruct0 and tofunargs we take a list of nodes, transform them into
      a slice of Fields, set the fields on a type, then use the IterFields
      iterator to iterate over the list again to see if any of them are
      broken.
      
      As we know the slice of fielde-we just created it-we can combine these two
      interations into one pass over the fields.
      
      Change-Id: I8b04c90fb32fd6c3b1752cfc607128a634ee06c5
      Reviewed-on: https://go-review.googlesource.com/21350Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      03731283
  2. 30 Mar, 2016 9 commits