mirror of
https://github.com/github/codeql.git
synced 2026-06-19 03:41:07 +02:00
Merge pull request #21877 from michaelnebel/csharp/spanaccessrange
C#: Extract `.Slice` method call when using a span in conjunction with a range.
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
using System.IO;
|
||||
using Microsoft.CodeAnalysis;
|
||||
using Microsoft.CodeAnalysis.CSharp;
|
||||
using Microsoft.CodeAnalysis.CSharp.Syntax;
|
||||
using Semmle.Extraction.Kinds;
|
||||
|
||||
@@ -8,7 +9,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
|
||||
internal abstract class ElementAccess : Expression<ExpressionSyntax>
|
||||
{
|
||||
protected ElementAccess(ExpressionNodeInfo info, ExpressionSyntax qualifier, BracketedArgumentListSyntax argumentList)
|
||||
: base(info.SetKind(GetKind(info.Context, qualifier)))
|
||||
: base(info.SetKind(GetKind(info.Context, info.Node, qualifier)))
|
||||
{
|
||||
this.qualifier = qualifier;
|
||||
this.argumentList = argumentList;
|
||||
@@ -17,6 +18,125 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
|
||||
private readonly ExpressionSyntax qualifier;
|
||||
private readonly BracketedArgumentListSyntax argumentList;
|
||||
|
||||
|
||||
private ISymbol? GetTargetSymbol()
|
||||
{
|
||||
return Context.GetSymbolInfo(base.Syntax).Symbol;
|
||||
}
|
||||
|
||||
private static void SetExprArgument(TextWriter trapFile, Expression left, Expression right)
|
||||
{
|
||||
trapFile.expr_argument(left, 0);
|
||||
trapFile.expr_argument(right, 0);
|
||||
}
|
||||
|
||||
private Expression MakeZeroFromEndExpression(IExpressionParentEntity parent, int child)
|
||||
{
|
||||
var info = new ExpressionInfo(
|
||||
Context,
|
||||
AnnotatedTypeSymbol.CreateNotAnnotated(Context.Compilation.GetSpecialType(SpecialType.System_Int32)),
|
||||
Location,
|
||||
ExprKind.INDEX,
|
||||
parent,
|
||||
child,
|
||||
isCompilerGenerated: true,
|
||||
null);
|
||||
|
||||
var index = new Expression(info);
|
||||
|
||||
MakeZeroLiteral(index, 0);
|
||||
return index;
|
||||
}
|
||||
|
||||
private Expression MakeZeroLiteral(IExpressionParentEntity parent, int child)
|
||||
{
|
||||
return Literal.CreateGenerated(Context, parent, child, Context.Compilation.GetSpecialType(SpecialType.System_Int32), 0, Location);
|
||||
}
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// It is assumed that either the input is
|
||||
/// 1. A normal expression that can be used as endpoint (e.g a constant like "3").
|
||||
/// 2. An index expression indicating that we should read from the end (e.g "^1").
|
||||
/// </summary>
|
||||
/// <param name="syntax">The syntax node representing the range endpoint.</param>
|
||||
/// <param name="parent">The parent expression entity.</param>
|
||||
/// <param name="child">The child index within the parent.</param>
|
||||
/// <returns>An expression representing the endpoint of a range to be used in conjunction with a slice operation.</returns>
|
||||
private Expression MakeFromRangeEndpoint(ExpressionSyntax syntax, IExpressionParentEntity parent, int child)
|
||||
{
|
||||
var info = new ExpressionNodeInfo(Context, syntax, parent, child);
|
||||
|
||||
return syntax.Kind() == SyntaxKind.IndexExpression
|
||||
? PrefixUnary.Create(info.SetKind(ExprKind.INDEX))
|
||||
: Factory.Create(info);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Determines whether the given method is a slice method, which is defined as a method with
|
||||
/// the name "Slice" or "Substring" and two parameters.
|
||||
/// </summary>
|
||||
/// <param name="method">The method symbol to check.</param>
|
||||
/// <returns>True if the method is a slice method; false otherwise.</returns>
|
||||
private bool IsSlice(IMethodSymbol method, out RangeExpressionSyntax? range)
|
||||
{
|
||||
range = null;
|
||||
|
||||
if (argumentList.Arguments.Count == 1)
|
||||
{
|
||||
range = argumentList.Arguments[0].Expression as RangeExpressionSyntax;
|
||||
}
|
||||
|
||||
return (method.Name == "Slice" || method.Name == "Substring")
|
||||
&& method.Parameters.Length == 2;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Populates a slice method call based on the given range.
|
||||
/// Roslyn translates indexer accesses with range expressions in the following way.
|
||||
/// 1. s[a..b] -> s.Slice(a, b - a)
|
||||
/// 2. s[..b] -> s.Slice(0, b)
|
||||
/// 3. s[a..] -> s.Slice(a, s.Length - a)
|
||||
/// 4. s[..] -> s.Slice(0, s.Length)
|
||||
/// However, it is possible that both the qualifier or the index endpoints may contain method calls.
|
||||
/// If we want to translate this accurately, we would need to introduce synthetic statements for qualifier and
|
||||
/// the endpoints, which should then be used in the slice method call.
|
||||
/// To avoid this, we translate as follows.
|
||||
/// 1. s[a..b] -> s.Slice(a, b)
|
||||
/// 2. s[..b] -> s.Slice(0, b)
|
||||
/// 3. s[a..] -> s.Slice(a, ^0)
|
||||
/// 4. s[..] -> s.Slice(0, ^0)
|
||||
///
|
||||
/// Even though index expressions can't technically be used in this way, they signal that we
|
||||
/// could perceive ^b as "length - b".
|
||||
///
|
||||
/// Call arguments are only populated when a range expression is directly available in
|
||||
/// the list of arguments.
|
||||
/// This means that cases like below are not handled.
|
||||
/// System.Range x = 1..3;
|
||||
/// s[x]
|
||||
/// </summary>
|
||||
/// <param name="trapFile">The trap file to write to.</param>
|
||||
/// <param name="slice">The slice method symbol.</param>
|
||||
/// <param name="range">The range expression syntax.</param>
|
||||
private void PopulateSlice(TextWriter trapFile, IMethodSymbol slice, RangeExpressionSyntax? range)
|
||||
{
|
||||
if (range is not null)
|
||||
{
|
||||
// Populate the call arguments
|
||||
var left = range.LeftOperand is ExpressionSyntax lsyntax
|
||||
? MakeFromRangeEndpoint(lsyntax, this, 0)
|
||||
: MakeZeroLiteral(this, 0);
|
||||
|
||||
var right = range.RightOperand is ExpressionSyntax rsyntax
|
||||
? MakeFromRangeEndpoint(rsyntax, this, 1)
|
||||
: MakeZeroFromEndExpression(this, 1);
|
||||
|
||||
SetExprArgument(trapFile, left, right);
|
||||
}
|
||||
trapFile.expr_call(this, Method.Create(Context, slice));
|
||||
}
|
||||
|
||||
protected override void PopulateExpression(TextWriter trapFile)
|
||||
{
|
||||
if (Kind == ExprKind.POINTER_INDIRECTION)
|
||||
@@ -30,11 +150,19 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
|
||||
else
|
||||
{
|
||||
Create(Context, qualifier, this, -1);
|
||||
|
||||
var target = GetTargetSymbol();
|
||||
if (target is IMethodSymbol method && IsSlice(method, out var range))
|
||||
{
|
||||
// When an indexer on a span or string is used in conjunction with a range expression, the compiler translates
|
||||
// this into a call to the "Slice" or "Substring" method.
|
||||
// In this case, we want to populate a slice/substring method call instead of an indexer access.
|
||||
PopulateSlice(trapFile, method, range);
|
||||
return;
|
||||
}
|
||||
|
||||
PopulateArguments(trapFile, argumentList, 0);
|
||||
|
||||
var symbolInfo = Context.GetSymbolInfo(base.Syntax);
|
||||
|
||||
if (symbolInfo.Symbol is IPropertySymbol indexer)
|
||||
if (target is IPropertySymbol { IsIndexer: true } indexer)
|
||||
{
|
||||
trapFile.expr_access(this, Indexer.Create(Context, indexer));
|
||||
}
|
||||
@@ -46,8 +174,11 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
|
||||
private static bool IsArray(ITypeSymbol symbol) =>
|
||||
symbol.TypeKind == Microsoft.CodeAnalysis.TypeKind.Array || symbol.IsInlineArray();
|
||||
|
||||
private static ExprKind GetKind(Context cx, ExpressionSyntax qualifier)
|
||||
private static ExprKind GetKind(Context cx, ExpressionSyntax syntax, ExpressionSyntax qualifier)
|
||||
{
|
||||
if (cx.GetSymbolInfo(syntax).Symbol is IMethodSymbol)
|
||||
return ExprKind.METHOD_INVOCATION;
|
||||
|
||||
var qualifierType = cx.GetType(qualifier);
|
||||
|
||||
// This is a compilation error, so make a guess and continue.
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Improved extraction of range-access expressions on spans and strings (for example, `a[0..3]`). These expressions are now extracted as `Slice` (span) or `Substring` (string) calls.
|
||||
29
csharp/ql/test/library-tests/spans/Slice.cs
Normal file
29
csharp/ql/test/library-tests/spans/Slice.cs
Normal file
@@ -0,0 +1,29 @@
|
||||
using System;
|
||||
|
||||
public class C
|
||||
{
|
||||
public void M(int a, int b)
|
||||
{
|
||||
var s = "hello world";
|
||||
var sub1 = s[1..a];
|
||||
var sub2 = s[..2];
|
||||
var sub3 = s[3..];
|
||||
var sub4 = s[..^4];
|
||||
var sub5 = s[a..^b];
|
||||
var sub6 = s[..];
|
||||
|
||||
Range range = 1..a;
|
||||
var sub7 = s[range];
|
||||
|
||||
Span<int> sp = null;
|
||||
var slice1 = sp[5..a];
|
||||
var slice2 = sp[..6];
|
||||
var slice3 = sp[7..];
|
||||
var slice4 = sp[..^8];
|
||||
var slice5 = sp[a..^b];
|
||||
var slice6 = sp[..];
|
||||
|
||||
Range range2 = 1..a;
|
||||
var slice7 = sp[range2];
|
||||
}
|
||||
}
|
||||
41
csharp/ql/test/library-tests/spans/slice.expected
Normal file
41
csharp/ql/test/library-tests/spans/slice.expected
Normal file
@@ -0,0 +1,41 @@
|
||||
methodArguments
|
||||
| Slice.cs:8:20:8:26 | call to method Substring | Substring(int, int) | 0 | 1 |
|
||||
| Slice.cs:8:20:8:26 | call to method Substring | Substring(int, int) | 1 | access to parameter a |
|
||||
| Slice.cs:9:20:9:25 | call to method Substring | Substring(int, int) | 0 | 0 |
|
||||
| Slice.cs:9:20:9:25 | call to method Substring | Substring(int, int) | 1 | 2 |
|
||||
| Slice.cs:10:20:10:25 | call to method Substring | Substring(int, int) | 0 | 3 |
|
||||
| Slice.cs:10:20:10:25 | call to method Substring | Substring(int, int) | 1 | ^0 |
|
||||
| Slice.cs:11:20:11:26 | call to method Substring | Substring(int, int) | 0 | 0 |
|
||||
| Slice.cs:11:20:11:26 | call to method Substring | Substring(int, int) | 1 | ^4 |
|
||||
| Slice.cs:12:20:12:27 | call to method Substring | Substring(int, int) | 0 | access to parameter a |
|
||||
| Slice.cs:12:20:12:27 | call to method Substring | Substring(int, int) | 1 | ^access to parameter b |
|
||||
| Slice.cs:13:20:13:24 | call to method Substring | Substring(int, int) | 0 | 0 |
|
||||
| Slice.cs:13:20:13:24 | call to method Substring | Substring(int, int) | 1 | ^0 |
|
||||
| Slice.cs:19:22:19:29 | call to method Slice | Slice(int, int) | 0 | 5 |
|
||||
| Slice.cs:19:22:19:29 | call to method Slice | Slice(int, int) | 1 | access to parameter a |
|
||||
| Slice.cs:20:22:20:28 | call to method Slice | Slice(int, int) | 0 | 0 |
|
||||
| Slice.cs:20:22:20:28 | call to method Slice | Slice(int, int) | 1 | 6 |
|
||||
| Slice.cs:21:22:21:28 | call to method Slice | Slice(int, int) | 0 | 7 |
|
||||
| Slice.cs:21:22:21:28 | call to method Slice | Slice(int, int) | 1 | ^0 |
|
||||
| Slice.cs:22:22:22:29 | call to method Slice | Slice(int, int) | 0 | 0 |
|
||||
| Slice.cs:22:22:22:29 | call to method Slice | Slice(int, int) | 1 | ^8 |
|
||||
| Slice.cs:23:22:23:30 | call to method Slice | Slice(int, int) | 0 | access to parameter a |
|
||||
| Slice.cs:23:22:23:30 | call to method Slice | Slice(int, int) | 1 | ^access to parameter b |
|
||||
| Slice.cs:24:22:24:27 | call to method Slice | Slice(int, int) | 0 | 0 |
|
||||
| Slice.cs:24:22:24:27 | call to method Slice | Slice(int, int) | 1 | ^0 |
|
||||
methodCalls
|
||||
| Slice.cs:3:14:3:14 | call to method <object initializer> | <object initializer>() |
|
||||
| Slice.cs:8:20:8:26 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:9:20:9:25 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:10:20:10:25 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:11:20:11:26 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:12:20:12:27 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:13:20:13:24 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:16:20:16:27 | call to method Substring | Substring(int, int) |
|
||||
| Slice.cs:19:22:19:29 | call to method Slice | Slice(int, int) |
|
||||
| Slice.cs:20:22:20:28 | call to method Slice | Slice(int, int) |
|
||||
| Slice.cs:21:22:21:28 | call to method Slice | Slice(int, int) |
|
||||
| Slice.cs:22:22:22:29 | call to method Slice | Slice(int, int) |
|
||||
| Slice.cs:23:22:23:30 | call to method Slice | Slice(int, int) |
|
||||
| Slice.cs:24:22:24:27 | call to method Slice | Slice(int, int) |
|
||||
| Slice.cs:27:22:27:31 | call to method Slice | Slice(int, int) |
|
||||
17
csharp/ql/test/library-tests/spans/slice.ql
Normal file
17
csharp/ql/test/library-tests/spans/slice.ql
Normal file
@@ -0,0 +1,17 @@
|
||||
import csharp
|
||||
|
||||
private string printExpr(Expr e) {
|
||||
e = any(IndexExpr index | result = "^" + index.getExpr().toString())
|
||||
or
|
||||
not e instanceof IndexExpr and
|
||||
result = e.toString()
|
||||
}
|
||||
|
||||
query predicate methodArguments(MethodCall mc, string target, int i, string arg) {
|
||||
target = mc.getTarget().toStringWithTypes() and
|
||||
arg = printExpr(mc.getArgument(i))
|
||||
}
|
||||
|
||||
query predicate methodCalls(MethodCall mc, string target) {
|
||||
target = mc.getTarget().toStringWithTypes()
|
||||
}
|
||||
@@ -1,2 +0,0 @@
|
||||
| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
|
||||
| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
|
||||
|
||||
@@ -7,7 +7,5 @@
|
||||
| Quality.cs:20:13:20:23 | access to property MyProperty6 | Call without target $@. | Quality.cs:20:13:20:23 | access to property MyProperty6 | access to property MyProperty6 |
|
||||
| Quality.cs:23:9:23:14 | access to event Event1 | Call without target $@. | Quality.cs:23:9:23:14 | access to event Event1 | access to event Event1 |
|
||||
| Quality.cs:23:9:23:30 | delegate call | Call without target $@. | Quality.cs:23:9:23:30 | delegate call | delegate call |
|
||||
| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
|
||||
| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
|
||||
| Quality.cs:38:16:38:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:38:16:38:26 | access to property MyProperty2 | access to property MyProperty2 |
|
||||
| Quality.cs:50:20:50:26 | object creation of type T | Call without target $@. | Quality.cs:50:20:50:26 | object creation of type T | object creation of type T |
|
||||
|
||||
@@ -23,10 +23,10 @@ public class Test
|
||||
Event1.Invoke(this, 5);
|
||||
|
||||
var str = "abcd";
|
||||
var sub = str[..3]; // TODO: this is not an indexer call, but rather a `str.Substring(0, 3)` call.
|
||||
var sub = str[..3];
|
||||
|
||||
Span<int> sp = null;
|
||||
var slice = sp[..3]; // TODO: this is not an indexer call, but rather a `sp.Slice(0, 3)` call.
|
||||
var slice = sp[..3];
|
||||
|
||||
Span<byte> guidBytes = stackalloc byte[16];
|
||||
guidBytes[08] = 1;
|
||||
|
||||
Reference in New Issue
Block a user