Skip to content

Commit

Permalink
Merge branch 'master' into ViewCFG
Browse files Browse the repository at this point in the history
  • Loading branch information
KengoTODA committed May 4, 2022
2 parents d73544a + 85ebe28 commit b5d0bab
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
- Updated documentation by adding parenthesis `()` to the negative odd check message ([#1995](https://github.com/spotbugs/spotbugs/issues/1995))

### Fixed
- Fixed reports to truncate existing files before writing new content ([#1950](https://github.com/spotbugs/spotbugs/issues/1950))
- Bumped Saxon-HE from 10.6 to 11.3 ([#1955](https://github.com/spotbugs/spotbugs/pull/1955), [#1999](https://github.com/spotbugs/spotbugs/pull/1999))
- Fixed traversal of nested archives governed by `-nested:true` ([#1930](https://github.com/spotbugs/spotbugs/pull/1930))
- Warnings of deprecated System::setSecurityManager calls on Java 17 ([#1983](https://github.com/spotbugs/spotbugs/pull/1983))
Expand All @@ -23,6 +24,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
* `THROWS_METHOD_THROWS_CLAUSE_THROWABLE` is reported when a method has Throwable in its throws clause (See [SEI CERT ERR07-J](https://wiki.sei.cmu.edu/confluence/display/java/ERR07-J.+Do+not+throw+RuntimeException%2C+Exception%2C+or+Throwable))
* 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))
* 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)
* New test detector `ViewCFG` to visualize the control-flow graph for `SpotBugs` developers

## 4.6.0 - 2022-03-08
Expand Down
2 changes: 1 addition & 1 deletion buildSrc/build.gradle.kts
Expand Up @@ -6,5 +6,5 @@ repositories {
gradlePluginPortal()
}
dependencies {
implementation("com.diffplug.gradle:goomph:3.36.0")
implementation("com.diffplug.gradle:goomph:3.36.2")
}
2 changes: 1 addition & 1 deletion eclipsePlugin-junit/build.gradle
Expand Up @@ -12,7 +12,7 @@ tasks.named('compileJava', JavaCompile).configure {
dependencies {
implementation project(':eclipsePlugin')
testImplementation 'junit:junit:4.13.2'
testImplementation 'org.mockito:mockito-core:4.5.0'
testImplementation 'org.mockito:mockito-core:4.5.1'
}

tasks.named('jacocoTestReport', JacocoReport).configure {
Expand Down
Expand Up @@ -36,6 +36,19 @@ public void handleOutputFilePathUsesGzip() throws IOException {
assertThat("GZip file should have -117 as its header", written[1], is((byte) -117));
}

@Test
public void handleOutputFileTruncatesExisting() throws IOException {
Path file = Files.createTempFile("spotbugs", ".html");
Files.writeString(file, "content");
TextUICommandLine commandLine = new TextUICommandLine();
SortingBugReporter reporter = new SortingBugReporter();
commandLine.handleOutputFilePath(reporter, "withMessages=" + file.toFile().getAbsolutePath());

reporter.finish();
byte[] written = Files.readAllBytes(file);
assertThat("Output file should be truncated to 0 bytes", written.length, is(0));
}

@Test
public void htmlReportWithOption() throws IOException {
Path xmlFile = Files.createTempFile("spotbugs", ".xml");
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 @@ -661,6 +661,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 @@ -1277,6 +1279,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 @@ -1746,6 +1746,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 @@ -8681,6 +8688,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 @@ -8707,6 +8724,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
Expand Up @@ -278,7 +278,8 @@ public boolean justPrintVersion() {
if (index >= 0) {
Path path = Paths.get(optionExtraPart.substring(index + 1));
try {
OutputStream oStream = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
OutputStream oStream = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE,
StandardOpenOption.TRUNCATE_EXISTING);
if ("gz".equals(Util.getFileExtension(path.toFile()))) {
oStream = new GZIPOutputStream(oStream);
}
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()));
}
}
}
2 changes: 1 addition & 1 deletion spotbugsTestCases/build.gradle
Expand Up @@ -24,7 +24,7 @@ dependencies {
api 'net.jcip:jcip-annotations:1.0'
implementation 'org.springframework:spring-core:5.3.19'
compileOnly 'javax.annotation:javax.annotation-api:1.3.2'
implementation 'org.checkerframework:checker-qual:3.21.4'
implementation 'org.checkerframework:checker-qual:3.22.0'

implementation 'junit:junit:4.13.2'
implementation 'org.testng:testng:7.5'
Expand Down
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 b5d0bab

Please sign in to comment.