[インデックス 14734] ファイルの概要

このコミットは、Goコンパイラのcmd/gcパッケージにおけるselectステートメントの評価順序に関するバグ修正を扱っています。具体的には、select文内で変数のロードが行われる際に、その変数の値が後続の関数呼び出しによって変更される可能性がある場合でも、コンパイラがその変数の値を保存する（一時変数に退避させる）必要がないと誤って判断していた問題を修正します。これにより、select文の実行中に予期せぬ変数の値の変更が発生する可能性がありました。

コミット

commit 1b3244e0dbbe547a0703d0380708f480a8f3c228
Author: Russ Cox <rsc@golang.org>
Date:   Sat Dec 22 16:46:01 2012 -0500

    cmd/gc: fix eval order in select
    
    Ordinary variable load was assumed to be not worth saving,
    but not if one of the function calls later might change
    its value.
    
    Fixes #4313.
    
    R=ken2
    CC=golang-dev
    https://golang.org/cl/6997047

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/1b3244e0dbbe547a0703d0380708f480a8f3c228

元コミット内容

cmd/gc: fix eval order in select

通常の変数ロードは、保存する価値がないと見なされていましたが、後続の関数呼び出しのいずれかがその値を変更する可能性がある場合はそうではありませんでした。

Issue #4313 を修正します。

変更の背景

Go言語のselectステートメントは、複数の通信操作（チャネルの送受信）を待機し、準備ができた最初の操作を実行するための強力な制御構造です。このステートメントのセマンティクスは、各ケースの式が評価される順序と、その評価がチャネル操作の選択にどのように影響するかに依存します。

このコミットの背景には、selectステートメント内で使用される変数の評価順序に関する潜在的なバグがありました。Goコンパイラ（cmd/gc）は、コードを最適化する際に、一時的な変数のロードが「保存する価値がない」（つまり、その値がすぐに必要とされ、後で変更される可能性がない）と判断することがあります。しかし、selectケース内でチャネル操作の引数として変数が使用され、かつそのチャネル操作の評価中に別の関数呼び出し（例えば、selectの他のケースの評価の一部として実行される関数）がその変数の値を変更する可能性がある場合、この最適化は誤った動作を引き起こす可能性がありました。

具体的には、select文の評価中に、あるチャネル操作の引数として使用される変数の値が、別のチャネル操作の準備のために実行される関数によって変更されてしまうと、最初のチャネル操作が古い（変更前の）値で実行されてしまうという問題です。これは、Goのメモリモデルと評価順序の保証に反する動作であり、予測不能なプログラムの挙動につながります。

Issue #4313 はこの問題点を指摘しており、このコミットはその修正を目的としています。テストケース test/fixedbugs/issue4313.go は、このバグがどのように顕在化するかを示しています。

前提知識の解説

このコミットを理解するためには、以下のGo言語およびコンパイラの概念に関する知識が必要です。

1. Go言語のselectステートメント: selectステートメントは、複数のチャネル操作を同時に待機し、準備ができた最初の操作を実行します。もし複数の操作が同時に準備できた場合、selectはランダムに1つを選択します。defaultケースが存在する場合、どのチャネル操作も準備できていない場合にdefaultケースが実行されます。 selectの各ケース内の式（チャネル式や送信値など）は、selectがどのケースを選択するかを決定する前に評価されます。この評価順序が重要です。

2. Goコンパイラ (cmd/gc): cmd/gcは、Go言語の公式コンパイラです。Goのソースコードを中間表現に変換し、最終的に実行可能なバイナリを生成します。コンパイルプロセスには、構文解析、型チェック、最適化、コード生成などが含まれます。

3. 評価順序 (Evaluation Order): Go言語の仕様では、式の評価順序が厳密に定義されています。特に、関数呼び出しの引数や複合リテラルの要素など、特定の操作における評価順序は、プログラムのセマンティクスを保証するために重要です。selectステートメントのケース内の式も、特定の順序で評価される必要があります。

4. Node構造体とninitリスト: Goコンパイラの内部では、プログラムの抽象構文木 (AST) がNode構造体で表現されます。各Nodeは、式、ステートメント、宣言などを表します。 ninitは、Node構造体の一部であり、そのNodeの評価に必要な初期化ステートメントのリスト（NodeList）を保持します。例えば、一時変数の宣言や、式の評価に伴う副作用のある操作などがninitリストに追加されます。コンパイラは、ninitリスト内のステートメントを、関連するNodeが評価される前に実行するようにスケジュールします。これは、式の評価順序を制御し、副作用が正しく発生するようにするために非常に重要です。

