From 7545fe74e3c2ea69384f199cb6c4fc80ddaa9719 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Tue, 29 Sep 2020 12:10:35 +0200
Subject: [PATCH] Java, C#: Refactor implicitSsaDefSign in sign analysis

---
 .../rangeanalysis/SignAnalysisCommon.qll         |  7 +++++++
 .../rangeanalysis/SignAnalysisSpecific.qll       | 13 ++++++++-----
 .../rangeanalysis/SignAnalysisCommon.qll         |  7 +++++++
 .../rangeanalysis/SignAnalysisSpecific.qll       | 16 +++++++++-------
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll
index 33ef5d3683f..37183f13f8d 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll
@@ -245,6 +245,13 @@ private Sign ssaDefSign(SsaVariable v) {
   )
 }
 
+/** Returns the sign of implicit SSA definition `v`. */
+private Sign implicitSsaDefSign(SsaVariable v) {
+  result = fieldSign(getImplicitSsaDeclaration(v))
+  or
+  anySign(result) and nonFieldImplicitSsaDefinition(v)
+}
+
 /** Gets a possible sign for `e`. */
 cached
 Sign exprSign(Expr e) {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll
index 00f68fa85df..5d0f92d6c2a 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll
@@ -122,11 +122,14 @@ private module Impl {
     )
   }
 
-  /** Returns the sign of implicit SSA definition `v`. */
-  Sign implicitSsaDefSign(Ssa::ImplicitDefinition v) {
-    result = fieldSign(v.getSourceVariable().getAssignable())
-    or
-    anySign(result) and not v.getSourceVariable().getAssignable() instanceof Field
+  /** Gets the variable underlying the implicit SSA variable `v`. */
+  Declaration getImplicitSsaDeclaration(Ssa::ImplicitDefinition v) {
+    result = v.getSourceVariable().getAssignable()
+  }
+
+  /** Holds if the variable underlying the implicit SSA variable `v` is not a field. */
+  predicate nonFieldImplicitSsaDefinition(Ssa::ImplicitDefinition v) {
+    not getImplicitSsaDeclaration(v) instanceof Field
   }
 
   /** Gets a possible sign for `f`. */
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll b/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll
index 33ef5d3683f..37183f13f8d 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisCommon.qll
@@ -245,6 +245,13 @@ private Sign ssaDefSign(SsaVariable v) {
   )
 }
 
+/** Returns the sign of implicit SSA definition `v`. */
+private Sign implicitSsaDefSign(SsaVariable v) {
+  result = fieldSign(getImplicitSsaDeclaration(v))
+  or
+  anySign(result) and nonFieldImplicitSsaDefinition(v)
+}
+
 /** Gets a possible sign for `e`. */
 cached
 Sign exprSign(Expr e) {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll b/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll
index cbca875c8c2..74466990725 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll
@@ -115,13 +115,15 @@ private module Impl {
     )
   }
 
-  /** Returns the sign of implicit SSA definition `v`. */
-  Sign implicitSsaDefSign(SsaVariable v) {
-    result = fieldSign(v.(SsaImplicitUpdate).getSourceVariable().getVariable())
-    or
-    result = fieldSign(v.(SsaImplicitInit).getSourceVariable().getVariable())
-    or
-    anySign(result) and exists(Parameter p | v.(SsaImplicitInit).isParameterDefinition(p))
+  /** Gets the variable underlying the implicit SSA variable `v`. */
+  Variable getImplicitSsaDeclaration(SsaVariable v) {
+    result = v.(SsaImplicitUpdate).getSourceVariable().getVariable() or
+    result = v.(SsaImplicitInit).getSourceVariable().getVariable()
+  }
+
+  /** Holds if the variable underlying the implicit SSA variable `v` is not a field. */
+  predicate nonFieldImplicitSsaDefinition(SsaImplicitInit v) {
+    exists(Parameter p | v.isParameterDefinition(p))
   }
 
   /** Gets a possible sign for `f`. */