From b8e7e1d15e64c4c290e1a280b3ca8ddb22d12717 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 9 Nov 2023 15:56:17 +0100 Subject: [PATCH] Java/C++: Share ssaUpdateStep. --- .../internal/semantic/analysis/RangeUtils.qll | 24 ----------------- .../code/java/dataflow/RangeAnalysis.qll | 4 --- .../semmle/code/java/dataflow/RangeUtils.qll | 19 ++----------- .../codeql/rangeanalysis/ModulusAnalysis.qll | 2 +- .../codeql/rangeanalysis/RangeAnalysis.qll | 4 +-- .../rangeanalysis/internal/RangeUtils.qll | 27 +++++++++++++++++++ 6 files changed, 31 insertions(+), 49 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll index 11624d5397f..229c2270e9e 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll @@ -9,30 +9,6 @@ private import RangeAnalysisImpl private import ConstantAnalysis module RangeUtil Lang> implements UtilSig { - /** - * Holds if `v` is an `SsaExplicitUpdate` that equals `e + delta`. - */ - predicate semSsaUpdateStep(SemSsaExplicitUpdate v, SemExpr e, D::Delta delta) { - exists(SemExpr defExpr | defExpr = v.getSourceExpr() | - defExpr.(SemCopyValueExpr).getOperand() = e and delta = D::fromFloat(0) - or - defExpr.(SemStoreExpr).getOperand() = e and delta = D::fromFloat(0) - or - defExpr.(SemAddOneExpr).getOperand() = e and delta = D::fromFloat(1) - or - defExpr.(SemSubOneExpr).getOperand() = e and delta = D::fromFloat(-1) - or - e = defExpr and - not ( - defExpr instanceof SemCopyValueExpr or - defExpr instanceof SemStoreExpr or - defExpr instanceof SemAddOneExpr or - defExpr instanceof SemSubOneExpr - ) and - delta = D::fromFloat(0) - ) - } - /** * Holds if `e1 + delta` equals `e2`. */ diff --git a/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll b/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll index bd7ab9411d3..b4029245d65 100644 --- a/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll +++ b/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll @@ -372,10 +372,6 @@ module JavaLangImpl implements LangSig { module Utils implements UtilSig { private import RangeUtils as RU - predicate semSsaUpdateStep(Sem::SsaExplicitUpdate v, Sem::Expr e, int delta) { - RU::ssaUpdateStep(v, e, delta) - } - predicate semValueFlowStep = RU::valueFlowStep/3; Sem::Type getTrackedTypeForSsaVariable(Sem::SsaVariable var) { diff --git a/java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll b/java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll index c9a7412011f..6261a601418 100644 --- a/java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll +++ b/java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll @@ -15,6 +15,8 @@ private predicate backEdge = U::backEdge/3; predicate ssaRead = U::ssaRead/2; +predicate ssaUpdateStep = U::ssaUpdateStep/3; + predicate guardDirectlyControlsSsaRead = U::guardDirectlyControlsSsaRead/3; predicate guardControlsSsaRead = U::guardControlsSsaRead/3; @@ -158,23 +160,6 @@ class ConstantStringExpr extends Expr { string getStringValue() { constantStringExpr(this, result) } } -/** - * Holds if `v` is an `SsaExplicitUpdate` that equals `e + delta`. - */ -predicate ssaUpdateStep(SsaExplicitUpdate v, Expr e, int delta) { - v.getDefiningExpr().(VariableAssign).getSource() = e and delta = 0 - or - v.getDefiningExpr().(PostIncExpr).getExpr() = e and delta = 1 - or - v.getDefiningExpr().(PreIncExpr).getExpr() = e and delta = 1 - or - v.getDefiningExpr().(PostDecExpr).getExpr() = e and delta = -1 - or - v.getDefiningExpr().(PreDecExpr).getExpr() = e and delta = -1 - or - v.getDefiningExpr().(AssignOp) = e and delta = 0 -} - /** * Holds if `e1 + delta` equals `e2`. */ diff --git a/shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll b/shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll index 66652b930ac..773937bc73d 100644 --- a/shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll +++ b/shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll @@ -30,7 +30,7 @@ module ModulusAnalysis< */ pragma[nomagic] private predicate valueFlowStepSsa(Sem::SsaVariable v, SsaReadPosition pos, Sem::Expr e, int delta) { - U::semSsaUpdateStep(v, e, D::fromInt(delta)) and pos.hasReadOfVar(v) + ssaUpdateStep(v, e, D::fromInt(delta)) and pos.hasReadOfVar(v) or exists(Sem::Guard guard, boolean testIsTrue | hasReadOfVarInlineLate(pos, v) and diff --git a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll index 302f322dd63..5f32aeec5c0 100644 --- a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll +++ b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll @@ -304,8 +304,6 @@ signature module LangSig { } signature module UtilSig { - predicate semSsaUpdateStep(Sem::SsaExplicitUpdate v, Sem::Expr e, DeltaParam::Delta delta); - predicate semValueFlowStep(Sem::Expr e2, Sem::Expr e1, DeltaParam::Delta delta); /** @@ -671,7 +669,7 @@ module RangeStage< Sem::SsaVariable v, SsaReadPosition pos, Sem::Expr e, D::Delta delta, boolean upper, SemReason reason ) { - semSsaUpdateStep(v, e, delta) and + ssaUpdateStep(v, e, delta) and pos.hasReadOfVar(v) and (upper = true or upper = false) and reason = TSemNoReason() diff --git a/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll b/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll index 80423c5ce51..aa09c94414e 100644 --- a/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll +++ b/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll @@ -57,6 +57,33 @@ module MakeUtils { ) } + /** + * Holds if `v` is an `SsaExplicitUpdate` that equals `e + delta`. + */ + predicate ssaUpdateStep(SsaExplicitUpdate v, Expr e, D::Delta delta) { + exists(Expr defExpr | defExpr = v.getDefiningExpr() | + defExpr.(CopyValueExpr).getOperand() = e and delta = D::fromFloat(0) + or + defExpr.(PostIncExpr).getOperand() = e and delta = D::fromFloat(1) + or + defExpr.(PreIncExpr).getOperand() = e and delta = D::fromFloat(1) + or + defExpr.(PostDecExpr).getOperand() = e and delta = D::fromFloat(-1) + or + defExpr.(PreDecExpr).getOperand() = e and delta = D::fromFloat(-1) + or + e = defExpr and + not ( + defExpr instanceof CopyValueExpr or + defExpr instanceof PostIncExpr or + defExpr instanceof PreIncExpr or + defExpr instanceof PostDecExpr or + defExpr instanceof PreDecExpr + ) and + delta = D::fromFloat(0) + ) + } + private newtype TSsaReadPosition = TSsaReadPositionBlock(BasicBlock bb) { exists(SsaVariable v | v.getAUse().getBasicBlock() = bb)