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

New Analyzer: Prevent behavioral change in built-in operators for IntPtr and UIntPtr #6153

Merged
merged 8 commits into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.NetCore.Analyzers.Runtime;

namespace Microsoft.NetCore.CSharp.Analyzers.Runtime
{
/// <summary>
///
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpPreventNumericIntPtrUIntPtrBehavioralChanges : PreventNumericIntPtrUIntPtrBehavioralChanges
{
private const string IntPtr = nameof(IntPtr);
private const string UIntPtr = nameof(UIntPtr);

protected override bool IsWithinCheckedContext(IOperation operation)
{
var parent = operation.Parent.Syntax;
while (parent != null)
{
switch (parent)
{
case CheckedExpressionSyntax expression:
return expression.Kind() == SyntaxKind.CheckedExpression;
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
case CheckedStatementSyntax statement:
return statement.Kind() == SyntaxKind.CheckedStatement;
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
case MethodDeclarationSyntax:
return false;
}

parent = parent.Parent;
}

return false;
}

protected override bool IsAliasUsed(ISymbol? symbol)
{
if (symbol != null)
{
foreach (SyntaxReference reference in symbol.DeclaringSyntaxReferences)
{
SyntaxNode definition = reference.GetSyntax();

while (definition is VariableDeclaratorSyntax)
{
definition = definition.Parent;
}

var type = GetType(definition);

if (IdentifierNameIsIntPtrOrUIntPtr(type))
{
return false;
}
}
}

return true;
}

private static bool IdentifierNameIsIntPtrOrUIntPtr(ExpressionSyntax? syntax) =>
syntax is IdentifierNameSyntax identifierName &&
identifierName.Identifier.Text is IntPtr or UIntPtr;

protected override bool IsAliasUsed(SyntaxNode syntax)
{
if (syntax is CastExpressionSyntax castSyntax)
{
if (IdentifierNameIsIntPtrOrUIntPtr(castSyntax.Expression) ||
IdentifierNameIsIntPtrOrUIntPtr(castSyntax.Type))
{
return false;
}
}

return true;
}

private static TypeSyntax? GetType(SyntaxNode syntax) =>
syntax switch
{
VariableDeclarationSyntax fieldDeclaration => fieldDeclaration.Type,
ParameterSyntax parameter => parameter.Type,
CastExpressionSyntax cast => cast.Type,
_ => null,
};
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ CA1853 | Performance | Info | DoNotGuardDictionaryRemoveByContainsKey, [Document
CA1854 | Performance | Info | PreferDictionaryTryGetValueAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854)
CA1855 | Performance | Info | UseSpanClearInsteadOfFillAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1855)
CA2019 | Reliability | Info | UseThreadStaticCorrectly, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2019)
CA2020 | Reliability | Warning | NumericIntPtrUIntPtrAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2020)
CA2259 | Usage | Warning | UseThreadStaticCorrectly, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2259)
CA5404 | Security | Disabled | DoNotDisableTokenValidationChecks, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5404)
CA5405 | Security | Disabled | DoNotAlwaysSkipTokenValidationInDelegates, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5405)
Original file line number Diff line number Diff line change
Expand Up @@ -1971,4 +1971,19 @@
<data name="UseSpanClearInsteadOfFillTitle" xml:space="preserve">
<value>Prefer 'Clear' over 'Fill'</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesDescription" xml:space="preserve">
<value>Built in '+', '-' and explicit casting operators now may or may not throw when overflowing, while it was before. Wrap the expression with 'checked' or 'unchecked' statements to restore old behavior.</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesOperatorNotThrowMessage" xml:space="preserve">
<value>Built in operator '{0}' now may not throw when overflowing in unchecked context. Wrap the expression with 'checked' statement to restore old behavior.</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesOperatorThrowsMessage" xml:space="preserve">
<value>Built in operator '{0}' now may throw when overflowing in checked context. Wrap the expression with 'unchecked' statement to restore old behavior.</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesTitle" xml:space="preserve">
<value>Prevent from behavioral change</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage" xml:space="preserve">
<value>Built in explicit conversion '{0}' now may throw when overflowing in checked context. Wrap the expression with 'unchecked' statement to restore old behavior.</value>
</data>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

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

