Author Topic: C::B and Purify  (Read 10405 times)

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
C::B and Purify
« on: January 27, 2006, 01:58:05 pm »
Hello,

I have used Purify (memory leaks detection tool from IBM) and I have got some interesting results, which are in the attachment (attention, the unpacked file is huge). Hope this could be of some help :).

Best wishes,
Michael

PS.: I have compressed the file using 7zip, but even with it I have to cut it in two parts :(. To unpack the file(s) please remove the .zip extension and use 7zip. If the administrators find that the file use too much space, then they can delete it after downloading it. If they not find the file(s) useful, just delete it(them).

[attachment deleted by admin]

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
Re: C::B and Purify
« Reply #1 on: January 27, 2006, 01:58:39 pm »
And here part2.

Michael


[attachment deleted by admin]

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: C::B and Purify
« Reply #2 on: January 27, 2006, 02:03:15 pm »
Cannot unzip it :(

EDIT:
Lol, my fault, too stupid to read your post properly...
« Last Edit: January 27, 2006, 02:04:48 pm by thomas »
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
Re: C::B and Purify
« Reply #3 on: January 27, 2006, 02:09:07 pm »
I have forgotten to say that I have used C::B rev1884 (in the devel directory).

Michael
 

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: C::B and Purify
« Reply #4 on: January 27, 2006, 02:10:07 pm »
Hmmm... I only see wxString and wxStringBuffer allocations there. But we never allocate wxStrings dynamically (except in two places: personalities, and the list of compilers, that's half a dozen strings alltogether...).

So maybe wxString as such is broken? But surely someone would already have noticed that?
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
Re: C::B and Purify
« Reply #5 on: January 27, 2006, 03:31:38 pm »
Hmmm... I only see wxString and wxStringBuffer allocations there. But we never allocate wxStrings dynamically (except in two places: personalities, and the list of compilers, that's half a dozen strings alltogether...).

There are a lot of wxString and wxStringBuffer allocations, but there are else too :). I have provided a collapsed view of the Purify report (rev1886 this time) which should be easier to read (at least I hope :)).

Michael


[attachment deleted by admin]

Offline Der Meister

  • Regular
  • ***
  • Posts: 307
Re: C::B and Purify
« Reply #6 on: January 27, 2006, 05:33:35 pm »
I just ran a little test on Linux using valgrind (www.valgrind.org). I'm using revision 1886 without any active plugin. I just opened Code::Blocks, openend the keybinder-project (without any open editor, just the start-here-page), closed it and then closed Code::Blocks.

The first thing I then noticed was the use of an unitialised variable:
Code
==7212== Conditional jump or move depends on uninitialised value(s)
==7212==    at 0x1B9EFA26: CompileTargetBase::SetOptionRelation(OptionsRelationType, OptionsRelation) (compiletargetbase.cpp:116)
==7212==    by 0x1BA7BA93: ProjectLoader::DoBuildTargetOptions(TiXmlElement*, ProjectBuildTarget*) (projectloader.cpp:429)
==7212==    by 0x1BA7C4EE: ProjectLoader::DoBuildTarget(TiXmlElement*) (projectloader.cpp:289)
==7212==    by 0x1BA7C6BA: ProjectLoader::DoBuild(TiXmlElement*) (projectloader.cpp:267)
==7212==    by 0x1BA7CEE0: ProjectLoader::Open(wxString const&) (projectloader.cpp:90)
==7212==    by 0x1B9D9721: cbProject::Open() (cbproject.cpp:269)
==7212==    by 0x1B9DA85B: cbProject::cbProject(wxString const&) (cbproject.cpp:75)
==7212==    by 0x1BA8B084: ProjectManager::LoadProject(wxString const&, bool) (projectmanager.cpp:616)
==7212==    by 0x807DA88: MainFrame::DoOpenProject(wxString const&, bool) (main.cpp:1122)
==7212==    by 0x807DFD4: MainFrame::OpenGeneric(wxString const&, bool) (main.cpp:1089)
==7212==    by 0x8087ECB: MainFrame::OnStartHereLink(wxCommandEvent&) (main.cpp:1339)
==7212==    by 0x1C35848A: wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&), wxEvent&) const (in /usr/lib/libwx_baseu-2.6.so.0.0.0)
This can be solved with this patch:
Code
Index: src/sdk/compiletargetbase.cpp
===================================================================
--- src/sdk/compiletargetbase.cpp       (revision 1886)
+++ src/sdk/compiletargetbase.cpp       (working copy)
@@ -34,7 +34,7 @@
     m_CompilerIdx(0)
 {
        //ctor
-    for (int i = 0; i < 4; ++i)
+    for (int i = 0; i < ortLast; ++i)
         m_OptionsRelation[i] = orAppendToParentOptions;
 
     // default "make" commands


And yes, valgringd reports also memory-leaks. Some about wxString, but also some about Code::Blocks itself. (See the attachment for a list)

Anyway, I fear that this is only the tip of the iceberg...

[attachment deleted by admin]
Real Programmers don't comment their code. If it was hard to write, it should be hard to understand.
Real Programmers don't write in BASIC. Actually, no programmers write in BASIC, after the age of 12.

takeshimiya

  • Guest
Re: C::B and Purify
« Reply #7 on: January 27, 2006, 05:45:46 pm »
It's great to use valgrind/purify to iron out lot's of bugs/memleaks/segfaults, because in linux it's almost unuseable.

Offline Der Meister

  • Regular
  • ***
  • Posts: 307
Re: C::B and Purify
« Reply #8 on: January 28, 2006, 02:45:05 pm »
I just fixed two of the memory leaks valgrind discovered (patch is at sf.net - #1417376).

However, there still remain a lot memory leaks. Three of them look as they could easily be fixed if there were not those damn wxArrays:
Code
6176 bytes in 8 blocks are definitely lost in loss record 130 of 148
==9113==    at 0x1B90524A: calloc (in /usr/lib/valgrind/vgpreload_memcheck.so)
==9113==    by 0x1BA6D092: ProjectFile::ProjectFile(cbProject*) (hashmap.h:101)
==9113==    by 0x1B9D7778: cbProject::AddFile(int, wxString const&, bool, bool, unsigned short) (blockallocated.h:147)
==9113==    by 0x1BA78995: ProjectLoader::DoUnits(TiXmlElement*) (projectloader.cpp:644)
==9113==    by 0x1BA7CFEC: ProjectLoader::Open(wxString const&) (projectloader.cpp:103)
==9113==    by 0x1B9D9721: cbProject::Open() (cbproject.cpp:273)
==9113==    by 0x1B9DA85B: cbProject::cbProject(wxString const&) (cbproject.cpp:79)
==9113==    by 0x1BA8B0C4: ProjectManager::LoadProject(wxString const&, bool) (projectmanager.cpp:622)
==9113==    by 0x807DAE8: MainFrame::DoOpenProject(wxString const&, bool) (main.cpp:1122)
==9113==    by 0x807E034: MainFrame::OpenGeneric(wxString const&, bool) (main.cpp:1089)
==9113==    by 0x8087F2B: MainFrame::OnStartHereLink(wxCommandEvent&) (main.cpp:1339)
==9113==    by 0x1C35848A: wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&), wxEvent&) const (in /usr/lib/libwx_baseu-2.6.so.0.0.0)

