Skip to content

G
golang
Switch branch/tag
  1. 10 Mar, 2018 7 commits
  3. 09 Mar, 2018 20 commits
  5. 08 Mar, 2018 13 commits
    • Austin Clements's avatar
      runtime: explain and enforce that _panic values live on the stack · 5d22cebb
      Austin Clements authored 
      It's a bit mysterious that _defer.sp is a uintptr that gets
stack-adjusted explicitly while _panic.argp is an unsafe.Pointer that
doesn't, but turns out to be critically important when a deferred
function grows the stack before doing a recover.

Add a comment explaining that this works because _panic values live on
the stack. Enforce this by marking _panic go:notinheap.

Change-Id: I9ca49e84ee1f86d881552c55dccd0662b530836b
Reviewed-on: https://go-review.googlesource.com/99735
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarMatthew Dempsky <mdempsky@google.com>
      5d22cebb
    • Austin Clements's avatar
      runtime: ensure abort actually crashes the process · 60a9e5d6
      Austin Clements authored 
      On all non-x86 arches, runtime.abort simply reads from nil.
Unfortunately, if this happens on a user stack, the signal handler
will dutifully turn this into a panicmem, which lets user defers run
and which user code can even recover from.

To fix this, add an explicit check to the signal handler that turns
faults in abort into hard crashes directly in the signal handler. This
has the added benefit of giving a register dump at the abort point.

Change-Id: If26a7f13790745ee3867db7f53b72d8281176d70
Reviewed-on: https://go-review.googlesource.com/93661
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      60a9e5d6
    • Austin Clements's avatar
      runtime: call abort instead of raw INT $3 or bad MOV · c950a90d
      Austin Clements authored 
      Everything except for amd64, amd64p32, and 386 currently defines and
uses an abort function. This CL makes these match. The next CL will
recognize the abort function to make this more useful.

Change-Id: I7c155871ea48919a9220417df0630005b444f488
Reviewed-on: https://go-review.googlesource.com/93660
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      c950a90d
    • Austin Clements's avatar
      runtime: make throw safer to call · 7f1b2738
      Austin Clements authored 
      Currently, throw may grow the stack, which means whenever we call it
from a context where it's not safe to grow the stack, we first have to
switch to the system stack. This is pretty easy to get wrong.

Fix this by making throw switch to the system stack so it doesn't grow
the stack and is hence safe to call without a system stack switch at
the call site.

The only thing this complicates is badsystemstack itself, which would
now go into an infinite loop before printing anything (previously it
would also go into an infinite loop, but would at least print the
error first). Fix this by making badsystemstack do a direct write and
then crash hard.

Change-Id: Ic5b4a610df265e47962dcfa341cabac03c31c049
Reviewed-on: https://go-review.googlesource.com/93659
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      7f1b2738
    • Austin Clements's avatar
      runtime: move unrecoverable panic handling to the system stack · 9d59234c
      Austin Clements authored 
      Currently parts of unrecoverable panic handling (notably, printing
panic messages) can happen on the user stack. This may grow the stack,
which is generally fine, but if we're handling a runtime panic, it's
better to do as little as possible in case the runtime is in an
inconsistent state.

Hence, this commit rearranges the handling of unrecoverable panics so
that it's done entirely on the system stack.

This is mostly a matter of shuffling code a bit so everything can move
into a systemstack block. The one slight subtlety is in the "panic
during panic" case, where we now depend on startpanic_m's caller to
print the stack rather than startpanic_m itself. To make this work,
startpanic_m now returns a boolean indicating that the caller should
avoid trying to print any panic messages and get right to the stack
trace. Since the caller is already in a position to do this, this
actually simplifies things a little.

Change-Id: Id72febe8c0a9fb31d9369b600a1816d65a49bfed
Reviewed-on: https://go-review.googlesource.com/93658
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      9d59234c
    • Austin Clements's avatar
      cmd/compile: simplify OpSlicemask optimization · da022da9
      Austin Clements authored 
      The previous CL introduced isConstDelta. Use it to simplify the
