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

コミット

コミットハッシュ: fd922c875d302a2234c62285051df561a16fc4e6
作成者: Ken Thompson ken@golang.org
日付: 2008年10月30日 14:32:04 -0700
メッセージ: "shift bug"

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

https://github.com/golang/go/commit/fd922c875d302a2234c62285051df561a16fc4e6

元コミット内容

commit fd922c875d302a2234c62285051df561a16fc4e6
Author: Ken Thompson <ken@golang.org>
Date:   Thu Oct 30 14:32:04 2008 -0700

    shift bug
    
    R=r
    OCL=18166
    CL=18166
---
 src/cmd/6g/gen.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/cmd/6g/gen.c b/src/cmd/6g/gen.c
index 39c9d6f38b..f4a15f2f20 100644
--- a/src/cmd/6g/gen.c
+++ b/src/cmd/6g/gen.c
@@ -1047,13 +1047,16 @@ cgen_shift(int op, Node *nl, Node *nr, Node *res)
 	regalloc(&n1, nr->type, &n1);
 
 	// clean out the CL register
-	if(rcl && !samereg(res, &n1)) {
+	if(rcl) {
 		regalloc(&n2, types[TINT64], N);
 		gins(AMOVQ, &n1, &n2);
 		regfree(&n1);
 
 		reg[D_CX] = 0;
-		cgen_shift(op, nl, nr, res);
+		if(samereg(res, &n1))
+			cgen_shift(op, nl, nr, &n2);
+		else
+			cgen_shift(op, nl, nr, res);
 		reg[D_CX] = rcl;
 
 		gins(AMOVQ, &n2, &n1);

変更の背景

このコミットは、Go言語の初期開発段階（2008年）において、6g コンパイラのコード生成器（code generator）におけるシフト演算の処理に関するバグを修正したものです。Ken Thompson氏によって発見・修正されたこのバグは、x86-64アーキテクチャでのシフト演算における特定の条件下でのレジスタ管理の問題を解決しています。

当時のGoコンパイラは各アーキテクチャごとに独立したコンパイラを持っており、6gは x86-64（AMD64）アーキテクチャ用のコンパイラでした。このコミットは、レジスタ割り当てとシフト演算の相互作用において発生する微妙な問題を修正したものです。

前提知識の解説

x86-64アーキテクチャにおけるシフト演算

x86-64アーキテクチャでは、シフト演算（左シフト、右シフト）において、シフト量（何ビット移動するか）は特定のレジスタにのみ格納できるという制約があります。具体的には：

CL レジスタ: シフト量を格納するための専用レジスタ（RCXレジスタの下位8ビット）
シフト命令: SHL、SHR、SAR などのシフト命令では、可変シフト量は CL レジスタからのみ読み取り可能

コンパイラのレジスタ割り当て

コンパイラは限られたCPUレジスタを効率的に利用するため、レジスタ割り当て（register allocation）を行います。この過程で：

レジスタの衝突: 複数の値が同じレジスタを必要とする場合
レジスタの保存・復元: 一時的に他の値のためにレジスタを明け渡す必要がある場合
アーキテクチャ制約: 特定の命令が特定のレジスタを要求する場合

if(rcl && !samereg(res, &n1)) {

この条件は以下を意味します：

rcl: CLレジスタに何らかの値が格納されている
!samereg(res, &n1): 結果のレジスタと n1 が異なるレジスタを指している

修正後の改善

修正後のコードでは：

if(rcl) {
    regalloc(&n2, types[TINT64], N);
    gins(AMOVQ, &n1, &n2);
    regfree(&n1);

    reg[D_CX] = 0;
    if(samereg(res, &n1))
        cgen_shift(op, nl, nr, &n2);
    else
        cgen_shift(op, nl, nr, res);
    reg[D_CX] = rcl;

    gins(AMOVQ, &n2, &n1);
}

この修正により：

CLレジスタに値がある場合は常にクリーンアップを実行
sameregチェックを内部に移動し、適切な引数でシフト演算を再帰呼び出し
レジスタの衝突を避けるため、必要に応じて一時レジスタ（n2）を使用

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

src/cmd/6g/gen.c:1047-1057

変更箇所は cgen_shift 関数内のCLレジスタ管理部分です：

修正前:

if(rcl && !samereg(res, &n1)) {
    // CLレジスタのクリーンアップ処理
    cgen_shift(op, nl, nr, res);
}

修正後:

if(rcl) {
    // CLレジスタのクリーンアップ処理
    if(samereg(res, &n1))
        cgen_shift(op, nl, nr, &n2);
    else
        cgen_shift(op, nl, nr, res);
}

コアとなるコードの解説

修正の詳細分析

条件判定の簡素化:
- if(rcl && !samereg(res, &n1)) → if(rcl)
- CLレジスタに値がある場合は常にクリーンアップを実行するよう変更
レジスタ衝突の適切な処理:
- samereg(res, &n1) チェックを内部に移動
- 同じレジスタを指している場合は一時レジスタ n2 を使用
- 異なるレジスタの場合は直接 res を使用
再帰呼び出しの最適化:
- レジスタ衝突の有無に応じて適切な引数でシフト演算を再実行
- レジスタの整合性を保ちながら正しいシフト演算を実現

バグの根本原因

このバグは、レジスタ割り当てにおいて以下の状況で発生していました：

CLレジスタに既に値が格納されている（rclが真）
結果レジスタ（res）と作業レジスタ（n1）が同じ物理レジスタを指している
従来の条件 !samereg(res, &n1) が偽となり、CLレジスタのクリーンアップが実行されない
結果として、シフト演算が正しく実行されないか、レジスタの内容が破壊される

修正により、レジスタの衝突状況に関わらず適切なCLレジスタ管理が行われるようになりました。

comemo