4376 (240 direct, 4136 indirect) bytes in 12 blocks are definitely lost in loss record 103 of 148
==9113==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==9113==    by 0x1BA68D4C: PluginManager::LoadPlugin(wxString const&) (pluginmanager.cpp:197)
==9113==    by 0x1BA696C9: PluginManager::ScanForPlugins(wxString const&) (pluginmanager.cpp:94)
==9113==    by 0x8071DAE: MainFrame::ScanForPlugins() (main.cpp:700)
==9113==    by 0x807C4F9: MainFrame::MainFrame(wxWindow*) (main.cpp:440)
==9113==    by 0x8062CB8: CodeBlocksApp::InitFrame() (app.cpp:229)
==9113==    by 0x8064147: CodeBlocksApp::OnInit() (app.cpp:412)
==9113==    by 0x8065080: wxAppConsole::CallOnInit() (app.h:87)
==9113==    by 0x1C39C8AF: wxEntry(int&, wchar_t**) (in /usr/lib/libwx_baseu-2.6.so.0.0.0)
==9113==    by 0x1C39CC1B: wxEntry(int&, char**) (in /usr/lib/libwx_baseu-2.6.so.0.0.0)
==9113==    by 0x80605E9: main (app.cpp:87)

==9113== 1600 (80 direct, 1520 indirect) bytes in 4 blocks are definitely lost in loss record 112 of 148
==9113==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==9113==    by 0x1BA68FD1: PluginManager::LoadPlugin(wxString const&) (pluginmanager.cpp:229)
==9113==    by 0x1BA696C9: PluginManager::ScanForPlugins(wxString const&) (pluginmanager.cpp:94)
==9113==    by 0x8071DAE: MainFrame::ScanForPlugins() (main.cpp:700)
==9113==    by 0x807C4F9: MainFrame::MainFrame(wxWindow*) (main.cpp:440)
==9113==    by 0x8062CB8: CodeBlocksApp::InitFrame() (app.cpp:229)
==9113==    by 0x8064147: CodeBlocksApp::OnInit() (app.cpp:412)
==9113==    by 0x8065080: wxAppConsole::CallOnInit() (app.h:87)
==9113==    by 0x1C39C8AF: wxEntry(int&, wchar_t**) (in /usr/lib/libwx_baseu-2.6.so.0.0.0)
==9113==    by 0x1C39CC1B: wxEntry(int&, char**) (in /usr/lib/libwx_baseu-2.6.so.0.0.0)
==9113==    by 0x80605E9: main (app.cpp:87)
(Note: I noticed that the line-numbers valgrind prints are not always correct - but they seem to be at least close to the problematic line.)
I'm not really familiar with wxArrays (Why are we even using them in these cases? Shouldn't be the STL-containers be good enough?), so I didn't try to solve this leaks. I looks as we need to iterate through the array in the destructor of the class that owns this array and call delete for every entry of the array.

