From e2e2c0e54042cecafcfa1cee46a22bf9032ac18f Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 18 Feb 2022 17:03:10 -0500 Subject: [PATCH] Fix a few bugs to make results of semantic sign analysis match the original AST analysis --- .../java/semantic/SemanticTypeSpecific.qll | 4 +- .../java/semantic/analysis/RangeUtils.qll | 13 +++++ .../semantic/analysis/SignAnalysisCommon.qll | 4 +- .../dataflow/sign-analysis/diff.ql | 50 +++---------------- 4 files changed, 23 insertions(+), 48 deletions(-) diff --git a/java/ql/lib/semmle/code/java/semantic/SemanticTypeSpecific.qll b/java/ql/lib/semmle/code/java/semantic/SemanticTypeSpecific.qll index 35e65966f24..217a22b4340 100644 --- a/java/ql/lib/semmle/code/java/semantic/SemanticTypeSpecific.qll +++ b/java/ql/lib/semmle/code/java/semantic/SemanticTypeSpecific.qll @@ -13,7 +13,7 @@ private int pointerSize() { result = 8 } predicate voidType(Type type) { type instanceof J::VoidType } predicate addressType(Type type, int byteSize) { - type instanceof J::ClassOrInterface and + (type instanceof J::RefType or type instanceof J::NullType) and byteSize = pointerSize() } @@ -37,7 +37,7 @@ predicate floatingPointType(Type type, int byteSize) { predicate integerType(Type type, int byteSize, boolean signed) { exists(J::PrimitiveType primType | primType = type | - primType.hasName("byte") and byteSize = 1 and signed = false + primType.hasName("byte") and byteSize = 1 and signed = true or primType.hasName("char") and byteSize = 2 and signed = false or diff --git a/java/ql/lib/semmle/code/java/semantic/analysis/RangeUtils.qll b/java/ql/lib/semmle/code/java/semantic/analysis/RangeUtils.qll index 069b1751b55..ee6a58d5a35 100644 --- a/java/ql/lib/semmle/code/java/semantic/analysis/RangeUtils.qll +++ b/java/ql/lib/semmle/code/java/semantic/analysis/RangeUtils.qll @@ -6,6 +6,7 @@ private import semmle.code.java.semantic.SemanticCFG private import semmle.code.java.semantic.SemanticExpr private import semmle.code.java.semantic.SemanticGuard private import semmle.code.java.semantic.SemanticSSA +private import semmle.code.java.semantic.SemanticType private import RangeAnalysisSpecific as Specific private import ConstantAnalysis @@ -122,3 +123,15 @@ predicate semValueFlowStep(SemExpr e2, SemExpr e1, int delta) { x.(SemConstantIntegerExpr).getIntValue() = -delta ) } + +/** + * Gets the type used to track the specified expression's range information. + * + * Usually, this just `e.getSemType()`, but the language can override this to track immutable boxed + * primitive types as the underlying primitive type. + */ +SemType getTrackedType(SemExpr e) { + result = Specific::getAlternateType(e) + or + not exists(Specific::getAlternateType(e)) and result = e.getSemType() +} diff --git a/java/ql/lib/semmle/code/java/semantic/analysis/SignAnalysisCommon.qll b/java/ql/lib/semmle/code/java/semantic/analysis/SignAnalysisCommon.qll index 3637adbcfe0..da515fe5f0d 100644 --- a/java/ql/lib/semmle/code/java/semantic/analysis/SignAnalysisCommon.qll +++ b/java/ql/lib/semmle/code/java/semantic/analysis/SignAnalysisCommon.qll @@ -47,7 +47,7 @@ private predicate unknownSign(SemExpr e) { or exists(SemCastExpr cast, SemType fromtyp | cast = e and - fromtyp = cast.getSemType() and + fromtyp = getTrackedType(cast.getExpr()) and not fromtyp instanceof SemNumericType ) or @@ -291,7 +291,7 @@ Sign semExprSign(SemExpr e) { s = specificSubExprSign(e) ) | - if e.getSemType() instanceof SemUnsignedIntegerType and s = TNeg() + if getTrackedType(e) instanceof SemUnsignedIntegerType and s = TNeg() then result = TPos() else result = s ) diff --git a/java/ql/test/library-tests/dataflow/sign-analysis/diff.ql b/java/ql/test/library-tests/dataflow/sign-analysis/diff.ql index e1ada324fae..8b425313d90 100644 --- a/java/ql/test/library-tests/dataflow/sign-analysis/diff.ql +++ b/java/ql/test/library-tests/dataflow/sign-analysis/diff.ql @@ -3,54 +3,16 @@ import semmle.code.java.dataflow.SignAnalysis import semmle.code.java.semantic.analysis.SignAnalysisCommon import semmle.code.java.semantic.SemanticExpr -string getSemSignString(Expr e) { - result = - concat(string sign | - exists(SemExpr semExpr | semExpr = getSemanticExpr(e) | - semPositive(semExpr) and - not semStrictlyPositive(semExpr) and - sign = "positive" - or - semNegative(semExpr) and - not semStrictlyNegative(semExpr) and - sign = "negative" - or - semStrictlyPositive(semExpr) and - sign = "strictlyPositive" - or - semStrictlyNegative(semExpr) and - sign = "strictlyNegative" - ) - | - sign, " " - ) +string getSemanticSign(Expr e) { + result = concat(string s | s = semExprSign(getSemanticExpr(e)).toString() | s, "") } -string getASTSignString(Expr e) { - result = - concat(string sign | - positive(e) and - not strictlyPositive(e) and - sign = "positive" - or - negative(e) and - not strictlyNegative(e) and - sign = "negative" - or - strictlyPositive(e) and - sign = "strictlyPositive" - or - strictlyNegative(e) and - sign = "strictlyNegative" - | - sign, " " - ) -} +string getASTSign(Expr e) { result = concat(string s | s = exprSign(e).toString() | s, "") } from Expr e, string semSign, string astSign where e.getEnclosingCallable().fromSource() and - semSign = getSemSignString(e) and - astSign = getASTSignString(e) and + semSign = getSemanticSign(e) and + astSign = getASTSign(e) and semSign != astSign -select e, astSign, semSign +select e, "AST sign was '" + astSign + "', but semantic sign was '" + semSign + "'."