|
1 From 08ea82529a12506da7b329c2d0f6af57b714514a Mon Sep 17 00:00:00 2001 |
|
2 From: Austin Clements <[email protected]> |
|
3 Date: Wed, 18 Nov 2015 14:10:40 -0500 |
|
4 Subject: [PATCH 48/63] [release-branch.go1.5] runtime: prevent sigprof during |
|
5 all stack barrier ops |
|
6 |
|
7 A sigprof during stack barrier insertion or removal can crash if it |
|
8 detects an inconsistency between the stkbar array and the stack |
|
9 itself. Currently we protect against this when scanning another G's |
|
10 stack using stackLock, but we don't protect against it when unwinding |
|
11 stack barriers for a recover or a memmove to the stack. |
|
12 |
|
13 This commit cleans up and improves the stack locking code. It |
|
14 abstracts out the lock and unlock operations. It uses the lock |
|
15 consistently everywhere we perform stack operations, and pushes the |
|
16 lock/unlock down closer to where the stack barrier operations happen |
|
17 to make it more obvious what it's protecting. Finally, it modifies |
|
18 sigprof so that instead of spinning until it acquires the lock, it |
|
19 simply doesn't perform a traceback if it can't acquire it. This is |
|
20 necessary to prevent self-deadlock. |
|
21 |
|
22 Updates #11863, which introduced stackLock to fix some of these |
|
23 issues, but didn't go far enough. |
|
24 |
|
25 Updates #12528. |
|
26 |
|
27 Change-Id: I9d1fa88ae3744d31ba91500c96c6988ce1a3a349 |
|
28 Reviewed-on: https://go-review.googlesource.com/17036 |
|
29 Reviewed-by: Russ Cox <[email protected]> |
|
30 Run-TryBot: Austin Clements <[email protected]> |
|
31 TryBot-Result: Gobot Gobot <[email protected]> |
|
32 Reviewed-on: https://go-review.googlesource.com/17057 |
|
33 --- |
|
34 src/runtime/mgcmark.go | 33 +++++++++++++++++++++++++++++++++ |
|
35 src/runtime/proc1.go | 25 +++++++++++++------------ |
|
36 2 files changed, 46 insertions(+), 12 deletions(-) |
|
37 |
|
38 diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go |
|
39 index 64cc1af..ac93e16 100644 |
|
40 --- a/src/runtime/mgcmark.go |
|
41 +++ b/src/runtime/mgcmark.go |
|
42 @@ -365,6 +365,8 @@ func scanstack(gp *g) { |
|
43 throw("g already has stack barriers") |
|
44 } |
|
45 |
|
46 + gcLockStackBarriers(gp) |
|
47 + |
|
48 case _GCmarktermination: |
|
49 if int(gp.stkbarPos) == len(gp.stkbar) { |
|
50 // gp hit all of the stack barriers (or there |
|
51 @@ -419,6 +421,9 @@ func scanstack(gp *g) { |
|
52 if gcphase == _GCmarktermination { |
|
53 gcw.dispose() |
|
54 } |
|
55 + if gcphase == _GCscan { |
|
56 + gcUnlockStackBarriers(gp) |
|
57 + } |
|
58 gp.gcscanvalid = true |
|
59 } |
|
60 |
|
61 @@ -572,6 +577,8 @@ func gcRemoveStackBarriers(gp *g) { |
|
62 print("hit ", gp.stkbarPos, " stack barriers, goid=", gp.goid, "\n") |
|
63 } |
|
64 |
|
65 + gcLockStackBarriers(gp) |
|
66 + |
|
67 // Remove stack barriers that we didn't hit. |
|
68 for _, stkbar := range gp.stkbar[gp.stkbarPos:] { |
|
69 gcRemoveStackBarrier(gp, stkbar) |
|
70 @@ -581,6 +588,8 @@ func gcRemoveStackBarriers(gp *g) { |
|
71 // adjust them. |
|
72 gp.stkbarPos = 0 |
|
73 gp.stkbar = gp.stkbar[:0] |
|
74 + |
|
75 + gcUnlockStackBarriers(gp) |
|
76 } |
|
77 |
|
78 // gcRemoveStackBarrier removes a single stack barrier. It is the |
|
79 @@ -627,6 +636,7 @@ func gcPrintStkbars(stkbar []stkbar) { |
|
80 // |
|
81 //go:nosplit |
|
82 func gcUnwindBarriers(gp *g, sp uintptr) { |
|
83 + gcLockStackBarriers(gp) |
|
84 // On LR machines, if there is a stack barrier on the return |
|
85 // from the frame containing sp, this will mark it as hit even |
|
86 // though it isn't, but it's okay to be conservative. |
|
87 @@ -635,6 +645,7 @@ func gcUnwindBarriers(gp *g, sp uintptr) { |
|
88 gcRemoveStackBarrier(gp, gp.stkbar[gp.stkbarPos]) |
|
89 gp.stkbarPos++ |
|
90 } |
|
91 + gcUnlockStackBarriers(gp) |
|
92 if debugStackBarrier && gp.stkbarPos != before { |
|
93 print("skip barriers below ", hex(sp), " in goid=", gp.goid, ": ") |
|
94 gcPrintStkbars(gp.stkbar[before:gp.stkbarPos]) |
|
95 @@ -658,6 +669,28 @@ func setNextBarrierPC(pc uintptr) { |
|
96 gp.stkbar[gp.stkbarPos].savedLRVal = pc |
|
97 } |
|
98 |
|
99 +// gcLockStackBarriers synchronizes with tracebacks of gp's stack |
|
100 +// during sigprof for installation or removal of stack barriers. It |
|
101 +// blocks until any current sigprof is done tracebacking gp's stack |
|
102 +// and then disallows profiling tracebacks of gp's stack. |
|
103 +// |
|
104 +// This is necessary because a sigprof during barrier installation or |
|
105 +// removal could observe inconsistencies between the stkbar array and |
|
106 +// the stack itself and crash. |
|
107 +func gcLockStackBarriers(gp *g) { |
|
108 + for !cas(&gp.stackLock, 0, 1) { |
|
109 + osyield() |
|
110 + } |
|
111 +} |
|
112 + |
|
113 +func gcTryLockStackBarriers(gp *g) bool { |
|
114 + return cas(&gp.stackLock, 0, 1) |
|
115 +} |
|
116 + |
|
117 +func gcUnlockStackBarriers(gp *g) { |
|
118 + atomicstore(&gp.stackLock, 0) |
|
119 +} |
|
120 + |
|
121 // TODO(austin): Can we consolidate the gcDrain* functions? |
|
122 |
|
123 // gcDrain scans objects in work buffers, blackening grey |
|
124 diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go |
|
125 index 55f1a24..54cb3eb 100644 |
|
126 --- a/src/runtime/proc1.go |
|
127 +++ b/src/runtime/proc1.go |
|
128 @@ -414,13 +414,7 @@ func scang(gp *g) { |
|
129 // the goroutine until we're done. |
|
130 if castogscanstatus(gp, s, s|_Gscan) { |
|
131 if !gp.gcscandone { |
|
132 - // Coordinate with traceback |
|
133 - // in sigprof. |
|
134 - for !cas(&gp.stackLock, 0, 1) { |
|
135 - osyield() |
|
136 - } |
|
137 scanstack(gp) |
|
138 - atomicstore(&gp.stackLock, 0) |
|
139 gp.gcscandone = true |
|
140 } |
|
141 restartg(gp) |
|
142 @@ -2500,11 +2494,6 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) { |
|
143 // Profiling runs concurrently with GC, so it must not allocate. |
|
144 mp.mallocing++ |
|
145 |
|
146 - // Coordinate with stack barrier insertion in scanstack. |
|
147 - for !cas(&gp.stackLock, 0, 1) { |
|
148 - osyield() |
|
149 - } |
|
150 - |
|
151 // Define that a "user g" is a user-created goroutine, and a "system g" |
|
152 // is one that is m->g0 or m->gsignal. |
|
153 // |
|
154 @@ -2571,8 +2560,18 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) { |
|
155 // transition. We simply require that g and SP match and that the PC is not |
|
156 // in gogo. |
|
157 traceback := true |
|
158 + haveStackLock := false |
|
159 if gp == nil || sp < gp.stack.lo || gp.stack.hi < sp || setsSP(pc) { |
|
160 traceback = false |
|
161 + } else if gp.m.curg != nil { |
|
162 + if gcTryLockStackBarriers(gp.m.curg) { |
|
163 + haveStackLock = true |
|
164 + } else { |
|
165 + // Stack barriers are being inserted or |
|
166 + // removed, so we can't get a consistent |
|
167 + // traceback right now. |
|
168 + traceback = false |
|
169 + } |
|
170 } |
|
171 var stk [maxCPUProfStack]uintptr |
|
172 n := 0 |
|
173 @@ -2615,7 +2614,9 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) { |
|
174 } |
|
175 } |
|
176 } |
|
177 - atomicstore(&gp.stackLock, 0) |
|
178 + if haveStackLock { |
|
179 + gcUnlockStackBarriers(gp.m.curg) |
|
180 + } |
|
181 |
|
182 if prof.hz != 0 { |
|
183 // Simple cas-lock to coordinate with setcpuprofilerate. |
|
184 -- |
|
185 2.6.1 |
|
186 |