Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

コミット

このコミットは、Goコンパイラ（cmd/gc）におけるappend組み込み関数のインライン展開時の引数保持に関するバグ修正です。具体的には、appendがインライン展開される際に、スライスを修正する操作が、そのスライスから読み取られる引数の値に影響を与えないようにするための変更です。これにより、appendの引数として渡されたスライス要素が、append操作の途中で上書きされてしまう問題を解決しています。

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

https://github.com/golang/go/commit/47b6197a011f201b6975407c36978dac0b1f87d7

元コミット内容

commit 47b6197a011f201b6975407c36978dac0b1f87d7
Author: Ian Lance Taylor <iant@golang.org>
Date:   Thu Mar 22 09:44:31 2012 -0700

    cmd/gc: when expanding append inline, preserve arguments
    
    Fixes #3369.
    
    R=golang-dev, gri, lvd, r
    CC=golang-dev
    https://golang.org/cl/5876044
---
 src/cmd/gc/walk.c        |  6 ++++++
 test/fixedbugs/bug428.go | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c
index 5c8282b52e..ff6f1d28bd 100644
--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -2358,6 +2358,12 @@ append(Node *n, NodeList **init)
 
  	walkexprlistsafe(n->list, init);
 
+	// walkexprlistsafe will leave OINDEX (s[n]) along if both s
+	// and n are name or literal, but those may index the slice we're
+	// modifying here.  Fix explicitly.
+	for(l = n->list; l; l=l->next)
+		l->n = cheapexpr(l->n, init);
+
  	nsrc = n->list->n;
  	argc = count(n->list) - 1;
  	if (argc < 1) {
diff --git a/test/fixedbugs/bug428.go b/test/fixedbugs/bug428.go
new file mode 100644
index 0000000000..298c455183
--- /dev/null
+++ b/test/fixedbugs/bug428.go
@@ -0,0 +1,19 @@
+// 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.
+
+// Test that when the compiler expands append inline it does not
+// overwrite a value before it needs it (issue 3369).
+
+package main
+
+func main() {
+	s := make([]byte, 5, 6)
+	copy(s, "12346")
+	s = append(s[:len(s)-1], '5', s[len(s)-1])
+	if string(s) != "123456" {
+		panic(s)
+	}
+}

変更の背景

この変更は、Go言語の組み込み関数であるappendがコンパイラによってインライン展開される際に発生する特定のバグを修正するために行われました。このバグは、Issue 3369として報告されており、appendの引数として渡されるスライス要素が、append操作の途中でそのスライス自体が変更されることによって、意図しない値に上書きされてしまうというものでした。

具体的には、s = append(s[:len(s)-1], '5', s[len(s)-1])のようなコードにおいて、s[len(s)-1]という引数の評価が、s[:len(s)-1]によってスライスsが再割り当てされた後に実行されると、古いsの最後の要素ではなく、新しい（短くなった）sの範囲外のメモリを指してしまう可能性がありました。コンパイラがappendをインライン展開する際、このような評価順序の依存関係を正しく処理できていなかったため、誤った結果が生じていました。

この修正の目的は、appendの引数が、append操作によって変更される可能性のあるスライスを参照している場合でも、その引数の値が正しく評価され、保持されるようにすることです。これにより、Goプログラムの予測可能性と正確性が向上します。

前提知識の解説

Go言語のスライス (Slice)

Go言語のスライスは、配列のセグメントを参照するデータ構造です。スライスは、内部的に配列へのポインタ、長さ（len）、容量（cap）の3つの要素で構成されます。

  • 長さ (Length): スライスに含まれる要素の数。len(s)で取得できます。
  • 容量 (Capacity): スライスの基底配列の先頭から、スライスが拡張できる最大要素数。cap(s)で取得できます。

スライスは動的なサイズ変更が可能ですが、これは新しい基底配列の割り当てと要素のコピーによって実現されます。append関数は、スライスの容量が不足した場合に、より大きな新しい基底配列を割り当て、既存の要素をコピーし、新しい要素を追加します。このとき、元のスライス変数は新しいスライスヘッダ（新しい基底配列へのポインタを含む）を指すように更新されます。

append組み込み関数

appendはGo言語の組み込み関数で、スライスに要素を追加するために使用されます。 newSlice = append(oldSlice, elem1, elem2, ...) のように使用し、oldSliceにelem1, elem2, ...を追加した新しいスライスを返します。 もしoldSliceの容量が足りない場合、appendは新しい、より大きな基底配列を割り当てて、既存の要素と新しい要素をその新しい配列にコピーします。このため、appendの呼び出し元で返り値を受け取ることが重要です。

コンパイラのインライン展開 (Inlining)

インライン展開とは、コンパイラ最適化の一種で、関数呼び出しをその関数の本体のコードで直接置き換えることです。これにより、関数呼び出しのオーバーヘッド（スタックフレームの作成、引数の渡し、戻り値の処理など）が削減され、プログラムの実行速度が向上する可能性があります。

Goコンパイラも、特定の条件（関数が小さい、呼び出し回数が多いなど）を満たす関数に対してインライン展開を行います。appendのような組み込み関数は、パフォーマンス上の理由から頻繁にインライン展開の対象となります。

OINDEXノード

Goコンパイラの内部では、プログラムのソースコードは抽象構文木（AST）として表現されます。OINDEXは、このASTにおけるインデックス操作（例: s[n]）を表すノードの一種です。

cheapexpr関数

cheapexprはGoコンパイラの内部関数で、式を評価し、その結果を一時変数に格納するなどの方法で、式の副作用を最小限に抑えたり、評価順序を制御したりするために使用されます。特に、式が複数回評価される可能性がある場合や、評価のタイミングが重要となる場合に利用されます。この関数は、式の評価が「安価」（副作用が少ない、または制御可能）であることを保証する目的で使われます。

技術的詳細

このコミットは、Goコンパイラのsrc/cmd/gc/walk.cファイル内のappend関数の処理ロジックを変更することで、Issue 3369で報告されたバグを修正しています。

バグの根本原因は、appendがインライン展開される際に、その引数リストに含まれる式（特にスライスインデックス操作OINDEX）の評価順序が、append操作によるスライスの再割り当てと競合する可能性があったことです。

例えば、s = append(s[:len(s)-1], '5', s[len(s)-1])という式を考えます。

  1. s[:len(s)-1]が評価され、新しいスライスが生成されます。
  2. appendがインライン展開され、必要に応じて新しい基底配列が割り当てられ、要素がコピーされます。この時点で、s変数は新しいスライスヘッダを指すように更新される可能性があります。
  3. その後、s[len(s)-1]が評価されます。もしsがステップ2で新しいスライスヘッダを指すように更新されていた場合、このs[len(s)-1]は、元のスライスの最後の要素ではなく、新しいスライスの（おそらく範囲外の）メモリ位置を参照してしまう可能性がありました。

修正は、walkexprlistsafeが引数リストを処理した後、特にOINDEXノード（s[n]のようなインデックス操作）に対して追加の処理を行うことで実現されています。

追加されたコードは以下の通りです。

	// walkexprlistsafe will leave OINDEX (s[n]) along if both s
	// and n are name or literal, but those may index the slice we're
	// modifying here.  Fix explicitly.
	for(l = n->list; l; l=l->next)
		l->n = cheapexpr(l->n, init);

このコードは、appendの引数リスト（n->list）をイテレートし、各引数式（l->n）に対してcheapexpr関数を適用しています。

cheapexpr関数は、引数式を評価し、その結果を一時変数に格納するなどの処理を行います。これにより、append操作によってスライスが再割り当てされる前に、s[len(s)-1]のような引数式が評価され、その値が一時的に保持されることが保証されます。結果として、append操作がスライスを変更した後でも、引数として渡された正しい値が使用されるようになります。

この修正により、appendのインライン展開が、引数の評価順序に関する潜在的な問題を回避し、より堅牢になります。

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

変更は主にsrc/cmd/gc/walk.cファイル内のappend関数にあります。

--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -2358,6 +2358,12 @@ append(Node *n, NodeList **init)
 
  	walkexprlistsafe(n->list, init);
 
+	// walkexprlistsafe will leave OINDEX (s[n]) along if both s
+	// and n are name or literal, but those may index the slice we're
+	// modifying here.  Fix explicitly.
+	for(l = n->list; l; l=l->next)
+		l->n = cheapexpr(l->n, init);
+
  	nsrc = n->list->n;
  	argc = count(n->list) - 1;
  	if (argc < 1) {

また、このバグを再現し、修正を検証するための新しいテストケースがtest/fixedbugs/bug428.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.

// Test that when the compiler expands append inline it does not
// overwrite a value before it needs it (issue 3369).

package main

func main() {
	s := make([]byte, 5, 6)
	copy(s, "12346")
	s = append(s[:len(s)-1], '5', s[len(s)-1])
	if string(s) != "123456" {
		panic(s)
	}
}

コアとなるコードの解説

src/cmd/gc/walk.cの変更

walk.cはGoコンパイラのバックエンドの一部であり、抽象構文木（AST）を走査し、コード生成のための準備を行う「ウォーカー」の役割を担っています。append関数は、append組み込み関数のASTノードを処理する部分です。

追加されたループは、appendの引数リストを反復処理します。 for(l = n->list; l; l=l->next): これは、append関数の引数リストを構成するNodeListを走査するための標準的なループです。n->listはappendの引数ノードのリストの先頭を指します。

l->n = cheapexpr(l->n, init);: この行が変更の核心です。

  • l->n: 現在処理している引数式を表すASTノードです。
  • cheapexpr(l->n, init): この関数が呼び出されます。cheapexprは、引数式l->nを評価し、その結果を一時変数に格納するなどの処理を行います。initは、初期化文のリストを構築するために使用されるポインタです。cheapexprは、必要に応じて一時変数の宣言や代入といった初期化文をこのリストに追加します。
  • l->n = ...: cheapexprが返した新しいノード（一時変数への参照など）で、元の引数ノードを置き換えます。

この変更により、appendの引数として渡される式（特にs[len(s)-1]のようなスライスインデックス操作）は、appendがスライスを再割り当てする前に評価され、その値が安全に一時変数に保存されるようになります。これにより、スライスの再割り当てが行われた後でも、引数の値が正しく参照されることが保証され、バグが修正されます。

test/fixedbugs/bug428.goのテストケース

このテストケースは、修正されたバグを具体的に再現し、修正が正しく機能することを確認するために作成されました。

s := make([]byte, 5, 6): 長さ5、容量6のバイトスライスsを作成します。 copy(s, "12346"): スライスsに初期値"12346"をコピーします。この時点でのsは[49 50 51 52 54]（ASCII値）です。 s = append(s[:len(s)-1], '5', s[len(s)-1]): この行がバグをトリガーする核心です。

  • s[:len(s)-1]はs[:4]となり、[49 50 51 52]（"1234"）という新しいスライスを生成します。
  • appendは、この[49 50 51 52]に'5'（ASCII 53）とs[len(s)-1]を追加しようとします。
  • バグがある場合、s[len(s)-1]が評価される前にsが新しいスライスヘッダを指すように更新されると、s[len(s)-1]は元のsの最後の要素（'6'）ではなく、誤った値を参照してしまう可能性がありました。
  • 修正後、s[len(s)-1]はappendがスライスを再割り当てする前に評価され、その値（'6'）が一時的に保持されます。
  • 結果として、appendは[49 50 51 52]に'5'と'6'を追加し、最終的に[49 50 51 52 53 54]（"123456"）というスライスが生成され、sに代入されます。

if string(s) != "123456" { panic(s) }: 最終的なスライスの内容が期待通り"123456"であるかを検証します。もしバグが修正されていなければ、この条件が真となり、テストは失敗します。

関連リンク

参考にした情報源リンク

  • Go言語の公式ドキュメント（スライス、append関数に関する情報）
  • Goコンパイラのソースコード（src/cmd/gc/walk.cの関連部分）
  • Go Issue Tracker（Issue 3369の詳細）
  • Go Code Review（CL 5876044のレビューコメント）
  • コンパイラのインライン展開に関する一般的な情報