Merge pull request #673 from owen-mc/refactor-returnvalue-n

Refactor `ReturnValue[n]` in data flow libraries

Author: owen-mc

ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll

@@ -822,32 +822,14 @@ module Private {
       )
     }
 
-    /** Holds if specification component `c` parses as return value `n`. */
-    predicate parseReturn(string c, int n) {
-      specSplit(_, c, _) and
-      (
-        c = "ReturnValue" and n = 0
-        or
-        c.regexpCapture("ReturnValue\\[([-0-9]+)\\]", 1).toInt() = n
-        or
-        exists(int n1, int n2 |
-          c.regexpCapture("ReturnValue\\[([-0-9]+)\\.\\.([0-9]+)\\]", 1).toInt() = n1 and
-          c.regexpCapture("ReturnValue\\[([-0-9]+)\\.\\.([0-9]+)\\]", 2).toInt() = n2 and
-          n = [n1 .. n2]
-        )
-      )
-    }
-
     private SummaryComponent interpretComponent(string c) {
       specSplit(_, c, _) and
       (
         exists(int pos | parseArg(c, pos) and result = SummaryComponent::argument(pos))
         or
         exists(int pos | parseParam(c, pos) and result = SummaryComponent::parameter(pos))
         or
-        exists(int pos |
-          parseReturn(c, pos) and result = SummaryComponent::return(getReturnKind(pos))
-        )
+        c = "ReturnValue" and result = SummaryComponent::return(getReturnValueKind())
         or
         result = interpretComponentSpecific(c)
       )
@@ -914,12 +896,15 @@ module Private {
       not exists(interpretComponent(c))
     }
 
-    private predicate inputNeedsReference(string c) { parseArg(c, _) }
+    private predicate inputNeedsReference(string c) {
+      parseArg(c, _) or
+      inputNeedsReferenceSpecific(c)
+    }
 
     private predicate outputNeedsReference(string c) {
       parseArg(c, _) or
       c = "ReturnValue" or
-      parseReturn(c, _)
+      outputNeedsReferenceSpecific(c)
     }
 
     private predicate sourceElementRef(InterpretNode ref, string output, string kind) {
@@ -959,13 +944,8 @@ module Private {
           c = "Parameter" or parseParam(c, pos)
         )
         or
-        exists(int pos |
-          node.asNode() = getAnOutNodeExt(mid.asCall(), TValueReturn(getReturnKind(pos)))
-        |
-          c = "ReturnValue" and pos = 0
-          or
-          parseReturn(c, pos)
-        )
+        c = "ReturnValue" and
+        node.asNode() = getAnOutNodeExt(mid.asCall(), TValueReturn(getReturnValueKind()))
         or
         interpretOutputSpecific(c, mid, node)
       )
@@ -982,14 +962,10 @@ module Private {
       |
         exists(int pos | node.asNode().(ArgNode).argumentOf(mid.asCall(), pos) | parseArg(c, pos))
         or
-        exists(int pos, ReturnNodeExt ret |
-          (
-            c = "ReturnValue" and pos = 0
-            or
-            parseReturn(c, pos)
-          ) and
+        exists(ReturnNodeExt ret |
+          c = "ReturnValue" and
           ret = node.asNode() and
-          ret.getKind().(ValueReturnKind).getKind() = getReturnKind(pos) and
+          ret.getKind().(ValueReturnKind).getKind() = getReturnValueKind() and
           mid.asCallable() = getNodeEnclosingCallable(ret)
         )
         or

ql/lib/semmle/go/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -8,6 +8,7 @@ private import DataFlowUtil
 private import FlowSummaryImpl::Private
 private import FlowSummaryImpl::Public
 private import semmle.go.dataflow.ExternalFlow
+private import DataFlowImplCommon
 
 private module FlowSummaries {
   private import semmle.go.dataflow.FlowSummary as F
@@ -65,6 +66,8 @@ predicate summaryElement(DataFlowCallable c, string input, string output, string
 /** Gets the summary component for specification component `c`, if any. */
 bindingset[c]
 SummaryComponent interpretComponentSpecific(string c) {
+  exists(int pos | parseReturn(c, pos) and result = SummaryComponent::return(getReturnKind(pos)))
+  or
   exists(Content content | parseContent(c, content) and result = SummaryComponent::content(content))
 }
 
@@ -92,8 +95,20 @@ private string getContentSpecificCsv(Content c) {
 /** Gets the textual representation of the content in the format used for flow summaries. */
 string getComponentSpecificCsv(SummaryComponent sc) {
   exists(Content c | sc = TContentSummaryComponent(c) and result = getContentSpecificCsv(c))
+  or
+  exists(ReturnKind rk, int n | n = rk.getIndex() |
+    sc = TReturnSummaryComponent(rk) and
+    result = "ReturnValue[" + n + "]" and
+    n != 0
+  )
 }
 
+/** Holds if input specification component `c` needs a reference. */
+predicate inputNeedsReferenceSpecific(string c) { none() }
+
+/** Holds if output specification component `c` needs a reference. */
+predicate outputNeedsReferenceSpecific(string c) { parseReturn(c, _) }
+
 private newtype TSourceOrSinkElement =
   TEntityElement(Entity e) or
   TAstElement(AstNode n)
@@ -144,7 +159,7 @@ predicate sinkElement(SourceOrSinkElement e, string input, string kind) {
 }
 
 /** Gets the return kind corresponding to specification `"ReturnValue"`. */
-ReturnKind getReturnValueKind() { any() }
+ReturnKind getReturnValueKind() { result = getReturnKind(0) }
 
 private newtype TInterpretNode =
   TElement(SourceOrSinkElement n) or
@@ -191,6 +206,10 @@ class InterpretNode extends TInterpretNode {
 /** Provides additional sink specification logic required for annotations. */
 pragma[inline]
 predicate interpretOutputSpecific(string c, InterpretNode mid, InterpretNode node) {
+  exists(int pos | node.asNode() = getAnOutNodeExt(mid.asCall(), TValueReturn(getReturnKind(pos))) |
+    parseReturn(c, pos)
+  )
+  or
   exists(Node n, SourceOrSinkElement e |
     n = node.asNode() and
     e = mid.asElement()
@@ -206,9 +225,32 @@ predicate interpretOutputSpecific(string c, InterpretNode mid, InterpretNode nod
 /** Provides additional source specification logic required for annotations. */
 pragma[inline]
 predicate interpretInputSpecific(string c, InterpretNode mid, InterpretNode n) {
+  exists(int pos, ReturnNodeExt ret |
+    parseReturn(c, pos) and
+    ret = n.asNode() and
+    ret.getKind().(ValueReturnKind).getKind() = getReturnKind(pos) and
+    mid.asCallable() = getNodeEnclosingCallable(ret)
+  )
+  or
   exists(DataFlow::Write fw, Field f |
     c = "" and
     f = mid.asElement().asEntity() and
     fw.writesField(_, f, n.asNode())
   )
 }
+
+/** Holds if specification component `c` parses as return value `n`. */
+predicate parseReturn(string c, int n) {
+  External::specSplit(_, c, _) and
+  (
+    c = "ReturnValue" and n = 0
+    or
+    c.regexpCapture("ReturnValue\\[([-0-9]+)\\]", 1).toInt() = n
+    or
+    exists(int n1, int n2 |
+      c.regexpCapture("ReturnValue\\[([-0-9]+)\\.\\.([0-9]+)\\]", 1).toInt() = n1 and
+      c.regexpCapture("ReturnValue\\[([-0-9]+)\\.\\.([0-9]+)\\]", 2).toInt() = n2 and
+      n = [n1 .. n2]
+    )
+  )
+}