Using uninitialized memory (valgrind report)

[oBFusCATed]

Here is what valgrind have spit http://pastebin.com/qscKUNt8
How to reproduce:
1. build
2. cd src && ./update
3. cd devel && export LD_LIBRARY_PATH=`pwd`
4. valgrind ./codeblocks -p debug
5. open codeblocks-unix.cbp

I'm not too familiar with the wxString's implementation, but the function mentioned in the reports is full of constructs that look suspicious.
Examples:

Code

while (*ptr <= ParserConsts::space_chr)
    ++ptr;
...
while (*(ptr+1) == ParserConsts::ptr_chr)
{
     baseArgs << *ptr; // append one more '*' to baseArgs
     ptr++; // next char
}
...
// skip white spaces and increase pointer
while (   *ptr     != ParserConsts::null
          && *(ptr+1) == ParserConsts::space_chr )
{
      ++ptr; // next char
}

I'm not familiar with this code, so it will be good if someone could look this up.

[ollydbg]

Can you tell me what is context of your test?
Did you use a 64 bit Linux? or 32 bit Linux? --->looking at your log, it looks like you are using 64bit.
32 bit wxWidgets or 64 bit wxWidgets library?
Did you use 2.8.12 version or 2.9.x version of wxWidgets?
I believe that the wxString implementation has changed from 2.8 -> 2.9+, this means using a raw pointer(usually wxChar *) will cause problems. Different system now use different kind of wxString.
See my related post here:http://forums.codeblocks.org/index.php/topic,18353.msg125462.html#msg125462

[ollydbg]

Ok, besides the potential issue about wxString in 2.9.x+, I found that the issue reported is definitely related to an unsafe pointer access.

Look at the code snippet:

Code

bool ParserThread::GetBaseArgs(const wxString& args, wxString& baseArgs)
{
    const wxChar* ptr = args;  // pointer to current char in args string
    wxString word;             // compiled word of last arg
    bool skip = false;         // skip the next char (do not add to stripped args)
    bool sym  = false;         // current char symbol
    bool one  = true;          // only one argument

    TRACE(_T("GetBaseArgs() : args='%s'."), args.wx_str());
    baseArgs.Alloc(args.Len() + 1);

    // Verify ptr is valid (still within the range of the string)
    while (*ptr != ParserConsts::null)
    {
        switch (*ptr)
        {
        case ParserConsts::eol_chr:
            while (*ptr <= ParserConsts::space_chr)   // this the the line parserthread.cpp:2859 reported by valgrind
                ++ptr;
            break;
        case ParserConsts::space_chr:
        ...

You see, when we meet an "eol_chr", we are going to run a loop:

Code

            while (*ptr <= ParserConsts::space_chr)   // this the the line parserthread.cpp:2859 reported by valgrind
                ++ptr;

You see, we don't check whether we have meet a "ParserConsts::null" in the loop, so I think the simple fix is adding this check. Any ideas?

[oBFusCATed]

Gentoo linux 64bit, wxGTK 2.8.12, Codeblocks 9399 or something like this.

If you look at my post you'll see that all code excerpts are from this function.

[ollydbg]

Gentoo linux 64bit, wxGTK 2.8.12, Codeblocks 9399 or something like this.

Ok, I see.

If you look at my post you'll see that all code excerpts are from this function.

Yes, there are many "Conditional jump or move depends on uninitialised value(s)" reports from valgrind, the reason is that after the pointer passes the eof (the null character at the end of the string, it becomes Dangling pointer.

[oBFusCATed]

Can they be fixed?

[ollydbg]

Can they be fixed?

Sure, the main point here is that avoid using a raw pointer. In wx 2.9.x+ under Linux, the UTF8 is used to store characters, so we should use index to access each element of the wxString.

[ollydbg]

the main point here is that avoid using a raw pointer. In wx 2.9.x+ under Linux, the UTF8 is used to store characters, so we should use index to access each element of the wxString.

Ok, a little off topic, I see that the new wxString now use fixed width encoding since 2012-05-13, so using a raw pointer(wchar_t *) is OK in all systems. See my post: Re: wxString support in wxWidgets 3.0 problem?

[oBFusCATed]

I don't know if it is OK, because I'm not too familiar with wxString in both 2.8 and 2.9, but what I'm sure is that all these constructs in my first post are broken and dangerous.

[ollydbg]

...but what I'm sure is that all these constructs in my first post are broken and dangerous.

Yes, the bug in first post should be fixed.
I just debugged a little under wx2.8.12, and see that the wxString is always terminated by Null characters, see the figures and document in the webpage: http://docs.wxwidgets.org/trunk/overview_string.html

[ollydbg]

Code

 src/plugins/codecompletion/parser/parserthread.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/plugins/codecompletion/parser/parserthread.cpp b/src/plugins/codecompletion/parser/parserthread.cpp
index b614aca..221b548 100644
--- a/src/plugins/codecompletion/parser/parserthread.cpp
+++ b/src/plugins/codecompletion/parser/parserthread.cpp
@@ -2856,7 +2856,7 @@ bool ParserThread::GetBaseArgs(const wxString& args, wxString& baseArgs)
         switch (*ptr)
         {
         case ParserConsts::eol_chr:
-            while (*ptr <= ParserConsts::space_chr)
+            while (*ptr != ParserConsts::null && *ptr <= ParserConsts::space_chr)
                 ++ptr;
             break;
         case ParserConsts::space_chr:
@@ -2876,7 +2876,7 @@ bool ParserThread::GetBaseArgs(const wxString& args, wxString& baseArgs)
         case ParserConsts::ptr_chr: // handle pointer args
             // handle multiple pointer like in: main (int argc, void** argv)
             // or ((int *, char ***))
-            while (*(ptr+1) == ParserConsts::ptr_chr)
+            while (*(ptr+1) != ParserConsts::null && *(ptr+1) == ParserConsts::ptr_chr)
             {
                 baseArgs << *ptr; // append one more '*' to baseArgs
                 ptr++; // next char

Does the patch above fix the issue reported by valgrind? (I just add some null check when accessing by pointer de-reference)
Thanks.

[ollydbg]

Committed in r9416, I hope it fix the bug.