5. safeexprとlocalexpr: コンパイラの最適化フェーズで使用される関数です。

   • safeexpr(Node *n, NodeList **init): 式nが安全に評価できることを保証します。もしnが副作用を持つか、複数回評価されると問題が生じる可能性がある場合、nの値を一時変数に退避させるなどの処理を行い、その初期化ステートメントをinitリストに追加します。
   • localexpr(Node *n, Type *t, NodeList **init): 式nの値を、指定された型tのローカル変数に格納します。これは、nが複雑な式である場合や、その値が後で変更される可能性がある場合に、その値を「固定」するために使用されます。この関数は、nが既に適切なローカル変数である場合は何もしません。

6. ONAMEノードとaddrtakenフラグ: ONAMEノードは、変数や関数名などの識別子を表します。 n->addrtakenフラグは、その変数のアドレスが取得されたことがあるかどうかを示します。アドレスが取得された変数は、ポインタを介して間接的に変更される可能性があるため、コンパイラはより慎重に扱います。

技術的詳細

このコミットの技術的な核心は、selectステートメントのケース内でチャネル操作の引数として使用される変数の評価と、その値の「固定」に関するコンパイラの挙動の修正です。

Goコンパイラは、selectステートメントを処理する際に、各caseを内部的に変換します。この変換プロセスでは、チャネル操作（送受信）がselectsendやselectrecvといったランタイム関数呼び出しに置き換えられます。これらのランタイム関数は、チャネル操作の準備状況をチェックし、実際に操作を実行します。

問題は、これらのランタイム関数に渡される引数（特に送信値や受信バッファ）が、select文の評価中に他のcaseの評価によって変更される可能性がある場合に発生しました。コンパイラは、通常の変数ロードを「保存する価値がない」と判断し、一時変数への退避を行わないことがありました。しかし、selectの文脈では、あるcaseの評価が別のcaseの評価に影響を与える可能性があるため、この仮定は危険でした。

このコミットでは、以下の主要な変更が行われています。

1. src/cmd/gc/order.c の変更: orderstmt関数は、ステートメントの評価順序を決定し、必要な初期化ステートメントをninitリストに追加する役割を担います。

   • OSELRECV2 (select receive with two return values, i.e., v, ok := <-ch) と OSEND (channel send) のケースで、orderexpr関数の第2引数がout（現在のステートメントリスト）からl->n->ninit（現在のselectケースの初期化リスト）に変更されています。これは、これらのチャネル操作の引数（受信バッファや送信値）の評価に伴う初期化ステートメントが、selectケースの初期化リストに確実に追加されるようにするためです。これにより、これらの引数がselectの他の部分の評価によって影響を受ける前に、その値が「固定」されるようになります。

2. src/cmd/gc/select.c の変更: walkselect関数は、selectステートメントをウォークし、ランタイム関数呼び出しに変換する主要な場所です。

   • r->nbody = cas->ninit; が r->ninit = cas->ninit; に変更されています。これは、selectケースの初期化リストが、生成されるifステートメントのninitリストに正しく引き継がれるようにするためです。
   • selectdefault, selectsend, selectrecv, selectrecv2 のランタイム関数呼び出し (mkcall または mkcall1) の第3引数（初期化リスト）が &init から &r->ninit に変更されています。これは、これらのランタイム関数呼び出しの引数（チャネル、送信値、受信バッファなど）の評価に伴う初期化ステートメントが、selectケース全体の初期化リストに確実に追加されるようにするためです。
   • OSEND のケースで、n->left = safeexpr(n->left, &r->ninit); の行が n->left = localexpr(safeexpr(n->left, &r->ninit), n->left->type, &r->ninit); に変更されています。これは、チャネル送信操作のチャネル式 (n->left) が、localexprによってローカル変数に「固定」されるようにするためです。これにより、チャネル式が評価された後、その値が他のselectケースの評価によって変更されることを防ぎます。

