Author: spouliot
Date: 2008-02-15 14:19:05 -0500 (Fri, 15 Feb 2008)
New Revision: 95794
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/ChangeLog
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/GetLastErrorMustBeCalledRightAfterPInvokeRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/MarshalStringsInPInvokeDeclarationsRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/PInvokeShouldNotBeVisibleRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/UseManagedAlternativesToPInvokeRule.cs
Log:
2008-02-15 Sebastien Pouliot <[EMAIL PROTECTED]>
* GetLastErrorMustBeCalledRightAfterPInvokeRule.cs
* MarshalStringsInPInvokeDeclarationsRule.cs
* PInvokeShouldNotBeVisibleRule.cs
* UseManagedAlternativesToPInvokeRule.cs:
Update rules wrt framework changes.
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/ChangeLog
===================================================================
--- trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/ChangeLog
2008-02-15 19:17:14 UTC (rev 95793)
+++ trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/ChangeLog
2008-02-15 19:19:05 UTC (rev 95794)
@@ -1,3 +1,11 @@
+2008-02-15 Sebastien Pouliot <[EMAIL PROTECTED]>
+
+ * GetLastErrorMustBeCalledRightAfterPInvokeRule.cs
+ * MarshalStringsInPInvokeDeclarationsRule.cs
+ * PInvokeShouldNotBeVisibleRule.cs
+ * UseManagedAlternativesToPInvokeRule.cs:
+ Update rules wrt framework changes.
+
2008-01-30 Sebastien Pouliot <[EMAIL PROTECTED]>
* GetLastErrorMustBeCalledRightAfterPInvokeRule.cs: Changed to
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/GetLastErrorMustBeCalledRightAfterPInvokeRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/GetLastErrorMustBeCalledRightAfterPInvokeRule.cs
2008-02-15 19:17:14 UTC (rev 95793)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/GetLastErrorMustBeCalledRightAfterPInvokeRule.cs
2008-02-15 19:19:05 UTC (rev 95794)
@@ -26,17 +26,21 @@
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//
+using System;
using System.Collections.Generic;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Gendarme.Framework;
+using Gendarme.Framework.Helpers;
using Gendarme.Framework.Rocks;
namespace Gendarme.Rules.Interoperability {
- public class GetLastErrorMustBeCalledRightAfterPInvokeRule :
IMethodRule {
+ [Problem ("GetLastError() should be called immediately after this the
P/Invoke call.")]
+ [Solution ("Move the call to GetLastError just after the P/Invoke
call.")]
+ public class GetLastErrorMustBeCalledRightAfterPInvokeRule : Rule,
IMethodRule {
struct Branch {
public readonly Instruction Instruction;
@@ -49,6 +53,8 @@
}
}
+ private const string Message = "GetLastError() should be called
immediately after this the PInvoke call.";
+
private const string GetLastError = "System.Int32
System.Runtime.InteropServices.Marshal::GetLastWin32Error()";
private List<string> AllowedCalls;
@@ -62,7 +68,7 @@
List<Branch> branches = new List<Branch> ();
- private bool CheckPInvoke (MethodDefinition pinvoke,
Instruction startInstruction)
+ private bool CheckPInvoke (Instruction startInstruction)
{
branches.Clear ();
branches.Add (new Branch (startInstruction.Next,
false));
@@ -102,8 +108,9 @@
break;
if (alternatives != null) {
- if (alternatives is
Instruction) {
- branches.AddIfNew (new
Branch ((Instruction) alternatives, dirty));
+ Instruction alt_ins =
(alternatives as Instruction);
+ if (alt_ins != null) {
+ branches.AddIfNew (new
Branch (alt_ins, dirty));
} else {
Instruction [] alts =
(Instruction []) alternatives;
foreach (Instruction
altIns in alts)
@@ -124,13 +131,12 @@
return true;
}
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
+ // rule does not apply if the method has no IL
if (!method.HasBody)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
- MessageCollection results = null;
-
foreach (Instruction ins in method.Body.Instructions) {
switch (ins.OpCode.Code) {
case Code.Call:
@@ -150,21 +156,18 @@
break;
// check if GetLastError is called near
enough this pinvoke call
- if (CheckPInvoke (pinvoke, ins))
+ if (CheckPInvoke (ins))
break;
- if (results == null)
- results = new MessageCollection
();
- Location loc = new Location (method,
ins.Offset);
- Message msg = new Message
("GetLastError() should be called immediately after this the PInvoke call.",
loc, MessageType.Error);
- results.Add (msg);
+ // code might not work if an error
occurs
+ Runner.Report (method, ins,
Severity.High, Confidence.High, String.Empty);
break;
default:
break;
}
}
- return results;
+ return Runner.CurrentRuleResult;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/MarshalStringsInPInvokeDeclarationsRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/MarshalStringsInPInvokeDeclarationsRule.cs
2008-02-15 19:17:14 UTC (rev 95793)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/MarshalStringsInPInvokeDeclarationsRule.cs
2008-02-15 19:19:05 UTC (rev 95794)
@@ -33,38 +33,39 @@
namespace Gendarme.Rules.Interoperability {
- public class MarshalStringsInPInvokeDeclarationsRule : IMethodRule {
+ [Problem ("Marshaling information for string types is incomplete and
what is required may be different from what you expected the default to be.")]
+ [Solution ("Add [DllImport CharSet=] on the method or [MarshalAs] on
the parameter(s)")]
+ public class MarshalStringsInPInvokeDeclarationsRule : Rule,
IMethodRule {
private static bool IsStringOrSBuilder (TypeReference reference)
{
- TypeReference original = reference.GetOriginalType ();
- return original.FullName == "System.String" ||
original.FullName == "System.Text.StringBuilder";
+ switch (reference.GetOriginalType ().FullName) {
+ case "System.String":
+ case "System.Text.StringBuilder":
+ return true;
+ default:
+ return false;
+ }
}
- private static void AddBadParameterMessage (ref
MessageCollection messages, ParameterDefinition param)
+ public RuleResult CheckMethod (MethodDefinition method)
{
- if (messages == null)
- messages = new MessageCollection ();
+ // rule does not apply to non-pinvoke methods
+ if (!method.IsPInvokeImpl)
+ return RuleResult.DoesNotApply;
- Location loc = new Location (param.Method as
MethodDefinition);
- string text = string.Format ("Parameter '{0}', of type
'{1}', does not have [MarshalAs] attribute, yet no [DllImport CharSet=] is set
for the method '{2}'.",
- param.Name, param.ParameterType.Name,
param.Method.Name);
- Message msg = new Message (text, loc,
MessageType.Error);
- messages.Add (msg);
- }
+ if (!method.PInvokeInfo.IsCharSetNotSpec)
+ return RuleResult.Success;
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
- {
- if (!method.IsPInvokeImpl ||
!method.PInvokeInfo.IsCharSetNotSpec)
- return runner.RuleSuccess;
-
- MessageCollection messages = runner.RuleSuccess;
foreach (ParameterDefinition parameter in
method.Parameters) {
if (IsStringOrSBuilder
(parameter.ParameterType) && (parameter.MarshalSpec == null)) {
- AddBadParameterMessage (ref messages,
parameter);
+ string text = string.Format ("Parameter
'{0}', of type '{1}', does not have [MarshalAs] attribute, yet no [DllImport
CharSet=] is set for the method '{2}'.",
+ parameter.Name,
parameter.ParameterType.Name, parameter.Method.Name);
+ Runner.Report (parameter,
Severity.High, Confidence.Total, text);
}
}
- return messages;
+
+ return Runner.CurrentRuleResult;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/PInvokeShouldNotBeVisibleRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/PInvokeShouldNotBeVisibleRule.cs
2008-02-15 19:17:14 UTC (rev 95793)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/PInvokeShouldNotBeVisibleRule.cs
2008-02-15 19:19:05 UTC (rev 95794)
@@ -31,32 +31,30 @@
using Mono.Cecil;
using Gendarme.Framework;
+using Gendarme.Framework.Rocks;
namespace Gendarme.Rules.Interoperability {
- public class PInvokeShouldNotBeVisibleRule : IMethodRule {
+ [Problem ("P/Invoke declarations should not be visible outside of the
assembly.")]
+ [Solution ("Wrap the p/invoke call into a managed class/method and
include parameters, and result, validation(s).")]
+ public class PInvokeShouldNotBeVisibleRule : Rule, IMethodRule {
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
+ // rule does not apply to non-p/invoke
if (!method.IsPInvokeImpl)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
+
+ // rule applies
+
+ // ok if method is not visible (this include it's
declaring type too)
+ if (!method.IsVisible ())
+ return RuleResult.Success;
- if (!method.IsPublic)
- return runner.RuleSuccess;
-
- TypeDefinition type = (TypeDefinition)
method.DeclaringType;
-
- while (type.IsNested) {
- if (!type.IsNestedPublic)
- return runner.RuleSuccess;
- type = (TypeDefinition) type.DeclaringType;
- }
- if (!type.IsPublic)
- return runner.RuleSuccess;
-
- Location loc = new Location (method);
- Message msg = new Message ("P/Invoke declarations
should not be visible outside of the assembly.", loc, MessageType.Warning);
- return new MessageCollection (msg);
+ // code will work (low) but it's bad design
(non-fx-like validations) and makes
+ // it easier to expose security vulnerabilities
+ Runner.Report (method, Severity.Low, Confidence.Total,
String.Empty);
+ return RuleResult.Failure;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/UseManagedAlternativesToPInvokeRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/UseManagedAlternativesToPInvokeRule.cs
2008-02-15 19:17:14 UTC (rev 95793)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Interoperability/UseManagedAlternativesToPInvokeRule.cs
2008-02-15 19:19:05 UTC (rev 95794)
@@ -37,7 +37,9 @@
namespace Gendarme.Rules.Interoperability {
- public class UseManagedAlternativesToPInvokeRule : IMethodRule {
+ [Problem ("There is a potential managed API for the p/invoke
declaration.")]
+ [Solution ("Use the suggested managed alternative to your p/invoke call
and remove it's declaration.")]
+ public class UseManagedAlternativesToPInvokeRule : Rule, IMethodRule {
private struct PInvokeCall {
@@ -89,7 +91,7 @@
}
}
- private class ManagedAlternatives {
+ private sealed class ManagedAlternatives {
private List<string> alternatives = new List<string> ();
private TargetRuntime runtime;
@@ -122,59 +124,45 @@
}
}
- private static Dictionary<PInvokeCall, ManagedAlternatives>
managedAlternatives;
+ private static Dictionary<PInvokeCall, ManagedAlternatives>
managedAlternatives =
+ new Dictionary<PInvokeCall, ManagedAlternatives> () {
+ { new PInvokeCall ("kernel32.dll", "Sleep"),
new ManagedAlternatives ("System.Threading.Thread::Sleep ()") },
+ { new PInvokeCall ("kernel32.dll",
"FindFirstFile"), new ManagedAlternatives ("System.IO.Directory::GetDirectories
()", "System.IO.Directory::GetFiles ()",
"System.IO.Directory::GetFileSystemEntries ()") },
+ { new PInvokeCall ("kernel32.dll", "ReadFile"),
new ManagedAlternatives ("System.IO.FileStream") },
+ { new PInvokeCall ("kernel32.dll",
"WaitForMultipleObjects"), new ManagedAlternatives
("System.Threading.WaitHandle::WaitAny ()",
"System.Threading.WaitHandle::WaitAll ()") },
+ { new PInvokeCall ("kernel32.dll",
"GetLastError"), new ManagedAlternatives
("System.Runtime.InteropServices.Marshal::GetLastWin32Error ()") },
+ { new PInvokeCall ("user32.dll", "MessageBox"),
new ManagedAlternatives ("System.Windows.Forms.MessageBox::Show ()") },
+ { new PInvokeCall ("kernel32.dll", "Beep"), new
ManagedAlternatives (TargetRuntime.NET_2_0, "System.Console::Beep ()") },
+ { new PInvokeCall ("winmm.dll", "PlaySound"),
new ManagedAlternatives (TargetRuntime.NET_2_0, "System.Media.SoundPlayer") }
+ };
- static UseManagedAlternativesToPInvokeRule ()
- {
- managedAlternatives = new Dictionary<PInvokeCall,
ManagedAlternatives> ();
- managedAlternatives.Add (new PInvokeCall
("kernel32.dll", "Sleep"),
- new ManagedAlternatives
("System.Threading.Thread::Sleep ()"));
- managedAlternatives.Add (new PInvokeCall
("kernel32.dll", "FindFirstFile"),
- new ManagedAlternatives
("System.IO.Directory::GetDirectories ()", "System.IO.Directory::GetFiles ()",
"System.IO.Directory::GetFileSystemEntries ()"));
- managedAlternatives.Add (new PInvokeCall
("kernel32.dll", "ReadFile"),
- new ManagedAlternatives
("System.IO.FileStream"));
- managedAlternatives.Add (new PInvokeCall
("kernel32.dll", "WaitForMultipleObjects"),
- new ManagedAlternatives
("System.Threading.WaitHandle::WaitAny ()",
"System.Threading.WaitHandle::WaitAll ()"));
- managedAlternatives.Add (new PInvokeCall
("kernel32.dll", "GetLastError"),
- new ManagedAlternatives
("System.Runtime.InteropServices.Marshal::GetLastWin32Error ()"));
- managedAlternatives.Add (new PInvokeCall ("user32.dll",
"MessageBox"),
- new ManagedAlternatives
("System.Windows.Forms.MessageBox::Show ()"));
- managedAlternatives.Add (new PInvokeCall
("kernel32.dll", "Beep"),
- new ManagedAlternatives
(TargetRuntime.NET_2_0, "System.Console::Beep ()"));
- managedAlternatives.Add (new PInvokeCall ("winmm.dll",
"PlaySound"),
- new ManagedAlternatives
(TargetRuntime.NET_2_0, "System.Media.SoundPlayer"));
- }
-
private static ManagedAlternatives GetManagedAlternatives
(MethodDefinition method, TargetRuntime runtime)
{
string moduleName = method.PInvokeInfo.Module.Name;
PInvokeCall callInfo = new PInvokeCall (moduleName,
method.Name);
if (managedAlternatives.ContainsKey (callInfo)) {
ManagedAlternatives alts = managedAlternatives
[callInfo];
- if (IsRuntimeVersionGreaterOrEqual (runtime,
alts.Runtime))
+ if (runtime >= alts.Runtime)
return alts;
}
return null;
}
- private static bool IsRuntimeVersionGreaterOrEqual
(TargetRuntime a, TargetRuntime b)
+ public RuleResult CheckMethod (MethodDefinition method)
{
- return (int) a >= (int) b; // as of now it works (and
should work further)
- }
-
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
- {
+ // rule does not apply to non-pinvoke methods
if (!method.IsPInvokeImpl)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
+ // rule apply, looks for alternatives
+
ManagedAlternatives alternatives =
GetManagedAlternatives (method, method.DeclaringType.Module.Assembly.Runtime);
if (alternatives == null)
- return runner.RuleSuccess;
+ return RuleResult.Success;
- Location loc = new Location (method);
string message = string.Format ("Do not perform
platform-dependent call ({0}) if it can be avoided. Use (one of) the following
alternative(s) provided by .NET Framework: {1}.", method.Name, alternatives);
- Message msg = new Message (message, loc,
MessageType.Warning);
- return new MessageCollection (msg);
+ Runner.Report (method, Severity.Low, Confidence.High,
message);
+ return RuleResult.Failure;
}
}
}
_______________________________________________
Mono-patches maillist - [email protected]
http://lists.ximian.com/mailman/listinfo/mono-patches
