PVS-Studio Static Analyzerを使用したReiser4コードの確認

良い一日！



この記事では、PVS-Studio静的アナライザーの無料版（無料およびオープンソースプロジェクト用）の使用に専念します。 Reiser4ファイルシステムとそのユーティリティのソースコードを確認します。



この記事を読んでくれる皆さん、少なくとも耳を傾けて、PVS-Studio静的コードアナライザーについて聞いたことを願っています。 あなたがそれらの1人でない場合は、 このリンクをたどってください。この静的アナライザーについて簡単に読むことができます。



また、開発会社にはHabrahabrに関する公式ブログがあり、さまざまなオープンソースプロジェクトの検証に関するレポートが頻繁に掲載されています。



カーネルwikiで Reiser4について少し読むことができます 。

ユーティリティ、具体的にはlibaalライブラリから始めましょう。 次に、reiser4progsユーティリティをチェックし、最後にカーネルにコードチェックを残します。

事前準備

まず、PVS-Studioをインストールする必要があります。 公式サイトでは、debおよびrpmパッケージ、およびプログラムのアーカイブだけを見つけることができます。 最も便利な方法でインストールします。



次に、どういうわけか無料ライセンスを使用する必要があります。 開いているプロジェクトの場合、各ソースファイルの先頭に次の行を追加する必要があります（ヘッダーファイルには必要ありません）。

// This is an open source non-commercial project. Dear PVS-Studio, please check it. // PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これらの行を各ファイルに手動で追加しないように、bashに小さなスクリプトを作成します。 これらの目的のために、sedストリーミングテキストエディターを使用します。

 #!/usr/bin/bash for str in $(find $1 -name '*.c'); do sed -i -e '1 s/^/\/\/ This is an open source non-commercial project. Dear PVS-Studio, please check it.\n\/\/ PVS-Studio Static Code Analyzer for C, C++ and C\#: http:\/\/www.viva64.com\n\n/;' $str done
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

便宜上、PVS-Studioをビルドおよび実行する別のスクリプトを作成します。

 #!/usr/bin/bash pvs-studio-analyzer trace -- make -j9 || exit 1 pvs-studio-analyzer analyze -o log.log -j9 || exit 1 plog-converter -a GA:1,2 -t tasklist log.log || exit 1
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これで、ソースコードを分析する準備ができました。 libaalライブラリーから始めましょう。

libaal-1.0.7を確認してください

libaalは、reiser4progsが使用するReiser4構造抽象化ライブラリです。





標準データ型の再宣言に関連する警告は別として、考えられる問題は、 src / bitops.cファイルの68、129、139行目のみです 。



V629 Consider inspecting the 'byte_nr << 3' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type.







