Repository files navigation

References to range loop variables found on GitHub.

The findings in the issue tracker of this repository were gathered in order to better understand the impact ofthis proposed change.

The findings in this repository may be inaccurate, and/or contain broken links, as the tool used to produce them remains under active development. There are several known false-negatives which can arise using the current tool, so the absence of a finding must not be construed as safety. For more information, seethis issue tracker.

This repository is "issue-only". It exists to capture the findings ofa static analysis bot, which reads Go repositories found on GitHub and captures potential problems aroundrange loop variable references.

Range-loop capture is the reason this code prints4, 4, 4, 4, instead of what you might expect.

xs:= []int{1,2,3,4}for_,x:=rangexs {gofunc() {fmt.Printf("%d",x)    }()}

But why raise issues for all these code examples?

Members of the Go language team have indicated a willingness to modify the behavior of range loop variable capture to make Go more intuitive. This change could be made despite thestrong backwards compatibility guarantee of Go version 1,only if we can ensure that the change will not result in incorrect behavior in current programs.

To make that determination, a large number of "real world" Go programs would need to be vetted. If we find that, in every case, the current compiler behavior results in an undesirable outcome (aka bugs), we can consider makinga change to the language.

The goal of thegithub-vet project is to motivate such a change by gathering static analysis results from Go code hosted in publicly available GitHub repositories, and crowd-sourcing their human analysis.

We're glad you're here! We need the help of the Go community to examine our bot's findings to determine if they represent a bug or not.

Each instance we've gathered falls into one of three buckets (explained further below).

1. Bug 👎 - as written, the captured loop variable could lead to undesirable behavior.
2. Mitigated 👍 - the captured loop variable does not escape the block of the for loop, or if it does, it is handled in such a way that no undesirable behavior can occur.
3. Desirable Behavior 🚀 - the current range loop semantics are required for the correct operation of the program. Changing the behavior of the compileras outlined in this issue would make this code incorrect.

Findings can be classified by the community by marking the listed emoji reactions to each finding. A separate bot gathers the community assessment and uses it to label each instance automatically, in addition to input from experts.

We don't currently have any examples of desirable behavior which depends on the current semantics of range-loop captures. The goal of this project is to determine whether such an example exists in real-world code.

We don't think such an example does exist. This means that any examples marked as 'Desirable' will be faced with a high degree of scrutiny. Don't let this discourage you from reporting it. Just be aware that we may disagree, so is important to engage with a sense of professional detachment. Be humble and keep learning.

An example is a bug when the captured loop variable causes "undesirable" or "confusing" behavior. These terms are somewhat subjective as they depend on the programmer's intent. While we can'tknow the programmer's original intent, wecan ask whether the actual behavior would be better achieved in a different way.

For example, the below finding is an example of undesirable behavior. A reference to the loop variableaction is captured in the goroutine started within the block of the for loop. There is a race-condition between updating the value ofaction at the beginning of the loop and reading the value ofaction within the goroutine. Iflen(d.workers) > 1, it is usually the case that the only action to be run will be the last entry visited in the range loop. We mark this as a bug because if that were the intent, it would have been written differently.

forqueueName,action:=ranged.workers {q:=d.queue.Job[queueName]gofunc() {for {action(q)}}()}

As another example, in the below finding, undesirable behavior also occurs. Updates to the value ofn race against the start of the goroutine, ensuring there is no guarantee that.Run() will be called on every value ing.nodes, which is clearly the intent of the range loop.

for_,n:=rangeg.nodes {wg.Add(1)gofunc() {err:=n.Run()iferr!=nil {c<-err    }  }()}

These examples aren't bugs because they have been written in such a way that the capture is mitigated. "Mitigated" examples aren't as interesting as desirable behavior, but we want to mark them separately, so we can consider whether the static analysis could detect them automatically.

For instance, the below finding is 'mitigated' by explicitly passing thereader variable into the function started via a goroutine.

for_,reader:=rangedec.rs {wg.Add(1)gofunc(r io.Reader) {deferwg.Done()tris,err:=dec.newDecoderFunc(r).Decode()select {caseresults<-&result{tris:tris,err:err,reader:r}:case<-done:return}}(reader)}

Another common instance of mitigated use is found in tests like the below finding. Thedone channel in this example is used to ensure that the goroutine finishes before the value oftt is updated. This means that the value oftt is not overwritten before the goroutine completes, so there is no race condition.

fori,tt:=rangetests {txn:=s.Write()done:=make(chanstruct{},1)gofunc() {tt()done<-struct{}{}}()select {case<-done:t.Fatalf("#%d: operation failed to be blocked",i)case<-time.After(10*time.Millisecond):}txn.End()select {case<-done:case<-time.After(10*time.Second):testutil.FatalStack(t,fmt.Sprintf("#%d: operation failed to be unblocked",i))}}

To mark a finding, simply add the appropriate emoji reaction to the issue.

VetBot does a lot of work to try and prove that no reference to a range-loop variable is improperly used. To do so, it checks the broad classes of errors seen below (along with a minimal example).

for_,i:=range []int{1,2,3} {gofunc() {fmt.Print(i)// prints 3,3,3 (usually)   }()}

Reported asrange-loop variable i used in defer or goroutine at line 3.

varx*intfor_,i:=range []int{1,2,3} {x=&i}

Reported asreference to i is reassigned at line 3.

varintPtr*intfuncunsafe(x*int) {intPtr=x}for_,i:=range []int{1,2,3} {unsafe(&i)}

Reported asfunction call at line 7 may store a reference to i.

A Type I error would also be reported in caseunsafe uses the value ofx inside a composite literal (i.e.Foo{1, "bar", &x}).

funcunsafe(x*int) {gofunc() {fmt.Println(x)  }()}for_,i:=range []int{1,2,3} {unsafe(&i)}

Reported asfunction call which takes a reference to i at line 8 may start a goroutine.

At the time of this writing, a Type II error would be reported whetherunsafe uses the value ofx inside the goroutine or not.

Both the Type I and II errors reported above are traced inductively throughout the call graph. Roughly speaking, if a functionf calls a functiong which is marked as unsafe, thenf is also considered unsafe.

For more information on the static analysis used to produce these findings, seethe documentation for the VetBot