From 3722f902e7f64d047184ba4d43aabe61f1ef3556 Mon Sep 17 00:00:00 2001 From: gonczmisi Date: Tue, 10 May 2022 00:02:52 +0200 Subject: [PATCH 1/8] Introduced detector for rule VNA-00 and VNA-02 with test cases --- CHANGELOG.md | 2 + ...ibilityOnSharedPrimitiveVariablesTest.java | 100 +++++++++ ...mpoundOperationsOnSharedVariablesTest.java | 207 ++++++++++++++++++ spotbugs/etc/findbugs.xml | 6 + spotbugs/etc/messages.xml | 56 +++++ ...dVisibilityOnSharedPrimitiveVariables.java | 158 +++++++++++++ .../CompoundOperationsOnSharedVariables.java | 207 ++++++++++++++++++ .../MultiThreadedCodeIdentifierUtils.java | 106 +++++++++ ...poundAdditionOnSharedVolatileVariable.java | 13 ++ .../CompoundDivisionOnSharedVariable.java | 13 ++ ...CompoundIANDOperationOnSharedVariable.java | 13 ++ .../CompoundIOROperationOnSharedVariable.java | 13 ++ .../CompoundLeftShiftingOnSharedVariable.java | 17 ++ ...dLogicalRightShiftingOnSharedVariable.java | 13 ++ .../CompoundModuloOnSharedVariable.java | 14 ++ ...ompoundMultiplicationOnSharedVariable.java | 13 ++ ...mpoundOperationOnSharedAtomicVariable.java | 15 ++ ...undPostDecrementationOnSharedVariable.java | 13 ++ ...undPostIncrementationOnSharedVariable.java | 13 ++ ...oundPreDecrementationOnSharedVariable.java | 13 ++ ...oundPreIncrementationOnSharedVariable.java | 13 ++ ...CompoundRightShiftingOnSharedVariable.java | 19 ++ .../CompoundSubtractionOnSharedVariable.java | 13 ++ .../CompoundXOROperationOnSharedVariable.java | 13 ++ ...lockCompoundOperationOnSharedVariable.java | 15 ++ ...thodCompoundOperationOnSharedVariable.java | 13 ++ .../sharedPrimitiveVariables/AtomicField.java | 21 ++ .../FieldWithBadVisibilityRunnable.java | 20 ++ .../FieldWithBadVisibilityThread.java | 20 ++ .../SynchronizedBlock.java | 22 ++ ...chronizedBlockAndBadVisibilityOnField.java | 25 +++ .../SynchronizedMethod.java | 20 ++ ...hronizedMethodAndBadVisibilityOnField.java | 23 ++ .../VolatileField.java | 19 ++ 34 files changed, 1261 insertions(+) create mode 100644 spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/BadVisibilityOnSharedPrimitiveVariablesTest.java create mode 100644 spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindCompoundOperationsOnSharedVariablesTest.java create mode 100644 spotbugs/src/main/java/edu/umd/cs/findbugs/detect/BadVisibilityOnSharedPrimitiveVariables.java create mode 100644 spotbugs/src/main/java/edu/umd/cs/findbugs/detect/CompoundOperationsOnSharedVariables.java create mode 100644 spotbugs/src/main/java/edu/umd/cs/findbugs/util/MultiThreadedCodeIdentifierUtils.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundAdditionOnSharedVolatileVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundDivisionOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundIANDOperationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundIOROperationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundLeftShiftingOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundLogicalRightShiftingOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundModuloOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundMultiplicationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundOperationOnSharedAtomicVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundPostDecrementationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundPostIncrementationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundPreDecrementationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundPreIncrementationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundRightShiftingOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundSubtractionOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/CompoundXOROperationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/SynchronizedBlockCompoundOperationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/compoundOperationOnSharedVariables/SynchronizedMethodCompoundOperationOnSharedVariable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/AtomicField.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/FieldWithBadVisibilityRunnable.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/FieldWithBadVisibilityThread.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/SynchronizedBlock.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/SynchronizedBlockAndBadVisibilityOnField.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/SynchronizedMethod.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/SynchronizedMethodAndBadVisibilityOnField.java create mode 100644 spotbugsTestCases/src/java/multithreaded/sharedPrimitiveVariables/sharedPrimitiveVariables/VolatileField.java diff --git a/CHANGELOG.md b/CHANGELOG.md index e2a0e90dffb..95e0e24826f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ This is the changelog for SpotBugs. This follows [Keep a Changelog v1.0.0](http: Currently the versioning policy of this project follows [Semantic Versioning v2.0.0](http://semver.org/spec/v2.0.0.html). ## Unreleased - 2022-??-?? +### Added +* New detector `BadVisibilityOnSharedPrimitiveVariables` for new bug type `SPV_BAD_VISIBILITY_ON_SHARED_PRIMITIVE_VARIABLES` (See [SEI CERT rule VNA00-J](https://wiki.sei.cmu.edu/confluence/display/java/VNA00-J.+Ensure+visibility+when+accessing+shared+primitive+variables)) ## 4.7.0 - 2022-04-14 ### Changed diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/BadVisibilityOnSharedPrimitiveVariablesTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/BadVisibilityOnSharedPrimitiveVariablesTest.java new file mode 100644 index 00000000000..1e98288d5a6 --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/BadVisibilityOnSharedPrimitiveVariablesTest.java @@ -0,0 +1,100 @@ +package edu.umd.cs.findbugs.detect; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.SystemProperties; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; +import org.junit.Test; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; + +public class BadVisibilityOnSharedPrimitiveVariablesTest extends AbstractIntegrationTest { + + @Test + public void failurePath_fieldWithBadVisibility_whenOtherMethodHasSynchronizedBlock() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/SynchronizedBlockAndBadVisibilityOnField.class"); + + assertSPVNumOfBugs(1); + assertSPVBug("SynchronizedBlockAndBadVisibilityOnField", "shutdown", 18); + } + + @Test + public void failurePath_fieldWithBadVisibility_whenOtherMethodIsSynchronized() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/SynchronizedMethodAndBadVisibilityOnField.class"); + + assertSPVNumOfBugs(1); + assertSPVBug("SynchronizedMethodAndBadVisibilityOnField", "shutdown", 18); + } + + @Test + public void failurePath_fieldWithBadVisibility_whenClassExtendsThread() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/FieldWithBadVisibilityThread.class"); + + assertSPVNumOfBugs(1); + assertSPVBug("FieldWithBadVisibilityThread", "shutdown", 19); + } + + @Test + public void failurePath_fieldWithBadVisibility_whenClassImplementsRunnable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/FieldWithBadVisibilityRunnable.class"); + + assertSPVNumOfBugs(1); + assertSPVBug("FieldWithBadVisibilityRunnable", "shutdown", 19); + } + + @Test + public void happyPath_atomicField() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/AtomicField.class"); + + assertSPVNumOfBugs(0); + } + + @Test + public void happyPath_volatileField() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/VolatileField.class"); + + assertSPVNumOfBugs(0); + } + + @Test + public void happyPath_synchronizedBlock() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/SynchronizedBlock.class"); + + assertSPVNumOfBugs(0); + } + + @Test + public void happyPath_synchronizedMethod() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/sharedPrimitiveVariables/SynchronizedMethod.class"); + + assertSPVNumOfBugs(0); + } + + private void assertSPVNumOfBugs(int num) { + final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder() + .bugType("SPV_BAD_VISIBILITY_ON_SHARED_PRIMITIVE_VARIABLES") + .build(); + assertThat(getBugCollection(), containsExactly(num, bugTypeMatcher)); + } + + private void assertSPVBug(String className, String methodName, int line) { + final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder() + .bugType("SPV_BAD_VISIBILITY_ON_SHARED_PRIMITIVE_VARIABLES") + .inClass(className) + .inMethod(methodName) + .atLine(line) + .build(); + assertThat(getBugCollection(), hasItem(bugInstanceMatcher)); + } + +} diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindCompoundOperationsOnSharedVariablesTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindCompoundOperationsOnSharedVariablesTest.java new file mode 100644 index 00000000000..a3b7d8d5b74 --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindCompoundOperationsOnSharedVariablesTest.java @@ -0,0 +1,207 @@ +package edu.umd.cs.findbugs.detect; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.SystemProperties; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; +import org.junit.Test; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.hamcrest.Matchers.hasItem; + +import static org.hamcrest.MatcherAssert.assertThat; + +public class FindCompoundOperationsOnSharedVariablesTest extends AbstractIntegrationTest { + + @Test + public void happyPath_compoundOperation_onSharedAtomicVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundOperationOnSharedAtomicVariable.class"); + + assertCOSVNumOfBugs(0); + } + + @Test + public void happyPath_compoundOperationInsideSynchronizedBlock_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/SynchronizedBlockCompoundOperationOnSharedVariable.class"); + + assertCOSVNumOfBugs(0); + } + + @Test + public void happyPath_compoundOperationInsideSynchronizedMethod_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/SynchronizedMethodCompoundOperationOnSharedVariable.class"); + + assertCOSVNumOfBugs(0); + } + + // --num + @Test + public void reportsBugFor_compoundPreDecrementation_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundPreDecrementationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundPreDecrementationOnSharedVariable", "getNum", 11); + } + + // num-- + @Test + public void reportsBugFor_compoundPostDecrementation_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundPostDecrementationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundPostDecrementationOnSharedVariable", "getNum", 11); + } + + // ++num + @Test + public void reportsBugFor_compoundPreIncrementation_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundPreIncrementationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundPreIncrementationOnSharedVariable", "getNum", 11); + } + + // num++ + @Test + public void reportsBugFor_compoundPostIncrementation_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundPostIncrementationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundPostIncrementationOnSharedVariable", "getNum", 11); + } + + // &= + @Test + public void reportsBugFor_compoundIAND_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundIANDOperationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundIANDOperationOnSharedVariable", "getNum", 11); + } + + // |= + @Test + public void reportsBugFor_compoundIOR_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundIOROperationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundIOROperationOnSharedVariable", "getNum", 11); + } + + // >>>= + @Test + public void reportsBugFor_compoundLogicalRightShifting_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundLogicalRightShiftingOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundLogicalRightShiftingOnSharedVariable", "getNum", 11); + } + + // >>= + @Test + public void reportsBugFor_compoundRightShifting_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundRightShiftingOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundRightShiftingOnSharedVariable", "getNum", 11); + } + + // <<= + @Test + public void reportsBugFor_compoundLeftShifting_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundLeftShiftingOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundLeftShiftingOnSharedVariable", "getNum", 11); + } + + // %= + @Test + public void reportsBugFor_compoundModulo_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundModuloOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundModuloOnSharedVariable", "getNum", 12); + } + + // *= + @Test + public void reportsBugFor_compoundMultiplication_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundMultiplicationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundMultiplicationOnSharedVariable", "getNum", 11); + } + + // /= + @Test + public void reportsBugFor_compoundDivision_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundDivisionOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundDivisionOnSharedVariable", "getNum", 11); + } + + // -= + @Test + public void reportsBugFor_compoundSubtraction_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundSubtractionOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundSubtractionOnSharedVariable", "getNum", 11); + } + + + // += + @Test + public void reportsBugFor_compoundAddition_onSharedVolatileVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundAdditionOnSharedVolatileVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundAdditionOnSharedVolatileVariable", "getNum", 11); + } + + // ^= + @Test + public void reportsBugFor_compoundXOROperation_onSharedVariable() { + SystemProperties.setProperty("ful.debug", "true"); + performAnalysis("multithreaded/compoundOperationOnSharedVariables/CompoundXOROperationOnSharedVariable.class"); + + assertCOSVNumOfBugs(1); + assertCOSVBug("CompoundXOROperationOnSharedVariable", "getFlag", 11); + } + + private void assertCOSVNumOfBugs(int num) { + final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder() + .bugType("COSV_COMPOUND_OPERATIONS_ON_SHARED_VARIABLES") + .build(); + assertThat(getBugCollection(), containsExactly(num, bugTypeMatcher)); + } + + private void assertCOSVBug(String className, String methodName, int line) { + final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder() + .bugType("COSV_COMPOUND_OPERATIONS_ON_SHARED_VARIABLES") + .inClass(className) + .inMethod(methodName) + .atLine(line) + .build(); + assertThat(getBugCollection(), hasItem(bugInstanceMatcher)); + } +} diff --git a/spotbugs/etc/findbugs.xml b/spotbugs/etc/findbugs.xml index 8d60497ae13..118d3133918 100644 --- a/spotbugs/etc/findbugs.xml +++ b/spotbugs/etc/findbugs.xml @@ -669,6 +669,10 @@ reports="PERM_SUPER_NOT_CALLED_IN_GETPERMISSIONS"/> + + + +
+ + Detector for patterns where a shared primitive variable in one thread + may not yield the value of the most recent write to the variable from another thread. + Consequently, the thread may observe a stale value of the shared variable. +

+ ]]> +
+
+ +
+ + Description of the detector.... +

+ ]]> +
+