namespace Microsoft.NetCore.Analyzers.Runtime
{
using static MicrosoftNetCoreAnalyzersResources;
/// <summary>
/// CA2020: Detects Behavioral Changes introduced by new Numeric IntPtr UIntPtr feature
/// </summary>
public abstract class PreventNumericIntPtrUIntPtrBehavioralChanges : DiagnosticAnalyzer
{
internal const string RuleId = "CA2020";

internal static readonly DiagnosticDescriptor OperatorThrowsRule = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesTitle)),
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesOperatorThrowsMessage)),
DiagnosticCategory.Reliability,
RuleLevel.BuildWarning,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved

internal static readonly DiagnosticDescriptor ConversionThrowsRule = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesTitle)),
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage)),
DiagnosticCategory.Reliability,
RuleLevel.BuildWarning,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor NotThrowRule = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesTitle)),
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesOperatorNotThrowMessage)),
DiagnosticCategory.Reliability,
RuleLevel.BuildWarning,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(OperatorThrowsRule, NotThrowRule);

protected abstract bool IsWithinCheckedContext(IOperation operation);

protected abstract bool IsAliasUsed(ISymbol? syntaxReferences);

protected abstract bool IsAliasUsed(SyntaxNode syntax);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(context =>
{
if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemRuntimeCompilerServicesRuntimeFeature, out var runtimeFeatureType) ||
!runtimeFeatureType.GetMembers("NumericIntPtr").OfType<IFieldSymbol>().Any())
{
// Numeric IntPtr feature not available
return;
}

