diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/ResourceInMultipleThreadsDetector.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/ResourceInMultipleThreadsDetector.java index 40986c66162..b9556fb18ad 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/ResourceInMultipleThreadsDetector.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/ResourceInMultipleThreadsDetector.java @@ -18,6 +18,7 @@ package edu.umd.cs.findbugs.detect; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -27,8 +28,16 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import edu.umd.cs.findbugs.ba.AnalysisContext; +import edu.umd.cs.findbugs.ba.CFG; +import edu.umd.cs.findbugs.ba.CFGBuilderException; +import edu.umd.cs.findbugs.ba.ClassContext; +import edu.umd.cs.findbugs.ba.Location; import edu.umd.cs.findbugs.ba.XFactory; +import edu.umd.cs.findbugs.ba.Hierarchy; +import edu.umd.cs.findbugs.ba.JavaClassAndMethod; import org.apache.bcel.Const; +import org.apache.bcel.Repository; import org.apache.bcel.classfile.Attribute; import org.apache.bcel.classfile.BootstrapMethods; import org.apache.bcel.classfile.ConstantInvokeDynamic; @@ -45,6 +54,11 @@ import edu.umd.cs.findbugs.util.CollectionAnalysis; import edu.umd.cs.findbugs.util.MethodAnalysis; import edu.umd.cs.findbugs.util.MutableClasses; +import org.apache.bcel.generic.ConstantPoolGen; +import org.apache.bcel.generic.INVOKEDYNAMIC; +import org.apache.bcel.generic.Instruction; +import org.apache.bcel.generic.InstructionHandle; +import org.apache.bcel.generic.InvokeInstruction; public class ResourceInMultipleThreadsDetector extends OpcodeStackDetector { @@ -76,9 +90,46 @@ public void visit(JavaClass obj) { for (Method m : obj.getMethods()) { doVisitMethod(m); } + + for (XMethod xm : methodsUsedInThreads) { + // it's a nest member, an anonymous class implementing Runnable + if (!xm.getClassName().equals(obj.getClassName())) { + collectMethods(xm); + } + } firstPass = false; } + private void collectMethods(XMethod xMethod) { + try { + JavaClass javaClass = Repository.lookupClass(xMethod.getClassName()); + ClassContext classContext = AnalysisContext.currentAnalysisContext().getClassContext(javaClass); + Method method = Arrays.stream(javaClass.getMethods()) + .filter(m -> m.getName().equals(xMethod.getName()) && m.getSignature().equals(xMethod.getSignature())) + .findFirst().orElse(null); + if (method != null) { + CFG cfg = classContext.getCFG(method); + ConstantPoolGen cpg = classContext.getConstantPoolGen(); + + for (Location location : cfg.orderedLocations()) { + InstructionHandle handle = location.getHandle(); + Instruction instruction = handle.getInstruction(); + + if (instruction instanceof InvokeInstruction && !(instruction instanceof INVOKEDYNAMIC)) { + XMethod calledMethod = XFactory.createXMethod((InvokeInstruction) instruction, cpg); + calledMethodsByMethods.computeIfAbsent(getXMethod(), v -> new HashSet<>()).add(calledMethod); + addToMethodsUsedInThreads(calledMethod); + } + } + } + + } catch (ClassNotFoundException e) { + bugReporter.reportMissingClass(e); + } catch (CFGBuilderException e) { + bugReporter.logError("Detector ResourceInMultipleThreadsDetector caught exception while analyzing an anonymous class", e); + } + } + @Override public void visit(Method method) { synchronizedBlock = method.isSynchronized(); @@ -121,6 +172,24 @@ private void collectMethodsUsedInThreads(int seen) { } else if ((seen == Const.INVOKEVIRTUAL || seen == Const.INVOKEINTERFACE || seen == Const.INVOKESPECIAL || seen == Const.INVOKESTATIC)) { XMethod calledMethod = getXMethodOperand(); if (calledMethod != null) { + if ("java.lang.Thread".equals(calledMethod.getClassName()) && Const.CONSTRUCTOR_NAME.equals(calledMethod.getName())) { + int stackIdx = getStackIndexOfRunnable(calledMethod.getSignature()); + if (stackIdx >= 0) { + OpcodeStack.Item stackItem = getStack().getStackItem(stackIdx); + try { + JavaClass runnableClass = stackItem.getJavaClass(); + if (runnableClass != null && runnableClass.isAnonymous()) { + // java.lang.Runnable.run() method is the relevant one + JavaClassAndMethod runMethod = Hierarchy.findMethod(runnableClass, "run", "()V"); + if (runMethod != null) { + methodsUsedInThreads.add(runMethod.toXMethod()); + } + } + } catch (ClassNotFoundException e) { + bugReporter.logError(String.format("Could not find class during analyzing %s with ResourceInMultipleThreadsDetector", getClassName()), e); + } + } + } calledMethodsByMethods.computeIfAbsent(getXMethod(), v -> new HashSet<>()).add(calledMethod); if (methodsUsedInThreads.contains(getXMethod()) @@ -138,6 +207,26 @@ private void addToMethodsUsedInThreads(XMethod methodToAdd) { } } + /** + * Get the stack index of java.lang.Runnable type if the provided signature is a valid signature of java.lang.Thread's constructors. + * @param signature the provided method signature + * @return the stack index of the java.lang.Runnable type + */ + private static int getStackIndexOfRunnable(String signature) { + switch (signature) { + case "(Ljava/lang/Runnable;)V": + case "(Ljava/lang/ThreadGroup;Ljava/lang/Runnable;)V": + return 0; + case "(Ljava/lang/Runnable;Ljava/lang/String;)V": + case "(Ljava/lang/ThreadGroup;Ljava/lang/Runnable;Ljava/lang/String;)V": + return 1; + case "(Ljava/lang/ThreadGroup;Ljava/lang/Runnable;Ljava/lang/String;J)V": + return 2; + default: + return -1; + } + } + /** * Ignore a special case where a Thread is passed to the {@code java.lang.Runtime} class, * so it is used as a shutdown hook. @@ -176,6 +265,12 @@ private void collectFieldsUsedInThreads(int seen) throws CheckedAnalysisExceptio createOrUpdateFieldData(field, true, getMethod(), getXMethodOperand()); } } + } else if ((seen == Const.GETFIELD || seen == Const.GETSTATIC) +// && !MethodAnalysis.isDuplicatedLocation(getMethodDescriptor(), getPC()) +// && methodsUsedInThreads.contains(getMethodDescriptor()) + ) { + OpcodeStack.Item stackItem = getStack().getStackItem(0); + } else if ((seen == Const.INVOKEVIRTUAL || seen == Const.INVOKEINTERFACE || seen == Const.INVOKESPECIAL || seen == Const.INVOKESTATIC) && getXMethodOperand() != null && getStack().getStackDepth() > 0 && !MethodAnalysis.isDuplicatedLocation(getMethodDescriptor(), getPC()) diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/util/NestedAccessUtil.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/util/NestedAccessUtil.java index 980679b7fa4..f4e92103f30 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/util/NestedAccessUtil.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/util/NestedAccessUtil.java @@ -156,6 +156,20 @@ private static String[] getNestMemberClassNames(JavaClass javaClass) { return null; } + public static List getNestMemberClasses(JavaClass javaClass) throws ClassNotFoundException { + List list = new ArrayList<>(); + if (javaClass != null) { + String[] nestMemberClassNames = getNestMemberClassNames(javaClass); + if (nestMemberClassNames != null) { + for (String nestMember : nestMemberClassNames) { + JavaClass nestMemberClass = Repository.lookupClass(nestMember); + list.add(nestMemberClass); + } + } + } + return list; + } + @CheckForNull private static String getHostDottedClassName(JavaClass javaClass) { String hostClassName = getHostClassName(javaClass);