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

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

このコミットは、Go言語のコードフォーマッタであるgofmtが、抽象構文木（AST）内のnilノードを処理する際にパニックを起こす問題を修正するものです。具体的には、item.fieldのような式に対して同一性変換を適用しようとした際に発生するパニックを防ぐための変更が加えられました。この修正により、gofmtの堅牢性が向上し、特定の不正なAST構造を持つコードに対しても安定して動作するようになります。

コミット

commit 2ba079868204837891e531eecb4215eecfee8ff7
Author: Rémy Oudompheng <oudomphe@phare.normalesup.org>
Date:   Fri Nov 11 14:11:30 2011 -0800

    gofmt: leave nil nodes of the AST unchanged.
    
    Without this check, gofmt panics when trying to apply
    the identity transformation on "item.field" expressions.
    Fixes #2410.
    
    R=rsc, gri
    CC=golang-dev, remy
    https://golang.org/cl/5376061
---
 src/cmd/gofmt/gofmt_test.go            |  1 +
 src/cmd/gofmt/rewrite.go               |  4 ++--
 src/cmd/gofmt/testdata/rewrite3.golden | 12 ++++++++++++\n src/cmd/gofmt/testdata/rewrite3.input  | 12 ++++++++++++\n 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go
index 4432a178bc..303c4f1e1c 100644
--- a/src/cmd/gofmt/gofmt_test.go
+++ b/src/cmd/gofmt/gofmt_test.go
@@ -76,6 +76,7 @@ var tests = []struct {
 	{"testdata/old.input\", \"\"},\n \t{\"testdata/rewrite1.input\", \"-r=Foo->Bar\"},\n \t{\"testdata/rewrite2.input\", \"-r=int->bool\"},\n+\t{\"testdata/rewrite3.input\", \"-r=x->x\"},\n \t{\"testdata/stdin*.input\", \"-stdin\"},\n \t{\"testdata/comments.input\", \"\"},\n \t{\"testdata/import.input\", \"\"},\ndiff --git a/src/cmd/gofmt/rewrite.go b/src/cmd/gofmt/rewrite.go
index 25049f8f8c..60a4a7b49f 100644
--- a/src/cmd/gofmt/rewrite.go
+++ b/src/cmd/gofmt/rewrite.go
@@ -159,8 +159,8 @@ func match(m map[string]reflect.Value, pattern, val reflect.Value) bool {
 \tif m != nil && pattern.IsValid() && pattern.Type() == identType {\n \t\tname := pattern.Interface().(*ast.Ident).Name\n \t\tif isWildcard(name) && val.IsValid() {\n-\t\t\t// wildcards only match expressions\n-\t\t\tif _, ok := val.Interface().(ast.Expr); ok {\n+\t\t\t// wildcards only match valid (non-nil) expressions.\n+\t\t\tif _, ok := val.Interface().(ast.Expr); ok && !val.IsNil() {\n \t\t\t\tif old, ok := m[name]; ok {\n \t\t\t\t\treturn match(nil, old, val)\n \t\t\t\t}\ndiff --git a/src/cmd/gofmt/testdata/rewrite3.golden b/src/cmd/gofmt/testdata/rewrite3.golden
new file mode 100644
index 0000000000..0d16d16011
--- /dev/null
+++ b/src/cmd/gofmt/testdata/rewrite3.golden
@@ -0,0 +1,12 @@
+// Copyright 2011 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.
+
+package main
+
+// Field tags are *ast.BasicLit nodes that are nil when the tag is
+// absent. These nil nodes must not be mistaken for expressions,
+// the rewriter should not try to dereference them. Was issue 2410.
+type Foo struct {
+	Field int
+}
diff --git a/src/cmd/gofmt/testdata/rewrite3.input b/src/cmd/gofmt/testdata/rewrite3.input
new file mode 100644
index 0000000000..0d16d16011
--- /dev/null
+++ b/src/cmd/gofmt/testdata/rewrite3.input
@@ -0,0 +1,12 @@
+// Copyright 2011 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.
+
+package main
+
+// Field tags are *ast.BasicLit nodes that are nil when the tag is
+// absent. These nil nodes must not be mistaken for expressions,
+// the rewriter should not try to dereference them. Was issue 2410.
+type Foo struct {
+	Field int
+}

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

https://github.com/golang/go/commit/2ba079868204837891e531eecb4215eecfee8ff7

元コミット内容

gofmt: leave nil nodes of the AST unchanged.

Without this check, gofmt panics when trying to apply
the identity transformation on "item.field" expressions.
Fixes #2410.

R=rsc, gri
CC=golang-dev, remy
https://golang.org/cl/5376061

変更の背景

このコミットは、Go言語の公式フォーマッタであるgofmtが特定の状況下でパニック（プログラムの異常終了）を起こすバグを修正するために行われました。具体的には、gofmtがコードの抽象構文木（AST）を処理する際に、nil（ヌル）であるべきノードを誤って有効な式として扱おうとすると、パニックが発生していました。

問題は、item.fieldのような式、特に構造体のフィールドタグなど、AST上ではnilになりうる部分に対して、gofmtが「同一性変換」（つまり、何も変更しない変換）を適用しようとした際に顕在化しました。gofmtの内部処理では、ASTノードが有効な式であるかどうかをチェックしますが、このチェックが不十分であったため、nilのノードが式として認識され、その後の処理でnilポインタ参照が発生し、パニックに至っていました。

このバグは、Goの内部イシュートラッカーで「#2410」として報告されており、gofmtの安定性と信頼性を確保するために修正が必要でした。

前提知識の解説

gofmt

gofmtは、Go言語のソースコードを自動的にフォーマットするためのツールです。Go言語には厳格なコーディングスタイルガイドラインがあり、gofmtはそのガイドラインに沿ってコードを整形することで、Goコミュニティ全体で一貫したコードスタイルを維持するのに役立っています。開発者はgofmtを使用することで、インデント、スペース、改行などのスタイルに関する議論を避け、コードの可読性を高めることができます。gofmtはGoのツールチェインの一部として提供されており、通常はgo fmtコマンドを通じて利用されます。

抽象構文木 (AST: Abstract Syntax Tree)

抽象構文木（AST）は、プログラミング言語のソースコードの抽象的な構文構造を木構造で表現したものです。コンパイラやインタプリタは、ソースコードを直接処理するのではなく、まずソースコードを解析（パース）してASTを生成します。ASTは、コードの論理的な構造を反映しており、コメントや空白などの構文上の詳細を省略しています。

Go言語では、go/astパッケージがASTの構造を定義しており、go/parserパッケージがソースコードからASTを構築する機能を提供します。gofmtのようなツールは、このASTを操作してコードの整形やリファクタリングを行います。ASTの各ノードは、変数、関数、式、ステートメントなどのコード要素を表します。

Goにおけるnil

Go言語におけるnilは、ポインタ、インターフェース、マップ、スライス、チャネルなどの参照型変数が「ゼロ値」または「何も指していない」状態であることを示します。nilは他の言語のnullに似ていますが、Goでは型によってnilが意味するものが異なります。

例えば、ポインタがnilである場合、それは有効なメモリアドレスを指していません。nilポインタをデリファレンス（参照解除）しようとすると、ランタイムパニックが発生し、プログラムがクラッシュします。このコミットで修正された問題は、まさにこのnilポインタデリファレンスに起因するものでした。

reflectパッケージ

Goのreflectパッケージは、実行時にプログラムの構造を検査（リフレクション）するための機能を提供します。これにより、変数の型、値、メソッドなどを動的に調べたり、変更したりすることが可能になります。

このコミットのコードでは、reflect.Value型が使用されています。reflect.Valueは、Goの任意の型の値を表すことができます。val.IsValid()はreflect.Valueが有効な値を表しているか（ゼロ値でないか）をチェックし、val.IsNil()はreflect.Valueがnilであるかどうかをチェックします。gofmtは、ASTノードをreflect.Valueとして扱い、その型や値を動的に検査することで、リライト処理を行っています。

技術的詳細

gofmtのrewrite.goファイルは、コードのリライト（書き換え）ロジックを担っています。このファイル内のmatch関数は、パターンマッチングを通じてASTノードを比較し、必要に応じて変換を適用します。

問題の核心は、match関数内でワイルドカード（_やxなどのプレースホルダー）がASTノードにマッチするかどうかを判断する部分にありました。ワイルドカードは通常、式（ast.Expr）にマッチすることを意図しています。しかし、GoのASTでは、特定の要素（例えば、構造体のフィールドタグが省略された場合など）がnilのast.BasicLitノードとして表現されることがあります。

以前のコードでは、val.IsValid()のチェックは行われていましたが、val.IsNil()のチェックが欠けていました。そのため、valが有効なreflect.Valueであり、かつast.Expr型にキャスト可能であると判断された場合でも、その実体がnilである可能性がありました。

// 変更前
if _, ok := val.Interface().(ast.Expr); ok {
    // ...
}

// 変更後
if _, ok := val.Interface().(ast.Expr); ok && !val.IsNil() {
    // ...
}

val.Interface().(ast.Expr)は、reflect.Valueがラップしている実際の値をast.Exprインターフェース型に変換しようとします。この変換は、valがnilであっても、その基底の型がast.Exprインターフェースを満たすポインタ型であれば成功する可能性があります（例: (*ast.BasicLit)(nil)）。しかし、その後にvalが指す値（この場合はnil）をデリファレンスしようとすると、パニックが発生します。

このコミットでは、!val.IsNil()という追加のチェックが導入されました。これにより、valがast.Expr型にキャスト可能であるだけでなく、実際にnilではない有効な式である場合にのみ、ワイルドカードのマッチングとそれに続く処理が行われるようになりました。これにより、nilポインタデリファレンスによるパニックが回避されます。

テストケースとして追加されたtestdata/rewrite3.inputは、フィールドタグが省略された構造体type Foo struct { Field int }を含んでいます。このような構造体は、AST上ではnilの*ast.BasicLitノードを持つことがあり、これが以前のgofmtのパニックを引き起こす原因となっていました。このテストケースは、修正が正しく機能することを確認するために使用されます。

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

変更はsrc/cmd/gofmt/rewrite.goファイル内のmatch関数にあります。

--- a/src/cmd/gofmt/rewrite.go
+++ b/src/cmd/gofmt/rewrite.go
@@ -159,8 +159,8 @@ func match(m map[string]reflect.Value, pattern, val reflect.Value) bool {
 	if m != nil && pattern.IsValid() && pattern.Type() == identType {
 		name := pattern.Interface().(*ast.Ident).Name
 		if isWildcard(name) && val.IsValid() {
-			// wildcards only match expressions
-			if _, ok := val.Interface().(ast.Expr); ok {
+			// wildcards only match valid (non-nil) expressions.
+			if _, ok := val.Interface().(ast.Expr); ok && !val.IsNil() {
 				if old, ok := m[name]; ok {
 					return match(nil, old, val)
 				}

コアとなるコードの解説

変更の核心は、if _, ok := val.Interface().(ast.Expr); ok の条件に && !val.IsNil() が追加された点です。

  • 変更前: if _, ok := val.Interface().(ast.Expr); ok この行は、valが表す値がast.Exprインターフェース型に変換可能かどうかをチェックしていました。valがnilポインタであっても、その基底の型がast.Exprインターフェースを満たすポインタ型であれば、このチェックはtrueを返してしまう可能性がありました。例えば、(*ast.BasicLit)(nil)のような値はast.Exprインターフェースを満たしますが、実体はnilです。

  • 変更後: if _, ok := val.Interface().(ast.Expr); ok && !val.IsNil() この変更により、valがast.Exprインターフェース型に変換可能であることに加えて、valがnilではない（つまり、有効な値を指している）ことも同時にチェックされるようになりました。val.IsNil()メソッドは、reflect.Valueがラップしている値がnilであるかどうかを正確に判断します。

この追加された!val.IsNil()チェックによって、gofmtはnilのASTノードを有効な式として誤って処理しようとすることがなくなり、結果としてnilポインタデリファレンスによるパニックが回避されるようになりました。これは、gofmtがASTをより堅牢に、かつ安全に処理するための重要な修正です。

関連リンク

  • Go CL (Code Review) リンク: https://golang.org/cl/5376061
  • Go Issue #2410: このコミットメッセージで参照されている#2410は、Goの内部イシュートラッカーの番号である可能性が高いです。公開されているGitHubリポジトリのIssue #2410は、golang/vscode-goに関するものであり、このgofmtのパニックとは直接関係ありません。

参考にした情報源リンク

  • Web search results for "gofmt panic nil AST nodes issue 2410" (Medium.com, Joeshaw.org articles)
  • Go言語の公式ドキュメント（go/ast, go/parser, reflectパッケージに関する情報）