debugging a cygheap leak

Eric Blake ebb9@byu.net
Tue May 20 02:15:00 GMT 2008


According to Christopher Faylor on 5/1/2008 10:10 AM:
>>      1 [main] m4 18392
>> \\?\C:\cygwin\home\eblake\m4-head\build\src\.libs\m4.exe: *** fatal
>> error - cmalloc would have returned NULL
>>
>> Any ideas on how to go about debugging this?  It looks like a memory leak 
>> regression in 1.7.0.
> 
> Build the DLL with --enable-debugging and CFLAGS=-g, if you can isolate
> the test to just one invocation of m4 then:
> 
> set CYGWIN_DEBUG=m4
> 
> and do a backtrace from there.

Thanks.  I finally isolated it, and it is a true leak in cygwin1.dll. 
path_conv has some bugs - it fails to zero-initialize various members in 
some constructor paths.  It also uses path_conv::check() for two purposes 
- as part of the constructor, but also for subsequent expansion of the 
object with more details.  Therefore, in the current code base, it is 
impossible to know whether check() has previously been called, because you 
can't tell whether the pointer is uninitialized or previously allocated.

This patch shows that path_conv::check() is being called more than once, 
in at least one code path triggered by dlopen().  And, since each 
invocation of check() blindly resets wide_path and normalized_path to 
NULL, this leads to cmalloc leaks if the previous call happened to 
allocate these fields.  I still haven't managed to trim the m4/libtool 
testcase into something smaller, but hopefully this backtrace helps you 
spot a way to fix the problem.

The correct patch should not need the addition of 'bool built' (hence, I'm 
posting here and not the -patches list, and no ChangeLog); rather, it is 
to demonstrate the bug.

Index: cygwin/path.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/path.cc,v
retrieving revision 1.496
diff -u -p -r1.496 path.cc
--- cygwin/path.cc	13 May 2008 13:44:04 -0000	1.496
+++ cygwin/path.cc	19 May 2008 22:39:33 -0000
@@ -817,6 +817,7 @@ path_conv::check (const char *src, unsig
    memset (&dev, 0, sizeof (dev));
    fs.clear ();
    normalized_path = NULL;
+  built = true;
    int component = 0;		// Number of translated components

    if (!(opt & PC_NULLEMPTY))
Index: cygwin/path.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/path.h,v
retrieving revision 1.118
diff -u -p -r1.118 path.h
--- cygwin/path.h	13 May 2008 13:44:04 -0000	1.118
+++ cygwin/path.h	19 May 2008 22:39:33 -0000
@@ -208,7 +208,7 @@ class path_conv

    path_conv (const device& in_dev): fileattr (INVALID_FILE_ATTRIBUTES),
       wide_path (NULL), path_flags (0), known_suffix (NULL), error (0),
-     dev (in_dev)
+     dev (in_dev), built (false)
      {
        strcpy (path, in_dev.native);
      }
@@ -216,24 +216,27 @@ class path_conv
    path_conv (int, const char *src, unsigned opt = PC_SYM_FOLLOW,
  	     const suffix_info *suffixes = NULL)
    {
+    built = false;
      check (src, opt, suffixes);
    }

    path_conv (const UNICODE_STRING *src, unsigned opt = PC_SYM_FOLLOW,
  	     const suffix_info *suffixes = NULL)
    {
+    built = false;
      check (src, opt | PC_NULLEMPTY, suffixes);
    }

    path_conv (const char *src, unsigned opt = PC_SYM_FOLLOW,
  	     const suffix_info *suffixes = NULL)
    {
+    built = false;
      check (src, opt | PC_NULLEMPTY, suffixes);
    }

    path_conv (): fileattr (INVALID_FILE_ATTRIBUTES), wide_path (NULL),
-  		path_flags (0), known_suffix (NULL), error (0),
-		normalized_path (NULL)
+		path_flags (0), known_suffix (NULL), error (0),
+           normalized_path (NULL), built (false)
      {path[0] = '\0';}

    ~path_conv ();
@@ -283,6 +286,7 @@ class path_conv
    void set_normalized_path (const char *) __attribute__ ((regparm
(2)));
    DWORD get_symlink_length () { return symlink_length; };
   private:
+  bool built;
    DWORD symlink_length;
    char path[NT_MAX_PATH];
  };


With the patch installed, I reran the m4 testcase, and here's the gdb 
session proving the double call to check() and thus the memory leak of 
wide_path:


Breakpoint 1, path_conv::check (this=0x224840,
     src=0x6a8c18 "/home/eblake/m4-head/build/modules/m4.la", opt=161,
     suffixes=0x612058e0) at ../../../../winsup/cygwin/path.cc:815
815       wide_path = NULL;
Current language:  auto; currently c++
(gdb) cond 1 built
(gdb) c
Continuing.

Breakpoint 1, path_conv::check (this=0x2248b0, src=0x690280 "libltdl.a",
     opt=33, suffixes=0x61205888) at
../../../../winsup/cygwin/path.cc:815
815       wide_path = NULL;
(gdb) p wide_path
$2 = (PWCHAR) 0x612a9014
(gdb) bt
#0  path_conv::check (this=0x2248b0, src=0x690280 "libltdl.a", opt=33,
     suffixes=0x61205888) at ../../../../winsup/cygwin/path.cc:815
#1  0x610cb280 in perhaps_suffix (prog=0x690280 "libltdl.a",
buf=@0x2248b0,
     err=@0x224820, opt=15) at ../../../../winsup/cygwin/spawn.cc:69
#2  0x610cb669 in find_exec (name=0x690280 "libltdl.a", buf=@0x2248b0,
     mywinenv=0x6119a959 "/usr/lib", opt=15, known_suffix=0x0)
     at ../../../../winsup/cygwin/spawn.cc:115
#3  0x6114b213 in check_path_access (mywinenv=0x6119a959 "/usr/lib",
     name=0x690280 "libltdl.a", buf=@0x2248b0)
     at ../../../../winsup/cygwin/dlfcn.cc:34
#4  0x610209ca in get_full_path_of_dll (str=0x6bdf58 "libltdl.a",
     name=0x690280 "libltdl.a") at ../../../../winsup/cygwin/dlfcn.cc:63
#5  0x61020b6c in dlopen (name=0x6bdf58 "libltdl.a")
     at ../../../../winsup/cygwin/dlfcn.cc:91
...

-- 
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net



More information about the Cygwin-developers mailing list