Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffer.BlockCopy Analyzer #5242

Merged
merged 13 commits into from
Aug 13, 2021
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ CA1846 | Performance | Info | PreferAsSpanOverSubstring, [Documentation](https:/
CA1847 | Performance | Info | UseStringContainsCharOverloadWithSingleCharactersAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)
CA1848 | Performance | Hidden | LoggerMessageDefineAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1848)
CA2017 | Reliability | Warning | LoggerMessageDefineAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2017)
CA2018 | Reliability | Warning | BufferBlockCopyLengthAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2018)
CA2250 | Usage | Info | UseCancellationTokenThrowIfCancellationRequested, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2250)
CA2251 | Usage | Hidden | UseStringEqualsOverStringCompare, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2251)
CA2252 | Usage | Info | DetectPreviewFeatureAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2252)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,15 @@
<value>and all other platforms</value>
<comment>This call site is reachable on: 'windows' 10.0.2000 and later, and all other platforms</comment>
</data>
<data name="BufferBlockCopyLengthMessage" xml:space="preserve">
<value>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</value>
</data>
<data name="BufferBlockCopyLengthTitle" xml:space="preserve">
<value>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</value>
</data>
<data name="BufferBlockCopyDescription" xml:space="preserve">
<value>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</value>
</data>
<data name="PreferAsSpanOverSubstringDescription" xml:space="preserve">
<value>'AsSpan' is more efficient then 'Substring'. 'Substring' performs an O(n) string copy, while 'AsSpan' does not and has a constant cost.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Runtime
{
/// <summary>
/// Check for the intended use of .Length on arrays passed into Buffer.BlockCopy
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class BufferBlockCopyLengthAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA2018";
private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.BufferBlockCopyLengthTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.BufferBlockCopyLengthMessage), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.BufferBlockCopyDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));

internal static DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(RuleId,
s_localizableTitle,
s_localizableMessage,
DiagnosticCategory.Reliability,
RuleLevel.BuildWarning,
s_localizableDescription,
isPortedFxCopRule: false,
isDataflowRule: false);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(context =>
{
if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemBuffer, out INamedTypeSymbol? bufferType))
{
return;
}

INamedTypeSymbol arrayType = context.Compilation.GetSpecialType(SpecialType.System_Array);
INamedTypeSymbol int32Type = context.Compilation.GetSpecialType(SpecialType.System_Int32);

// Buffer.BlockCopy(Array src, int srcOffset, Array dst, int dstOffset, int count)
ParameterInfo[] blockCopyParameterInfo = {
ParameterInfo.GetParameterInfo(arrayType, false, 1, false),
ParameterInfo.GetParameterInfo(int32Type, false, 1, false),
ParameterInfo.GetParameterInfo(arrayType, false, 1, false),
ParameterInfo.GetParameterInfo(int32Type, false, 1, false),
ParameterInfo.GetParameterInfo(int32Type, false, 1, false)};

IMethodSymbol? blockCopyMethod = bufferType
.GetMembers("BlockCopy")
.OfType<IMethodSymbol>()
.GetFirstOrDefaultMemberWithParameterInfos(blockCopyParameterInfo);

if (blockCopyMethod is null)
{
return;
}

IPropertySymbol arrayLengthProperty = arrayType
.GetMembers("Length")
.OfType<IPropertySymbol>()
.FirstOrDefault();

