2024 年、私たちは豊富なプロジェクトを分析し、その発見をブログで共有してきました。さあ、大晦日です。お祝いの話をするときです。私たちは、オープンソース プロジェクトで検出された最も興味深い Java エラーを集めて、皆さんにお届けします!
私たちは、PVS-Studio で検出された最も興味深いバグを投稿するという伝統を長年守ってきましたが、2020 年以降、Java 関連のトップはありませんでした。今回はヴィンテージルーブリックを復活させてみました。心地よい毛布と温かいお茶を手元に用意していただければ幸いです。私はあなたのために 10 匹以上の面白い虫を選びました。ランク付け方法は次のとおりです:
独自のプログラミングの知恵を使った 10 の楽しいストーリーを楽しみましょう — もうすぐ大晦日がやってきます :)
10 位では、最初のコード スニペットが両手を広げて私たちを歓迎します。
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
このコードのバグにより、より早く翌年に移動できるため、これを大晦日のトップに含めずにはいられませんでした :) バグの原因はどこにあると思いますか?
SimpleDateFormat コンストラクターに渡される引数を見てみましょう。それは有効だと思われますか？この記事の執筆日 (2024 年 10 月 12 日) など、ほぼすべての日付を渡すと、コードは正しい値 20241210 を返します。
ただし、2024 年 12 月 29 日を過ぎると、代わりに 20251229 が返されるため、巧妙に早めに新年に移行します。ちなみに、時間を遡ることも可能です。
これは、SimpleDateFormat の引数内の Y 文字が週番号に基づいて年を表すために発生します。つまり、新年の少なくとも 4 日が含まれる場合、1 週間は最初の週とみなされます。つまり、週が日曜日に始まる場合、3 日早く新年に突入することができます。
これを修正するには、大文字の Y を小文字の y に置き換えるだけです。もっと詳しく知りたいですか?このトピックに投稿全体を捧げました。
このエラーに対する PVS-Studio の警告は次のとおりです:
V6122 'Y' (週年) パターンの使用が検出されました: おそらく 'y' (年) を使用することが意図されていました。 SkeinParameters.java 246
週番号の詳細のため、テストはこのエラーを見つけるのに最適な手段ではありません。では、なぜこのような話題のバグが最後に来るのでしょうか?その理由は、警告が Bouncy Castle の実際のバージョンからのものではなく、テスト ベースからのものであるためです。古いソースはまだそこに残っており、このバグは長い間修正されてきました。これは過去からの敬礼です、またタイムトラベルです:)
9 番目には、GeoServer 分析からの警告があります:
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// ....
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key); // <=
Assert.assertEquals(newValue, oldValue);
}
}
PVS-Studio の警告は次のとおりです:
V6027 変数 'newValue'、'oldValue' は、同じ関数の呼び出しを通じて初期化されます。おそらくエラーか、最適化されていないコードです。 DataAccessRuleDAOTest.java 110、DataAccessRuleDAOTest.java 111
このようなエラーの何が興味深いのでしょうか? 4 つの点の背後に何が隠されているかを明らかにしましょう:
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
コードが何らかの理由で機能しないというコメントがあります。正直、初めて見たときは笑ってしまいました。
ただし、このコメントはかなり曖昧です。おそらく、比較がダウンしている間の失敗を防ぐために、テストが意図的にそのように書かれていると考えられます。ただし、コードは 10 年以上この状態にあり、いくつかの疑問が生じます。この曖昧さがあるため、私はそれを上位にランク付けしませんでした。
JBullet のバグを足元のショットと呼ぶことができない場合、どのバグをそう呼ぶことができるのかわかりません。記事のエラーは次のとおりです:
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// ....
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key); // <=
Assert.assertEquals(newValue, oldValue);
}
}
エラーの場所を特定するために PVS-Studio の警告は必要ないと思います。とにかく、念のため、ここにあります:
V6026 この値はすでに「proxy1」変数に割り当てられています。ハッシュオーバーラップペアキャッシュ.java 233
はい、これは恥ずかしいほど単純な間違いです。しかし、そのシンプルさがさらに面白いものにしています。それでも、これには独自の物語があります。
JBullet ライブラリは C/C Bullet ライブラリからの移植であり、同様の関数があります。
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// properties equality does not seem to work...
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key);
Assert.assertEquals(newValue, oldValue);
}
}
このコードが正しく書かれていることが簡単にわかります。 gitblame から判断すると、本来は正しく書かれています。コードをある言語から別の言語に移植したときにエラーが発生したことが判明しました。
豊かな歴史と驚くほど気取らない雰囲気を評価して、私はこの警告を 8 位に与えます。足を撃たれるバグが C 関連であることが判明したことを楽しんでいただけたでしょうか。
確かに、次の警告はさまざまな理由から私の心を温かくします。以下は、GeoGebra チェックのコード スニペットです:
@Override
public BroadphasePair findPair(BroadphaseProxy proxy0, BroadphaseProxy proxy1) {
BulletStats.gFindPairs++;
if (proxy0.getUid() > proxy1.getUid()) {
BroadphaseProxy tmp = proxy0;
proxy0 = proxy1;
proxy1 = proxy0;
}
....
}
自分でエラーを見つけてみてください。覗き見されないように、警告と説明をネタバレ部分に隠しました。
V6107 定数 0.7071067811865 が使用されています。結果の値は不正確になる可能性があります。 Math.sqrt(0.5) の使用を検討してください。 DrawAngle.java 303 実際には、0.7071067811865 は魔法の数字ではなく、単に 0.5 の平方根を四捨五入した結果です。しかし、この精度の低下はどれほど重大なのでしょうか? GeoGebra は数学者向けに作られたソフトウェアであり、精度が高くても問題はないようです。 なぜ私はこの虫がそんなに好きなのですか? まず、カンファレンスでは、他のコード部分で同様のエラーを見つけるように参加者に依頼することがよくあります。定数の中にバグが隠されている場合に、彼らがコードを注意深く分析するのを見るのはいつも楽しいものです。 2 番目に、これは Java アナライザー用に実装された最初の診断ルールです。だからこそ、偏見があることを承知の上で、これをトップに含めずにはいられず、7 位に入れました :)答え
まず、PVS-Studio の警告を見てみましょう。
次の警告は、DBeaver チェックに基づいて最初の記事から抜粋したものですが、すぐには注意を引かないかもしれません。コードの一部を次に示します:
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
PVS-Studio アナライザーが検出した内容は次のとおりです:
V6082 安全でない二重チェックロック。フィールドは volatile として宣言する必要があります。 TaskImpl.java 59、TaskImpl.java317
この特定の警告については特別なことは何もありませんが、それでも非常に興味深いと思います。重要なのは、適用された Double-Checked Locking パターンが機能しないということです。コツは何ですか？これは20年前にも関連していました:)
このトピックについてさらに詳しく知りたい場合は、記事全文を読むことをお勧めします。とりあえず、簡単な概要を説明しましょう。
ダブルチェック ロック パターンは、マルチスレッド環境で遅延初期化を実装するために使用されます。 「重量」チェックの前に、「軽量」チェックが同期ブロックなしで実行されます。リソースは両方のチェックに合格した場合にのみ作成されます。
ただし、このアプローチでは、オブジェクトの作成は非アトミックであり、プロセッサーとコンパイラーは操作順序を変更できます。したがって、別のスレッドが部分的に作成されたオブジェクトを誤って受け取り、そのオブジェクトの操作を開始する可能性があり、それが誤った動作を引き起こす可能性があります。このエラーはめったに発生しないため、開発者にとってデバッグは大きな課題となります。
ここにひねりがあり、このパターンは JDK 5 までは機能しませんでした。JDK 5 以降では、happens-before 原則のおかげで操作の並べ替えという潜在的な問題を解決するために volatile キーワードが導入されました。アナライザーは、このキーワードを追加する必要があると警告します。
ただし、とにかくこのパターンは避けたほうが良いでしょう。ハードウェアと JVM のパフォーマンスはそれ以来大幅に進歩し、同期操作はそれほど遅くなくなりました。ただし、DCL パターンを誤って実装すると、依然としてよくある落とし穴があり、前述の重大な結果を招く可能性があります。これは、私たちのアナライザーが依然として古いプロジェクトでこのような過失エラーを検出しているという事実を裏付けています。
5 位には、別の DBeaver 警告が含まれており、これについては記事を用意しました。見てみましょう:
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// ....
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key); // <=
Assert.assertEquals(newValue, oldValue);
}
}
説明は次のとおりです:
V6030 「&」演算子の右側にあるメソッドは、左側のオペランドの値に関係なく呼び出されます。おそらく、「&&」を使用する方がよいでしょう。 ExasolTableColumnManager.java 79、DB2TableColumnManager.java 77
開発者は論理 && とビット単位の & を混同しました。これらの動作は異なります。式内の条件はビット単位の AND の後で終了しません。 短絡評価はビット単位の AND では機能しません。したがって、 exasolTableBase != null が false を返しても、実行スレッドは exasolTableBase.getClass() に到達し、NPE が発生します。
わかりました、それはただのタイプミスです、先に進みましょう? DBeaver にはそのような警告がたくさんあります。たくさん。多くは比較的無害ですが、好奇心旺盛な読者のために、以下にいくつかの例を残しました:
ExasolConnectionManager.java: ExasolDataSource.java:エラーを引き起こさないビット演算の使用
ExasolSecurityPolicy.java:
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// ....
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key); // <=
Assert.assertEquals(newValue, oldValue);
}
}
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// properties equality does not seem to work...
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key);
Assert.assertEquals(newValue, oldValue);
}
}
さらに詳しく調査した結果、私のチームは、開発者がパフォーマンスを微細に最適化しようとしていたのではないかと推測しました。全体像については、記事をご覧ください。ここでは概要を紹介します。
重要な点は、ビット単位の演算が 分岐予測 に依存しないため、論理演算と比較してより高速な実行が可能になる可能性があるということです。
驚いたことに、独自のベンチマークがこの主張を裏付けています。
この表は、各操作の種類にかかる時間を示しています。これを信頼すると、ビット単位の演算は論理演算より 40% 高速であるようです。
なぜこの話題を取り上げたのでしょうか?潜在的な微細最適化のコストを強調するため。
まず、開発者が分岐予測を発明したのには理由がありました。それを放棄するにはコストがかかりすぎるからです。したがって、値には正規分布があり、実際のケースでは観察される可能性は低いため、ベンチマークはおそらく高速に動作します。
第二に、短絡評価メカニズムを放棄すると、コストが大幅に増加する可能性があります。上記のスポイラーの 3 番目の例を見ると、最速ではない contains 操作がすぐに停止するのではなく、常に実行されることがわかります。
第三に、章の最初からそのようなエラーに対して白紙の状態を与えます。
全体として、マイクロ最適化の価格に関する警告的な話は、トップ 5 を開くのに十分な説得力があると思います。
自動テストは、さまざまなエラーに対する究極の安全策として考えられることがよくあります。しかし、時々、「テスト自体は誰が行うのですか?」と尋ねたくなることがあります。 GeoServer チェックからの別の警告もこれを示しています。コードの一部を次に示します:
@Override
public BroadphasePair findPair(BroadphaseProxy proxy0, BroadphaseProxy proxy1) {
BulletStats.gFindPairs++;
if (proxy0.getUid() > proxy1.getUid()) {
BroadphaseProxy tmp = proxy0;
proxy0 = proxy1;
proxy1 = proxy0;
}
....
}
PVS-Studio の警告:
V6060 「e」参照は、null に対して検証される前に使用されました。 ResourceAccessManagerWCSTest.java 186、ResourceAccessManagerWCSTest.java 193
V6060 は冗長コードに対して発行されることが多いため、一見すると、この警告はアナライザーにとって最も興味深い警告ではないように思えるかもしれません。それでも、私は彼らの魅力に基づいて候補者を選ぶと約束しました。したがって、この事件は見た目よりもはるかに興味深いものです。
最初は、テスト ロジックが間違っているように見えるかもしれません。これは、変数 e が catch 演算子から取得され、その後変更されないため、null になることはありません。野良編集を行って、if(e == nul) 条件の then 分岐を到達不能として削除するだけです。しかし、それは根本的に間違っています。獲物はもう分かりましたか?
キーは、例外オブジェクトを含むコード内のもう 1 つの変数、つまり se にあります。ループ本体内で変更される値を持ちます。したがって、条件には e の代わりに se 変数が存在するはずであると簡単に推測できます。
このエラーにより then ブランチが実行されなくなるため、例外がないことがわかりません。さらに悪いことに、変数名が非常に似ているため、コードレビューでそのようなエラーに気づくのはかなり困難です。
この話から得られる知恵は 2 つあります:
このような貴重な教訓を提供したことに対して、私はこの警告を 4 位に与えます。
上位 3 つの勝者は、NetBeans チェックからの警告に属します。以前のコード スニペットは比較的コンパクトだったので、長いコード スニペットを見てみましょう:
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
最後は自分でバグを見つけてみてください。お待ちしています...
検索していますか?
いいですね！ iDesc.neighbor != null || という式にのみエラーを見つけた人iDesc.index == iDesc.index、残念ですが、負けました :)
確かに問題はありますが、トップほど興味深いものではありません。はい、ここには 2 つの間違いがあります。少し騙されました。しかし、ちょっとしたいたずらのない休日とは何でしょうか？ :)
アナライザーはここで i^i 式のエラーを検出し、次の警告を発行しました:
V6001 '^' 演算子の左右に同一の部分式 'i' があります。 LayoutFeeder.java 3897
2 つの同一の値の排他的論理和は常に 0 になるため、XOR 演算は意味がありません。簡単に復習するために、XOR の真理値表を次に示します。
| a | b | a^b |
|---|---|---|
| 0 | 0 | 0 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
| 1 | 1 | 0 |
言い換えれば、演算はオペランドが異なる場合にのみ真となります。値が同じなので、すべてのビットが同じになります。
なぜ私はこのバグをそんなに気に入ったのでしょうか? i^1 演算がありますが、これは i^i とほぼ同じです。したがって、上記で正しい i^1 がすでに確認されているため、コード レビューでこのエラーを見逃すのは非常に簡単です。
あなたのことは知りませんが、次のような有名な言葉を思い出します。
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
そうでないと、単純なタイプミスで退屈なバージョンを無視しない限り、それがどのようにしてコードに組み込まれたのかを説明するのは困難です。もしそのバグを見つけることができたなら、自分の背中をたたいてください。あるいは、コメントであなたの探偵スキルを共有してください :)
DBeaver の記事の 1 つ目と 3 つ目のエラーはすでに示しましたので、2 つ目は省略します。私は訂正します。以下は 2 番目の記事から抜粋したものです。
PVS-Studio アナライザーは、サブクラスでオーバーライドされる TextWithOpen クラスのコンストラクターから isBinaryContents が呼び出されるのを好みません。
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// ....
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key); // <=
Assert.assertEquals(newValue, oldValue);
}
}
だから何？これはオーバーライドされますが、大したことではありません。それはコードの匂いのように思えますが、重大なものではありません。少なくとも、私はそう思っていました。このバグとの闘いを記事にしました。
TextWithOpen には多くのサブクラスがあり、そのうちの 1 つが TextWithOpenFile です。そこで、メソッドは実際にオーバーライドされ、false の代わりに、スーパークラスにないフィールドが返されます。
@Test
public void testStore() {
Properties newProps = dao.toProperties();
// properties equality does not seem to work...
Assert.assertEquals(newProps.size(), props.size());
for (Object key : newProps.keySet()) {
Object newValue = newProps.get(key);
Object oldValue = newProps.get(key);
Assert.assertEquals(newValue, oldValue);
}
}
まだ怪しいですか?このクラスのコンストラクターはどのようなものですか?
@Override
public BroadphasePair findPair(BroadphaseProxy proxy0, BroadphaseProxy proxy1) {
BulletStats.gFindPairs++;
if (proxy0.getUid() > proxy1.getUid()) {
BroadphaseProxy tmp = proxy0;
proxy0 = proxy1;
proxy1 = proxy0;
}
....
}
それに気づきましたか?バイナリ フィールドは、スーパークラス コンストラクターが呼び出された後に初期化されます。ただし、サブクラス フィールドを参照する isBinaryContents メソッドへの呼び出しがあります!
PVS-Studio の警告は次のとおりです:
V6052 'TextWithOpen' 親クラス コンストラクターでオーバーライドされた 'isBinaryContents' メソッドを呼び出すと、初期化されていないデータが使用される可能性があります。検査フィールド: バイナリ。 TextWithOpenFile.java(77)、TextWithOpen.java 59
なかなか面白い写真ですね。一見すると、開発者はベスト プラクティスに従っているように見えました。つまり、保守が不可能なスパゲッティ コードを避け、テンプレート メソッド パターンを通じて正規の OOP を実装しようとしました。しかし、このような単純なパターンを実装する場合でも、間違いを犯す可能性があり、それが実際に起こったのです。私の意見では、このような（誤った）シンプルさの美しさは、確実に 2 位です。
喜んでください！ステージ初登場！競争は熾烈でしたが、選択を迫られました。熟考した結果、NetBeans チェックからの警告を受け入れることにしました。最後のコード スニペットを紹介します:
public Builder setPersonalisation(Date date, ....) {
....
final OutputStreamWriter
out = new OutputStreamWriter(bout, "UTF-8");
final DateFormat
format = new SimpleDateFormat("YYYYMMdd");
out.write(format.format(date));
....
}
このようなバグを一目で見つけることは不可能であると私は確信しています。もちろん、あなた自身がこの間違いを犯した場合を除きます。お待たせしません。PVS-Studio の警告は次のとおりです。
V6009 バッファ容量は、char 値を使用して「47」に設定されます。おそらく、「/」記号がバッファーに配置されるはずでした。 IgnoreUnignoreCommand.java 107
実際、このエラーは驚くほど初歩的なものです。StringBuilder コンストラクターには、char を受け入れるオーバーロードがありません。それではどのコンストラクターが呼び出されるでしょうか?開発者は、String を受け入れるオーバーロードが呼び出され、StringBuilder の初期値がこのスラッシュになると考えたようです。
ただし、暗黙的な型変換が発生し、int を受け入れる型コンストラクターが呼び出されます。この場合、これは StringBuilder の初期サイズを表します。 char を引数として渡しても、最終的な文字列には含まれないため、機能的には何も影響しません。初期サイズを超えた場合、例外やその他の副作用が発生することなく、自然に増加するだけです。
でも、ちょっと待ってください、私は 2 つの間違いについて言及しましたよね? 2 番目はどこにあり、それらはどのように接続されているのでしょうか?これを見つけるには、メソッド本体を読み取って、このコードが何を行うのかを理解する必要があります。
ファイルまたはディレクトリへの絶対パスを生成します。コードに基づくと、結果のパスは次のようになります。
コードは非常に正しいようです。それが問題なのです。コードは実際に期待どおりに機能します:) しかし、文字を文字列に置き換えてエラーを修正すると、正しい結果の代わりに次の結果が得られます:
言い換えると、文字列の最後に余分なスラッシュが追加されます。上記のコードは毎回行の先頭に新しいテキストを追加するため、これは最後になります。
したがって、2 番目のエラーは、このスラッシュが引数としてコンストラクターに渡されたことです。ただし、誰かがチェックせずに文字を文字列に置き換えることを決定した場合、問題が発生する可能性があるため、このようなエラーを過小評価することはできません。
このようにして、エラーの先頭に正しく動作するコードが表示されます。新年の奇跡、何を期待していましたか？ :)
私のバグの話を楽しんで読んでいただければ幸いです。特定のストーリーが印象に残った場合、またはランキングの調整に関する提案がある場合は、コメント欄でお気軽にご意見を共有してください。次回の参考にさせていただきます :)
他の言語に興味がある場合は、2024 年の C# のトップ バグをここでチェックしてみてください。新しいトップに注目してください。
これらのエラーはすべて PVS-Studio アナライザーで検出され、最新バージョン (7.34) がリリースされました。このリンクから試すことができます。
コードの品質に関する新しい記事を常にご覧いただくために、購読することをお勧めします。
明けましておめでとうございます!
以上が4 つの最も興味深い Java エラーの上位の詳細内容です。詳細については、PHP 中国語 Web サイトの他の関連記事を参照してください。
![[Web フロントエンド] Node.js クイック スタート](https://img.php.cn/upload/course/000/000/067/662b5d34ba7c0227.png)
