2016年5月31日火曜日

NT-Monitorの外部オブジェクト参照にバグがあるんじゃないかという話

NT-Monitorとは、CQ出版社が出版しているInterface2013年1月号に掲載されたマイコン用のモニタプログラムです。シェルの部分はNT-Shellを用い、その上にモニタプログラムとしてNT-Monitorが乗る形を想定しました。記事掲載時点ではFM3マイコンに対するモニタプログラムとして構成し、その時点ではコールバックに渡される外部オブジェクト参照を使用しない実装になっていました。

2013年から約三年後の2016年現在、そういえば久しぶりに懐かしいコードを引っ張り出してみようと見てみたところ、限りなく怪しい香りのする実装が幾つも見られます。そのひとつがタイプセーフではない外部オブジェクト参照の実装。ちょっと載せてみたいシステムがあったので使ってみたところ、インターフェースに渡したはずのオブジェクトへのポインタが、異なるアドレスを指す何かに代わってコールバックされます。「あー、何だこりゃ」というのが事の始まり。


NT-Monitorは、元々単一スレッド上で永久に動作し続けるものであるとの前提で設計しました。唯一のAPIは上記に示すntm_executeで、シェルオブジェクトとシリアルI/O関数へのポインタを渡すと後は勝手にシェルを駆動しながら処理してくれるというものです。

システム内部に複数のシリアルインターフェースが存在し、かつそれらを別々のモニタープログラムが使用する事も念頭に入れ、UARTなどのインターフェースに対するハンドラへのポインタなどを格納できるようにしたのがextobjです。

が、ここにどうしようもない情けないバグがあるんじゃないかとコードを眺めていて気付きました。

まず、NT-Monitorの最上位階層であるntmモジュールを見たユーザは、当然のように自分がextobjへ渡したオブジェクトへのポインタが、そのままserial_readやserial_writeへ渡されることを期待します。そうでなければ、それが一体なんであるのか上記のAPIから知る由もありません。

しかし、実際にNT-Monitorの実装を見るとのっけから次のようなコードが見えます。何やら突然現れたntmcmd_extobj_tにserial_readとserial_writeを格納し、更にはユーザが上位のAPIで与えたextobjをuser_extobjとして保存しています。そして、この準備の終わった新しい外部参照用オブジェクトへのポインタをシェルモジュールであるntmshellに渡しています。


このままセオリー通りに事が進むと、ntmshellから呼び出されるserial_readとserial_writeは、最上位でユーザーが渡したextobjとは異なるものになってしまいそうな気もします。確認してみると・・・


ntmshell_executeで呼び出されるp->func_writeは、最上位で言うところのserial_writeです。そして、そのシリアル出力関数に渡される外部参照オブジェクトへのポインタはp->extobjを示しており、これは先ほどの確認にあったようにNT-Monitorの内部で定義されたntmcmd_extobj_tである事がわかります。ということで、のっけからユーザーが渡したものと違うアドレスがコールバック関数に渡ってしまう事になります。あらら。

一方で更に物事を無駄に複雑にしているのが以下の記述です。



なんと、シリアル書き込み関数に対してマクロが用意されており、そのマクロを使用した箇所に関しては、user_extobjが渡るようになっています。どうしてここは正しいんだ。と過去の自分に言いたい。

つまり、同じシリアル通信関数でも、呼び出される個所によって期待するアドレスが渡ったり、はたまた間違ったアドレスが渡ったりします。

実はこの話、凡ミスのようにも見えますが、問題の根本には全体設計のまずさがあります。そもそも、シェルインターフェースを提供するNT-Shellと、モニタ機能を提供するNT-Monitorのシステム境界が曖昧です。本来であればシェルの機能とモニタの機能は完全に分離できるはずですが、あまり綺麗に分離できているとは言い難く、結果的にこれらの二層に渡るオブジェクトの取り違えにまで発展してしまっています。

冒頭「シェルの部分はNT-Shellを用い、その上にモニタプログラムとしてNT-Monitorが乗る形を想定しました。」と書いている事からもわかるように、本来の依存関係はNT-MonitorからNT-Shellへの使用依存ですが、実装を見ると内包になっています。これも気持ちが悪い。

ということで、NT-ShellもNT-MonitorもそろそろAPIの見直しなども含めて考えたくなってきました。