Bug 46103 - Review various possible bugs detected by Jtest
Summary: Review various possible bugs detected by Jtest
Status: NEW
Alias: None
Product: Batik - Now in Jira
Classification: Unclassified
Component: Test Tools (show other bugs)
Version: 1.7
Hardware: All All
: P3 minor
Target Milestone: ---
Assignee: Batik Developer's Mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-27 18:24 UTC by Mirko Raner
Modified: 2008-11-21 04:10 UTC (History)
1 user (show)



Attachments
Jtest Static Analysis for Apache Batik (84.86 KB, application/octet-stream)
2008-10-27 18:27 UTC, Mirko Raner
Details
Jtest Static Analysis for Apache Batik (trunk) (71.09 KB, application/octet-stream)
2008-10-28 14:27 UTC, Mirko Raner
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mirko Raner 2008-10-27 18:24:49 UTC
Hello, Batik Developers!

I work as a Systems Engineer for Parasoft, and we regularly test our Automated Error Prevention products on open source projects. We just finished running our flagship product, Jtest, on some projects and I chose to have a look at Apache Batik. This exercise already helped us to find some false positives in our analysis, but I thought I'd share the results with the developer community as well. That way everybody benefits. My initial run using Parasoft's Recommended Rules produced over 6000 warnings, but since nobody would look at such a large amount of warnings I boiled it down to a configuration that included only the 10 most essential analysis rules and produced 66 warnings.

I have attached the report that was produced by Jtest. While it is possible that all the detected bugs are effectively inconsequential the report points to a number of problems where code clearly does not do what it is supposed to do. So, I definitely suggest that somebody familiar with the code base have a look at it.

The report is based on the sources for Batik 1.7, so some of the line numbers might have shifted in the meantime. Please let us know if this analysis proved valuable for you or if you found any false positives or have other related feedback!
Comment 1 Mirko Raner 2008-10-27 18:27:05 UTC
Created attachment 22781 [details]
Jtest Static Analysis for Apache Batik
Comment 2 Helder Magalhães 2008-10-28 02:19:32 UTC
This seems interesting input. :-)

Could a similar report be created against the trunk code [1]?

Thank you for sharing,

 Helder Magalhães

[1] http://xmlgraphics.apache.org/batik/download.cgi#Subversion+repository
Comment 3 info 2008-10-28 04:51:20 UTC
Thanks for the effort and thanks for sharing the results.

You might wonder, why the use of such a sophisticated tool like JTest doesnt produce more 'sophisticated' bug-warnings: in the last two years the Batik code-base has been analysed from time to time with Findbugs and the detectors in the IntelliJ IDE. So most of the interesting (and mechanically findable) bugs have been found and removed already...
The names of the authors show, that these are parts of Batik, which havent been touched since quite a while - 'stable code'.

Most of the 'sync on non-final' - warnings are known to me - no urgent action needed. I'd prefer to make those fields final or private, but that changes the API of the class. 
I will have a look on those conditional expressions with fixed outcome.
 
If you want to test the capabilities of your tool, you might try an older version of Batik - there have been some interesting bugs. 

OTOH, when you have a new, state-of-the-art detector waiting to see some non-trivial real world code, then Batik is the right food for it.

greetings
dieter von holten



Comment 4 Mirko Raner 2008-10-28 14:27:10 UTC
Created attachment 22785 [details]
Jtest Static Analysis for Apache Batik (trunk)

As requested by Helder, I re-ran a slightly modified configuration on the SVN trunk over lunch and attached the report. I took out the TRS.SOUF rule about possible synchronization problems, as Dieter mentioned that he was well aware of the issue. In turn, I enabled a rule for detecting the broken Double Checked Locking pattern, which promptly found a violation. Also, I added detection of unused package-private fields and methods. Unless there is some fancy Reflection API access going on somewhere those fields and methods can probably be removed.

The attached results were all produced by simple static analysis, i.e. pattern matching assisted by some additional collected information about type hierarchies, mutability, etc. Jtest can also perform a full flow analysis, which is essentially a partial simulation of executed code and finds bugs that are caused by scenarios that may involve multiple classes. In the reports I attached, this type of analysis was not enabled, mainly for two reasons: (1) it can obviously take a very long time to run, which would exceed the scope of our internal testing of open source projects, and (2) the bugs indeed turn out to be more "sophisticated", involving multiple correlated stack traces which are typically very hard to explore without the corresponding UI in Jtest. Anyway, if I get a chance and can find a machine with some spare CPU cycles I can try running flow analysis and send you the report. But don't hold your breath for that right now.
Comment 5 Helder Magalhães 2008-11-21 04:10:59 UTC
(In reply to comment #4)
> As requested by Helder, I re-ran a slightly modified configuration on the SVN
> trunk over lunch and attached the report.

Thank you. I've only quickly review, given that I don't have enough Batik expertize for further analyzing the potential issues, but it definitely deserves a deeper analysis and/or follow up actions. ;-)


> Anyway, if I get a chance and can find a machine with some spare CPU cycles
> I can try  running flow analysis and send you the report.

Yes, it would also be great whenever possible. :-)