Gendarme 2.6 Preview 1 is ready for download

[Sebastien Pouliot]

Good News Gendarmes!

The win32 installer and 'binary zip' files for Gendarme 2.6 preview 1
are now available in ohloh [1].

The win32 installer is new (not based on the previous installers) so
you either have to remove previous Gendarme versions *or* you can keep
them in parallel (2.6 will install in another directory so both will
remain functional).

It's unlikely that there will be many previews for 2.6 so your testing
is very appreciated.

Thanks,
Sebastien

[1] http://www.ohloh.net/p/gendarme

[MarkusSchaber]

Hi,

On 5 Dez., 18:29, Sebastien Pouliot <sebastien.poul...@gmail.com>
wrote:
> Good News Gendarmes!

Yes, Indeed!

> The win32 installer and 'binary zip' files for Gendarme 2.6 preview 1
> are now available in ohloh [1].

Thanks for your work! I'll see whether I find time to re-test our
assembly with the new version, and see whether it does not crash this
time. :-)

Thanks,
Markus

[MarkusSchaber]

Hi,

On 7 Dez., 09:02, MarkusSchaber <m...@soloplan.de> wrote:
> > The win32 installer and 'binary zip' files for Gendarme 2.6 preview 1
> > are now available in ohloh [1].
>
> Thanks for your work! I'll see whether I find time to re-test our
> assembly with the new version, and see whether it does not crash this
> time. :-)

I just tested it, it works fine with our DLL now.

Thanks for your work!,
markus

[MarkusSchaber]

Hi,

I just tested it on a directory with over 100 DLLs, thinking that this
all are .NET dlls. But it seems that there are some non-.NET DLLs in
this directory. Instead of rejecting them, the Gendarme UI crashes
with an uncaught exception:

Mono.Cecil.Binary.ImageFormatException: The image is not a managed
assembly
bei Mono.Cecil.StructureReader..ctor(ImageReader ir)
bei Mono.Cecil.StructureReader..ctor(ImageReader ir, Boolean
manifestOnly)
bei Mono.Cecil.AssemblyFactory.GetAssembly(String file)
bei Gendarme.Wizard.UpdateAssemblies()
bei Gendarme.Wizard.PrepareAnalyze()
bei Gendarme.Wizard.UpdateAnalyzeUI()
bei System.Windows.Forms.Control.OnClick(EventArgs e)
bei System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent)
bei System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons
button, Int32 clicks)
bei System.Windows.Forms.Control.WndProc(Message& m)
bei System.Windows.Forms.ButtonBase.WndProc(Message& m)
bei System.Windows.Forms.Button.WndProc(Message& m)
bei System.Windows.Forms.Control.ControlNativeWindow.WndProc
(Message& m)
bei System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32
msg, IntPtr wparam, IntPtr lparam)

Maybe you could catch this exception, log a warning and otherwise
ignore the assembly.

[MarkusSchaber]

Hi,

