Merge master into next.

adityasharad · adityasharad · commit 7bc729a7dcab · 2018-12-14T10:16:47.000Z
diff --git a/change-notes/1.20/analysis-csharp.md b/change-notes/1.20/analysis-csharp.md
@@ -17,6 +17,8 @@
 
 ## Changes to code extraction
 
+* Initializers of `stackalloc` arrays are now extracted.
+
 ## Changes to QL libraries
 
 ## Changes to the autobuilder
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ArrayCreation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ArrayCreation.cs
@@ -9,55 +9,35 @@ abstract class ArrayCreation<SyntaxNode> : Expression<SyntaxNode> where SyntaxNo
         protected ArrayCreation(ExpressionNodeInfo info) : base(info) { }
     }
 
-    class StackAllocArrayCreation : ArrayCreation<StackAllocArrayCreationExpressionSyntax>
+    abstract class ExplicitArrayCreation<SyntaxNode> : ArrayCreation<SyntaxNode> where SyntaxNode : ExpressionSyntax
     {
-        StackAllocArrayCreation(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.ARRAY_CREATION)) { }
+        protected ExplicitArrayCreation(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.ARRAY_CREATION)) { }
 
-        public static Expression Create(ExpressionNodeInfo info) => new StackAllocArrayCreation(info).TryPopulate();
+        protected abstract ArrayTypeSyntax TypeSyntax { get; }
+
+        public abstract InitializerExpressionSyntax  Initializer { get;  }
 
         protected override void Populate()
         {
-            var arrayType = Syntax.Type as ArrayTypeSyntax;
-
-            if (arrayType == null)
-            {
-                cx.ModelError(Syntax, "Unexpected array type");
-                return;
-            }
-
             var child = 0;
+            var explicitlySized = false;
 
-            foreach (var rank in arrayType.RankSpecifiers.SelectMany(rs => rs.Sizes))
+            if (TypeSyntax is null)
             {
-                Create(cx, rank, this, child++);
+                cx.ModelError(Syntax, "Array has unexpected type syntax");
             }
 
-            cx.Emit(Tuples.explicitly_sized_array_creation(this));
-        }
-    }
-
-    class ExplicitArrayCreation : ArrayCreation<ArrayCreationExpressionSyntax>
-    {
-        ExplicitArrayCreation(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.ARRAY_CREATION)) { }
-
-        public static Expression Create(ExpressionNodeInfo info) => new ExplicitArrayCreation(info).TryPopulate();
-
-        protected override void Populate()
-        {
-            var child = 0;
-            bool explicitlySized = false;
-
-            foreach (var rank in Syntax.Type.RankSpecifiers.SelectMany(rs => rs.Sizes))
+            foreach (var rank in TypeSyntax.RankSpecifiers.SelectMany(rs => rs.Sizes))
             {
                 if (rank is OmittedArraySizeExpressionSyntax)
                 {
                     // Create an expression which simulates the explicit size of the array
 
-                    if (Syntax.Initializer != null)
+                    if (!(Initializer is null))
                     {
                         // An implicitly-sized array must have an initializer.
                         // Guard it just in case.
-                        var size = Syntax.Initializer.Expressions.Count;
+                        var size = Initializer.Expressions.Count;
 
                         var info = new ExpressionInfo(
                             cx,
@@ -79,16 +59,38 @@ protected override void Populate()
                 }
                 child++;
             }
-            if (Syntax.Initializer != null)
+            if (!(Initializer is null))
             {
-                ArrayInitializer.Create(new ExpressionNodeInfo(cx, Syntax.Initializer, this, -1));
+                ArrayInitializer.Create(new ExpressionNodeInfo(cx, Initializer, this, -1));
             }
 
             if (explicitlySized)
                 cx.Emit(Tuples.explicitly_sized_array_creation(this));
         }
     }
 
+    class NormalArrayCreation : ExplicitArrayCreation<ArrayCreationExpressionSyntax>
+    {
+        private NormalArrayCreation(ExpressionNodeInfo info) : base(info) { }
+
+        protected override ArrayTypeSyntax TypeSyntax => Syntax.Type;
+
+        public override InitializerExpressionSyntax Initializer => Syntax.Initializer;
+
+        public static Expression Create(ExpressionNodeInfo info) => new NormalArrayCreation(info).TryPopulate();
+    }
+
+    class StackAllocArrayCreation : ExplicitArrayCreation<StackAllocArrayCreationExpressionSyntax>
+    {
+        StackAllocArrayCreation(ExpressionNodeInfo info) : base(info) { }
+
+        protected override ArrayTypeSyntax TypeSyntax => Syntax.Type as ArrayTypeSyntax;
+
+        public override InitializerExpressionSyntax Initializer => Syntax.Initializer;
+
+        public static Expression Create(ExpressionNodeInfo info) => new StackAllocArrayCreation(info).TryPopulate();
+    }
+
     class ImplicitArrayCreation : ArrayCreation<ImplicitArrayCreationExpressionSyntax>
     {
         ImplicitArrayCreation(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.ARRAY_CREATION)) { }
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Factory.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Factory.cs
@@ -86,7 +86,7 @@ internal static Expression Create(ExpressionNodeInfo info)
                         return ExplicitObjectCreation.Create(info);
 
                     case SyntaxKind.ArrayCreationExpression:
-                        return ExplicitArrayCreation.Create(info);
+                        return NormalArrayCreation.Create(info);
 
                     case SyntaxKind.ObjectInitializerExpression:
                         return ObjectInitializer.Create(info);
diff --git a/csharp/ql/test/library-tests/csharp7.3/ArrayCreations.expected b/csharp/ql/test/library-tests/csharp7.3/ArrayCreations.expected
@@ -0,0 +1,6 @@
+| csharp73.cs:9:20:9:49 | array creation of type Char* | 0 | csharp73.cs:9:20:9:49 | 2 |
+| csharp73.cs:10:20:10:45 | array creation of type Char* | 0 | csharp73.cs:10:36:10:36 | 1 |
+| csharp73.cs:11:20:11:37 | array creation of type Char[] | 0 | csharp73.cs:11:20:11:37 | 1 |
+| csharp73.cs:12:20:12:38 | array creation of type Char* | 0 | csharp73.cs:12:36:12:37 | 10 |
+| csharp73.cs:13:20:13:31 | array creation of type Char[] | 0 | csharp73.cs:13:29:13:30 | 10 |
+| csharp73.cs:21:23:21:33 | array creation of type Int32[] | 0 | csharp73.cs:21:31:21:32 | 10 |
diff --git a/csharp/ql/test/library-tests/csharp7.3/ArrayCreations.ql b/csharp/ql/test/library-tests/csharp7.3/ArrayCreations.ql
@@ -0,0 +1,4 @@
+import csharp
+
+from ArrayCreation creation, int i
+select creation, i, creation.getLengthArgument(i)
diff --git a/csharp/ql/test/library-tests/csharp7.3/ArrayElements.expected b/csharp/ql/test/library-tests/csharp7.3/ArrayElements.expected
@@ -0,0 +1,4 @@
+| csharp73.cs:9:20:9:49 | array creation of type Char* | 0 | csharp73.cs:9:40:9:42 | x |
+| csharp73.cs:9:20:9:49 | array creation of type Char* | 1 | csharp73.cs:9:45:9:47 | y |
+| csharp73.cs:10:20:10:45 | array creation of type Char* | 0 | csharp73.cs:10:41:10:43 | x |
+| csharp73.cs:11:20:11:37 | array creation of type Char[] | 0 | csharp73.cs:11:33:11:35 | x |
diff --git a/csharp/ql/test/library-tests/csharp7.3/ArrayElements.ql b/csharp/ql/test/library-tests/csharp7.3/ArrayElements.ql
@@ -0,0 +1,4 @@
+import csharp
+
+from ArrayCreation array, int i
+select array, i, array.getInitializer().getElement(i)
diff --git a/csharp/ql/test/library-tests/csharp7.3/csharp73.cs b/csharp/ql/test/library-tests/csharp7.3/csharp73.cs
@@ -0,0 +1,52 @@
+// semmle-extractor-options: /langversion:latest
+
+using System;
+
+class StackAllocs
+{
+    unsafe void Fn()
+    {
+        var arr1 = stackalloc char[] { 'x', 'y' };
+        var arr2 = stackalloc char[1] { 'x' };
+        var arr3 = new char[] { 'x' };
+        var arr4 = stackalloc char[10];
+        var arr5 = new char[10];
+    }
+}
+
+class PinnedReference
+{
+    unsafe void F()
+    {
+        Span<int> t = new int[10];
+        // This line should compile and generate a call to t.GetPinnableReference()
+        // fixed (int * p = t)
+        {
+        }
+    }
+}
+
+class UnmanagedConstraint<T> where T : unmanaged
+{
+}
+
+class EnumConstraint<T> where T : System.Enum
+{
+}
+
+class DelegateConstraint<T> where T : System.Delegate
+{
+}
+
+class ExpressionVariables
+{
+    ExpressionVariables(out int x)
+    {
+        x = 5;
+    }
+
+    public ExpressionVariables() : this(out int x)
+    {
+        Console.WriteLine($"x is {x}");
+    }
+}
diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
@@ -109,15 +109,26 @@ predicate whitelist(Expr e) {
 
 /**
  * Holds if `e` is part of a conditional node `cond` that evaluates
- * `e` and checks its value for truthiness.
+ * `e` and checks its value for truthiness, and the return value of `e`
+ * is not used for anything other than this truthiness check.
  */
-predicate isConditional(ASTNode cond, Expr e) {
+predicate isExplicitConditional(ASTNode cond, Expr e) {
   e = cond.(IfStmt).getCondition() or
   e = cond.(LoopStmt).getTest() or
   e = cond.(ConditionalExpr).getCondition() or
-  e = cond.(LogicalBinaryExpr).getLeftOperand() or
-  // Include `z` in `if (x && z)`.
-  isConditional(_, cond) and e = cond.(Expr).getUnderlyingValue().(LogicalBinaryExpr).getRightOperand()
+  isExplicitConditional(_, cond) and e = cond.(Expr).getUnderlyingValue().(LogicalBinaryExpr).getAnOperand()
+}
+
+/**
+ * Holds if `e` is part of a conditional node `cond` that evaluates
+ * `e` and checks its value for truthiness.
+ *
+ * The return value of `e` may have other uses besides the truthiness check,
+ * but if the truthiness check always goes one way, it still indicates an error.
+ */
+predicate isConditional(ASTNode cond, Expr e) {
+  isExplicitConditional(cond, e) or
+  e = cond.(LogicalBinaryExpr).getLeftOperand()
 }
 
 from ASTNode cond, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -687,6 +687,7 @@ private predicate flowsTo(PathNode flowsource, DataFlow::Node source,
  * Holds if `nd` is reachable from a source under `cfg` along a path summarized by
  * `summary`.
  */
+pragma[nomagic]
 private predicate reachableFromSource(DataFlow::Node nd, DataFlow::Configuration cfg,
                                       PathSummary summary) {
   exists (FlowLabel lbl |
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
@@ -311,6 +311,25 @@ class FunctionNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
   Function getFunction() {
     result = astNode
   }
+
+  /**
+   * Gets the function whose `this` binding a `this` expression in this function refers to,
+   * which is the nearest enclosing non-arrow function.
+   */
+  FunctionNode getThisBinder() {
+    result.getFunction() = getFunction().getThisBinder()
+  }
+
+  /**
+   * Gets the dataflow node holding the value of the receiver passed to the given function.
+   *
+   * Has no result for arrow functions, as they ignore the receiver argument.
+   *
+   * To get the data flow node for `this` in an arrow function, consider using `getThisBinder().getReceiver()`.
+   */
+  ThisNode getReceiver() {
+    result.getBinder() = this
+  }
 }
 
 /** A data flow node corresponding to an object literal expression. */
diff --git a/javascript/ql/test/library-tests/Receiver/GetReceiver.expected b/javascript/ql/test/library-tests/Receiver/GetReceiver.expected
@@ -0,0 +1,6 @@
+| tst.js:1:1:13:1 | functio ... \\n  };\\n} | tst.js:1:1:1:0 | this |
+| tst.js:2:3:2:21 | function inner() {} | tst.js:2:3:2:2 | this |
+| tst.js:5:11:5:10 | () {} | tst.js:5:11:5:10 | this |
+| tst.js:6:11:6:15 | () {} | tst.js:6:11:6:10 | this |
+| tst.js:10:15:10:19 | () {} | tst.js:10:15:10:14 | this |
+| tst.js:11:15:11:20 | (x) {} | tst.js:11:15:11:14 | this |
diff --git a/javascript/ql/test/library-tests/Receiver/GetReceiver.ql b/javascript/ql/test/library-tests/Receiver/GetReceiver.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from DataFlow::FunctionNode function
+select function, function.getReceiver()
diff --git a/javascript/ql/test/library-tests/Receiver/tst.js b/javascript/ql/test/library-tests/Receiver/tst.js
@@ -0,0 +1,13 @@
+function f() {
+  function inner() {}
+  let arrow = () => 5;
+  
+  class C {
+    method() {}
+  };
+  
+  let obj = {
+    get getter() {},
+    set setter(x) {}
+  };
+}
diff --git a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected
@@ -21,6 +21,7 @@
 | UselessConditional.js:101:18:101:18 | x | This use of variable 'x' always evaluates to false. |
 | UselessConditional.js:102:19:102:19 | x | This use of variable 'x' always evaluates to false. |
 | UselessConditional.js:103:23:103:23 | x | This use of variable 'x' always evaluates to false. |
+| UselessConditional.js:109:15:109:16 | {} | This expression always evaluates to true. |
 | UselessConditionalGood.js:58:12:58:13 | x2 | This use of variable 'x2' always evaluates to false. |
 | UselessConditionalGood.js:69:12:69:13 | xy | This use of variable 'xy' always evaluates to false. |
 | UselessConditionalGood.js:85:12:85:13 | xy | This use of variable 'xy' always evaluates to false. |
diff --git a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.js b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.js
@@ -104,4 +104,9 @@ async function awaitFlow(){
     }
 });
 
+(function(x,y) {
+    let obj = (x && {}) || y; // OK
+    if ((x && {}) || y) {} // NOT OK
+});
+
 // semmle-extractor-options: --experimental