context.RegisterOperationAction(context =>
{
if (context.Operation is IBinaryOperation binaryOperation &&
binaryOperation.IsAdditionOrSubstractionOperation(out var binaryOperator) &&
binaryOperation.IsChecked)
{
if ((binaryOperation.LeftOperand.Type?.SpecialType == SpecialType.System_IntPtr ||
binaryOperation.LeftOperand.Type?.SpecialType == SpecialType.System_UIntPtr) &&
IsConversionFromInt32(binaryOperation.RightOperand) &&
!IsAliasUsed(GetSymbol(binaryOperation.LeftOperand)))
{
context.ReportDiagnostic(binaryOperation.CreateDiagnostic(OperatorThrowsRule, binaryOperator));
}
}
else if (context.Operation is IConversionOperation conversionOperation)
{
var operation = conversionOperation.WalkDownConversion(c => c.IsImplicit); // get innermost converesion
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
if (operation is IConversionOperation explicitConversion &&
explicitConversion.OperatorMethod == null) // Built in conversion
{
if (IsWithinCheckedContext(explicitConversion)) // explicitConversion.IsChecked somehow not working
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
{
if (IsIntPtrToOrFromVoidPtrConversion(explicitConversion.Type, explicitConversion.Operand.Type) &&
!IsAliasUsed(GetSymbol(explicitConversion.Operand)))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(ConversionThrowsRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
else if (IsIntPtrToOrFromVoidPtrConversion(explicitConversion.Operand.Type, explicitConversion.Type) &&
!IsAliasUsed(explicitConversion.Syntax))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(ConversionThrowsRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
}
else // unchecked context
{
if ((IsLongToIntPtrConversion(explicitConversion.Type, explicitConversion.Operand.Type) ||
IsULongToUIntPtrConversion(explicitConversion.Type, explicitConversion.Operand.Type)) &&
!IsAliasUsed(explicitConversion.Syntax))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(NotThrowRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
else if ((IsIntPtrToIntConversion(explicitConversion.Type, explicitConversion.Operand.Type) ||
IsUIntPtrToUIntConversion(explicitConversion.Type, explicitConversion.Operand.Type)) &&
!IsAliasUsed(GetSymbol(explicitConversion.Operand)))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(NotThrowRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
}
}
}
},
OperationKind.Binary, OperationKind.Conversion);
});

static string PopulateConversionString(ITypeSymbol type, ITypeSymbol operand)
{
string typeName = type.Name;
string operandName = operand.Name;

if (type is IPointerTypeSymbol pointer)
{
typeName = $"*{pointer.PointedAtType.Name}";
}

if (operand is IPointerTypeSymbol pointerOp)
{
operandName = $"*{pointerOp.PointedAtType.Name}";
}

return $"({typeName}){operandName}";
}

static ISymbol? GetSymbol(IOperation operation) =>
operation switch
{
IFieldReferenceOperation fieldReference => fieldReference.Field,
IParameterReferenceOperation parameter => parameter.Parameter,
ILocalReferenceOperation local => local.Local,
_ => null,
};

static bool IsConversionFromInt32(IOperation operation) =>
operation is IConversionOperation conversion &&
conversion.Operand.Type.SpecialType == SpecialType.System_Int32;

static bool IsIntPtrToOrFromVoidPtrConversion(ITypeSymbol pointerType, ITypeSymbol intPtrType) =>
intPtrType.SpecialType == SpecialType.System_IntPtr &&
pointerType is IPointerTypeSymbol pointer && pointer.PointedAtType.SpecialType == SpecialType.System_Void;

static bool IsLongToIntPtrConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_IntPtr &&
operandType.SpecialType == SpecialType.System_Int64;

static bool IsIntPtrToIntConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_Int32 &&
operandType.SpecialType == SpecialType.System_IntPtr;

static bool IsULongToUIntPtrConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_UIntPtr &&
operandType.SpecialType == SpecialType.System_UInt64;

static bool IsUIntPtrToUIntConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_UInt32 &&
operandType.SpecialType == SpecialType.System_UIntPtr;
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -2092,6 +2092,31 @@
<target state="translated">Zvažte možnost použít metodu StringBuilder.Append(char) tam, kde je to možné</target>
<note />
</trans-unit>
<trans-unit id="PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage">
<source>Built in explicit conversion '{0}' now may throw when overflowing in checked context. Wrap the expression with 'unchecked' statement to restore old behavior.</source>
<target state="new">Built in explicit conversion '{0}' now may throw when overflowing in checked context. Wrap the expression with 'unchecked' statement to restore old behavior.</target>
<note />
</trans-unit>
<trans-unit id="PreventNumericIntPtrUIntPtrBehavioralChangesDescription">
<source>Built in '+', '-' and explicit casting operators now may or may not throw when overflowing, while it was before. Wrap the expression with 'checked' or 'unchecked' statements to restore old behavior.</source>
<target state="new">Built in '+', '-' and explicit casting operators now may or may not throw when overflowing, while it was before. Wrap the expression with 'checked' or 'unchecked' statements to restore old behavior.</target>
<note />
</trans-unit>
<trans-unit id="PreventNumericIntPtrUIntPtrBehavioralChangesOperatorNotThrowMessage">
<source>Built in operator '{0}' now may not throw when overflowing in unchecked context. Wrap the expression with 'checked' statement to restore old behavior.</source>
<target state="new">Built in operator '{0}' now may not throw when overflowing in unchecked context. Wrap the expression with 'checked' statement to restore old behavior.</target>
<note />
</trans-unit>
<trans-unit id="PreventNumericIntPtrUIntPtrBehavioralChangesOperatorThrowsMessage">
<source>Built in operator '{0}' now may throw when overflowing in checked context. Wrap the expression with 'unchecked' statement to restore old behavior.</source>
<target state="new">Built in operator '{0}' now may throw when overflowing in checked context. Wrap the expression with 'unchecked' statement to restore old behavior.</target>
<note />
</trans-unit>
<trans-unit id="PreventNumericIntPtrUIntPtrBehavioralChangesTitle">
<source>Prevent from behavioral change</source>
<target state="new">Prevent from behavioral change</target>
<note />
</trans-unit>
<trans-unit id="ProvideCorrectArgumentToEnumHasFlagDescription">
<source>'Enum.HasFlag' method expects the 'enum' argument to be of the same 'enum' type as the instance on which the method is invoked and that this 'enum' is marked with 'System.FlagsAttribute'. If these are different 'enum' types, an unhandled exception will be thrown at runtime. If the 'enum' type is not marked with 'System.FlagsAttribute' the call will always return 'false' at runtime.</source>
<target state="translated">Metoda Enum.HasFlag očekává, že argument enum bude mít stejný typ enum jako instance, ve které se metoda zavolala, a že tento výčet enum bude označený jako System.FlagsAttribute. Pokud existuje více různých typů enum, vyvolá se za běhu neošetřená výjimka. Pokud se typ enum neoznačí jako System.FlagsAttribute, volání vždy vrátí false.</target>
Expand Down
Loading