OpSlicemask optimization in the prove pass. This passes toolstash
-cmp.

Change-Id: If2aa762db4cdc0cd1c581a536340530a9831081b
Reviewed-on: https://go-review.googlesource.com/87481Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      da022da9
    • Austin Clements's avatar
      cmd/compile: add fence-post implications to prove · 6436270d
      Austin Clements authored 
      This adds four new deductions to the prove pass, all related to adding
or subtracting one from a value. This is the first hint of actual
arithmetic relations in the prove pass.

The most effective of these is

   x-1 >= w && x > min  ⇒  x > w

This helps eliminate bounds checks in code like

  if x > 0 {
    // do something with s[x-1]
  }

Altogether, these deductions prove an additional 260 branches in std
and cmd. Furthermore, they will let us eliminate some tricky
compiler-inserted panics in the runtime that are interfering with
static analysis.

Fixes #23354.

Change-Id: I7088223e0e0cd6ff062a75c127eb4bb60e6dce02
Reviewed-on: https://go-review.googlesource.com/87480Reviewed-by: 's avatarKeith Randall <khr@golang.org>
Reviewed-by: 's avatarAlexandru Moșoi <alexandru@mosoi.ro>
      6436270d
    • Austin Clements's avatar
      cmd/compile: derive unsigned limits from signed limits in prove · 941fc129
      Austin Clements authored 
      This adds a few simple deductions to the prove pass' fact table to
derive unsigned concrete limits from signed concrete limits where
possible.

This tweak lets the pass prove 70 additional branch conditions in std
and cmd.

This is based on a comment from the recently-deleted factsTable.get:
"// TODO: also use signed data if lim.min >= 0".

Change-Id: Ib4340249e7733070f004a0aa31254adf5df8a392
Reviewed-on: https://go-review.googlesource.com/87479Reviewed-by: 's avatarAlexandru Moșoi <alexandru@mosoi.ro>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      941fc129
    • Austin Clements's avatar
      cmd/compile: make prove pass use unsatisfiability · 669db2ce
      Austin Clements authored 
      Currently the prove pass uses implication queries. For each block, it
collects the set of branch conditions leading to that block, and
queries this fact table for whether any of these facts imply the
block's own branch condition (or its inverse). This works remarkably
well considering it doesn't do any deduction on these facts, but it
has various downsides:

1. It requires an implementation both of adding facts to the table and
   determining implications. These are very nearly duals of each
   other, but require separate implementations. Likewise, the process
   of asserting facts of dominating branch conditions is very nearly
   the dual of the process of querying implied branch conditions.

2. It leads to less effective use of derived facts. For example, the
   prove pass currently derives facts about the relations between len
   and cap, but can't make use of these unless a branch condition is
   in the exact form of a derived fact. If one of these derived facts
   contradicts another fact, it won't notice or make use of this.

This CL changes the approach of the prove pass to instead use
*contradiction* instead of implication. Rather than ever querying a
branch condition, it simply adds branch conditions to the fact table.
If this leads to a contradiction (specifically, it makes the fact set
unsatisfiable), that branch is impossible and can be cut. As a result,

1. We can eliminate the code for determining implications
   (factsTable.get disappears entirely). Also, there is now a single
   implementation of visiting and asserting branch conditions, since
   we don't have to flip them around to treat them as facts in one
   place and queries in another.

2. Derived facts can be used effectively. It doesn't matter *why* the
   fact table is unsatisfiable; a contradiction in any of the facts is
   enough.

3. As an added benefit, it's now quite easy to avoid traversing beyond
   provably-unreachable blocks. In contrast, the current
   implementation always visits all blocks.

The prove pass already has nearly all of the mechanism necessary to
compute unsatisfiability, which means this both simplifies the code
and makes it more powerful.

The only complication is that the current implication procedure has a
hack for dealing with the 0 <= Args[0] condition of OpIsInBounds and
OpIsSliceInBounds. We replace this with asserting the appropriate fact
when we process one of these conditions. This seems much cleaner
anyway, and works because we can now take advantage of derived facts.

