Skip to content

Commit

Permalink
Improper warning - Potential null pointer access
Browse files Browse the repository at this point in the history 
Fixes eclipse-jdt#1461
Sets the flow info reach mode to FlowInfo.UNREACHABLE_BY_NULLANALYSIS
after 'Object o = null;if (Objects.isNull(o)) return;', 'Object o = "";if (Objects.nonNull(o)) return;

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
  • Loading branch information
@snjeza
snjeza committed Dec 7, 2023
1 parent e620362 commit 0aa77d6
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 1 deletion.
31 changes: 30 additions & 1 deletion ...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.ast;

import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.*;
import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.ASSIGNMENT_CONTEXT;
import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.INVOCATION_CONTEXT;
import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.VANILLA_CONTEXT;

import java.util.HashMap;
import java.util.function.BiConsumer;
Expand All @@ -75,6 +77,7 @@
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
import org.eclipse.jdt.internal.compiler.codegen.Opcodes;
import org.eclipse.jdt.internal.compiler.flow.ConditionalFlowInfo;
import org.eclipse.jdt.internal.compiler.flow.FlowContext;
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
import org.eclipse.jdt.internal.compiler.flow.UnconditionalFlowInfo;
Expand Down Expand Up @@ -223,14 +226,17 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
case ARG_NONNULL_IF_TRUE:
recordFlowUpdateOnResult(((SingleNameReference) argument).localVariableBinding(), true, false);
flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
flowInfo = setUnreachable(flowInfo, flowContext, argument, false);
break;
case ARG_NONNULL_IF_TRUE_NEGATABLE:
recordFlowUpdateOnResult(((SingleNameReference) argument).localVariableBinding(), true, true);
flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
flowInfo = setUnreachable(flowInfo, flowContext, argument, false);
break;
case ARG_NULL_IF_TRUE:
recordFlowUpdateOnResult(((SingleNameReference) argument).localVariableBinding(), false, true);
flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
flowInfo = setUnreachable(flowInfo, flowContext, argument, true);
break;
default:
flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
Expand Down Expand Up @@ -264,6 +270,29 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
flowContext.expireNullCheckedFieldInfo(); // no longer trust this info after any message send
return flowInfo;
}

private FlowInfo setUnreachable(FlowInfo flowInfo, FlowContext flowContext, Expression argument,
boolean nonNull) {
if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0) {
int status = argument.nullStatus(flowInfo, flowContext);
if (status == FlowInfo.NULL) {
flowInfo = FlowInfo.conditional(flowInfo.copy(), flowInfo.copy());
if (nonNull) {
((ConditionalFlowInfo) flowInfo).initsWhenFalse().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
} else {
((ConditionalFlowInfo) flowInfo).initsWhenTrue().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
}
} else if (status == FlowInfo.NON_NULL) {
flowInfo = FlowInfo.conditional(flowInfo.copy(), flowInfo.copy());
if (nonNull) {
((ConditionalFlowInfo) flowInfo).initsWhenTrue.setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
} else {
((ConditionalFlowInfo) flowInfo).initsWhenFalse.setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
}
}
}
return flowInfo;
}
public void recordFlowUpdateOnResult(LocalVariableBinding local, boolean nonNullIfTrue, boolean negatable) {
this.flowUpdateOnBooleanResult = (f, result) -> {
if (result || negatable) {
Expand Down
139 changes: 139 additions & 0 deletions ....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18568,4 +18568,143 @@ void m(String packageName, IPS[] packages) {
"""
});
}
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1461
public void testGH1461() {
if (this.complianceLevel < ClassFileConstants.JDK15) return;
runNegativeTest(
new String[] {
"X.java",
"""
import java.util.Objects;
public class X {
public void test() {
String name = null;
if (Objects.isNull(name)) {
System.out.println("Name is null");
return;
}
System.out.println(name.substring(0, 4));
}
}
"""
},
"""
----------
1. WARNING in X.java (at line 9)
System.out.println(name.substring(0, 4));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Dead code
----------
""");
}
//https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1461
public void testGH1461SuppressWarnings() {
if (this.complianceLevel < ClassFileConstants.JDK15) return;
runConformTest(
new String[] {
"X.java",
"""
import java.util.Objects;
@SuppressWarnings("unused")
public class X {
public void test() {
String name = null;
if (Objects.isNull(name)) {
System.out.println("Name is null");
return;
}
System.out.println(name.substring(0, 4));
}
}
"""
},
"");
}
//https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1461
public void testGH1461_a() {
if (this.complianceLevel < ClassFileConstants.JDK15) return;
runNegativeTest(
new String[] {
"X.java",
"""
import java.util.Objects;
public class X {
public void test() {
String name = "name";
if (Objects.nonNull(name)) {
System.out.println("Name is null");
return;
}
System.out.println(name.substring(0, 4));
}
}
"""
},
"""
----------
1. WARNING in X.java (at line 9)
System.out.println(name.substring(0, 4));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Dead code
----------
""");
}
//https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1461
public void testGH1461_b() {
if (this.complianceLevel < ClassFileConstants.JDK15) return;
runNegativeTest(
new String[] {
"X.java",
"""
import java.util.Objects;
public class X {
public void test() {
String name = null;
if (!Objects.nonNull(name)) {
System.out.println("Name is null");
return;
}
System.out.println(name.substring(0, 4));
}
}
"""
},
"""
----------
1. WARNING in X.java (at line 9)
System.out.println(name.substring(0, 4));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Dead code
----------
""");
}
//https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1461
public void testGH1461_c() {
if (this.complianceLevel < ClassFileConstants.JDK15) return;
runNegativeTest(
new String[] {
"X.java",
"""
public class X {
public void test() {
Foo foo = new Foo();
if (Bar.class.isInstance(foo)) {
return;
}
System.out.println("Hello, world!");
}
}
class Foo extends Bar{}
class Bar{}
"""
},
"""
----------
1. WARNING in X.java (at line 7)
System.out.println("Hello, world!");
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Dead code
----------
""");
}
}

0 comments on commit 0aa77d6

Please sign in to comment.