doxygen-develop

[Doxygen-develop] Memory underrun in util.cpp:5584

[Michael M. <Mic...@tt...>]

Hi there,

I took the doxygen-1.4.4-20050815.tar.gz CVS tarball and ran it under
valgrind.  I found a byte being accessed before the start of a buffer:

  // search for trailing empty lines
  int b=l-1,bi=-1;
  p=s.data()+b;
  while (b>=0)
  {
    c=*--p;  <------ Can read before s.data
    if (c==' ' || c=='\t' || c=='\r') b--;
    else if (c=='\n') bi=b,b--;
    else break;
  }

I think the problem is that --p occurs before the dereference, and so the
code would be better written as:

  while (b>=0)
  {
    c=*p; 
    p--;
    if (c==' ' || c=='\t' || c=='\r') b--;

This avoids p[-1] being accessed if something like "\n" is in the buffer.

I've not put it into Bugzilla, but let me know if you would prefer it filed
there instead.

Regards,

Mike

[Doxygen-develop] Use of uninitialised variable in classdef.cpp:2627

[Michael M. <Mic...@tt...>]

Hi,

Another from valgrind and the doxygen-1.4.4-20050815.tar.gz CVS tarball.

classdef.cpp:50:

// constructs a new class definition
ClassDef::ClassDef(
    const char *defFileName,int defLine,
    const char *nm,CompoundType ct,
    const char *lref,const char *fName,
    bool isSymbol)
 :
Definition(defFileName,defLine,removeRedundantWhiteSpace(nm),0,0,isSymbol)
{
  m_compType=ct;
  QCString compoundName=compoundTypeString(); 

  ...

Unfortunately compoundTypeString() does the following:

if (m_compType==Interface && m_isObjC) return "class";


Unlike m_compType, m_isObjC hasn't yet been setup.

Cheers,

Mike

[Doxygen-develop] Re: Use of uninitialised variable in classdef.cpp:2627

[Dimitri v. H. <di...@st...>]

On Fri, Oct 21, 2005 at 04:39:04PM +0100, Michael McTernan wrote:
> Hi,
> 
> Another from valgrind and the doxygen-1.4.4-20050815.tar.gz CVS tarball.
> 
> classdef.cpp:50:
> 
> // constructs a new class definition
> ClassDef::ClassDef(
>     const char *defFileName,int defLine,
>     const char *nm,CompoundType ct,
>     const char *lref,const char *fName,
>     bool isSymbol)
>  :
> Definition(defFileName,defLine,removeRedundantWhiteSpace(nm),0,0,isSymbol)
> {
>   m_compType=ct;
>   QCString compoundName=compoundTypeString(); 
> 
>   ...
> 
> Unfortunately compoundTypeString() does the following:
> 
> if (m_compType==Interface && m_isObjC) return "class";
> 
> 
> Unlike m_compType, m_isObjC hasn't yet been setup.

Indeed, will be corrected.

Regards, 
  Dimitri

Re: [Doxygen-develop] Memory underrun in util.cpp:5584

[Dimitri v. H. <di...@st...>]

On Fri, Oct 21, 2005 at 03:54:21PM +0100, Michael McTernan wrote:
> Hi there,
> 
> I took the doxygen-1.4.4-20050815.tar.gz CVS tarball and ran it under
> valgrind.  I found a byte being accessed before the start of a buffer:
> 
>   // search for trailing empty lines
>   int b=l-1,bi=-1;
>   p=s.data()+b;
>   while (b>=0)
>   {
>     c=*--p;  <------ Can read before s.data
>     if (c==' ' || c=='\t' || c=='\r') b--;
>     else if (c=='\n') bi=b,b--;
>     else break;
>   }
> 
> I think the problem is that --p occurs before the dereference, and so the
> code would be better written as:
> 
>   while (b>=0)
>   {
>     c=*p; 
>     p--;
>     if (c==' ' || c=='\t' || c=='\r') b--;
> 
> This avoids p[-1] being accessed if something like "\n" is in the buffer.
> 
> I've not put it into Bugzilla, but let me know if you would prefer it filed
> there instead.

If you look at version 1.4.5 or later, you'll see the code had been 
changed to:

------------------------------------------------------
  // search for trailing empty lines
  int b=l-1,bi=-1;
  p=s.data()+b;
  while (b>=0)
  {
    c=*p; p--;
    if (c==' ' || c=='\t' || c=='\r') b--;
    else if (c=='\n') bi=b,b--;
    else break;
  } 
------------------------------------------------------

so that's exactly what you are proposing ;-)

Regards,
  Dimitri

RE: [Doxygen-develop] Memory underrun in util.cpp:5584

[Michael M. <Mic...@tt...>]

Excellent!  Many thanks :)

BTW, did you see bug 319427:
http://bugzilla.gnome.org/show_bug.cgi?id=319427 

I just checked the CVS tarball for 22/10/2005, and think this is still an
issue.  I think the solution is just to move the assignment of m_isObjC =
FALSE to the start of the function, where m_compType is setup.

Regards,

Mike

> -----Original Message-----
> From: Dimitri van Heesch [mailto:di...@st...]
> Sent: 22 October 2005 13:18
> To: Michael McTernan
> Cc: dox...@li...
> Subject: Re: [Doxygen-develop] Memory underrun in util.cpp:5584
> 
> On Fri, Oct 21, 2005 at 03:54:21PM +0100, Michael McTernan wrote:
> > Hi there,
> >
> > I took the doxygen-1.4.4-20050815.tar.gz CVS tarball and ran it under
> > valgrind.  I found a byte being accessed before the start of a buffer:
> >
> >   // search for trailing empty lines
> >   int b=l-1,bi=-1;
> >   p=s.data()+b;
> >   while (b>=0)
> >   {
> >     c=*--p;  <------ Can read before s.data
> >     if (c==' ' || c=='\t' || c=='\r') b--;
> >     else if (c=='\n') bi=b,b--;
> >     else break;
> >   }
> >
> > I think the problem is that --p occurs before the dereference, and so
> the
> > code would be better written as:
> >
> >   while (b>=0)
> >   {
> >     c=*p;
> >     p--;
> >     if (c==' ' || c=='\t' || c=='\r') b--;
> >
> > This avoids p[-1] being accessed if something like "\n" is in the
> buffer.
> >
> > I've not put it into Bugzilla, but let me know if you would prefer it
> filed
> > there instead.
> 
> If you look at version 1.4.5 or later, you'll see the code had been
> changed to:
> 
> ------------------------------------------------------
>   // search for trailing empty lines
>   int b=l-1,bi=-1;
>   p=s.data()+b;
>   while (b>=0)
>   {
>     c=*p; p--;
>     if (c==' ' || c=='\t' || c=='\r') b--;
>     else if (c=='\n') bi=b,b--;
>     else break;
>   }
> ------------------------------------------------------
> 
> so that's exactly what you are proposing ;-)
> 
> Regards,
>   Dimitri