3. src/cmd/gc/subr.c の変更: localexpr関数は、式をローカル変数に変換する役割を担います。

   • if(n->op == ONAME && の条件に !n->addrtaken && が追加されています。これは、ONAMEノード（変数）が既にローカル変数であり、かつそのアドレスが取得されていない場合にのみ、localexprがその変数をそのまま返すようにするためです。もし変数のアドレスが取得されている場合（addrtakenがtrue）、その変数はポインタを介して間接的に変更される可能性があるため、localexprは新しい一時変数を作成して値をコピーする必要があります。この変更により、localexprは、アドレスが取得された変数に対しても、その値を安全に「固定」できるようになります。

これらの変更により、selectステートメント内のチャネル操作の引数として使用される変数の値が、selectの評価中に他の部分の副作用によって予期せず変更されることがなくなります。特に、localexprの改善と、ninitリストへの初期化ステートメントの適切な追加が、この問題の解決に貢献しています。

コアとなるコードの変更箇所

src/cmd/gc/order.c

--- a/src/cmd/gc/order.c
+++ b/src/cmd/gc/order.c
@@ -276,11 +276,11 @@ orderstmt(Node *n, NodeList **out)
 			case OSELRECV2:
 				orderexprinplace(&r->left);
 				orderexprinplace(&r->ntest);
-				orderexpr(&r->right->left, out);
+				orderexpr(&r->right->left, &l->n->ninit);
 				break;
 			case OSEND:
-				orderexpr(&r->left, out);
-				orderexpr(&r->right, out);
+				orderexpr(&r->left, &l->n->ninit);
+				orderexpr(&r->right, &l->n->ninit);
 				break;
 			}
 		}

src/cmd/gc/select.c

--- a/src/cmd/gc/select.c
+++ b/src/cmd/gc/select.c
@@ -297,15 +297,15 @@ walkselect(Node *sel)
 		setlineno(cas);
 		n = cas->left;
 		r = nod(OIF, N, N);
-		r->nbody = cas->ninit;
+		r->ninit = cas->ninit;
 		cas->ninit = nil;
 		if(n != nil) {
-			r->nbody = concat(r->nbody, n->ninit);
+			r->ninit = concat(r->ninit, n->ninit);
 			n->ninit = nil;
 		}
 		if(n == nil) {
 			// selectdefault(sel *byte);
-			r->ntest = mkcall("selectdefault", types[TBOOL], &init, var);
+			r->ntest = mkcall("selectdefault", types[TBOOL], &r->ninit, var);
 		} else {
 			switch(n->op) {
 			default:
@@ -313,25 +313,25 @@ walkselect(Node *sel)
 	
 			case OSEND:
 				// selectsend(sel *byte, hchan *chan any, elem *any) (selected bool);
-				n->left = safeexpr(n->left, &r->ninit);
+				n->left = localexpr(safeexpr(n->left, &r->ninit), n->left->type, &r->ninit);
 				n->right = localexpr(n->right, n->left->type->type, &r->ninit);
 				n->right = nod(OADDR, n->right, N);
 				n->right->etype = 1;  // pointer does not escape
 				typecheck(&n->right, Erv);
 				r->ntest = mkcall1(chanfn("selectsend", 2, n->left->type), types[TBOOL],
-					&init, var, n->left, n->right);
+					&r->ninit, var, n->left, n->right);
 				break;
 
 			case OSELRECV:
 				// selectrecv(sel *byte, hchan *chan any, elem *any) (selected bool);
 				r->ntest = mkcall1(chanfn("selectrecv", 2, n->right->left->type), types[TBOOL],
-					&init, var, n->right->left, n->left);
+					&r->ninit, var, n->right->left, n->left);
 				break;
 
 			case OSELRECV2:
 				// selectrecv2(sel *byte, hchan *chan any, elem *any, received *bool) (selected bool);
 				r->ntest = mkcall1(chanfn("selectrecv2", 2, n->right->left->type), types[TBOOL],
-					&init, var, n->right->left, n->left, n->ntest);
+					&r->ninit, var, n->right->left, n->left, n->ntest);
 				break;
 			}
 		}

src/cmd/gc/subr.c

