Author: spouliot
Date: 2008-02-15 13:21:20 -0500 (Fri, 15 Feb 2008)
New Revision: 95781
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/ChangeLog
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/DoubleCheckLockingRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/NonConstantStaticFieldsShouldNotBeVisibleRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/WriteStaticFieldFromInstanceMethodRule.cs
Log:
2008-02-15 Sebastien Pouliot <[EMAIL PROTECTED]>
* DoubleCheckLockingRule.cs
* NonConstantStaticFieldsShouldNotBeVisibleRule.cs
* WriteStaticFieldFromInstanceMethodRule.cs:
Update rules wrt framework changes.
NonConstantStaticFieldsShouldNotBeVisibleRule.
[Backport of r95541]
Modified: trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/ChangeLog
===================================================================
--- trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/ChangeLog
2008-02-15 18:18:43 UTC (rev 95780)
+++ trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/ChangeLog
2008-02-15 18:21:20 UTC (rev 95781)
@@ -1,7 +1,15 @@
+2008-02-15 Sebastien Pouliot <[EMAIL PROTECTED]>
+
+ * DoubleCheckLockingRule.cs
+ * NonConstantStaticFieldsShouldNotBeVisibleRule.cs
+ * WriteStaticFieldFromInstanceMethodRule.cs:
+ Update rules wrt framework changes.
+
2008-02-12 Sebastien Pouliot <[EMAIL PROTECTED]>
* Gendarme.Rules.Concurrency.xml.in: Fix copy-paste mistake for
- NonConstantStaticFieldsShouldNotBeVisibleRule
+ NonConstantStaticFieldsShouldNotBeVisibleRule.
+ [Backport of r95541]
2008-01-21 Sebastien Pouliot <[EMAIL PROTECTED]>
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/DoubleCheckLockingRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/DoubleCheckLockingRule.cs
2008-02-15 18:18:43 UTC (rev 95780)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/DoubleCheckLockingRule.cs
2008-02-15 18:21:20 UTC (rev 95781)
@@ -1,15 +1,33 @@
-/*
-* DoubleCheckLockingRule.cs: looks for instances of double-check locking.
-*
-* Authors:
-* Aaron Tomb <[EMAIL PROTECTED]>
-*
-* Copyright (c) 2005 Aaron Tomb and the contributors listed
-* in the ChangeLog.
-*
-* This is free software, distributed under the MIT/X11 license.
-* See the included MIT.X11 file for details.
-**********************************************************************/
+//
+// Gendarme.Rules.Concurrency.DoubleCheckLockingRule.cs:
+// looks for instances of double-check locking.
+//
+// Authors:
+// Aaron Tomb <[EMAIL PROTECTED]>
+// Sebastien Pouliot <[EMAIL PROTECTED]>
+//
+// Copyright (c) 2005 Aaron Tomb
+// Copyright (C) 2008 Novell, Inc (http://www.novell.com)
+//
+// Permission is hereby granted, free of charge, to any person obtaining
+// a copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to
+// permit persons to whom the Software is furnished to do so, subject to
+// the following conditions:
+//
+// The above copyright notice and this permission notice shall be
+// included in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+//
using System;
using System.Collections;
@@ -19,12 +37,17 @@
namespace Gendarme.Rules.Concurrency {
- public class DoubleCheckLockingRule : IMethodRule {
+ // note: the rule only report a single double-lock per method
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ [Problem ("This method uses the unreliable double-check locking
technique.")]
+ [Solution ("Remove the lock check that occurs outside of the protected
region.")]
+ public class DoubleCheckLockingRule : Rule, IMethodRule {
+
+ public RuleResult CheckMethod (MethodDefinition method)
{
+ // rule doesn't apply if the method has no IL
if (!method.HasBody)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
Hashtable comparisons = new Hashtable ();
InstructionCollection insns = method.Body.Instructions;
@@ -51,10 +74,8 @@
if(insn.Offset >=
(int)monitorOffsetList[mcount - 1])
continue;
- Location location = new
Location (method, insn.Offset);
- Message message = new
Message ("A double check locking detected",
- location,
MessageType.Warning);
- return new
MessageCollection (message);
+ Runner.Report (method,
insn, Severity.Medium, Confidence.High, String.Empty);
+ return
RuleResult.Failure;
}
}
comparisons[insns[i]] = twoBefore;
@@ -65,7 +86,7 @@
if(mcount > 0)
monitorOffsetList.RemoveAt(monitorOffsetList.Count - 1);
}
- return runner.RuleSuccess;
+ return RuleResult.Success;
}
private static bool IsMonitorMethod(Instruction insn, string
methodName)
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/NonConstantStaticFieldsShouldNotBeVisibleRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/NonConstantStaticFieldsShouldNotBeVisibleRule.cs
2008-02-15 18:18:43 UTC (rev 95780)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/NonConstantStaticFieldsShouldNotBeVisibleRule.cs
2008-02-15 18:21:20 UTC (rev 95781)
@@ -3,8 +3,10 @@
//
// Authors:
// Andreas Noever <[EMAIL PROTECTED]>
+// Sebastien Pouliot <[EMAIL PROTECTED]>
//
// (C) 2008 Andreas Noever
+// Copyright (C) 2008 Novell, Inc (http://www.novell.com)
//
// Permission is hereby granted, free of charge, to any person obtaining
// a copy of this software and associated documentation files (the
@@ -33,25 +35,25 @@
namespace Gendarme.Rules.Concurrency {
- public class NonConstantStaticFieldsShouldNotBeVisibleRule : ITypeRule {
+ [Problem ("This type has some static fields that are not constant. They
may represent problems in multithreaded applications.")]
+ [Solution ("Change the field to read-only, mark it [ThreadStatic] or
make it non visible outside the assembly.")]
+ public class NonConstantStaticFieldsShouldNotBeVisibleRule : Rule,
ITypeRule {
- public MessageCollection CheckType (TypeDefinition type, Runner
runner)
+ public const string Message = "This static field is not
InitOnly (readonly). Multithreaded access to this field needs to be
syncronized.";
+
+ public RuleResult CheckType (TypeDefinition type)
{
+ // rule does not apply to interface and enumerations
if (type.IsInterface || type.IsEnum)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
- MessageCollection results = null;
foreach (FieldDefinition field in type.Fields) {
if (field.IsStatic && field.IsVisible () &&
!field.IsInitOnly && !field.IsLiteral) {
- if (results == null)
- results = new MessageCollection
();
- Location loc = new Location (field);
- Message msg = new Message ("This static
field is not InitOnly (readonly). Multithreaded access to this field needs to
be syncronized.", loc, MessageType.Warning);
- results.Add (msg);
+ Runner.Report (field, Severity.Medium,
Confidence.High, Message);
}
}
- return results;
+ return Runner.CurrentRuleResult;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/WriteStaticFieldFromInstanceMethodRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/WriteStaticFieldFromInstanceMethodRule.cs
2008-02-15 18:18:43 UTC (rev 95780)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/WriteStaticFieldFromInstanceMethodRule.cs
2008-02-15 18:21:20 UTC (rev 95781)
@@ -4,7 +4,7 @@
// Authors:
// Sebastien Pouliot <[EMAIL PROTECTED]>
//
-// Copyright (C) 2006 Novell, Inc (http://www.novell.com)
+// Copyright (C) 2006,2008 Novell, Inc (http://www.novell.com)
//
// Permission is hereby granted, free of charge, to any person obtaining
// a copy of this software and associated documentation files (the
@@ -32,45 +32,43 @@
using Mono.Cecil;
using Mono.Cecil.Cil;
+
using Gendarme.Framework;
+using Gendarme.Framework.Rocks;
namespace Gendarme.Rules.Concurrency {
- public class WriteStaticFieldFromInstanceMethodRule : IMethodRule {
+ [Problem ("This instance method writes to static fields. This may cause
problem with multiple instances and in multithreaded applications.")]
+ [Solution ("Move initialization to the static constructor or ensure
appropriate locking.")]
+ public class WriteStaticFieldFromInstanceMethodRule : Rule, IMethodRule
{
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
- // #1 - rule apply only if the method has a body (e.g.
p/invokes, icalls don't)
- if (!method.HasBody)
- return runner.RuleSuccess;
+ // rule does not apply if
+ // - the method has no body (e.g. p/invokes, icalls
don't)
+ // - the method is static
+ // - the method was generated by the compiler or a tool
+ if (!method.HasBody || method.IsStatic ||
method.IsGeneratedCode ())
+ return RuleResult.DoesNotApply;
- // #2 - rule apply only is the method isn't static
(static method can modify static fields)
- if (method.IsStatic)
- return runner.RuleSuccess;
-
// *** ok, the rule applies! ***
- MessageCollection results = null;
-
- // #2 - look for stsfld instructions on static fields
+ // look for stsfld instructions on static fields
+
foreach (Instruction ins in method.Body.Instructions) {
- switch (ins.OpCode.Name) {
- case "stsfld":
+ switch (ins.OpCode.Code) {
+ case Code.Stsfld:
FieldDefinition fd = (ins.Operand as
FieldDefinition);
- if ((fd != null) && fd.IsStatic) {
- Location loc = new Location
(method, ins.Offset);
- string text = String.Format
("The field '{0}' ({1}) is being set in an instance method.", fd.Name,
fd.Attributes);
- Message msg = new Message
(text, loc, MessageType.Warning);
- if (results == null)
- results = new
MessageCollection (msg);
- else
- results.Add (msg);
+ // skip instance fields and generated
static field (likely by the compiler)
+ if ((fd != null) && fd.IsStatic &&
!fd.IsGeneratedCode ()) {
+ string text = String.Format
("The static field '{0}', of type '{1}'. is being set in an instance method.",
fd.Name, fd.FieldType);
+ Runner.Report (method, ins,
Severity.Medium, Confidence.High, text);
}
break;
}
}
- return results;
+ return Runner.CurrentRuleResult;
}
}
}
_______________________________________________
Mono-patches maillist - [email protected]
http://lists.ximian.com/mailman/listinfo/mono-patches