I just tried Gendarme on some other assemblies, using the default
options (except checking the "unlimited" with various results:

- IKVM.Runtime.DLL Version=0.38.0.2 (Worked okay, foudn 214 defects)

- IKVM.OpenJDK.ClassLibrary Version 0.30.0.2 (Seems to hang
indefinitely after finding 4284 defects)

- jts-1.10.dll (translated from jts-1.10.jar via ikvmc.exe): Aborted
with an "unexpected error" after finding 317 defects:
Rule:
Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule

Target: System.Boolean
com.vividsolutions.jts.algorithm.BoundaryNodeRule/
Mod2BoundaryNodeRule::isInBoundary(System.Int32)

Stack trace: System.ArgumentOutOfRangeException: Der Index lag
außerhalb des Bereichs. Er muss nicht negativ und kleiner als die
Auflistung sein.
Parametername: index
bei System.ThrowHelper.ThrowArgumentOutOfRangeException()
bei System.Collections.Generic.List`1.RemoveAt(Int32 index)
bei
Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule.CheckMethod
(MethodDefinition method)
bei Gendarme.Framework.Runner.OnMethod(RunnerEventArgs e)
bei Gendarme.Framework.Runner.Run()
bei Gendarme.GuiRunner.Execute()

As JTS is free software, I can send you this DLL for further analysis.

- Another one of our in-house DLLs:
Stack trace: System.InvalidOperationException: Der Vorgang ist
aufgrund des aktuellen Zustands des Objekts ungültig.
bei Mono.Cecil.Pdb.PdbCciReader.PopulateFunctions(String file)
bei Mono.Cecil.Pdb.PdbCciReader..ctor(ModuleDefinition module,
String file)
bei Mono.Cecil.Pdb.PdbFactory.CreateReader(ModuleDefinition module,
String assemblyFileName)
bei Gendarme.Framework.Rocks.ModuleRocks.LoadDebuggingSymbols
(ModuleDefinition self)
bei Gendarme.Framework.Runner.Initialize()
bei Gendarme.GuiRunner.Execute()

And another suggestion: Allow drag-and-drop of DLLs from the explorer
(or maybe also VS) onto the Gendarme Wizard.

[Sebastien Pouliot]

Great to hear!

Sebastien

[Sebastien Pouliot]

On Tue, 2009-12-08 at 10:05 -0800, MarkusSchaber wrote:
> Hi,
>
> I just tried Gendarme on some other assemblies, using the default
> options (except checking the "unlimited" with various results:
>
> - IKVM.Runtime.DLL Version=0.38.0.2 (Worked okay, foudn 214 defects)
>
> - IKVM.OpenJDK.ClassLibrary Version 0.30.0.2 (Seems to hang
> indefinitely after finding 4284 defects)

What's the size of the .dll ? if it's large then some rules (like the
smells duplication) can take a LOT of time.

> - jts-1.10.dll (translated from jts-1.10.jar via ikvmc.exe): Aborted
> with an "unexpected error" after finding 317 defects:
> Rule:
> Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule
>
> Target: System.Boolean
> com.vividsolutions.jts.algorithm.BoundaryNodeRule/
> Mod2BoundaryNodeRule::isInBoundary(System.Int32)
>
> Stack trace: System.ArgumentOutOfRangeException: Der Index lag
> außerhalb des Bereichs. Er muss nicht negativ und kleiner als die
> Auflistung sein.
> Parametername: index
> bei System.ThrowHelper.ThrowArgumentOutOfRangeException()
> bei System.Collections.Generic.List`1.RemoveAt(Int32 index)
> bei
> Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule.CheckMethod
> (MethodDefinition method)
> bei Gendarme.Framework.Runner.OnMethod(RunnerEventArgs e)
> bei Gendarme.Framework.Runner.Run()
> bei Gendarme.GuiRunner.Execute()
>
> As JTS is free software, I can send you this DLL for further analysis.

Markus, can you add the file to the group files section (or open a bug
report on http://bugzilla.novell.com, Mono Tools, Gendarme).

Rolf, can you have a look at it (after the full moon ;-)

>
> - Another one of our in-house DLLs:
> Stack trace: System.InvalidOperationException: Der Vorgang ist
> aufgrund des aktuellen Zustands des Objekts ungültig.
> bei Mono.Cecil.Pdb.PdbCciReader.PopulateFunctions(String file)
> bei Mono.Cecil.Pdb.PdbCciReader..ctor(ModuleDefinition module,
> String file)
> bei Mono.Cecil.Pdb.PdbFactory.CreateReader(ModuleDefinition module,
> String assemblyFileName)
> bei Gendarme.Framework.Rocks.ModuleRocks.LoadDebuggingSymbols
> (ModuleDefinition self)
> bei Gendarme.Framework.Runner.Initialize()
> bei Gendarme.GuiRunner.Execute()

That looks like a Cecil bug. Could you share the .dll / .pdb with
Cecil's author (jb) ?

As for Gendarme I can catch the exception and report it (maybe even
ignore the debugging symbols). Right now the "report" is too simple - I
need to rework this (2.8) so the analysis can go on and report
"warning" (not defects) for such "non-fatal" cases.

> And another suggestion: Allow drag-and-drop of DLLs from the explorer

I'll have a quick look (after 2.6). However keep in mind the wizard was
never meant to be a full featured tool (it would be nice to have one)
but a simple way, with a GUI, to run Gendarme.

In other words I prefer to keep my personal time (contributions are
welcome :-) on other things, including a better gui runner, than
improving the wizard (since anyway it won't stay simple if we keep
adding stuff into it ;-)

Thanks a lot for your feedback!
Sebastien

[Sebastien Pouliot]

Fixed in SVN (r147897/8).

This is not ideal but, for 2.6, non-managed assemblies will simply be
ignored (better than crashing the wizard :-). Hopefully we'll think of
something better for 2.8.

Thanks for the report!
Sebastien

[MarkusSchaber]

Hi, Sebastien,

On 9 Dez., 03:42, Sebastien Pouliot <sebastien.poul...@gmail.com>
wrote:

> > - IKVM.OpenJDK.ClassLibrary Version 0.30.0.2 (Seems to hang
> > indefinitely after finding 4284 defects)
>
> What's the size of the .dll ? if it's large then some rules (like the
> smells duplication) can take a LOT of time.

Its rather large, about 30 MB. I'm just runnign it again in the
background today, and see whether it comes to an end (quadcore CPUs
rule!).

> > - jts-1.10.dll (translated from jts-1.10.jar via ikvmc.exe): Aborted
> > with an "unexpected error" after finding 317 defects:
> > Rule:

[...]

> > As JTS is free software, I can send you this DLL for further analysis.
> Markus, can you add the file to the group files section (or open a bug
> report on http://bugzilla.novell.com, Mono Tools, Gendarme).

I tried to upload the file here, but I did not find any possibility to
do it. Do I need some permission / clearing for file upload?

> > - Another one of our in-house DLLs:
> > Stack trace: System.InvalidOperationException: Der Vorgang ist
> > aufgrund des aktuellen Zustands des Objekts ungültig.
> >    bei Mono.Cecil.Pdb.PdbCciReader.PopulateFunctions(String file)
> >    bei Mono.Cecil.Pdb.PdbCciReader..ctor(ModuleDefinition module,
> > String file)
> >    bei Mono.Cecil.Pdb.PdbFactory.CreateReader(ModuleDefinition module,
> > String assemblyFileName)
> >    bei Gendarme.Framework.Rocks.ModuleRocks.LoadDebuggingSymbols
> > (ModuleDefinition self)
> >    bei Gendarme.Framework.Runner.Initialize()
> >    bei Gendarme.GuiRunner.Execute()
>
> That looks like a Cecil bug. Could you share the .dll / .pdb with
> Cecil's author (jb) ?

I'll see whether I get permission for this, as it is one of our
proprietary DLLs. I might also be able to track it down to a small
example, as I did with the last DLL, but I cannot give any timeframe,
because I don't get any official time budget for that here. :-(

> As for Gendarme I can catch the exception and report it (maybe even
> ignore the debugging symbols). Right now the "report" is too simple - I
> need to rework this (2.8) so the analysis can go on and report
> "warning" (not defects) for such "non-fatal" cases.

It might be that the debugging symbols (pdb) are from a different
compiler run than the DLL itself, so this mimatch might be the cause
of the exception.

Thanks for your work!

[MarkusSchaber]

Hi,

On 9 Dez., 08:44, MarkusSchaber <m...@soloplan.de> wrote:

> On 9 Dez., 03:42, Sebastien Pouliot <sebastien.poul...@gmail.com>
> wrote:

> > > - Another one of our in-house DLLs:
> > > Stack trace: System.InvalidOperationException: Der Vorgang ist
> > > aufgrund des aktuellen Zustands des Objekts ungültig.
> > >    bei Mono.Cecil.Pdb.PdbCciReader.PopulateFunctions(String file)
> > >    bei Mono.Cecil.Pdb.PdbCciReader..ctor(ModuleDefinition module,
> > > String file)
> > >    bei Mono.Cecil.Pdb.PdbFactory.CreateReader(ModuleDefinition module,
> > > String assemblyFileName)
> > >    bei Gendarme.Framework.Rocks.ModuleRocks.LoadDebuggingSymbols
> > > (ModuleDefinition self)
> > >    bei Gendarme.Framework.Runner.Initialize()
> > >    bei Gendarme.GuiRunner.Execute()
>
> > That looks like a Cecil bug. Could you share the .dll / .pdb with
> > Cecil's author (jb) ?
>
> I'll see whether I get permission for this, as it is one of our
> proprietary DLLs. I might also be able to track it down to a small
> example, as I did with the last DLL, but I cannot give any timeframe,
> because I don't get any official time budget for that here. :-(
>

> It might be that the debugging symbols (pdb) are from a different
> compiler run than the DLL itself, so this mimatch might be the cause
> of the exception.

The pdf file has a very different date than the dll. Removing the .pdb
file allows Gendarme to anlayze it without any error. The same is when
I rebuild the files. This was caused by some script we use to update
the DLLs from the autobuilder, and which don't update the pdbs
accordingly.

Maybe Cecil should detect that situation (pdb and dll do not match),
and either throw a defined error / warning, or ignore the pdb.

[MarkusSchaber]

Hi,

On 9 Dez., 08:44, MarkusSchaber <m...@soloplan.de> wrote:
> > > - IKVM.OpenJDK.ClassLibrary Version 0.30.0.2 (Seems to hang
> > > indefinitely after finding 4284 defects)
>
> > What's the size of the .dll ? if it's large then some rules (like the
> > smells duplication) can take a LOT of time.
>
> Its rather large, about 30 MB. I'm just runnign it again in the
> background today, and see whether it comes to an end (quadcore CPUs
> rule!).

I did leave it running for some hours now, and it finally crashed,
after finding 6249846(!) defects:

Rule:
Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule

Target: System.String java.lang.Object::instancehelper_toString
(System.Object)

[Sebastien Pouliot]

On Tue, 2009-12-08 at 23:44 -0800, MarkusSchaber wrote:
> Hi, Sebastien,
>
> On 9 Dez., 03:42, Sebastien Pouliot <sebastien.poul...@gmail.com>
> wrote:
> > > - IKVM.OpenJDK.ClassLibrary Version 0.30.0.2 (Seems to hang
> > > indefinitely after finding 4284 defects)
> >
> > What's the size of the .dll ? if it's large then some rules (like the
> > smells duplication) can take a LOT of time.
>
> Its rather large, about 30 MB. I'm just runnign it again in the
> background today, and see whether it comes to an end (quadcore CPUs
> rule!).
>
> > > - jts-1.10.dll (translated from jts-1.10.jar via ikvmc.exe): Aborted
> > > with an "unexpected error" after finding 317 defects:
> > > Rule:
> [...]
> > > As JTS is free software, I can send you this DLL for further analysis.
> > Markus, can you add the file to the group files section (or open a bug
> > report on http://bugzilla.novell.com, Mono Tools, Gendarme).
>
> I tried to upload the file here, but I did not find any possibility to
> do it. Do I need some permission / clearing for file upload?

You should be able to upload it now. I'll keep a copy of it in my
"testcases" directory for use in the future.

Thanks
Sebastien

[Sebastien Pouliot]

Ah, that makes sense.

> Maybe Cecil should detect that situation (pdb and dll do not match),
> and either throw a defined error / warning, or ignore the pdb.

I wonder if the exception is about that mismatch ?!? I'll ask JB (cc).
If the exception is right it needs a better description :-)

Sebastien

[Sebastien Pouliot]

On Wed, 2009-12-09 at 04:14 -0800, MarkusSchaber wrote:
> Hi,
>
> On 9 Dez., 08:44, MarkusSchaber <m...@soloplan.de> wrote:
> > > > - IKVM.OpenJDK.ClassLibrary Version 0.30.0.2 (Seems to hang
> > > > indefinitely after finding 4284 defects)
> >
> > > What's the size of the .dll ? if it's large then some rules (like the
> > > smells duplication) can take a LOT of time.
> >
> > Its rather large, about 30 MB. I'm just runnign it again in the
> > background today, and see whether it comes to an end (quadcore CPUs
> > rule!).
>
> I did leave it running for some hours now, and it finally crashed,
> after finding 6249846(!) defects:

Heh, I don't think this is a good example (unless the tool maker wants
to select a few rules to get better generated code).

A lot of the rules (e.g. naming) won't like how Java promotes its API.
Also a lot of code is "compiler" generated (I'll bet without a generated
code attribute) and likely contains quite a bit of code duplication.

OTOH it makes a very good test case :-)

> Rule:
> Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule
>
> Target: System.String java.lang.Object::instancehelper_toString
> (System.Object)
>
> Stack trace: System.ArgumentOutOfRangeException: Der Index lag
> außerhalb des Bereichs. Er muss nicht negativ und kleiner als die
> Auflistung sein.
> Parametername: index
> bei System.ThrowHelper.ThrowArgumentOutOfRangeException()
> bei System.Collections.Generic.List`1.RemoveAt(Int32 index)
> bei
> Gendarme.Rules.Interoperability.DelegatesPassedToNativeCodeMustIncludeExceptionHandlingRule.CheckMethod
> (MethodDefinition method)
> bei Gendarme.Framework.Runner.OnMethod(RunnerEventArgs e)
> bei Gendarme.Framework.Runner.Run()
> bei Gendarme.GuiRunner.Execute()

Ok, that looks identical to the one reported earlier in this message
thread, right ? because it looks easier to debug from the former test
case ;-)

Thanks again!
Sebastien

[Jb Evain]

Hey,

On Wed, Dec 9, 2009 at 9:05 AM, MarkusSchaber <m...@soloplan.de> wrote:
>> > > Stack trace: System.InvalidOperationException: Der Vorgang ist
>> > > aufgrund des aktuellen Zustands des Objekts ungültig.
>> > >    bei Mono.Cecil.Pdb.PdbCciReader.PopulateFunctions(String file)
>> > >    bei Mono.Cecil.Pdb.PdbCciReader..ctor(ModuleDefinition module,
>> > > String file)
>> > >    bei Mono.Cecil.Pdb.PdbFactory.CreateReader(ModuleDefinition module,
>> > > String assemblyFileName)
>> > >    bei Gendarme.Framework.Rocks.ModuleRocks.LoadDebuggingSymbols
>> > > (ModuleDefinition self)
>> > >    bei Gendarme.Framework.Runner.Initialize()
>> > >    bei Gendarme.GuiRunner.Execute()
>>
>> > That looks like a Cecil bug. Could you share the .dll / .pdb with
>> > Cecil's author (jb) ?

That's indeed a pdb/assembly version mismatch. Which obviously needs a
better description.

--
Jb Evain <j...@nurv.fr>

[Sebastien Pouliot]

Thanks for confirming!

I think I'll ignore the InvalidOperationException exception when calling
LoadDebuggingSymbols for 2.6 and review this (along with the other
cases) for something better in 2.8.

Sebastien

[MarkusSchaber]

Hi, Sebastien,

On 10 Dez., 02:48, Sebastien Pouliot <sebastien.poul...@gmail.com>
wrote:

I did upload the jts-1.0.dll, but the ikvm library seems to hit the
file size limit.

However, you should be able to download the ikvm version from
Sourceforge: http://sourceforge.net/projects/ikvm/files/

HTH,
markus

[MarkusSchaber]

Hi, Sebastien,

On 10 Dez., 02:59, Sebastien Pouliot <sebastien.poul...@gmail.com>
wrote:

> OTOH it makes a very good test case :-)

Yes, indeed. In case you maintain some automatically running test
suite (unit tests etc.), you should add whatever free software DLL
for .NET you can get. :-)


Markus

[Sebastien Pouliot]

On Dec 10, 7:54 am, Sebastien Pouliot <sebastien.poul...@gmail.com>
wrote:

Ok, fixed in SVN. It looks like we already ignored most error but the
new CCI implementation of the PDB reader use InvalidOperationException
(while the old COM based one did not). In any case the "warnings" are
something to re-work more globally in the future.

Sebastien

[Sebastien Pouliot]

On Dec 10, 9:28 am, MarkusSchaber <m...@soloplan.de> wrote:
> Hi, Sebastien,
>
> On 10 Dez., 02:48, Sebastien Pouliot <sebastien.poul...@gmail.com>
> wrote:
>
> > On Tue, 2009-12-08 at 23:44 -0800, MarkusSchaber wrote:
> > > I tried to upload the file here, but I did not find any possibility to
> > > do it. Do I need some permission / clearing for file upload?
>
> > You should be able to upload it now. I'll keep a copy of it in my
> > "testcases" directory for use in the future.
>
> I did upload the jts-1.0.dll, but the ikvm library seems to hit the
> file size limit.

Thanks, I got it now.

> However, you should be able to download the ikvm version from
> Sourceforge:http://sourceforge.net/projects/ikvm/files/

I'll try to get Rolf to fix it on the above first :-) IKVM is huge
and, like I said, generated code is not a good target for Gendarme
(but it would be for Cecil).

> Yes, indeed. In case you maintain some automatically running test
> suite (unit tests etc.), you should add whatever free software DLL
> for .NET you can get. :-)

It's not as automatic as I would like. Mono is upgrading it's bots so
we should be able to get the tests run at each build. However the
servers are pretty occupied since there are lots of other things to
build. That rules out automating the analysis against large set of
assemblies. I do keep a small (and growing) set of assemblies that
have proven useful to debug gendarme in the past - but it's mostly a
personal archive.

Sebastien

[Rolf Bjarne Kvinge]

> I'll try to get Rolf to fix it on the above first :-) IKVM is huge
> and, like I said, generated code is not a good target for Gendarme
> (but it would be for Cecil).

I had a look, and the code is using branches to choose different ways
to use what's on the stack, and since the rule doesn't bother with the
branches, it ends up with a negative stack size since it evaluated all
ways the stack was used, in pseudo code something like this:

load int into stack location 1
load int into stack location 2
if foo then
add ints (consuming stack location 1 and 2, putting result into
stack location 1)
else
subtract ints (consuming stack location 1 and 2, putting result into
stack location 1)
endif
store result (consuing stack location 1)

The rule goes into both the add and subtract paths, ending up with a
negative stack size at the store instruction.

Unfortunately fixing this will make the rule a lot more complex, so
the quick fix would be to detect that something bad happened and just
stop evaluating that method :|

Rolf

[Sebastien Pouliot]

On Dec 11, 6:47 pm, Rolf Bjarne Kvinge <rolfbja...@gmail.com> wrote:
> > I'll try to get Rolf to fix it on the above first :-) IKVM is huge
> > and, like I said, generated code is not a good target for Gendarme
> > (but it would be for Cecil).
>
> I had a look, and the code is using branches to choose different ways
> to use what's on the stack, and since the rule doesn't bother with the
> branches, it ends up with a negative stack size since it evaluated all
> ways the stack was used, in pseudo code something like this:
>
> load int into stack location 1
> load int into stack location 2
> if foo then
>    add ints (consuming stack location 1 and 2, putting result into
> stack location 1)
> else
>   subtract ints (consuming stack location 1 and 2, putting result into
> stack location 1)
> endif
> store result (consuing stack location 1)
>
> The rule goes into both the add and subtract paths, ending up with a
> negative stack size at the store instruction.
>
> Unfortunately fixing this will make the rule a lot more complex,

It's unlikely that such code will be found (often) so it does not look
like it's worth the extra complexity (for both the rule maintenance
and the extra time it would require to "normal" code).

> so
> the quick fix would be to detect that something bad happened and just
> stop evaluating that method :|

Yes. Please add a FIXME there. I'm looking into adding "analysis
warnings" (for 2.8) and it could be a case where the rule could report
the offending code.

Thanks
Sebastien