--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -2040,11 +2040,13 @@ cheapexpr(Node *n, NodeList **init)
 
 /*
  * return n in a local variable of type t if it is not already.
+ * the value is guaranteed not to change except by direct
+ * assignment to it.
  */
 Node*
 localexpr(Node *n, Type *t, NodeList **init)
 {
-\tif(n->op == ONAME &&\
+\tif(n->op == ONAME && !n->addrtaken &&\
 		(n->class == PAUTO || n->class == PPARAM || n->class == PPARAMOUT) &&\
 		convertop(n->type, t, nil) == OCONVNOP)\
 		return n;

test/fixedbugs/issue4313.go

// run

// Copyright 2012 The Go Authors.  All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Order of operations in select.

package main

func main() {
	c := make(chan int, 1)
	x := 0
	select {
	case c <- x: // should see x = 0, not x = 42 (after makec)
	case <-makec(&x): // should be evaluated only after c and x on previous line
	}
	y := <-c
	if y != 0 {
		panic(y)
	}
}

func makec(px *int) chan bool {
	if false { for {} }
	*px = 42
	return make(chan bool, 0)
}

コアとなるコードの解説

このコミットの核心は、selectステートメントの評価セマンティクスを正しく実装するために、コンパイラがチャネル操作の引数として使用される変数の値を適切に「固定」することです。

1. order.c の変更: orderexpr(&r->right->left, out); から orderexpr(&r->right->left, &l->n->ninit); への変更は、OSELRECV2 および OSEND 操作の引数（チャネルや送信値）の評価によって生成される初期化ステートメントが、selectケース自身の初期化リスト (l->n->ninit) に追加されるようにします。これにより、これらの引数の評価が、select文全体の評価コンテキスト内で正しく順序付けられ、他のケースの副作用によって影響を受けないようになります。

2. select.c の変更:

   • r->nbody = cas->ninit; から r->ninit = cas->ninit; への変更は、selectケースの初期化リストが、そのケースに対応するifステートメントの初期化リストに正しく伝播されることを保証します。これは、selectの内部変換において、初期化ステートメントが適切な場所に配置されるために重要です。
   • mkcall / mkcall1 の第3引数が &init から &r->ninit に変更されたのは、selectランタイム関数呼び出しの引数（チャネル、送信値、受信バッファなど）の評価に伴う初期化ステートメントが、生成されるifステートメントの初期化リスト (r->ninit) に追加されるようにするためです。これにより、これらの引数の値が、ランタイム関数が呼び出される前に確実に評価され、固定されます。
   • OSEND ケースにおける n->left = localexpr(safeexpr(n->left, &r->ninit), n->left->type, &r->ninit); の追加は特に重要です。これは、チャネル送信操作のチャネル式 (n->left) が、localexprによってローカル変数に「固定」されることを意味します。safeexprは式が安全に評価されることを保証し、その結果がlocalexprに渡されます。localexprは、その値を一時変数にコピーすることで、チャネル式が評価された後、その値が他のselectケースの評価によって変更されることを防ぎます。

3. subr.c の変更: localexpr関数における !n->addrtaken の追加は、localexprの挙動をより正確にします。以前は、ONAMEノード（変数）がローカル変数であれば、その変数をそのまま返していました。しかし、もしその変数のアドレスが取得されている場合（n->addrtakenがtrue）、その変数はポインタを介して間接的に変更される可能性があります。この変更により、localexprは、アドレスが取得された変数に対しては、新しい一時変数を作成して値をコピーするようになります。これにより、localexprが返す値が、後続の操作によって予期せず変更されることがなくなります。

test/fixedbugs/issue4313.go は、このバグを再現するためのテストケースです。 main関数内で、c <- x と <-makec(&x) の2つのケースを持つselect文があります。 makec関数は、引数として渡されたポインタpxが指すxの値を42に変更します。 もしc <- xのxがselectの評価中にmakecによって変更されてしまうと、チャネルcに送信される値が0ではなく42になってしまいます。 このコミットの修正により、c <- xのxの値はselectが評価される前に「固定」されるため、makecがxの値を変更しても、チャネルcには正しく0が送信されるようになります。テストケースは、最終的にチャネルから受信した値が0であることをアサートすることで、この修正が正しく機能することを確認しています。

関連リンク

• Go言語のselectステートメントに関する公式ドキュメント: https://go.dev/ref/spec#Select_statements
• Go言語のメモリモデル: https://go.dev/ref/mem

参考にした情報源リンク

• Go言語のソースコード (特に src/cmd/gc ディレクトリ): https://github.com/golang/go
• Go Issue Tracker (GitHub): https://github.com/golang/go/issues (ただし、Issue #4313 は直接見つからなかったため、内部的なものか、古いトラッカーに存在した可能性があります。)
• Go Code Review (Gerrit): https://go.dev/cl/6997047 (コミットメッセージに記載されているGerritの変更リストへのリンク)
• Goコンパイラの内部構造に関する一般的な情報源 (例: "Go Compiler Internals" などの書籍やブログ記事)