This has no measurable effect on compiler performance.

Effectiveness:

There is exactly one condition in all of std and cmd that this fails
to prove that the old implementation could: (int64(^uint(0)>>1) < x)
in encoding/gob. This can never be true because x is an int, and it's
basically coincidence that the old code gets this. (For example, it
fails to prove the similar (x < ^int64(^uint(0)>>1)) condition that
immediately precedes it, and even though the conditions are logically
unrelated, it wouldn't get the second one if it hadn't first processed
the first!)

It does, however, prove a few dozen additional branches. These come
from facts that are added to the fact table about the relations
between len and cap. These were almost never queried directly before,
but could lead to contradictions, which the unsat-based approach is
able to use.

There are exactly two branches in std and cmd that this implementation
proves in the *other* direction. This sounds scary, but is okay
because both occur in already-unreachable blocks, so it doesn't matter
what we chose. Because the fact table logic is sound but incomplete,
it fails to prove that the block isn't reachable, even though it is
able to prove that both outgoing branches are impossible. We could
turn these blocks into BlockExit blocks, but it doesn't seem worth the
trouble of the extra proof effort for something that happens twice in
all of std and cmd.

Tests:

This CL updates test/prove.go to change the expected messages because
it can no longer give a "reason" why it proved or disproved a
condition. It also adds a new test of a branch it couldn't prove
before.

It mostly guts test/sliceopt.go, removing everything related to slice
bounds optimizations and moving a few relevant tests to test/prove.go.
Much of this test is actually unreachable. The new prove pass figures
this out and doesn't try to prove anything about the unreachable
parts. The output on the unreachable parts is already suspect because
anything can be proved at that point, so it's really just a regression
test for an algorithm the compiler no longer uses.

This is a step toward fixing #23354. That issue is quite easy to fix
once we can use derived facts effectively.

Change-Id: Ia48a1b9ee081310579fe474e4a61857424ff8ce8
Reviewed-on: https://go-review.googlesource.com/87478Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      669db2ce
    • Austin Clements's avatar
      cmd/compile: simplify limit logic in prove · 2e9cf5f6
      Austin Clements authored 
      This replaces the open-coded intersection of limits in the prove pass
with a general limit intersection operation. This should get identical
results except in one case where it's more precise: when handling an
equality relation, if the value is *outside* the existing range, this
will reduce the range to empty rather than resetting it. This will be
important to a follow-up CL where we can take advantage of empty
ranges.

For #23354.

Change-Id: I3d3d75924f61b1da1cb604b3a9d189b26fb3a14e
Reviewed-on: https://go-review.googlesource.com/87477
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
Reviewed-by: 's avatarAlexandru Moșoi <alexandru@mosoi.ro>
      2e9cf5f6
    • Austin Clements's avatar
      cmd/compile: more String methods for prove types · 44e20b64
      Austin Clements authored 
      These aid in debugging.

Change-Id: Ieb38c996765f780f6103f8c3292639d408c25123
Reviewed-on: https://go-review.googlesource.com/87476
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
      44e20b64
    • Austin Clements's avatar
      cmd/compile: minor comment improvements/corrections · 491f409a
      Austin Clements authored 
      Change-Id: Ie0934f1528d58d4971cdef726d3e2d23cf3935d3
Reviewed-on: https://go-review.googlesource.com/87475
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarDavid Chase <drchase@google.com>
Reviewed-by: 's avatarKeith Randall <khr@golang.org>
Reviewed-by: 's avatarAlexandru Moșoi <alexandru@mosoi.ro>
      491f409a
    • Matthew Dempsky's avatar
      Revert "cmd/compile: cleanup nodpc and nodfp" · b55eedd1
      Matthew Dempsky authored 
      This reverts commit dcac984b.

Reason for revert: broke LR architectures (arm64, ppc64, s390x)

Change-Id: I531d311c9053e81503c8c78d6cf044b318fc828b
Reviewed-on: https://go-review.googlesource.com/99695
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarAustin Clements <austin@google.com>
      b55eedd1