diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs
index ced4b6eba2a..a8b6aff7ca1 100644
--- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs
+++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs
@@ -31,6 +31,7 @@ OperationKindEx.FieldReference when operation.ToFieldReference() is var fieldRef
OperationKindEx.LocalReference => operation.ToLocalReference().Local,
OperationKindEx.ParameterReference => operation.ToParameterReference().Parameter,
OperationKindEx.Argument => operation.ToArgument().Value.TrackedSymbol(),
+ OperationKindEx.DeclarationExpression => IDeclarationExpressionOperationWrapper.FromOperation(operation).Expression.TrackedSymbol(),
_ => null
};
@@ -102,6 +103,38 @@ public static IOperation UnwrapConversion(this IOperation operation)
return operation;
}
+ ///
+ public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) =>
+ ArgumentValue(invocation.Arguments, parameterName);
+
+ ///
+ public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) =>
+ ArgumentValue(objectCreation.Arguments, parameterName);
+
+ ///
+ public static IOperation ArgumentValue(this IPropertyReferenceOperationWrapper propertyReference, string parameterName) =>
+ ArgumentValue(propertyReference.Arguments, parameterName);
+
+ ///
+ public static IOperation ArgumentValue(this IRaiseEventOperationWrapper raiseEvent, string parameterName) =>
+ ArgumentValue(raiseEvent.Arguments, parameterName);
+
+ ///
+ /// Returns the argument value corresponding to . For parameter an IArrayCreationOperation is returned.
+ ///
+ private static IOperation ArgumentValue(ImmutableArray arguments, string parameterName)
+ {
+ foreach (var operation in arguments)
+ {
+ var argument = operation.ToArgument();
+ if (argument.Parameter.Name == parameterName)
+ {
+ return argument.Value;
+ }
+ }
+ return null;
+ }
+
private static T? As(this IOperation operation, OperationKind kind, Func fromOperation) where T : struct =>
operation.Kind == kind ? fromOperation(operation) : null;
}
diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs
index e4cd44b82cc..80efcc64dc2 100644
--- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs
+++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs
@@ -94,6 +94,25 @@ public override ProgramState[] PostProcess(SymbolicContext context)
};
}
}
+ else if (context.Operation.Instance.AsObjectCreation() is { } objectCreation
+ && objectCreation.Type.Is(KnownType.System_Threading_Mutex)
+ && context.Operation.Parent.AsAssignment() is { } assignment
+ && assignment.Target.TrackedSymbol() is { } symbol
+ && objectCreation.ArgumentValue("initiallyOwned") is { } initiallyOwned
+ && context.State[initiallyOwned] is { } initiallyOwnedValue
+ && initiallyOwnedValue.HasConstraint(BoolConstraint.True))
+ {
+ lastSymbolLock[symbol] = objectCreation.ToSonar();
+ return objectCreation.ArgumentValue("createdNew") is { } createdNew
+ && createdNew.TrackedSymbol() is { } trackedCreatedNew
+ ? new[]
+ {
+ AddLock(context, objectCreation.WrappedOperation).Preserve(symbol).SetSymbolConstraint(trackedCreatedNew, BoolConstraint.True),
+ context.SetSymbolConstraint(trackedCreatedNew, BoolConstraint.False),
+ }
+ : AddLock(context, objectCreation.WrappedOperation).Preserve(symbol).ToArray();
+ }
+
return base.PostProcess(context);
}
@@ -131,21 +150,7 @@ ProgramState ProcessCondition(ISymbol lockSymbol)
protected override ProgramState PostProcessSimple(SymbolicContext context)
{
- if (context.Operation.Instance.AsObjectCreation() is { } objectCreation)
- {
- if (objectCreation.Type.Is(KnownType.System_Threading_Mutex)
- && context.Operation.Parent.AsAssignment() is { } assignment
- && objectCreation.Arguments.Length > 0
- && objectCreation.Arguments.First().ToArgument().Value is { } firstArgument
- && context.State[firstArgument] is { } firstArgumentValue
- && firstArgumentValue.HasConstraint(BoolConstraint.True)
- && assignment.Target.TrackedSymbol() is { } symbol)
- {
- lastSymbolLock[symbol] = objectCreation.ToSonar();
- return AddLock(context, objectCreation.WrappedOperation).Preserve(symbol);
- }
- }
- else if (context.Operation.Instance.AsInvocation() is { } invocation)
+ if (context.Operation.Instance.AsInvocation() is { } invocation)
{
// ToDo: we ignore the number of parameters for now.
if (invocation.TargetMethod.IsAny(KnownType.System_Threading_Monitor, "Enter", "TryEnter"))
@@ -191,7 +196,7 @@ private ProgramState AddLock(SymbolicContext context, ISymbol symbol)
}
lastSymbolLock[symbol] = context.Operation;
- return context.SetSymbolConstraint(symbol, LockConstraint.Held).Preserve(symbol);
+ return context.SetSymbolConstraint(symbol, LockConstraint.Held).SetSymbolConstraint(symbol, ObjectConstraint.NotNull).Preserve(symbol);
}
private ProgramState RemoveLock(SymbolicContext context, ISymbol symbol)
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs
index 71a8977a9d2..510e2a99ab4 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs
@@ -18,6 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
+using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Operations;
using SonarAnalyzer.SymbolicExecution.Roslyn;
using StyleCop.Analyzers.Lightup;
@@ -59,11 +60,140 @@ public void TrackedSymbol_FieldReference_IsFieldSymbol(string assignment)
assignmentTarget.TrackedSymbol().Should().Be(fieldReferenceSymbol);
}
+ [DataTestMethod]
+ [DataRow(@"(int i, int j) = (1, 1)")]
+ [DataRow(@"(var i, var j) = (1, 1)")]
+ [DataRow(@"int.TryParse(string.Empty, out int value)")]
+ [DataRow(@"int.TryParse(string.Empty, out var value)")]
+ public void TrackedSymbol_DeclarationExpression(string assignment)
+ {
+ var code = $"public class C {{ void Method() {{ {assignment}; }} }}";
+ var graph = TestHelper.CompileCfgCS(code);
+ var allDeclarations = graph.Blocks[1].Operations.SelectMany(x => x.DescendantsAndSelf()).Where(x => x.Kind == OperationKindEx.DeclarationExpression).Select(IDeclarationExpressionOperationWrapper.FromOperation).ToArray();
+ allDeclarations.Should().NotBeEmpty();
+ allDeclarations.Should().AllSatisfy(x =>
+ x.WrappedOperation.TrackedSymbol().Should().NotBeNull().And.BeAssignableTo()
+ .Which.GetSymbolType().Should().NotBeNull().And.BeAssignableTo()
+ .Which.SpecialType.Should().Be(SpecialType.System_Int32));
+ }
+
+ [TestMethod]
+ public void TrackedSymbol_DeclarationExpression_Tuple()
+ {
+ var code = $"public class C {{ void Method() {{ var (i, j) = (1, 1); }} }}";
+ var graph = TestHelper.CompileCfgCS(code);
+ var allDeclarations = graph.Blocks[1].Operations.SelectMany(x => x.DescendantsAndSelf()).Where(x => x.Kind == OperationKindEx.DeclarationExpression).Select(IDeclarationExpressionOperationWrapper.FromOperation).ToArray();
+ var declaration = allDeclarations.Should().ContainSingle().Which.WrappedOperation;
+ declaration.Kind.Should().Be(OperationKindEx.DeclarationExpression);
+ var declarationExpression = IDeclarationExpressionOperationWrapper.FromOperation(declaration).Expression;
+ declarationExpression.Kind.Should().Be(OperationKindEx.Tuple);
+ declaration.TrackedSymbol().Should().BeNull();
+ }
+
[TestMethod]
public void TrackedSymbol_SimpleAssignment_IsNull()
{
var simpleAssignment = TestHelper.CompileCfgBodyCS("var a = true; bool b; b = a;").Blocks[1].Operations[0];
simpleAssignment.TrackedSymbol().Should().BeNull();
}
+
+ [DataTestMethod]
+ [DataRow("""1, "Test" """, "intParam", 1)]
+ [DataRow("""1, "Test" """, "stringParam", "Test")]
+ [DataRow("""1, "Test" """, "optionalBoolParam", true)]
+ [DataRow("""1, "Test" """, "optionalIntParam", 1)]
+ [DataRow("""1, "Test", true """, "optionalBoolParam", true)]
+ [DataRow("""stringParam: "Test", intParam: 1 """, "stringParam", "Test")]
+ [DataRow("""stringParam: "Test", intParam: 1, optionalIntParam: 2 """, "optionalIntParam", 2)]
+ public void ArgumentValue_ObjectCreation(string objectCreationArguments, string parameterName, object expected)
+ {
+ var testClass = $$"""
+ class Test
+ {
+ Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1) { }
+
+ static void Create() =>
+ new Test({{objectCreationArguments}});
+ }
+ """;
+ var (tree, model) = TestHelper.CompileCS(testClass);
+ var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single();
+ var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation));
+ operation.ArgumentValue(parameterName).Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(expected);
+ }
+
+ [DataTestMethod]
+ [DataRow(""" """)]
+ [DataRow(""" "param1", "param2" """, "param1", "param2")]
+ [DataRow(""" null, null """, null, null)]
+ [DataRow(""" new[] {"param1", "param2"} """, "param1", "param2")]
+ public void ArgumentValue_ObjectCreation_Params(string arguments, params string[] expected)
+ {
+ var testClass = $$"""
+ class Test
+ {
+ Test(params string[] stringParams) { }
+
+ static void Create() =>
+ new Test({{arguments}});
+ }
+ """;
+ var (tree, model) = TestHelper.CompileCS(testClass);
+ var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single();
+ var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation));
+ var argument = operation.ArgumentValue("stringParams").Should().NotBeNull().And.BeAssignableTo().Subject;
+ var argumentArray = IArrayCreationOperationWrapper.FromOperation(argument);
+ var result = argumentArray.Initializer.ElementValues.Select(x => x.ConstantValue.Value).ToArray();
+ result.Should().BeEquivalentTo(expected);
+ }
+
+ [TestMethod]
+ public void ArgumentValue_Invocation()
+ {
+ const string testClass = """
+ class Test
+ {
+ static void M(string stringParam) => M("param");
+ }
+ """;
+ var (tree, model) = TestHelper.CompileCS(testClass);
+ var invocation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single();
+ var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocation));
+ operation.ArgumentValue("stringParam").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be("param");
+ }
+
+ [TestMethod]
+ public void ArgumentValue_PropertyReference()
+ {
+ const string testClass = """
+ class Test
+ {
+ int this[int index] => this[1];
+ }
+ """;
+ var (tree, model) = TestHelper.CompileCS(testClass);
+ var elementAccess = tree.GetRoot().DescendantNodesAndSelf().OfType().Single();
+ var operation = IPropertyReferenceOperationWrapper.FromOperation(model.GetOperation(elementAccess));
+ operation.ArgumentValue("index").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(1);
+ }
+
+ [TestMethod]
+ public void ArgumentValue_RaiseEvent()
+ {
+ const string testClass = """
+ Imports System
+ Public Class C
+ Event SomeEvent As EventHandler
+ Public Sub M()
+ RaiseEvent SomeEvent(Nothing, EventArgs.Empty)
+ End Sub
+ End Class
+ """;
+ var (tree, model) = TestHelper.CompileVB(testClass);
+ var raiseEvent = tree.GetRoot().DescendantNodesAndSelf().OfType().Single();
+ var operation = IRaiseEventOperationWrapper.FromOperation(model.GetOperation(raiseEvent));
+ operation.ArgumentValue("sender").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().BeNull();
+ operation.ArgumentValue("e").Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(OperationKindEx.FieldReference);
+ }
}
}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs
index cb268ad80c6..463861374eb 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs
@@ -94,7 +94,7 @@ public static void CompliantFromDocs(string mutexName, MutexSecurity mutexSecuri
// Enter the mutex, and hold it until the program exits.
try
{
- m.WaitOne(); // Noncompliant FP
+ m.WaitOne();
}
catch (UnauthorizedAccessException ex)
{
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs
index c503d7fd517..769963b62ee 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs
@@ -23,8 +23,6 @@ public void Noncompliant(Foo foo)
var m3 = Mutex.OpenExisting("x");
m3.WaitOne(); // Noncompliant
- var m4 = new Mutex();
-
foo.instanceMutex.WaitOne(); // FN
Foo.staticMutex.WaitOne(); // Noncompliant
@@ -35,7 +33,6 @@ public void Noncompliant(Foo foo)
m1.ReleaseMutex();
m2.ReleaseMutex();
m3.ReleaseMutex();
- m4.ReleaseMutex();
foo.instanceMutex.ReleaseMutex();
Foo.staticMutex.ReleaseMutex();
}
@@ -47,6 +44,34 @@ public void Noncompliant(Foo foo)
m3.Dispose();
}
+ public void MutexWasCreatedAndReleased(bool arg)
+ {
+ var m = new Mutex(initiallyOwned: true, "bar", out var wasCreated); // Noncompliant
+ if (wasCreated && arg)
+ {
+ m.ReleaseMutex();
+ }
+ }
+
+ public void MutexWasCreatedAndNotReleased(bool arg)
+ {
+ var m = new Mutex(initiallyOwned: true, "bar", out var wasCreated); // Compliant
+ if (!wasCreated && arg)
+ {
+ // wasCreated is false here, indicating that the lock is NOT (yet) held
+ m.ReleaseMutex(); // This is a release without a previous lock. This is not covered by this rule.
+ }
+ }
+
+ public void MutexWasCreatedWithoutOwnership(bool arg)
+ {
+ var m = new Mutex(initiallyOwned: false, "bar", out var wasCreated); // Compliant. The lock must still be acquired by requesting it.
+ if (wasCreated && arg)
+ {
+ m.ReleaseMutex();
+ }
+ }
+
public void Noncompliant2(Mutex paramMutex, Mutex paramMutex2)
{
// 'true' means it owns the mutex if no exception gets thrown
@@ -339,11 +364,11 @@ public void MutextAquireByConstructor_MultipleVariableDeclaration()
public void MutextAquireByConstructor_SwitchExpression(int x)
{
var m = x switch
- {
- 1 => new Mutex(),
- 2 => new Mutex(new bool()),
- 3 => new Mutex(true, "bar", out var mutextCreated), // FN
- };
+ {
+ 1 => new Mutex(),
+ 2 => new Mutex(new bool()),
+ 3 => new Mutex(true, "bar", out var mutextCreated), // FN
+ };
if (cond)
{