死にたくなるほど間抜けなバグに振り回される(その2)

要するに「短整数から整数への格上げに気をつけよう」ということなのか?

何をやっていたか?

データの順番を管理する方法の1つに、「各データにシーケンス番号をつける」というのがある。

これは例えばネットワーク通信のパケットみたいに何が何時どんな順番で到着するか分からないものを管理する時に使える。パケット中にシーケンス番号を格納するフィールドを設けておいて、送信側でシーケンス番号をつけてパケットを送り出す。

  • パケットが送信した順番で届くとは限らないが、順番が入れ替わったかどうかシーケンス番号から判断できる。
  • あるパケットが行方不明になって到着しない可能性があるが、シーケンス番号から抜けを判断できる。

UDPである程度の通信品質を持つプロトコルを自前で実装する場合、このような処理を自分で書くことになる。ちなみにTCPは内部でこれらの処理を行っているので、通信品質は高い*1

ところでシーケンス番号による順番の管理を実装する場合、整数型変数の大きさが有限である点に気をつけなくてはならない。

基本的にシーケンス番号は増加する一方だが、変数に格納できる値は有限なので、どこかで変数がオーバーフローしてしまう。そこで、ある程度大きな変数でシーケンス番号を保持して、且つ変数がいつかオーバーフローすることを前提として実装する。

例えばあるデータが別のデータよりも新しいかどうかシーケンス番号から判定するとする。次のコードではどちらかの変数がオーバーフローした直後に正しく判定できない。

struct DATA {
	int seqnum;     /* シーケンス番号 */
	/* その他データ等(省略) */
};
struct DATA new, old;

/* 中略 */

if (new.seqnum > old.seqnum) {
	/* データnewはoldより新しい */
}

ではどうするか? シーケンス番号を保持する変数同士の差分から判定する。符号付のデータ型でシーケンス番号を保持していれば、この方法で何とかなる。

if ((new.seqnum - old.seqnum) > 0) {
	/* データnewはoldより新しい */
}

「符号付データ型のオーバーフロー」とか「オーバーフロー時に特定の挙動になることを前提としている」等の事実には目をつぶることにする。

何が起きたか?

さてここからが本題。

あるデータが遅れて到着したかどうか判定する為に、次のようなコードを書いた。

/* typedef short int16_t; */

struct DATA {
	int16_t seqnum;     /* シーケンス番号 */
	/* その他データ等(省略) */
};

int16_t last_seqnum;    /* 到着済みデータ中で最新のシーケンス番号 */
struct DATA data;       /* 到着したデータを格納 */

/* 中略 */

if ((last_seqnum - data.seqnum) >= 0) {
	/* どうやらこのデータは遅れて到着したようだ */
}

しかしWindowsアプリ(32bit環境)に組み込んでテストしたところ、うまく判定できない場合があるようだった。例えばlast_seqnumか32767でdata.seqnumが-32760(オーバーフローして負の値になっている)の時、遅れてないはずなのに遅れたと判定されてしまった。

で、どうしたか?

ちょっと悩んで気づいたのはint16_t型がshort型をtypedefしたデータ型だったということ。

この判定方法は「求めた差分のデータ型は元の変数のデータ型と同じである」という前提で実装されている。どういうことかというと、

last_seqnum - data.seqnum
→ (short)32767 - (short)(-32760)
→ (short)32767 + (short)32760
→ (short)-9 /* 本当は65527だけど、オーバーフローしてこうなった */

となることを想定している。これなら「if (-9 >= 0)」となるので、if文に引っかからない。

ところがANSI Cの規格としては、short型の変数は短整数から整数への格上げによってint型に型変換されるので、どうも次のようになるらしい。

last_seqnum - data.seqnum
→ (short)32767 - (short)(-32760)
→ (int)32767 - (int)(-32760)
→ (int)32767 + (int)32760
→ (int)65527

なんと答えは65527。「if (65527 >= 0)」となるので、見事にif文の条件に引っかかってしまう。

ではどう対処したかというと、強引にキャストで誤魔化した。

if ((int16_t) (last_seqnum - data.seqnum) >= 0) {
	/* どうやらこのデータは遅れて到着したようだ */
}

事情があってint16_t型を使用しているので、どうしようもないのだが……これも微妙なコードだよなぁ。

*1:もちろんそれだけじゃないけど