Refactor fieldSign

Author: tamasvajk

csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll

@@ -277,6 +277,20 @@ private Sign implicitSsaDefSign(SsaVariable v) {
   anySign(result) and nonFieldImplicitSsaDefinition(v)
 }
 
+/** Gets a possible sign for `f`. */
+Sign fieldSign(Field f) {
+  if not fieldWithUnknownSign(f)
+  then
+    result = exprSign(getAssignedValueToField(f))
+    or
+    fieldIncrementOperationOperand(f) and result = fieldSign(f).inc()
+    or
+    fieldDecrementOperationOperand(f) and result = fieldSign(f).dec()
+    or
+    result = specificFieldSign(f)
+  else anySign(result)
+}
+
 /** Gets a possible sign for `e`. */
 cached
 Sign exprSign(Expr e) {

csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll

@@ -36,6 +36,8 @@ module Private {
 
   class ExprWithPossibleValue = CS::Expr;
 
+  class Field = CS::Field;
+
   predicate ssaRead = SU::ssaRead/2;
 }
 
@@ -133,22 +135,29 @@ private module Impl {
     not getImplicitSsaDeclaration(v) instanceof Field
   }
 
-  /** Gets a possible sign for `f`. */
-  Sign fieldSign(Field f) {
-    if f.fromSource() and f.isEffectivelyPrivate()
-    then
-      result = exprSign(f.getAnAssignedValue())
-      or
-      any(IncrementOperation inc).getOperand() = f.getAnAccess() and result = fieldSign(f).inc()
-      or
-      any(DecrementOperation dec).getOperand() = f.getAnAccess() and result = fieldSign(f).dec()
-      or
-      exists(AssignOperation a | a.getLValue() = f.getAnAccess() | result = exprSign(a))
-      or
-      not exists(f.getInitializer()) and result = TZero()
-    else anySign(result)
+  /** Returned an expression that is assigned to `f`. */
+  Expr getAssignedValueToField(Field f) {
+    result = f.getAnAssignedValue() or
+    result = any(AssignOperation a | a.getLValue() = f.getAnAccess())
   }
 
+  /** Holds if `f` can have any sign. */
+  predicate fieldWithUnknownSign(Field f) { not f.fromSource() or not f.isEffectivelyPrivate() }
+
+  /** Holds if `f` is accessed in an increment operation. */
+  predicate fieldIncrementOperationOperand(Field f) {
+    any(IncrementOperation inc).getOperand() = f.getAnAccess()
+  }
+
+  /** Holds if `f` is accessed in a decrement operation. */
+  predicate fieldDecrementOperationOperand(Field f) {
+    any(DecrementOperation dec).getOperand() = f.getAnAccess()
+  }
+
+  /** Returns possible signs of `f` based on the declaration. */
+  pragma[inline]
+  Sign specificFieldSign(Field f) { not exists(f.getInitializer()) and result = TZero() }
+
   /**
    * Holds if `e` has type `NumericOrCharType`, but the sign of `e` is unknown.
    */

java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll

@@ -277,6 +277,20 @@ private Sign implicitSsaDefSign(SsaVariable v) {
   anySign(result) and nonFieldImplicitSsaDefinition(v)
 }
 
+/** Gets a possible sign for `f`. */
+Sign fieldSign(Field f) {
+  if not fieldWithUnknownSign(f)
+  then
+    result = exprSign(getAssignedValueToField(f))
+    or
+    fieldIncrementOperationOperand(f) and result = fieldSign(f).inc()
+    or
+    fieldDecrementOperationOperand(f) and result = fieldSign(f).dec()
+    or
+    result = specificFieldSign(f)
+  else anySign(result)
+}
+
 /** Gets a possible sign for `e`. */
 cached
 Sign exprSign(Expr e) {

java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll

@@ -40,6 +40,8 @@ module Private {
 
   class ExprWithPossibleValue = J::Literal;
 
+  class Field = J::Field;
+
   predicate ssaRead = RU::ssaRead/2;
 
   predicate guardControlsSsaRead = RU::guardControlsSsaRead/3;
@@ -128,22 +130,31 @@ private module Impl {
     exists(Parameter p | v.isParameterDefinition(p))
   }
 
-  /** Gets a possible sign for `f`. */
-  Sign fieldSign(Field f) {
-    result = exprSign(f.getAnAssignedValue())
-    or
-    exists(PostIncExpr inc | inc.getExpr() = f.getAnAccess() and result = fieldSign(f).inc())
-    or
-    exists(PreIncExpr inc | inc.getExpr() = f.getAnAccess() and result = fieldSign(f).inc())
-    or
-    exists(PostDecExpr inc | inc.getExpr() = f.getAnAccess() and result = fieldSign(f).dec())
-    or
-    exists(PreDecExpr inc | inc.getExpr() = f.getAnAccess() and result = fieldSign(f).dec())
-    or
-    exists(AssignOp a | a.getDest() = f.getAnAccess() | result = exprSign(a))
-    or
-    anySign(result) and exists(ReflectiveFieldAccess rfa | rfa.inferAccessedField() = f)
-    or
+  /** Returned an expression that is assigned to `f`. */
+  Expr getAssignedValueToField(Field f) {
+    result = f.getAnAssignedValue() or
+    result = any(AssignOp a | a.getDest() = f.getAnAccess())
+  }
+
+  /** Holds if `f` can have any sign. */
+  predicate fieldWithUnknownSign(Field f) {
+    exists(ReflectiveFieldAccess rfa | rfa.inferAccessedField() = f)
+  }
+
+  /** Holds if `f` is accessed in an increment operation. */
+  predicate fieldIncrementOperationOperand(Field f) {
+    any(PostIncExpr inc).getExpr() = f.getAnAccess() or
+    any(PreIncExpr inc).getExpr() = f.getAnAccess()
+  }
+
+  /** Holds if `f` is accessed in a decrement operation. */
+  predicate fieldDecrementOperationOperand(Field f) {
+    any(PostDecExpr dec).getExpr() = f.getAnAccess() or
+    any(PreDecExpr dec).getExpr() = f.getAnAccess()
+  }
+
+  /** Returns possible signs of `f` based on the declaration. */
+  Sign specificFieldSign(Field f) {
     if f.fromSource()
     then not exists(f.getInitializer()) and result = TZero()
     else