最近、ソースコードの静的解析に絡む仕事をあれこれやっている。
理想と現実という言葉がよく合う難しい仕事である。
昔のソースコードはコーディング規約を守っていないものが多いし、
修正するにしても、修正せずとも妥当とするにせよ、量が多すぎて全部は回りきらない。
仕事でそういうことをやっている人なら知っているかもしれないが……
静的解析というのはプログラムを動かさずに記述の妥当性を確認する手法である。
こういう書き方をすると意図と異なる動きになりやすいですよと。
そういうのをあれこれ列挙してくれるわけである。
ただ、それが実際に意図しない動きになるかというと微妙なところで、
ほとんど空振り三振、でも稀に難しい球をヒットにしてしまうみたいな。
他の手法ではなかなか見つからない潜在的なバグを発見できることがあるのは価値である。
その指摘の中に「ラップアラウンドが発生する可能性がある」というものがある。
INT30-C. 符号無し整数の演算結果がラップアラウンドしないようにする (JPCERTCC)
CERT-Cというセキュアコーディング標準にあるルール(必ず適用するべき規則)の1つで、
このページの下の方にリスク評価として 深刻性:高、可能性:高、修正コスト:高 と書かれている。
潜在的にかなり危ないが後から修正するのは大変というルールである。
だからコーディング時点で注意して作りましょうねということなんだけど……
具体的にこんなのが危険だと指摘されたという例を書く。
size_t len = flash_end - flash_top; flash_read(flash_top, ram_top_ptr, len);
フラッシュメモリのアドレスflash_topからflash_end-1までのデータをRAM領域に転送する。
このときのデータ長lenをアドレスの差し引きで計算しているのだが、
もしもflash_top > flash_endだったらどうなるかというと、
size_tは負の数を表せないため、最大値にラップアラウンドして超巨大な数字になってしまう。
すると想定外のメモリアクセスが発生してシステムは破壊されかねない。
こういう場合を想定した処理になっていないですねという指摘である。
flash_top, flash_endに入る値の組み合わせが限られているなら、
そうなることはないと断言できるが、入りうる数字がいろいろある場合、
あるいは悪意を持って数字を入れられた場合には問題になる。
この件はflash_top, flash_endなどに入る数字のチェックも抜けてて、
0x000000≦ flash_top ≦ flash_end ≦ 0xFFFFFF
のような大小関係を保証するような処理を加えて打開している。
ということで指摘を受けて堅牢なソースコードに改善できた一例である。
一方で実態として問題ないのに指摘されることも多くて、
それは一定の記述で書かれたものでなければ静的解析では未対策とみなされるためである。
エラー回数カウンタで0x00から0xFFまでカウントアップしたら、そこで止まるという処理をこう書いていた。
cnt = (cnt!=0xFFu) ? (cnt+1u) : 0xFFu;
これは元々ラップアラウンドを意識したコードなんですよね。
何も考えずに cnt+=1u; と書くと0xFF→0x00でカウンタが戻ってしまう。
こうなると累積エラー回数があたかも0回に見えてしまうので、最大値以上はカウントしないと。
ところがこのコードですら「ラップアラウンドが発生する可能性がある」という指摘が出てしまう。
解析ツールが対策済みと検出できる書き方がいくつか書かれていて……
if(cnt > 0xFEu){
cnt = 0xFFu;
}else{
cnt += 1u;
}
とすると、対策済みとして指摘されなくなった。
実効的な意味はさっきの三項演算子を使った書き方と同じなんだけどね。
さっきフラッシュメモリの大小関係を保証する処理を入れたと書いたが、
そういう変更をしたところで結局は指摘事項としては残っている。、
とりあえず怪しそうなところは全部列挙しているけどねというのに近い。
それにしてももうちょっとやりようはあるんじゃないかとも思うが。
とりあえず僕が書いた新規部分はラップアラウンド対策はやったんですよね。
「修正コスト:高」は本当にその通りで、少しの書換でどうにかなる話ではない。
これを他に展開するのはかなり難しいだろうとは思うのである。
一方で潜在的な問題が多いことを認識するにはよい指摘だったことは事実である。
既存コードも含めて点検する価値が高いこともわかったので難しい。
少なくとももっと明確に危ないのから優先的にやったほうがいいとは思いますね。
ただ、これも既存部分は手を付けにくいですよね。
マクロに関する指摘ってのも多いわけだけど。
マクロの使い方が従来と全く変わっていなければ特に問題ないはずで、
マクロを従来と異なる使い方をしたときに思わぬ解釈が生じかねないというものは、
果たして手を入れるのがよいのか、そのままがよいのか難しい。
なぜならば、思わぬ解釈でも結果として正しく動いている可能性もあるからである。
最終的には残った指摘部分は全部問題なしと説明できるなければいけないけど、
現実的には困難なので、何か濃淡を付けた対応は必要になりそう。
そこで何を優先するかというのは勘というのもありそうですけどね。