Anyway, especially these problems could have been easily avoided if smart pointers were used (Well, someone said, raw pointers in containers are evil. First I didn't believe it but during the last projects I was working on I learned that this is really true...  :? ), but changing this now would possible be a lot of work.
Real Programmers don't comment their code. If it was hard to write, it should be hard to understand.
Real Programmers don't write in BASIC. Actually, no programmers write in BASIC, after the age of 12.

takeshimiya

  • Guest
Re: C::B and Purify
« Reply #9 on: January 28, 2006, 03:52:35 pm »
Shouldn't be the STL-containers be good enough?), so I didn't try to solve this leaks. I looks as we need to iterate through the array in the destructor of the class that owns this array and call delete for every entry of the array.
Half C::B is written with wx containers, and the other half with stl containers.
That's mostly because historical reasons: wx2.x adviced to try not use the STL at all, because compilers could not support well the STL/templates.
But we are in 2006 now, and the situation has changed (the compilers improved a lot), so that advice is no longer true.

In fact, I hope there will be a transition to use STL whenever possible, because wxWidgets 3.0 (aka wxTNG) will change radically to that way. And it has been said that a wxWidgets <-> boost cooperation is high likely to be done, so wx will use STL and boost instead of reinvent the well. It has been said that even the wxTNG will be sent to boost as a proposal to the GUI library. :D

Anyway, especially these problems could have been easily avoided if smart pointers were used (Well, someone said, raw pointers in containers are evil. First I didn't believe it but during the last projects I was working on I learned that this is really true...  :? ), but changing this now would possible be a lot of work.

Agreed, if one wants to use smart pointers (boost::shared_ptr) right now with wxWidgets, check this: http://wxforum.shadonet.com/viewtopic.php?t=4947

I modified the boost files, so that it will automatically call Destroy instead of delete for objects derived with wxWindow. So now, when you write code, it will automatically call Destroy:

wxWidgets example:
Code: cpp
#include <boost/shared_ptr.hpp>
typedef boost::shared_ptr wx_ptr;
wx_ptr<wxMessageDialog> ptr(new wxMessageDialog(...));


I think there could be done in C::B a transition from wx-containers to stl-containers, and pointers to smart-pointers, either slowly, or all at the same time, creating a new branch, just like when we made the UNICODE transition.

That will for sure create a C::B with almost 0 probability of having mem-leaks, and at the same time contributors like Der Meister can provide a patch because he understands stl containers but not wx containers.  :D
« Last Edit: January 28, 2006, 03:58:36 pm by Takeshi Miya »

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: C::B and Purify
« Reply #10 on: January 28, 2006, 06:07:09 pm »
I just fixed two of the memory leaks valgrind discovered (patch is at sf.net - #1417376).
I committed your patch together with the modifications I made to ProjectManager. Maybe there are still some leaks remaining, but unlike before, they are hardly noticeable now.
I am getting a rather positive attitude towards this problem :)
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
Re: C::B and Purify
« Reply #11 on: January 28, 2006, 09:30:01 pm »
I committed your patch together with the modifications I made to ProjectManager. Maybe there are still some leaks remaining, but unlike before, they are hardly noticeable now.

That is a good job :D. After testing it with rev1891 (see here for more info) I have remarked some small problems. The memory used seems not to be always and/or fully returned to the OS.

Michael