Merge pull request #14757 from aschackmull/rangeanalysis/simplify

aschackmull · web-flow · commit dc4dda1dbcc7 · 2023-11-13T13:16:04.000+01:00
Rangeanalysis: Misc simplifications
diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll
@@ -22,8 +22,6 @@ class SemSsaExplicitUpdate extends SemSsaVariable {
 
   SemSsaExplicitUpdate() { Specific::explicitUpdate(this, sourceExpr) }
 
-  final SemExpr getSourceExpr() { result = sourceExpr }
-
   final SemExpr getDefiningExpr() { result = sourceExpr }
 }
 
diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/ConstantAnalysis.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/ConstantAnalysis.qll
@@ -14,7 +14,7 @@ private predicate constantIntegerExpr(SemExpr e, int val) {
   // Copy of another constant
   exists(SemSsaExplicitUpdate v, SemExpr src |
     e = v.getAUse() and
-    src = v.getSourceExpr() and
+    src = v.getDefiningExpr() and
     constantIntegerExpr(src, val)
   )
   or
diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisConstantSpecific.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisConstantSpecific.qll
@@ -22,25 +22,7 @@ module CppLangImplConstant implements LangSig<Sem, FloatDelta> {
   predicate hasConstantBound(SemExpr e, float bound, boolean upper) { none() }
 
   /**
-   * Holds if `e >= bound + delta` (if `upper = false`) or `e <= bound + delta` (if `upper = true`).
+   * Holds if `e2 >= e1 + delta` (if `upper = false`) or `e2 <= e1 + delta` (if `upper = true`).
    */
-  predicate hasBound(SemExpr e, SemExpr bound, float delta, boolean upper) { none() }
-
-  /**
-   * Gets the type that range analysis should use to track the result of the specified expression,
-   * if a type other than the original type of the expression is to be used.
-   *
-   * This predicate is commonly used in languages that support immutable "boxed" types that are
-   * actually references but whose values can be tracked as the type contained in the box.
-   */
-  SemType getAlternateType(SemExpr e) { none() }
-
-  /**
-   * Gets the type that range analysis should use to track the result of the specified source
-   * variable, if a type other than the original type of the expression is to be used.
-   *
-   * This predicate is commonly used in languages that support immutable "boxed" types that are
-   * actually references but whose values can be tracked as the type contained in the box.
-   */
-  SemType getAlternateTypeForSsaVariable(SemSsaVariable var) { none() }
+  predicate additionalBoundFlowStep(SemExpr e2, SemExpr e1, float delta, boolean upper) { none() }
 }
diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisImpl.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisImpl.qll
@@ -1,7 +1,6 @@
 private import RangeAnalysisConstantSpecific
 private import RangeAnalysisRelativeSpecific
 private import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.FloatDelta
-private import RangeUtils
 private import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExpr
 private import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticCFG
 private import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticGuard
@@ -88,6 +87,10 @@ module Sem implements Semantic {
 
   class AddressType = SemAddressType;
 
+  SemType getExprType(SemExpr e) { result = e.getSemType() }
+
+  SemType getSsaType(SemSsaVariable var) { result = var.getType() }
+
   class SsaVariable = SemSsaVariable;
 
   class SsaPhiNode = SemSsaPhiNode;
@@ -103,7 +106,7 @@ module Sem implements Semantic {
 
 module SignAnalysis implements SignAnalysisSig<Sem> {
   private import SignAnalysisCommon as SA
-  import SA::SignAnalysis<FloatDelta, Util>
+  import SA::SignAnalysis<FloatDelta>
 }
 
 module ConstantBounds implements BoundSig<SemLocation, Sem, FloatDelta> {
@@ -166,18 +169,16 @@ private module ModulusAnalysisInstantiated implements ModulusAnalysisSig<Sem> {
   class ModBound = AllBounds::SemBound;
 
   private import codeql.rangeanalysis.ModulusAnalysis as MA
-  import MA::ModulusAnalysis<SemLocation, Sem, FloatDelta, AllBounds, Util>
+  import MA::ModulusAnalysis<SemLocation, Sem, FloatDelta, AllBounds>
 }
 
-module Util = RangeUtil<FloatDelta, CppLangImplConstant>;
-
 module ConstantStage =
   RangeStage<SemLocation, Sem, FloatDelta, ConstantBounds, FloatOverflow, CppLangImplConstant,
-    SignAnalysis, ModulusAnalysisInstantiated, Util>;
+    SignAnalysis, ModulusAnalysisInstantiated>;
 
 module RelativeStage =
   RangeStage<SemLocation, Sem, FloatDelta, RelativeBounds, FloatOverflow, CppLangImplRelative,
-    SignAnalysis, ModulusAnalysisInstantiated, Util>;
+    SignAnalysis, ModulusAnalysisInstantiated>;
 
 private newtype TSemReason =
   TSemNoReason() or
diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisRelativeSpecific.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisRelativeSpecific.qll
@@ -54,25 +54,7 @@ module CppLangImplRelative implements LangSig<Sem, FloatDelta> {
   predicate hasConstantBound(SemExpr e, float bound, boolean upper) { none() }
 
   /**
-   * Holds if `e >= bound + delta` (if `upper = false`) or `e <= bound + delta` (if `upper = true`).
+   * Holds if `e2 >= e1 + delta` (if `upper = false`) or `e2 <= e1 + delta` (if `upper = true`).
    */
-  predicate hasBound(SemExpr e, SemExpr bound, float delta, boolean upper) { none() }
-
-  /**
-   * Gets the type that range analysis should use to track the result of the specified expression,
-   * if a type other than the original type of the expression is to be used.
-   *
-   * This predicate is commonly used in languages that support immutable "boxed" types that are
-   * actually references but whose values can be tracked as the type contained in the box.
-   */
-  SemType getAlternateType(SemExpr e) { none() }
-
-  /**
-   * Gets the type that range analysis should use to track the result of the specified source
-   * variable, if a type other than the original type of the expression is to be used.
-   *
-   * This predicate is commonly used in languages that support immutable "boxed" types that are
-   * actually references but whose values can be tracked as the type contained in the box.
-   */
-  SemType getAlternateTypeForSsaVariable(SemSsaVariable var) { none() }
+  predicate additionalBoundFlowStep(SemExpr e2, SemExpr e1, float delta, boolean upper) { none() }
 }
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
diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/SignAnalysisCommon.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/SignAnalysisCommon.qll
@@ -11,10 +11,9 @@ private import RangeAnalysisImpl
 private import SignAnalysisSpecific as Specific
 private import semmle.code.cpp.rangeanalysis.new.internal.semantic.Semantic
 private import ConstantAnalysis
-private import RangeUtils
 private import Sign
 
-module SignAnalysis<DeltaSig D, UtilSig<Sem, D> Utils> {
+module SignAnalysis<DeltaSig D> {
   private import codeql.rangeanalysis.internal.RangeUtils::MakeUtils<Sem, D>
 
   /**
@@ -39,7 +38,7 @@ module SignAnalysis<DeltaSig D, UtilSig<Sem, D> Utils> {
 
   /** An SSA definition whose sign is determined by the sign of that definitions source expression. */
   private class ExplicitSignDef extends FlowSignDef instanceof SemSsaExplicitUpdate {
-    final override Sign getSign() { result = semExprSign(super.getSourceExpr()) }
+    final override Sign getSign() { result = semExprSign(super.getDefiningExpr()) }
   }
 
   /** An SSA Phi definition, whose sign is the union of the signs of its inputs. */
@@ -148,7 +147,7 @@ module SignAnalysis<DeltaSig D, UtilSig<Sem, D> Utils> {
       not this instanceof ConstantSignExpr and
       (
         // Only track numeric types.
-        Utils::getTrackedType(this) instanceof SemNumericType
+        Sem::getExprType(this) instanceof SemNumericType
         or
         // Unless the language says to track this expression anyway.
         Specific::trackUnknownNonNumericExpr(this)
@@ -203,7 +202,7 @@ module SignAnalysis<DeltaSig D, UtilSig<Sem, D> Utils> {
 
   /** An expression of an unsigned type. */
   private class UnsignedExpr extends FlowSignExpr {
-    UnsignedExpr() { Utils::getTrackedType(this) instanceof SemUnsignedIntegerType }
+    UnsignedExpr() { Sem::getExprType(this) instanceof SemUnsignedIntegerType }
 
     override Sign getSignRestriction() {
       result = TPos() or
@@ -276,7 +275,7 @@ module SignAnalysis<DeltaSig D, UtilSig<Sem, D> Utils> {
     override SemUnboxExpr cast;
 
     UnboxSignExpr() {
-      exists(SemType fromType | fromType = Utils::getTrackedType(cast.getOperand()) |
+      exists(SemType fromType | fromType = Sem::getExprType(cast.getOperand()) |
         // Only numeric source types are handled here.
         fromType instanceof SemNumericType
       )
@@ -471,7 +470,7 @@ module SignAnalysis<DeltaSig D, UtilSig<Sem, D> Utils> {
   Sign semExprSign(SemExpr e) {
     exists(Sign s | s = e.(SignExpr).getSign() |
       if
-        Utils::getTrackedType(e) instanceof SemUnsignedIntegerType and
+        Sem::getExprType(e) instanceof SemUnsignedIntegerType and
         s = TNeg() and
         not Specific::ignoreTypeRestrictions(e)
       then result = TPos()
diff --git a/cpp/ql/test/library-tests/ir/modulus-analysis/ModulusAnalysis.ql b/cpp/ql/test/library-tests/ir/modulus-analysis/ModulusAnalysis.ql
@@ -2,17 +2,14 @@ import cpp
 import codeql.rangeanalysis.ModulusAnalysis
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.Semantic
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticLocation
-import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeUtils
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.FloatDelta
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysisRelativeSpecific
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysisImpl
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExprSpecific
 import semmle.code.cpp.ir.IR as IR
 import TestUtilities.InlineExpectationsTest
 
-module ModulusAnalysisInstantiated =
-  ModulusAnalysis<SemLocation, Sem, FloatDelta, ConstantBounds,
-    RangeUtil<FloatDelta, CppLangImplRelative>>;
+module ModulusAnalysisInstantiated = ModulusAnalysis<SemLocation, Sem, FloatDelta, ConstantBounds>;
 
 module ModulusAnalysisTest implements TestSig {
   string getARelevantTag() { result = "mod" }
diff --git a/cpp/ql/test/library-tests/ir/sign-analysis/SignAnalysis.ql b/cpp/ql/test/library-tests/ir/sign-analysis/SignAnalysis.ql
@@ -1,15 +1,13 @@
 import cpp
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.SignAnalysisCommon
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.Semantic
-import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeUtils
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.FloatDelta
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysisRelativeSpecific
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExprSpecific
 import semmle.code.cpp.ir.IR as IR
 import TestUtilities.InlineExpectationsTest
 
-module SignAnalysisInstantiated =
-  SignAnalysis<FloatDelta, RangeUtil<FloatDelta, CppLangImplRelative>>;
+module SignAnalysisInstantiated = SignAnalysis<FloatDelta>;
 
 module SignAnalysisTest implements TestSig {
   string getARelevantTag() { result = "sign" }
diff --git a/java/ql/lib/semmle/code/java/dataflow/NullGuards.qll b/java/ql/lib/semmle/code/java/dataflow/NullGuards.qll
@@ -6,7 +6,6 @@ import java
 import SSA
 private import semmle.code.java.controlflow.internal.GuardsLogic
 private import semmle.code.java.frameworks.apache.Collections
-private import RangeUtils
 private import IntegerGuards
 
 /** Gets an expression that is always `null`. */
diff --git a/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll b/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll
@@ -241,6 +241,10 @@ module Sem implements Semantic {
     AddressType() { none() }
   }
 
+  Type getExprType(Expr e) { result = e.getType() }
+
+  Type getSsaType(SsaVariable var) { result = var.getSourceVariable().getType() }
+
   final private class FinalSsaVariable = SSA::SsaVariable;
 
   class SsaVariable extends FinalSsaVariable {
@@ -281,7 +285,7 @@ module Modulus implements ModulusAnalysisSig<Sem> {
   class ModBound = Bound;
 
   private import codeql.rangeanalysis.ModulusAnalysis as Mod
-  import Mod::ModulusAnalysis<Location, Sem, IntDelta, Bounds, Utils>
+  import Mod::ModulusAnalysis<Location, Sem, IntDelta, Bounds>
 }
 
 module IntDelta implements DeltaSig {
@@ -332,7 +336,7 @@ module JavaLangImpl implements LangSig<Sem, IntDelta> {
   /**
    * Holds if `e2 >= e1 + delta` (if `upper = false`) or `e2 <= e1 + delta` (if `upper = true`).
    */
-  predicate hasBound(Sem::Expr e2, Sem::Expr e1, int delta, boolean upper) {
+  predicate additionalBoundFlowStep(Sem::Expr e2, Sem::Expr e1, int delta, boolean upper) {
     exists(RandomDataSource rds |
       e2 = rds.getOutput() and
       (
@@ -362,21 +366,9 @@ module JavaLangImpl implements LangSig<Sem, IntDelta> {
 
   predicate ignoreExprBound(Sem::Expr e) { none() }
 
-  Sem::Type getAlternateType(Sem::Expr e) { none() }
-
-  Sem::Type getAlternateTypeForSsaVariable(Sem::SsaVariable var) { none() }
-
   predicate javaCompatibility() { any() }
 }
 
-module Utils implements UtilSig<Sem, IntDelta> {
-  Sem::Type getTrackedTypeForSsaVariable(Sem::SsaVariable var) {
-    result = var.getSourceVariable().getType()
-  }
-
-  Sem::Type getTrackedType(Sem::Expr e) { result = e.getType() }
-}
-
 module Bounds implements BoundSig<Location, Sem, IntDelta> {
   class SemBound = Bound;
 
@@ -394,7 +386,7 @@ module Overflow implements OverflowSig<Sem, IntDelta> {
 }
 
 module Range =
-  RangeStage<Location, Sem, IntDelta, Bounds, Overflow, JavaLangImpl, SignInp, Modulus, Utils>;
+  RangeStage<Location, Sem, IntDelta, Bounds, Overflow, JavaLangImpl, SignInp, Modulus>;
 
 predicate bounded = Range::semBounded/5;
 
diff --git a/shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll b/shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll
@@ -14,8 +14,7 @@ private import codeql.util.Location
 private import RangeAnalysis
 
 module ModulusAnalysis<
-  LocationSig Location, Semantic Sem, DeltaSig D, BoundSig<Location, Sem, D> Bounds,
-  UtilSig<Sem, D> U>
+  LocationSig Location, Semantic Sem, DeltaSig D, BoundSig<Location, Sem, D> Bounds>
 {
   private import internal.RangeUtils::MakeUtils<Sem, D>
 
@@ -254,8 +253,7 @@ module ModulusAnalysis<
     or
     exists(Sem::SsaVariable v, SsaReadPositionBlock bb |
       ssaModulus(v, bb, b, val, mod) and
-      e = v.getAUse() and
-      bb.getBlock() = e.getBasicBlock()
+      bb.getAnSsaRead(v) = e
     )
     or
     exists(Sem::Expr mid, int val0, int delta |
diff --git a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll

Original file line number	Diff line number	Diff line change
`@@ -22,8 +22,6 @@ class SemSsaExplicitUpdate extends SemSsaVariable {`
`22`	`22`
`23`	`23`	`SemSsaExplicitUpdate() { Specific::explicitUpdate(this, sourceExpr) }`
`24`	`24`
`25`		`- final SemExpr getSourceExpr() { result = sourceExpr }`
`26`		`-`
`27`	`25`	`final SemExpr getDefiningExpr() { result = sourceExpr }`
`28`	`26`	`}`
`29`	`27`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ private predicate constantIntegerExpr(SemExpr e, int val) {`
`14`	`14`	`// Copy of another constant`
`15`	`15`	`exists(SemSsaExplicitUpdate v, SemExpr src \|`
`16`	`16`	`e = v.getAUse() and`
`17`		`- src = v.getSourceExpr() and`
	`17`	`+ src = v.getDefiningExpr() and`
`18`	`18`	`constantIntegerExpr(src, val)`
`19`	`19`	`)`
`20`	`20`	`or`