それなりの規模のソフト開発における静的コード解析活用プラクティス

静的コード解析(スタイルチェック…特にバグの警告)について有効に使うためのプラクティスであまり語られていないところをまとめました。
#Static Code Analysis, Style Checker, 静的解析

Transcript

1. それなりの規模のソフト開発における 静的コード解析ツール活用プラクティス 2014/05/28　@masskaneko
2. 自己紹介 • 組込み系ソフトウェアエンジニア • コンシューマー～エンタープライズ両方 • ソフトウェア工学を実践するコンサルティング • 「SEPG」「SWET」「たんぽぽチーム」 と言えばなんとなく伝わるかも。 • SNSとか • @masskaneko, はてなブログ, Fb, LinkedIn
3. お話のスコープ • それなりの大きさの開発向け • 10人以上, 20万行以上, ライフ5年以上...だとか • C/C++, Java • スタイルチェック(特にバグの警告)について 有効に使うためのプラクティスであんまり 語られていないところ。 • ツールの例は話しますが、特定のツールの話はしません。 • 構造解析や形式手法の話はありません • セキュアプログラミングの話はありません
4. 静的コード解析で出力される警告の一部 • 特にユーザーの不利益に繋がりやすいもの • バッファーオーバーラン、メモリーリーク、NULL 参照、デッドロック、0除算、危険なキャスト… • よくない設計 • 到達不可能な文、未使用の変数、 非推奨ライブラリの使用、 for 文中でループ変数を計算、 default の無い switch 文…
5. ツール例 • 無償 • [C/C++] Cppcheck, CPAchecker • [Java] FindBugs, PMD • 商用 • Coverity Static Analysis • Grammatech CodeSonar • Mathworks PolySpace
6. ツール適用から修正までの流れ • バグを見つけてくれるわけではない。 「バグではないか？」と警告してくれるだけ。 • 全ての警告に対処する必要は無い。 コード 静的コード解析する 警告 フィルタリング レビューすべき 警告 修正された コード ※ 修正方法の検討 ユニットテスト レビュー 関連文書修正 なども含みます レビュー バグ
7. 警告は沢山出てくる 対象 言語 行数 警告数 警告数/1000行 MongoDB 2.6 C++ 551508 1285 2.3 Apache 2.4.9 C 95877 731 7.6 Maven 3.2.1 Java 42753 858 20.0 Cppcheck : 1.65 FindBugs : 2.0.3 SourceMonitor : 3.4.6.297 (行数の測定に使用. 正確には文の数) 適当にメジャーなOSSを3つピックアップした Cppcheck : 1.65 FindBugs : 2.0.3 SourceMonitor : 3.4.6.297 (行数の測定に使用. 正確には文の数)
8. 警告は沢山出てくる 対象 言語 行数 警告数 警告数/1000行 Telecom EMS/NMS system C++ 6M LOC 400k 66.6 Financial Web App Java JS 1M LOC 20k 20.0 全ての警告をレビューすることは非効率。 (注)先に示したOSSの例との単純比較はできない。行の数え方が異なる。 使用した静的コード解析ツールが同じとは限らない。 米Intuit社エンジニア John Ruberto 氏の発表(*2)によると
9. 警告をフィルタリング • 重要な機能に関係する箇所に絞る • 警告の種類で絞る • バグが起きそうな箇所に絞る(多発箇所) • バグが起きそうな箇所に絞る(他の案)
10. 重要な機能に関係する箇所に絞る • 何かもっともらしいけど…警告されたコードが どの機能に関係しているかわかりますか？ • 普段触れないコードも警告対象になる。 チームの誰に聞いてもわからなかったりして。 • grepとか構造解析ツールで頑張る。 • その機能はユーザーにとって重要ですか？ • ユーザーの使い方を把握しておこう • よく使われる機能であればレビュー対象にしよう
11. 警告の種類で絞る • 静的コード解析の目的、開発対象に求められる 品質、設計の傾向に合わせた警告セットを 決める • クリティカルバグ検出セット スタイルチェックセット CI向け標準セット 時間が無いときでもこれだけはセット…など。 • FindBugs, PMD だとあらかじめカテゴリが分けら れているが、そのまま適用するのが吉かは疑問。
12. バグが起きそうな箇所に絞る(多発箇所) • 今までに起きたバグの中で静的コード解析ツールが 検出の役に立ちそうなのはどれか？ それらのバグは「どこで」起こったのか？ 振り返ることができるBTSの運用をする。 • 「起きた箇所」はVCSと連携していれば容易 • どのようなテストで検出できたかを記録する欄を 設けて、その選択肢に「静的コード解析」を作っておく。 ※判断するのは意外と難しいかも • 多発箇所はこの先も起こると仮定。 • 多発箇所は警告セットを厳しめに。
13. バグが起きそうな箇所に絞る(他の案) • 複雑さを表すコードメトリクスを利用する • サイクロマティック複雑度とかLCOMとか • 複雑なコードほどバグが起きやすい • 警告数と複雑さ…因果は無いだろうけど相関はあるのでは？ • bugspots のスコアを利用する • リポジトリを食わせるとフォルダとファイル別にバグの起きやす さを示すスコアが出てくる。 • スコア計算式の変数は変更回数だけなので静的コード解析で見つ かるバグではないのでは。一工夫すれば使えるかも？ (注)筆者自信なし
14. 警告対象コードをレビューする • 普通のコードレビューと違うところ • 警告内容が正しいか人の目で確かめる。 正しい場合、修正すべきか判断する。 • 理解不十分なコードもレビュー対象になる。 (普段、追加や変更をする箇所だけではない。) • 警告あたりのレビュー時間は様々 • 悩むと警告1つに何時間も費やしかねない… • 10秒でわかるものもある
15. func( MyTable* p ) { if( (p->data == INVALID ) || (p == NULL) ) return; ... } 10秒でわかる例
16. func(*a, *b) { *p; *q; … p = foo(a->x, b->y); … q = baa(a->w, b->w); … … … … r = heck(p,q); … r->data = …; Warning: Null Pointer Dereference えーとまず r は p, q から求まって その p, q は…あーこれか heck() には return NULL; 無いから 絶対有効なアドレスが 入るはずなんだけどなー あれ？でも a, b って元はファイルから 読んでくるんだったよな… とりあえず foo, baa, heck 全部読むか うわッ！長ッ…！ 面倒だから全部NULLチェックするか… ってもしそれで抜けちゃったら そのあとはどうすれば… 悩むことも
17. 警告に関する修正優先度の例 修正 優先度 説明 修正方針 最高 警告された内容がユーザーの使用環境で 発生し、ユーザーに不利益をもたらす。 絶対に直す。 高 警告された内容はユーザーの使用環境で 発生するものの、ユーザーに不利益はも たらさない。 これも絶対に直したいが、やむを得 ない事情があれば見送る場合も。 中 ・警告された内容はユーザーの使用環境 では発生しないが、開発者環境では発生 させることができる。 ・バグではないが、バグを誘発しやすい スタイル。 将来のバグの素なので、 これらをいかに潰すかが肝。 無 ツールの誤検出。 本当に誤検出なのか、よく考える。 「ユーザー環境で発生しないから 誤検出だ」などと考えないこと。
18. 修正判断に自信が無い場合 • ユニットテストを書いてみよう • 自信が無いのは警告対象コードを理解してないから • ユニットテストを書けば理解が進む (*3) • バグが起こるという警告であれば、そのバグを起こそ うという姿勢でテストを書く • 他の人にレビューしてもらおう • 一人では見逃すバグがすぐ発見できるかも • チームでコードを共有する機会ができる • 相手に話しているうちに理解が進む
19. いつ静的コード解析ツールを使うの？ • いつでも、こまめに、がベター。 • CIに組み込むのがベスト。 • 解析時間が長く、レポートの容量が大きい場合は 週1だとかで良いのではないでしょうか。 • 軽量ツールならIDEに組み込む。 • 開発フェーズの早期から。 • 終盤で1回だと、目の前のクリティカルなバグだけ に注意が払われるので効果が薄い。
20. 最も身近な静的コード解析 • コンパイラのwarnings • わざわざ静的コード解析ツールを導入しなくても チェックできる項目があったりして。 • これならいつでもできる… はずなんですけど、見てますよね？ • コンパイルできればいいやと無視してると 静的コード解析ツールを導入したときに 今までのツケを払うことになる。
21. まとめ
22. 静的コード解析以外のプラクティスが 静的コード解析の成否につながる • コンパイラのwarningsに気を付けていれば 余分なレビューをしなくて済む • BTSでバグの発生箇所や静的コード解析検出可能性を 記録していれば警告のフィルタリングに役立つ。 • ユニットテスト等、警告対象コードをすぐに 実行できる環境があれば判断に自信を持てる。 • チームメンバー同士でコードの共通理解があれば レビューが楽になる。
23. バグ検出よりもコード品質を重視する 姿勢が静的コード解析の成否につながる • 目の前のバグを直すのは当たり前 • バグになりそうな箇所も直そう • ユーザー環境で発生するバグだけでなく、 開発者環境だけで発生するバグも直す。 • バグを誘発しやすいスタイルを直す。 • どういうコードが良いのか学ぶ機会になる。 (アンチパターンを見つけて避けられるように)
24. References • *1:【CEDEC 2012】静的コード解析ツールがバグを潰し、新 人を育てる http://www.inside-games.jp/article/2012/08/31/59369.html • *2: Introducing Static Analysis in a Mature Codebase http://www.slideshare.net/JohnRuberto/introducing-static-analysis-john-ruberto-jan-2014 • *3: レガシーコード改善ガイド 2009, 翔泳社 http://books.shoeisha.co.jp/book/b73170.html •