runtime: nowritebarrierrec doesn't cross systemstack #22384
Comments
|
/cc @RLH |
|
Change https://golang.org/cl/72772 mentions this issue: |
|
Change https://golang.org/cl/72770 mentions this issue: |
|
Change https://golang.org/cl/72771 mentions this issue: |
|
Change https://golang.org/cl/72773 mentions this issue: |
|
@aclements Do you have any thoughts on moving the interprocedural checks to a go/types-based checker? I'm wondering if that would be easier to maintain since the go/types API is better documented and also more stable. The intraprocedural stuff seems easy enough to keep in the compiler (e.g., nothing escaping to the heap, disallowing write barriers in nowritebarrier functions), but the interprocedural stuff seems to cause us trouble. |
|
@mdempsky, I'm not sure how that would work since it needs to know exactly where the compiler inserted write barriers, which go/types wouldn't know. |
|
@aclements Ah, right, I was thinking for a second that we explicitly mark "//go:nowritebarrier" on all functions that can't use write barriers, but I forgot the entire point of "//go:nowritebarrierrec" is to avoid that. I suppose the go/types checker could use compile -m (or whichever flag) output to identify which functions have write barriers, and feed that into its analysis. |
|
I thought of another reason doing this in go/types would be difficult: we want to see the implicit runtime calls that operations like copy, panic, etc. get lowered to. |
|
Yeah, I'm not thinking an exclusively go/types-based checker, but one that runs "compile -m" to first determine which functions contain write barriers (or whatever other interesting properties), and then go/types is used for understanding and validating the call graph. |
We're about to start tracking nowritebarrierrec through systemstack calls, which will reveal write barriers in startpanic_m prohibited by various callers. We actually can allow write barriers here because the write barrier is a no-op when we're panicking. Let the compiler know. Updates #22384. For #22460. Change-Id: Ifb3a38d3dd9a4125c278c3680f8648f987a5b0b8 Reviewed-on: https://go-review.googlesource.com/72770 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
We're about to start tracking nowritebarrierrec through systemstack calls, which will reveal write barriers in persistentalloc prohibited by various callers. The pointers manipulated by persistentalloc are always to off-heap memory, so this removes these write barriers statically by introducing a new go:notinheap type to represent generic off-heap memory. Updates #22384. For #22460. Change-Id: Id449d9ebf145b14d55476a833e7f076b0d261d57 Reviewed-on: https://go-review.googlesource.com/72771 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
We're about to start tracking nowritebarrierrec through systemstack calls, which detects that we're calling markroot (which has write barriers) from gchelper, which is called from the scheduler during STW apparently without a P. But it turns out that func helpgc, which wakes up blocked Ms to run gchelper, installs a P for gchelper to use. This means there *is* a P when gchelper runs, so it is allowed to have write barriers. Tell the compiler this by marking gchelper go:yeswritebarrierrec. Also, document the call to gchelper so I don't have to spend another half a day puzzling over how on earth this could possibly work before discovering the spooky action-at-a-distance in helpgc. Updates #22384. For #22460. Change-Id: I7394c9b4871745575f87a2d4fbbc5b8e54d669f7 Reviewed-on: https://go-review.googlesource.com/72772 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
When a function is marked go:nowritebarrierrec in the runtime, the compiler recursively disallows write barriers in functions called from it. However, this static check doesn't know that
systemstackcalls its argument. Since this is quite common in the runtime, we're actually failing to catch several write barriers in prohibited places (I found this the hard way when I made the write barrier a little more picky). I believe all of these are technically benign, but for subtle reasons. I don't like relying on subtle reasons.We should fix this analysis to understand
systemstackand fix the bad write barriers in the runtime. I have most of this done already, but some of the write barriers are proving difficult to remove, so this issue is to track this work.The text was updated successfully, but these errors were encountered: