From 0aa77d61d17bb6394ec595184277a53f0a04c69e Mon Sep 17 00:00:00 2001 From: Snjezana Peco Date: Thu, 5 Oct 2023 22:45:34 +0200 Subject: [PATCH] Improper warning - Potential null pointer access Fixes #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 --- .../internal/compiler/ast/MessageSend.java | 31 +++- .../regression/NullReferenceTest.java | 139 ++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java index 49002c44880..e5826e7de6e 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java @@ -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; @@ -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; @@ -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(); @@ -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) { diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java index c8f2847fe67..9f8db9aba74 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java @@ -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 + ---------- + """); +} } \ No newline at end of file