129行目と139行目のコードは次のとおりです。

 bit_t aal_find_next_set_bit(void *map, bit_t size, bit_t offset) { <...> unsigned int byte_nr = offset >> 3; <...> unsigned int nzb = aal_find_nzb(b, bit_nr); <...> if (nzb < 8) return (byte_nr << 3) + nzb; <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この場合、 unsigned int型の変数宣言をbit_t型に置き換えることにより、エラーを簡単に修正できます。



行68について：

 bit_t aal_find_first_zero_bit(void *map, bit_t size) <...> unsigned char *p = map; unsigned char *addr = map; <...> return (p - addr) << 3; <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

次に、PVSが突然（p-addr） 32ビットを考慮する理由について、私は途方に暮れています。 sizeof（）でさえ、明確な8バイトを生成します（amd64を使用しています）。

reiser4progs-1.2.1を確認してください

しかし、reiser4progsでは、いくつかの場所ですべてがより面白くて悲しくなります。 一般的に、Eduard Shishkinは次のように述べています。 したがって、この間違いの全体は驚くことではありません。」 実際、そのようなエラーが長年にわたって除去されていないことは驚くことではありません。



最初の重大なエラーは、ファイルplugin / key / key_short / key_short_repair.cに表示されます。



V616 The 'KEY_SHORT_BAND_MASK' named constant with the value of 0 is used in the bitwise operation.



 errno_t key_short_check_struct(reiser4_key_t *key) { <...> if (oid & KEY_SHORT_BAND_MASK) key_short_set_locality(key, oid & !KEY_SHORT_BAND_MASK); <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

KEY_SHORT_BAND_MASKは定数0xf000000000000000ull 、つまり この場合、否定のブール演算は嘘をつきます（Cでは0以外のすべてが真です）、すなわち 実際、0。著者は、ビット演算NOT- 〜を念頭に置いており、ブール演算を念頭に置いていませんでした。

このエラーは、いくつかの異なるファイルで数回繰り返されます。



以下は、次の形式のエラーを含むプラグイン/ハッシュ/tea_hash/tea_hash.cです。



V547 Expression 'len >= 16' is always false.







しかし...それは間違いではなく、ある種の魔法、またはあなたが魔法を信じていないなら汚いハックです。 なんで？ しかし、プロセッサ、OS、およびコードの作者の元のアイデアを深く知らなくても、そのようなコードが理解可能で明白であると考えられるかどうかを自分で判断してください。

 uint64_t tea_hash_build(unsigned char *name, uint32_t len) { <...> while(len >= 16) { <...> len -= 16; <...> } <...> if (len >= 12) { if (len >= 16) *(int *)0 = 0; <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

どうですか？ これは間違いではありませんが、ここで何が起こっているのか理解していないので、触れない方が良いです。 それを理解してみましょう。

コード*（int *）0 = 0; 通常のプログラムでは、 SIGSEGVになります。 カーネルに関する情報を探す場合、そのようなコードがカーネルで使用されるため、カーネルは oops（oops）を実行します。 このトピックに関する質問は、カーネル開発者のメーリングリストに掲載されており、トーバルズ自身がこれについて言及しています。 すなわち カーネルコードでそのような割り当てが何らかの未知の方法で実行された場合、おっとわかるでしょう。 「不可能」条件をチェックする理由は開発者の良心に残っていますが、上で述べたように、理解していない、触れないでください。

安全に確認できる唯一のことは、V547がトリガーする理由です。 式len> = 16は常にfalseです。 whileループは、 lenの値が16以上の間に実行され、ループの終わりに、各反復で16が減算されます。 すなわち 変数はlen = 16 * n + mとして表すことができます。ここで、 n、mは整数で、 m <16です。 サイクルの完了後、すべての16 * nが減算され、mが残ることが明らかになります。

他の同様の警告は同じパターンに従います。



ファイルplugin / sdext / sdext_plug / sdext_plug.cには、次のエラーがあります。



V595 The 'stat' pointer was utilized before it was verified against nullptr. Check lines: 18, 21.



 static void sdext_plug_info(stat_entity_t *stat) { <...> stat->info.digest = NULL; if (stat->plug->p.id.id != SDEXT_PSET_ID || !stat) return; <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここで、ありふれたタイプミスが起こるか、著者が何か他のものを持っていた。 チェック！Statはnullptrをチェックするように見えますが、2つの理由で意味がありません。 まず、上記のstatポインターが逆参照されています。 次に、標準に従って、この式は左から右に計算されます。実際にnullptrチェックである場合は、同じ条件ではポインターが逆参照されるため、条件の先頭に移動する必要があります。

ファイルplugin / item / cde40 / cde40_repair.cには、次の形式の操作がいくつかあります。



V547 Expression 'pol == 3' is always true.



 static errno_t cde40_pair_offsets_check(reiser4_place_t *place, uint32_t start_pos, uint32_t end_pos) { <...> if (end_offset == cde_get_offset(place, start_pos, pol) + ENTRY_LEN_MIN(S_NAME, pol) * count) { return 0; } <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

著者は、おそらくA ==（B + C）の形式の設計を念頭に置いていましたが、不注意に受け取った（A == B）+ C.



upd1。 私の間違い、混乱した優先度+と==



ファイルplugin / object / sym40 / sym40.cでエラーが発生しました-タイプミス：



V593 Consider reviewing the expression of the 'A = B < C' kind. The expression is calculated as following: 'A = (B < C)'.



 errno_t sym40_follow(reiser4_object_t *sym, reiser4_key_t *from, reiser4_key_t *key) { <...> if ((res = sym40_read(sym, path, size) < 0)) goto error; <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

問題は上記のものに似ています。 ブール式の結果が変数resに割り当てられていることがわかります。 明らかに、「機能」Cがここに適用されたため、式を（A = B）<Cの形式で書き換える必要があります。



タイプミスまたは不注意の別の代表。 ファイルlibreiser4 / flow.c ：



V555 The expression 'end - off > 0' will work as 'end != off'.



 int64_t reiser4_flow_write(reiser4_tree_t *tree, trans_hint_t *hint) { <...> uint64_t off; uint64_t end; <...> if (end - off > 0) { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

2つの整数変数があります。 その差は常にゼロ以上です。これは、コンピューターのメモリで整数を表すという観点から、プロセッサーの減算と加算が同じ演算（ 追加コード ）であるためです。 ほとんどの場合、条件をend> offに置き換える必要があります。



別の可能性のあるエラーはタイプミスです：



V547 Expression 'insert > 0' is always true.



 errno_t reiser4_flow_convert(reiser4_tree_t *tree, conv_hint_t *hint) { <...> for (hint->bytes = 0; insert > 0; insert -= conv) { <...> if (insert > 0) { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

コードはループ内にあり、ループ本体はinsert> 0の場合にのみ実行されます。 コードのこのセクションでは、条件のチェックインは常に真です。 間違いがあり、その結果、何か他のものが意図されていたか、または役に立たないチェックがあります。



V547 Expression 'ret' is always false.



 static errno_t repair_node_items_check(reiser4_node_t *node, place_func_t func, uint8_t mode, void *data) { <...> if ((ret = objcall(&key, check_struct) < 0)) return ret; if (ret) { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

前の条件では、フォームA =（B <0）の構築が見られますが、おそらく（A = B）<Cを意味していました。



ファイルlibrepair /semantic.cには、「ブラックマジック」の別の代表が存在する場合があります。



V612 An unconditional 'break' within a loop.



 static reiser4_object_t *cb_object_traverse(reiser4_object_t *parent, entry_hint_t *entry, void *data) { <...> while (sem->repair->mode == RM_BUILD && !attached) { <...> break; } <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この場合、 whileループは ifステートメントとして使用されます。 条件がtrueの場合、本体は1回実行されます（ ブレークが最後にあるため）、または条件がfalseの場合は満たされません。



次の前にどのプランが表示されるのか問題を推測してみてください。

そうです、これはタイプミスです-間違いです！ コードは「書き捨てられた」ように見えます。 今回はlibmisc / profile.cファイルのエラーは次のとおりです。



V528 It is odd that pointer to 'char' type is compared with the '\\0' value. Probably meant: *c + 1 == '\\0'.



 errno_t misc_profile_override(char *override) { <...> char *entry, *c; <...> if (c + 1 == '\0') { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ターミナルシンボルへのポインターを比較することは、間違いなく強力な解決策ですが、おそらく*（c + 1）== '\ 0'を念頭に置いていました。 option * c + 1 == '\ 0'はあまり意味がありません。



fprintf（）の使用に関するいくつかの警告を考慮してください。 警告自体は単純ですが、警告の内容を確認するには、いくつかのファイルをスキップする必要があります。

まず、 libmisc / ui.cファイルに移動します 。



V618 It's dangerous to call the 'fprintf' function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf("%s", str);









次のコードが表示されます。

 void misc_print_wrap(void *stream, char *text) { char *string, *word; <...> for (line_width = 0; (string = aal_strsep(&text, "\n")); ) { for (; (word = aal_strsep(&string, " ")); ) { if (line_width + aal_strlen(word) > screen_width) { fprintf(stream, "\n"); line_width = 0; } fprintf(stream, word); <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この関数の使用を探しています。 同じファイルにあります：

 void misc_print_banner(char *name) { char *banner; <...> if (!(banner = aal_calloc(255, 0))) return; aal_snprintf(banner, 255, BANNER); misc_print_wrap(stderr, banner); <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

BANNERを検索し、 include / misc / version.hファイルで見つけます。

 #define BANNER \ "Copyright (C) 2001-2005 by Hans Reiser, " \ "licensing governed by reiser4progs/COPYING."
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

すなわち 「注入」は発生しません。



ファイルprogs / debugfs / browse.cとprogs / debugfs / print.cの2番目の同様のエラーは同じコードを使用しているため、 browse.cのみを考慮してください。

 static errno_t debugfs_reg_cat(reiser4_object_t *object) { <...> char buff[4096]; <...> read = reiser4_object_read(object, buff, sizeof(buff)); if (read <= 0) break; printf(buff); <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

関数reiser4_object_read（）を探しています：

 int64_t reiser4_object_read( reiser4_object_t *object, /* object entry will be read from */ void *buff, /* buffer result will be stored in */ uint64_t n) /* buffer size */ { <...> return plugcall(reiser4_psobj(object), read, object, buff, n); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

plugcall（）の機能を探しています。これはマクロです：

 /* Checks if @method is implemented in @plug and calls it. */ #define plugcall(plug, method, ...) ({ \ aal_assert("Method \""#method"\" isn't implemented " \ "in "#plug"", (plug)->method != NULL); \ (plug)->method(__VA_ARGS__); \ })
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そしてもう一度、 メソッド（）が何をするのかを見つける必要があります。 そして、それはプラグに依存し、 プラグはreiser4_psobj（オブジェクト）です：

 #define reiser4_psobj(obj) \ ((reiser4_object_plug_t *)(obj)->info.pset.plug[PSET_OBJ])
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

それでもコードを調べてみると、これらもすべて一定の行であることがわかります。

 char *pset_name[PSET_STORE_LAST] = { [PSET_OBJ] = "object", [PSET_DIR] = "directory", [PSET_PERM] = "permission", [PSET_POLICY] = "formatting", [PSET_HASH] = "hash", [PSET_FIBRE] = "fibration", [PSET_STAT] = "statdata", [PSET_DIRITEM] = "diritem", [PSET_CRYPTO] = "crypto", [PSET_DIGEST] = "digest", [PSET_COMPRESS] = "compress", [PSET_CMODE] = "compressMode", [PSET_CLUSTER] = "cluster", [PSET_CREATE] = "create", };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そして、注射は機能しません。



残りのエラーは、上記で説明したものと同じ計画であるか、この場合は考慮すべき理由がないと考えられます。 たとえば、例のエラー、またはfprintの安全でない使用（理論的には、 書式文字列の挿入を使用できます） 。 この例は私にはあまり興味がありません。また、 fprintf（）のような関数の安全な使用は、私なしでは何度も考えられてきました 。

Reiser4をチェック

カーネル内のReiser4コードのチェックに直接進みます。 カーネル全体をアセンブルしないために、PVSを実行するようにスクリプトを変更して、Reiser4コードのみがアセンブルされるようにします。

 #!/usr/bin/bash pvs-studio-analyzer trace -- make SUBDIRS=fs/reiser4 -j9 || exit 1 pvs-studio-analyzer analyze -o log.log -j9 || exit 1 plog-converter -a GA:1,2 -t tasklist log.log || exit 1
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

したがって、 fs / reiser4フォルダーにあるソースコードのみを収集します 。





アセンブリは標準ヘッダーを使用せず、カーネルコードは興味がないため、カーネル自体のヘッダーで標準タイプをオーバーライドすることに関連するエラーと警告はスキップします。



邪魔になった最初のファイルはfs / reiser4 / carry.cです。



V522 Dereferencing of the null pointer 'reference' might take place. The null pointer is passed into 'add_op' function. Inspect the third argument. Check lines: 564, 703.



 static carry_op *add_op(carry_level * level, /* &carry_level to add node to */ pool_ordering order, /* where to insert: * at the beginning of @level; * before @reference; * after @reference; * at the end of @level */ carry_op * reference /* reference node for insertion */) { <...> result = (carry_op *) reiser4_add_obj(&level->pool->op_pool, &level->ops, order, &reference->header); <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この場合、 参照の NULLをチェックする必要があります 。なぜなら、 さらにコードでは、この関数の同様の呼び出しを確認できます。

 carry_op *node_post_carry(carry_plugin_info * info /* carry parameters * passed down to node * plugin */ , carry_opcode op /* opcode of operation */ , znode * node /* node on which this * operation will operate */ , int apply_to_parent_p /* whether operation will * operate directly on @node * or on it parent. */ ) { <...> result = add_op(info->todo, POOLO_LAST, NULL); <...>
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここで、 add_op（）は NULLの 参照値で明示的に呼び出され、カーネルはoopsを実行します。



次のエラー：



V591 Non-void function should return a value.



 static cmp_t carry_node_cmp(carry_level * level, carry_node * n1, carry_node * n2) { assert("nikita-2199", n1 != NULL); assert("nikita-2200", n2 != NULL); if (n1 == n2) return EQUAL_TO; while (1) { n1 = carry_node_next(n1); if (carry_node_end(level, n1)) return GREATER_THAN; if (n1 == n2) return LESS_THAN; } impossible("nikita-2201", "End of level reached"); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーは、関数がvoidではないことを示しているため、値を返す必要があります。 コードの最後の行から、このケースがエラーではないことが明らかになります。 whileの実行が停止した場合はエラーです。



V560 A part of conditional expression is always true: (result == 0).



 int lock_carry_node(carry_level * level /* level @node is in */ , carry_node * node/* node to lock */) { <...> result = 0; <...> if (node->parent && (result == 0)) { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここではすべてが単純であり、 結果の値は変化せず、テストは省略できます。 心配する必要はありません。



V1004 The 'ref' pointer was used unsafely after it was verified against nullptr. Check lines: 1191, 1210.



 carry_node *add_new_znode(znode * brother /* existing left neighbor of new * node */ , carry_node * ref /* carry node after which new * carry node is to be inserted * into queue. This affects * locking. */ , carry_level * doing /* carry queue where new node is * to be added */ , carry_level * todo /* carry queue where COP_INSERT * operation to add pointer to * new node will ne added */ ) { <...> /* There is a lot of possible variations here: to what parent new node will be attached and where. For simplicity, always do the following: (1) new node and @brother will have the same parent. (2) new node is added on the right of @brother */ fresh = reiser4_add_carry_skip(doing, ref ? POOLO_AFTER : POOLO_LAST, ref); <...> while (ZF_ISSET(reiser4_carry_real(ref), JNODE_ORPHAN)) { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

このチェックの本質は、三項演算子でrefの nullptrがチェックされ、次にnullptrをチェックせずにポインタの逆参照が発生する可能性があるreiser4_carry_real（）関数に渡されることです。 しかし、これはそうではありません。 reiser4_carry_real（）関数を考えてみましょう：

 znode *reiser4_carry_real(const carry_node * node) { assert("nikita-3061", node != NULL); return node->lock_handle.node; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

関数の本体でノードポインターのnullptrがチェックされているため、すべてが順序どおりになっていることがわかります。



続くのはfs / reiser4 / tree.cファイルの誤検知である可能性があります 。



V547 Expression 'child->in_parent.item_pos + 1 != 0' is always true.



 int find_child_ptr(znode * parent /* parent znode, passed locked */ , znode * child /* child znode, passed locked */ , coord_t * result /* where result is stored in */ ) { <...> if (child->in_parent.item_pos + 1 != 0) { <...> }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

item_pos広告を見つけて、それが何であるかを理解する必要があります。 いくつかのファイルを見ると、次の結果が得られます。

 struct znode { <...> parent_coord_t in_parent; <...> } __attribute__ ((aligned(16)));
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

 typedef struct parent_coord { <...> pos_in_node_t item_pos; } parent_coord_t;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

 typedef unsigned short pos_in_node_t;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

item_posが符号なしの整数であることがわかります。 つまり 条件trueがitem_posが可能な限り大きく、+ 1でオーバーフローが発生し、結果として変数がゼロになる場合は1つだけです。したがって、不等式条件はfalse falseです。



コメントでは、 Andrey2008がエラーの内容を示しています。 また、式がint型に変換される場合、 item_posの最大値の場合でも、オーバーフローは発生しないという事実から成ります。 式はint型に変換され、ゼロではなく、次の0xFFFF + 1 = 0x010000が得られます。



他のすべてのエラーは、上記で説明したエラーと同様であるか、誤検知であり、これも上記で検討されました。

結論

結論は簡単です。

まず、PVS-Studioはクールです。 適切なツールがあれば、より良い生産性で作業できます。 静的コードアナライザーとしてのPVS-Studioは、最高の側面から繰り返し実証されてきました。 開発者側の予期しない問題、タイプミス、または見落としを見つけて解決することができます。



次に、コードを慎重に記述します。 C言語の「ハック」を使用する必要はありません。少なくとも実際に正当化され、これなしでは不可能な場所では使用しないでください。 条件では、優先順位付けに加えて常に括弧を使用します.Cのスーパーデューパーハッカーでありスペシャリストであっても、オペレーターの正しい優先順位を忘れてしまい、多くのコードを一度に記述する場合は特に、多くのエラーが発生する可能性があるためです。

PVS-Studioの開発者に感謝

また、このような素晴らしいツールを開発者に感謝したいと思います！ 彼らは非常に一生懸命努力し、PVS-StudioをGNU / Linuxシステムに実装し、アナライザーの実装について考えました（詳細はこちらをご覧ください ）。 ビルドシステムにエレガントに統合され、ログを生成します。 アセンブリシステムに組み込みたくない場合は、 makeを実行することでコンパイラの実行を単に「キャッチ」できます。



そして最も重要なことは、学生、無料のプロジェクト、個々の開発者が無料で使用できる機会を与えてくれたことに感謝します！ これはすごい！