context.RegisterOperationAction(context =>
{
var invocationOperation = (IInvocationOperation)context.Operation;

if (!invocationOperation.TargetMethod.Equals(blockCopyMethod))
{
return;
}

ImmutableArray<IArgumentOperation> arguments = IOperationExtensions.GetArgumentsInParameterOrder(invocationOperation.Arguments);

IArgumentOperation sourceArgument = arguments[0];
IArgumentOperation destinationArgument = arguments[2];
IArgumentOperation countArgument = arguments[4];

bool CheckArrayLengthLocalReference(IArgumentOperation targetArgument, IPropertyReferenceOperation lengthPropertyArgument)
{
if (targetArgument.Value is IConversionOperation targetArgValue)
{
if (lengthPropertyArgument.Instance.GetReferencedMemberOrLocalOrParameter() == targetArgValue.Operand.GetReferencedMemberOrLocalOrParameter())
{
IArrayTypeSymbol countArgumentArrayTypeSymbol = (IArrayTypeSymbol)lengthPropertyArgument.Instance.Type;
if (countArgumentArrayTypeSymbol.ElementType.SpecialType != SpecialType.System_Byte &&
countArgumentArrayTypeSymbol.ElementType.SpecialType != SpecialType.System_SByte)
{
return true;
}
}
}
else if (targetArgument.Value is ILocalReferenceOperation local)
{
if (lengthPropertyArgument.Instance.GetReferencedMemberOrLocalOrParameter() == local.Local)
{
return true;
}
}
return false;
}

bool CheckLengthPropertyOnByteOrSByteArrays(IPropertyReferenceOperation countArgument)
{
if (countArgument.Property.Equals(arrayLengthProperty))
{
return CheckArrayLengthLocalReference(sourceArgument, countArgument) || CheckArrayLengthLocalReference(destinationArgument, countArgument);
}

return false;
}

if (countArgument.Value is IPropertyReferenceOperation countArgumentValue && CheckLengthPropertyOnByteOrSByteArrays(countArgumentValue))
{
context.ReportDiagnostic(countArgument.Value.CreateDiagnostic(Rule));
}
else
{
if (countArgument.Value is not ILocalReferenceOperation localReferenceOperation)
{
return;
}

SemanticModel semanticModel = countArgument.SemanticModel;
CancellationToken cancellationToken = context.CancellationToken;

ILocalSymbol localArgumentDeclaration = localReferenceOperation.Local;

SyntaxReference declaringSyntaxReference = localArgumentDeclaration.DeclaringSyntaxReferences.FirstOrDefault();
if (declaringSyntaxReference is null)
{
return;
}

if (semanticModel.GetOperationWalkingUpParentChain(declaringSyntaxReference.GetSyntax(cancellationToken), cancellationToken) is not IVariableDeclaratorOperation variableDeclaratorOperation)
{
return;
}

IVariableInitializerOperation variableInitializer = variableDeclaratorOperation.Initializer;

if (variableInitializer is not null && variableInitializer.Value is IPropertyReferenceOperation variableInitializerPropertyReference &&
CheckLengthPropertyOnByteOrSByteArrays(variableInitializerPropertyReference))
{
context.ReportDiagnostic(countArgument.Value.CreateDiagnostic(Rule));
}
else if (variableDeclaratorOperation.Parent is IVariableDeclarationOperation variableDeclarationOperation &&
variableDeclarationOperation.Initializer is not null &&
variableDeclarationOperation.Initializer.Value is IPropertyReferenceOperation variableInitializerPropertyReferenceVB &&
CheckLengthPropertyOnByteOrSByteArrays(variableInitializerPropertyReferenceVB))
{
context.ReportDiagnostic(countArgument.Value.CreateDiagnostic(Rule));
}
}
},
OperationKind.Invocation);
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@
<target state="translated">Nepoužívat nezabezpečený deserializátor BinaryFormatter</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyDescription">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyLengthMessage">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyLengthTitle">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</target>
<note />
</trans-unit>
<trans-unit id="CallGCSuppressFinalizeCorrectlyDescription">
<source>A method that is an implementation of Dispose does not call GC.SuppressFinalize; or a method that is not an implementation of Dispose calls GC.SuppressFinalize; or a method calls GC.SuppressFinalize and passes something other than this (Me in Visual Basic).</source>
<target state="translated">Metoda, která je implementací metody Dispose, nevolá GC.SuppressFinalize, nebo metoda, která není implementací metody Dispose, volá GC.SuppressFinalize, nebo metoda volá GC.SuppressFinalize a předává něco jiného než this (Me v jazyce Visual Basic).</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@
<target state="translated">Nicht den unsicheren BinaryFormatter zur Deserialisierung verwenden</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyDescription">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyLengthMessage">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyLengthTitle">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</target>
<note />
</trans-unit>
<trans-unit id="CallGCSuppressFinalizeCorrectlyDescription">
<source>A method that is an implementation of Dispose does not call GC.SuppressFinalize; or a method that is not an implementation of Dispose calls GC.SuppressFinalize; or a method calls GC.SuppressFinalize and passes something other than this (Me in Visual Basic).</source>
<target state="translated">Eine Methode, die eine Implementierung von "Dispose" ist, ruft "GC.SuppressFinalize" nicht auf, oder eine Methode, die keine Implementierung von "Dispose" ist, ruft "GC.SuppressFinalize" auf, oder eine Methode ruft "GC.SuppressFinalize" auf und übergibt etwas anderes als dies ("Me" in Visual Basic).</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@
<target state="translated">No usar el deserializador no seguro BinaryFormatter</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyDescription">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyLengthMessage">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument. Using 'Array.Length' may not match the number of bytes that needs to be copied.</target>
<note />
</trans-unit>
<trans-unit id="BufferBlockCopyLengthTitle">
<source>'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</source>
<target state="new">'Buffer.BlockCopy' expects the number of bytes to be copied for the 'count' argument</target>
<note />
</trans-unit>
<trans-unit id="CallGCSuppressFinalizeCorrectlyDescription">
<source>A method that is an implementation of Dispose does not call GC.SuppressFinalize; or a method that is not an implementation of Dispose calls GC.SuppressFinalize; or a method calls GC.SuppressFinalize and passes something other than this (Me in Visual Basic).</source>
<target state="translated">Un método que es una implementación de Dispose no llama a GC.SuppressFinalize, o un método que no es una implementación de Dispose llama a GC.SuppressFinalize, o un método llama a GC.SuppressFinalize y pasa algo distinto de "this" (Me en Visual Basic).</target>
Expand Down
Loading