Skip to content

Commit

Permalink
Added new detector for NUM09-J: Do not use floating-point variables a…
Browse files Browse the repository at this point in the history
…s loop counters (#1976)

* Added detector DontUseFloatsAsLoopCounters for NUM09-J: Do not use floating-point variables as loop counters

* Fixed wrong opcodes in isInductionVariable

* Removed isInductionVariable and rewrote message

* removed isFloatOrDouble

* Made BugReporter private final

Co-authored-by: Kengo TODA <skypencil@gmail.com>
  • Loading branch information
adrianturtoczki and KengoTODA committed May 4, 2022
1 parent 23f37b9 commit 85ebe28
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -25,6 +25,9 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
* New rule `PERM_SUPER_NOT_CALLED_IN_GETPERMISSIONS` to warn for custom class loaders who do not call their superclasses' `getPermissions()` in their `getPermissions()` method. This rule based on the SEI CERT rule *SEC07-J Call the superclass's getPermissions() method when writing a custom class loader*. ([#SEC07-J](https://wiki.sei.cmu.edu/confluence/display/java/SEC07-J.+Call+the+superclass%27s+getPermissions%28%29+method+when+writing+a+custom+class+loader))
* New rule `USC_POTENTIAL_SECURITY_CHECK_BASED_ON_UNTRUSTED_SOURCE` to detect cases where a non-final method of a non-final class is called from public methods of public classes and then the same method is called on the same object inside a doPrivileged block. Since the called method may have been overridden to behave differently on the first and second invocations this is a possible security check based on an unreliable source. This rule is based on *SEC02-J. Do not base security checks on untrusted sources*. ([#SEC02-J](https://wiki.sei.cmu.edu/confluence/display/java/SEC02-J.+Do+not+base+security+checks+on+untrusted+sources))

### Added
* New detector `DontUseFloatsAsLoopCounters` to detect usage of floating-point variables as loop counters (`FL_FLOATS_AS_LOOP_COUNTERS`), according to SEI CERT rules [NUM09-J. Do not use floating-point variables as loop counters](https://wiki.sei.cmu.edu/confluence/display/java/NUM09-J.+Do+not+use+floating-point+variables+as+loop+counters)

## 4.6.0 - 2022-03-08
### Fixed
- Fixed spotbugs build with ecj compiler ([#1903](https://github.com/spotbugs/spotbugs/issues/1903))
Expand Down
@@ -0,0 +1,38 @@
package edu.umd.cs.findbugs.detect;

import org.junit.Test;

import edu.umd.cs.findbugs.AbstractIntegrationTest;
import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;

import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
import static org.hamcrest.Matchers.hasItem;
import static org.junit.Assert.assertThat;

public class DontUseFloatsAsLoopCountersTest extends AbstractIntegrationTest {
@Test
public void testChecks() {
performAnalysis("DontUseFloatsAsLoopCounters.class");
assertNumOfEOSBugs(3);
assertBug("main", 5);
assertBug("main", 9);
assertBug("main", 12);
}

private void assertBug(String method, int line) {
final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
.bugType("FL_FLOATS_AS_LOOP_COUNTERS")
.inClass("DontUseFloatsAsLoopCounters")
.inMethod(method)
.atLine(line)
.build();
assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
}

private void assertNumOfEOSBugs(int num) {
final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
.bugType("FL_FLOATS_AS_LOOP_COUNTERS").build();
assertThat(getBugCollection(), containsExactly(num, bugTypeMatcher));
}
}
3 changes: 3 additions & 0 deletions spotbugs/etc/findbugs.xml
Expand Up @@ -659,6 +659,8 @@
reports="MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR,MC_OVERRIDABLE_METHOD_CALL_IN_CLONE"/>
<Detector class="edu.umd.cs.findbugs.detect.FindInstanceLockOnSharedStaticData" speed="fast"
reports="SSD_DO_NOT_USE_INSTANCE_LOCK_ON_SHARED_STATIC_DATA"/>
<Detector class="edu.umd.cs.findbugs.detect.DontUseFloatsAsLoopCounters" speed="fast"
reports="FL_FLOATS_AS_LOOP_COUNTERS"/>
<Detector class="edu.umd.cs.findbugs.detect.ThrowingExceptions" speed="fast"
reports="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION,THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION,THROWS_METHOD_THROWS_CLAUSE_THROWABLE" />
<Detector class="edu.umd.cs.findbugs.detect.PermissionsSuper"
Expand Down Expand Up @@ -1275,6 +1277,7 @@
<BugPattern abbrev="MC" type="MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR" category="MALICIOUS_CODE" />
<BugPattern abbrev="MC" type="MC_OVERRIDABLE_METHOD_CALL_IN_CLONE" category="MALICIOUS_CODE" />
<BugPattern abbrev="SSD" type="SSD_DO_NOT_USE_INSTANCE_LOCK_ON_SHARED_STATIC_DATA" category="CORRECTNESS" />
<BugPattern abbrev="FL" type="FL_FLOATS_AS_LOOP_COUNTERS" category="CORRECTNESS" />
<BugPattern abbrev="THROWS" type="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION" category="BAD_PRACTICE" />
<BugPattern abbrev="THROWS" type="THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION" category="BAD_PRACTICE" />
<BugPattern abbrev="THROWS" type="THROWS_METHOD_THROWS_CLAUSE_THROWABLE" category="BAD_PRACTICE" />
Expand Down
18 changes: 18 additions & 0 deletions spotbugs/etc/messages.xml
Expand Up @@ -1739,6 +1739,13 @@ factory pattern to create these objects.</p>
]]>
</Details>
</Detector>
<Detector class="edu.umd.cs.findbugs.detect.DontUseFloatsAsLoopCounters">
<Details>
<![CDATA[
<p>Checks for floats in loop counters.</p>
]]>
</Details>
</Detector>
<Detector class="edu.umd.cs.findbugs.detect.PermissionsSuper">
<Details>
<![CDATA[
Expand Down Expand Up @@ -8674,6 +8681,16 @@ object explicitly.</p>
</p>]]>
</Details>
</BugPattern>
<BugPattern type="FL_FLOATS_AS_LOOP_COUNTERS">
<ShortDescription>Do not use floating-point variables as loop counters</ShortDescription>
<LongDescription>Using floating-point loop counters can lead to unexpected behavior.</LongDescription>
<Details>
<![CDATA[<p>
Using floating-point variables should not be used as loop counters, as they are not precise, which may result in incorrect loops. A loop counter is a variable that is changed with each iteration and controls when the loop should terminate. It is decreased or increased by a fixed amount each iteration.</p>
<p>See rule <a href="https://wiki.sei.cmu.edu/confluence/display/java/NUM09-J.+Do+not+use+floating-point+variables+as+loop+counters">NUM09-J</a>.</p>
]]>
</Details>
</BugPattern>
<BugPattern type="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION">
<ShortDescription>Method intentionally throws RuntimeException.</ShortDescription>
<LongDescription>Method intentionally throws RuntimeException.</LongDescription>
Expand All @@ -8700,6 +8717,7 @@ object explicitly.</p>
<p>
Method lists Exception in its throws clause.<br>
When declaring a method, the types of exceptions in the throws clause should be the most specific.
Therefore, using Exception in the throws clause would force the caller to either use it in its own throws clause, or use it in a try-catch block (when it does not necessarily
contain any meaningful information about the thrown exception).<br><br>
Expand Down
@@ -0,0 +1,28 @@
package edu.umd.cs.findbugs.detect;

import org.apache.bcel.Const;

import edu.umd.cs.findbugs.BugInstance;
import edu.umd.cs.findbugs.BugReporter;
import edu.umd.cs.findbugs.StatelessDetector;
import edu.umd.cs.findbugs.bcel.OpcodeStackDetector;
import java.util.stream.Stream;

public class DontUseFloatsAsLoopCounters extends OpcodeStackDetector implements StatelessDetector {

private final BugReporter bugReporter;

public DontUseFloatsAsLoopCounters(BugReporter bugReporter) {
this.bugReporter = bugReporter;
}

@Override
public void sawOpcode(int seen) {
if ((seen == Const.GOTO || seen == Const.GOTO_W) &&
Stream.of(Const.FLOAD, Const.FLOAD_0, Const.FLOAD_1, Const.FLOAD_2, Const.FLOAD_3, Const.DLOAD, Const.DLOAD_0, Const.DLOAD_1,
Const.DLOAD_2, Const.DLOAD_3).anyMatch(x -> x == getCodeByte(getBranchTarget())) && (getBranchTarget() < getPC())) {
bugReporter.reportBug(new BugInstance(this, "FL_FLOATS_AS_LOOP_COUNTERS", NORMAL_PRIORITY)
.addClassAndMethod(this).addSourceLine(this, getBranchTarget()));
}
}
}
34 changes: 34 additions & 0 deletions spotbugsTestCases/src/java/DontUseFloatsAsLoopCounters.java
@@ -0,0 +1,34 @@
class DontUseFloatsAsLoopCounters {
public static void main(String[] args) {
//noncompliant
float x = 0.1f;
while (x<10){
System.out.println(x);
x++;
}
for (float y = 0.2f; y <= 1.0f; y += 0.1f) {
System.out.println(y);
}
for (double d = 0.2d; d <= 1.0d; d += 0.1d) {
System.out.println(d);
}
//compliant
for (int count = 1; count <= 10; count += 1) {
float q = count/10.0f;
System.out.println(q);
System.out.println(count);
}
int c = 0;
while (c<5){
c++;
}
boolean b = true;
while (b){
b = false;
}
int p = 1;
while (p<9){
p*=2;
}
}
}

0 comments on commit 85ebe28

Please sign in to comment.