public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* Extending /proc/*/maps
@ 2011-05-11  5:28 Ryan Johnson
  2011-05-11 10:32 ` Corinna Vinschen
  2011-05-11 11:15 ` Corinna Vinschen
  0 siblings, 2 replies; 25+ messages in thread
From: Ryan Johnson @ 2011-05-11  5:28 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

Hi all,

Please find attached three patches which extend the functionality of 
/proc/*/maps.

The first (proc-maps-files) makes format_process_maps report all 
reserved or committed address space, rather than just the parts occupied 
by dlls in the dll_list. It splits allocations when they have multiple 
sets of permissions (with proper file offsets when appropriate), 
displays the file name of all mapped images and files, and identifies 
shared memory segments.

The second (proc-maps-heaps) adds reporting of Windows heaps (or their 
bases, at least). Unfortunately there doesn't seem to be any efficient 
way to identify all virtual allocations which a heap owns.

The third (proc-maps-safe) adds a "safe" mode and helper function which 
allows to print the process map at early stages of process startup when 
cygwin1.dll is not initialized yet. It is provided in case anyone finds 
it helpful; I don't expect it to migrate upstream.

Changelog entries also attached...

NOTE 1: I do not attempt to identify PEB, TEB, or thread stacks. The 
first could be done easily enough, but the second and third require 
venturing into undocumented/private Windows APIs.

NOTE 2: If desired, we could easily extend format_process_maps further 
to report section names of mapped images (linux does this for .so 
files), using the pe/coff file introspection class that accompanies my 
fork patches (separate email). I did not implement it because I don't 
know if people want that functionality. I haven't needed it yet.

Thoughts?
Ryan



[-- Attachment #2: proc-maps-files.patch --]
[-- Type: text/plain, Size: 9154 bytes --]

# HG changeset patch
# Parent 1420f18fd5c5647e475df8339486020b456fb9d8
diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-05-10  Ryan Johnson  <ryan.johnson@cs.utoronto.ca>
+
+	* fhandler_process.cc (dos_drive_mappings, heap_info): New helper classes.
+	(format_process_maps): Reworked to report all mapped address space
+	in a process (committed or reserved), identifying the nature of
+	the mapping (mapped file/image, heap, shared memory) when
+	possible.
+	* autoload.cc: Register GetMappedFileNameW (psapi.dll)
+
 2011-04-15  Yaakov Selkowitz  <yselkowitz@users.sourceforge.net>
 
 	* thread.cc (pthread_setschedprio): New function.
diff --git a/autoload.cc b/autoload.cc
--- a/autoload.cc
+++ b/autoload.cc
@@ -422,6 +422,7 @@
 LoadDLLfunc (CoTaskMemFree, 4, ole32)
 
 LoadDLLfunc (EnumProcessModules, 16, psapi)
+LoadDLLfunc (GetMappedFileNameW, 16, psapi)
 LoadDLLfunc (GetModuleFileNameExW, 16, psapi)
 LoadDLLfunc (GetModuleInformation, 16, psapi)
 LoadDLLfunc (GetProcessMemoryInfo, 12, psapi)
diff --git a/fhandler_process.cc b/fhandler_process.cc
--- a/fhandler_process.cc
+++ b/fhandler_process.cc
@@ -527,6 +527,81 @@
   return len + 1;
 }
 
+struct dos_drive_mappings {
+  struct mapping {
+    mapping* next;
+    int len;
+    wchar_t drive_letter;
+    wchar_t mapping[1];
+  };
+  mapping* mappings;
+  
+  dos_drive_mappings ()
+    : mappings(0)
+  {
+    /* The logical drive strings buffer holds a list of (at most 26)
+       drive names separated by nulls and terminated by a double-null:
+
+       "a:\\\0b:\\\0...z:\\\0"
+
+       The annoying part is, QueryDosDeviceW wants only "x:" rather
+       than the "x:\" we get back from GetLogicalDriveStringsW, so
+       we'll have to strip out the trailing slash for each mapping.
+       
+       The returned mapping a native NT pathname (\Device\...) which
+       we can use to fix up the output of GetMappedFileNameW
+    */
+    static unsigned const DBUFLEN = 26*4;
+    wchar_t dbuf[DBUFLEN+1];
+    wchar_t pbuf[NT_MAX_PATH+1];
+    wchar_t drive[] = {L'x', L':', 0};
+    unsigned result = GetLogicalDriveStringsW (DBUFLEN*sizeof (wchar_t), dbuf);
+    if (!result)
+      debug_printf ("Failed to get logical DOS drive names: %lu", GetLastError ());
+    else if (result > DBUFLEN)
+      debug_printf ("Too many mapped drive letters: %u", result);
+    else
+      for (wchar_t* cur = dbuf; (*drive=*cur); cur = wcschr (cur, L'\0')+1)
+	if (QueryDosDeviceW (drive, pbuf, NT_MAX_PATH))
+	  {
+	    size_t plen = wcslen (pbuf);
+	    size_t psize = plen*sizeof (wchar_t);
+	    debug_printf ("DOS drive %ls maps to %ls", drive, pbuf);
+	    mapping* m = (mapping*) cmalloc (HEAP_FHANDLER, sizeof (mapping) + psize);
+	    m->next = mappings;
+	    m->len = plen;
+	    m->drive_letter = *drive;
+	    memcpy (m->mapping, pbuf, psize + sizeof (wchar_t));
+	    mappings = m;
+	  }
+	else
+	  debug_printf ("Unable to determine the native mapping for %ls (error %lu)",
+			drive,
+			GetLastError ());
+  }
+  
+  wchar_t* fixup_if_match (wchar_t* path) {
+    for (mapping* m = mappings; m; m = m->next)
+      if (!wcsncmp (m->mapping, path, m->len))
+	{
+	  path += m->len-2;
+	  path[0] = m->drive_letter;
+	  path[1] = L':';
+	  break;
+	}
+    return path;
+  }
+  
+  ~dos_drive_mappings () {
+    mapping* n = 0;
+    for (mapping* m = mappings; m; m = n)
+      {
+	n = m->next;
+	cfree (m);
+      }
+  }
+};
+
 static _off64_t
 format_process_maps (void *data, char *&destbuf)
 {
@@ -538,11 +613,23 @@
     return 0;
 
   _off64_t len = 0;
-  HMODULE *modules;
-  DWORD needed, i;
-  DWORD_PTR wset_size;
-  DWORD_PTR *workingset = NULL;
-  MODULEINFO info;
+  
+  union access {
+    char flags[8];
+    _off64_t word;
+  } a;
+
+  struct region {
+    access a;
+    char* abase;
+    char* rbase;
+    char* rend;
+  } cur = {{{'\0'}}, (char*)1, 0, 0};
+  
+  MEMORY_BASIC_INFORMATION mb;
+  dos_drive_mappings drive_maps;
+  struct __stat64 st;
+  long last_pass = 0;
 
   tmp_pathbuf tp;
   PWCHAR modname = tp.w_get ();
@@ -554,75 +641,86 @@
       cfree (destbuf);
       destbuf = NULL;
     }
-  if (!EnumProcessModules (proc, NULL, 0, &needed))
+  
+  /* Iterate over each VM region in the address space, coalescing
+     memory regions with the same permissions. Once we run out, do one
+     last_pass to trigger output of the last accumulated region. */
+  for(char* i = 0;
+      VirtualQueryEx (proc, i, &mb, sizeof(mb)) || (1 == ++last_pass);
+      i = cur.rend)
     {
-      __seterrno ();
-      len = -1;
-      goto out;
+      if (mb.State == MEM_FREE)
+	a.word = 0;
+      else
+	{
+	  static DWORD const RW = (PAGE_EXECUTE_READWRITE | PAGE_READWRITE
+				   | PAGE_EXECUTE_WRITECOPY | PAGE_WRITECOPY);
+	  DWORD p = mb.Protect;
+	  a = (access) {{
+	      (p & (RW | PAGE_EXECUTE_READ | PAGE_READONLY))?	'r' : '-',
+	      (p & (RW))?					'w' : '-',
+	      (p & (PAGE_EXECUTE_READWRITE | PAGE_EXECUTE_READ
+		    | PAGE_EXECUTE_WRITECOPY | PAGE_EXECUTE))?	'x' : '-',
+	      (p & (PAGE_GUARD))? 				's' : 'p',
+	      '\0', // zero-fill the remaining bytes
+	    }};
+	}
+
+      region next = {a,
+		     (char*) mb.AllocationBase,
+		     (char*) mb.BaseAddress,
+		     (char*) mb.BaseAddress+mb.RegionSize
+      };
+      
+      /* Windows permissions are more fine-grained than the unix rwxp,
+	 so we reduce clutter by manually coalescing regions sharing
+	 the same allocation base and effective permissions. */
+      bool newbase = (next.abase != cur.abase);
+      if (!last_pass && !newbase && next.a.word == cur.a.word)
+	  cur.rend = next.rend; // merge with previous
+      else
+	{
+	  // output the current region if it's "interesting"
+	  if (cur.a.word)
+	    {
+	      size_t newlen = strlen (posix_modname) + 62;
+	      if (len + newlen >= maxsize)
+		destbuf = (char *) crealloc_abort (destbuf,
+						   maxsize += roundup2 (newlen, 2048));
+	      int written = __small_sprintf (destbuf + len,
+					     "%08lx-%08lx %s %08lx %04x:%04x %U   ",
+					     cur.rbase, cur.rend, cur.a.flags,
+					     cur.rbase - cur.abase,
+					     st.st_dev >> 16,
+					     st.st_dev & 0xffff,
+					     st.st_ino);
+	      while (written < 62)
+		destbuf[len + written++] = ' ';
+	      len += written;
+	      len += __small_sprintf (destbuf + len, "%s\n", posix_modname);
+	    }
+	  // start of a new region (but possibly still the same allocation)
+	  cur = next;
+	  // if a new allocation, figure out what kind it is 
+	  if (newbase && !last_pass)
+	    {
+	      st.st_dev = 0;
+	      st.st_ino = 0;
+	      if ((mb.Type & (MEM_MAPPED | MEM_IMAGE)
+		   && GetMappedFileNameW (proc, cur.abase, modname, NT_MAX_PATH)))
+		{
+		  PWCHAR dosname = drive_maps.fixup_if_match (modname);
+		  if (mount_table->conv_to_posix_path (dosname, posix_modname, 0))
+		    wcstombs (posix_modname, dosname, NT_MAX_PATH);
+		  stat64 (posix_modname, &st);
+		}
+	      else if (mb.Type & MEM_MAPPED)
+		strcpy (posix_modname, "[shareable]");
+	      else
+		posix_modname[0] = 0;
+	    }
+	}
     }
-  modules = (HMODULE*) alloca (needed);
-  if (!EnumProcessModules (proc, modules, needed, &needed))
-    {
-      __seterrno ();
-      len = -1;
-      goto out;
-    }
-
-  QueryWorkingSet (proc, (void *) &wset_size, sizeof wset_size);
-  if (GetLastError () == ERROR_BAD_LENGTH)
-    {
-      workingset = (DWORD_PTR *) alloca (sizeof (DWORD_PTR) * ++wset_size);
-      if (!QueryWorkingSet (proc, (void *) workingset,
-			    sizeof (DWORD_PTR) * wset_size))
-	workingset = NULL;
-    }
-  for (i = 0; i < needed / sizeof (HMODULE); i++)
-    if (GetModuleInformation (proc, modules[i], &info, sizeof info)
-	&& GetModuleFileNameExW (proc, modules[i], modname, NT_MAX_PATH))
-      {
-	char access[5];
-	strcpy (access, "r--p");
-	struct __stat64 st;
-	if (mount_table->conv_to_posix_path (modname, posix_modname, 0))
-	  sys_wcstombs (posix_modname, NT_MAX_PATH, modname);
-	if (stat64 (posix_modname, &st))
-	  {
-	    st.st_dev = 0;
-	    st.st_ino = 0;
-	  }
-	size_t newlen = strlen (posix_modname) + 62;
-	if (len + newlen >= maxsize)
-	  destbuf = (char *) crealloc_abort (destbuf,
-					   maxsize += roundup2 (newlen, 2048));
-	if (workingset)
-	  for (unsigned i = 1; i <= wset_size; ++i)
-	    {
-	      DWORD_PTR addr = workingset[i] & 0xfffff000UL;
-	      if ((char *)addr >= info.lpBaseOfDll
-		  && (char *)addr < (char *)info.lpBaseOfDll + info.SizeOfImage)
-		{
-		  access[0] = (workingset[i] & 0x5) ? 'r' : '-';
-		  access[1] = (workingset[i] & 0x4) ? 'w' : '-';
-		  access[2] = (workingset[i] & 0x2) ? 'x' : '-';
-		  access[3] = (workingset[i] & 0x100) ? 's' : 'p';
-		}
-	    }
-	int written = __small_sprintf (destbuf + len,
-				"%08lx-%08lx %s %08lx %04x:%04x %U   ",
-				info.lpBaseOfDll,
-				(unsigned long)info.lpBaseOfDll
-				+ info.SizeOfImage,
-				access,
-				info.EntryPoint,
-				st.st_dev >> 16,
-				st.st_dev & 0xffff,
-				st.st_ino);
-	while (written < 62)
-	  destbuf[len + written++] = ' ';
-	len += written;
-	len += __small_sprintf (destbuf + len, "%s\n", posix_modname);
-      }
-out:
   CloseHandle (proc);
   return len;
 }

[-- Attachment #3: proc-maps-heaps.patch --]
[-- Type: text/plain, Size: 1817 bytes --]

# HG changeset patch
# Parent 830a4452f008bf467a58c4838a10840c16d34d2d
diff --git a/fhandler_process.cc b/fhandler_process.cc
--- a/fhandler_process.cc
+++ b/fhandler_process.cc
@@ -29,6 +29,7 @@
 #include <sys/param.h>
 #include <ctype.h>
 #include <psapi.h>
+#include <tlhelp32.h>
 
 #define _COMPILING_NEWLIB
 #include <dirent.h>
@@ -602,6 +603,51 @@
   }
 };
 
+struct heap_info {
+  struct heap {
+    heap* next;
+    void* base;
+  };
+  heap* heaps;
+
+  heap_info (DWORD pid)
+    : heaps (0)
+  {
+    HANDLE hHeapSnap = CreateToolhelp32Snapshot (TH32CS_SNAPHEAPLIST, pid);
+    HEAPLIST32 hl;
+    hl.dwSize = sizeof(hl);
+
+    if (hHeapSnap != INVALID_HANDLE_VALUE && Heap32ListFirst (hHeapSnap, &hl))
+      do
+	{
+	  heap* h = (heap*) cmalloc (HEAP_FHANDLER, sizeof (heap));
+	  *h = (heap) {heaps, (void*)hl.th32HeapID};
+	  heaps = h;
+	} while (Heap32ListNext (hHeapSnap, &hl));
+    CloseHandle (hHeapSnap);
+  }
+  
+  char* fill_if_match (void* base, char* dest ) {
+    long count = 0;
+    for(heap* h = heaps; h && ++count; h = h->next)
+      if(base == h->base)
+	{
+	  __small_sprintf (dest, "[heap %ld]", count);
+	  return dest;
+	}
+    return 0;
+  }
+  
+  ~heap_info ()  {
+    heap* n = 0;
+    for (heap* m = heaps; m; m = n)
+      {
+	n = m->next;
+	cfree (m);
+      }
+  }
+};
+
 static _off64_t
 format_process_maps (void *data, char *&destbuf)
 {
@@ -628,6 +674,7 @@
   
   MEMORY_BASIC_INFORMATION mb;
   dos_drive_mappings drive_maps;
+  heap_info heaps(p->dwProcessId);
   struct __stat64 st;
   long last_pass = 0;
 
@@ -716,7 +763,8 @@
 		}
 	      else if (mb.Type & MEM_MAPPED)
 		strcpy (posix_modname, "[shareable]");
-	      else
+	      else if (!(mb.Type & MEM_PRIVATE
+			 && heaps.fill_if_match (cur.abase, posix_modname)))
 		posix_modname[0] = 0;
 	    }
 	}

[-- Attachment #4: proc-maps-safe.patch --]
[-- Type: text/plain, Size: 3726 bytes --]

# HG changeset patch
# Parent 989c2873d1a584bd26eb3cbb4be26d0d4aa8e691

diff --git a/fhandler_process.cc b/fhandler_process.cc
--- a/fhandler_process.cc
+++ b/fhandler_process.cc
@@ -528,6 +528,17 @@
   return len + 1;
 }
 
+/* This function exists for debugging purposes: it can be called from
+   a debugger or calls to it embedded in cygwin code.
+
+   WARNING: if called early in process startup, be sure to set pass
+   make_safe=true or set use_safe_process_maps=true before making the
+   call. This tells format_process_maps to avoid using cygwin features
+   that may not be initialized yet, at the cost of a less detailed
+   process map.  */
+void print_process_maps (bool make_safe);
+static bool use_safe_process_maps = false;
+
 struct dos_drive_mappings {
   struct mapping {
     mapping* next;
@@ -540,6 +551,9 @@
   dos_drive_mappings ()
     : mappings(0)
   {
+    if (use_safe_process_maps)
+      return; // can't use psapi or heap yet...
+    
     /* The logical drive strings buffer holds a list of (at most 26)
        drive names separated by nulls and terminated by a double-null:
 
@@ -613,6 +627,9 @@
   heap_info (DWORD pid)
     : heaps (0)
   {
+    if (use_safe_process_maps)
+      return; // not safe to use the heap
+    
     HANDLE hHeapSnap = CreateToolhelp32Snapshot (TH32CS_SNAPHEAPLIST, pid);
     HEAPLIST32 hl;
     hl.dwSize = sizeof(hl);
@@ -648,6 +665,27 @@
   }
 };
 
+void
+print_process_maps (bool make_safe)
+{
+  bool saved = use_safe_process_maps;
+  use_safe_process_maps |= make_safe;
+  
+  _pinfo p;
+  p.dwProcessId = GetCurrentProcessId();
+  char* buf = 0;
+  DWORD done;
+  _off64_t len = format_process_maps(&p, buf);
+  WriteFile (GetStdHandle (STD_ERROR_HANDLE), buf, len, &done, 0);
+  FlushFileBuffers (GetStdHandle (STD_ERROR_HANDLE));
+  if (use_safe_process_maps)
+    VirtualFree (buf, 0, MEM_RELEASE);
+  else
+    cfree (buf);
+  
+  use_safe_process_maps = saved;
+}
+
 static _off64_t
 format_process_maps (void *data, char *&destbuf)
 {
@@ -688,6 +726,18 @@
       cfree (destbuf);
       destbuf = NULL;
     }
+
+  if (use_safe_process_maps)
+    {
+      maxsize = 256*1024;
+      destbuf = (char*) VirtualAlloc (NULL, maxsize, MEM_COMMIT, PAGE_READWRITE);
+      if (!destbuf)
+	{
+	  system_printf ("Error allocating virtual memory: %E");
+	  return 0;
+	}
+    }
+	
   
   /* Iterate over each VM region in the address space, coalescing
      memory regions with the same permissions. Once we run out, do one
@@ -732,8 +782,16 @@
 	    {
 	      size_t newlen = strlen (posix_modname) + 62;
 	      if (len + newlen >= maxsize)
-		destbuf = (char *) crealloc_abort (destbuf,
-						   maxsize += roundup2 (newlen, 2048));
+		{
+		  if (use_safe_process_maps)
+		    {
+		      system_printf ("truncating output of format_process_maps to fit available memory");
+		      return len;
+		    }
+		  else
+		    destbuf = (char *) crealloc_abort (destbuf,
+						       maxsize += roundup2 (newlen, 2048));
+		}
 	      int written = __small_sprintf (destbuf + len,
 					     "%08lx-%08lx %s %08lx %04x:%04x %U   ",
 					     cur.rbase, cur.rend, cur.a.flags,
@@ -753,7 +811,19 @@
 	    {
 	      st.st_dev = 0;
 	      st.st_ino = 0;
-	      if ((mb.Type & (MEM_MAPPED | MEM_IMAGE)
+	      if (use_safe_process_maps)
+		{
+		  char const* n;
+		  if (mb.Type & MEM_MAPPED)
+		    n = "[mapped file or shareable region]";
+		  else if (mb.Type & MEM_IMAGE)
+		    n = "[executable image]";
+		  else
+		    n = "";
+		  
+		  strcpy (posix_modname, n);
+		}
+	      else if ((mb.Type & (MEM_MAPPED | MEM_IMAGE)
 		   && GetMappedFileNameW (proc, cur.abase, modname, NT_MAX_PATH)))
 		{
 		  PWCHAR dosname = drive_maps.fixup_if_match (modname);

[-- Attachment #5: proc-changes.txt --]
[-- Type: text/plain, Size: 428 bytes --]

2011-05-11  Ryan Johnson  <ryan.johnson@cs.utoronto.ca>

	* fhandler_process.cc (format_process_maps): Reworked to report
	all mapped address space in a process (committed or reserved),
	identifying the nature of the mapping (mapped file/image, heap,
	shared memory) when possible.
	(dos_drive_mappings, heap_info): New helper classes
	(print_process_maps): debugging aid
	* autoload.cc: Register GetMappedFileNameW (psapi.dll)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11  5:28 Extending /proc/*/maps Ryan Johnson
@ 2011-05-11 10:32 ` Corinna Vinschen
  2011-05-11 12:53   ` Ryan Johnson
  2011-05-11 11:15 ` Corinna Vinschen
  1 sibling, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-11 10:32 UTC (permalink / raw)
  To: cygwin-patches

Hi Ryan,

On May 11 01:27, Ryan Johnson wrote:
> Hi all,
> 
> Please find attached three patches which extend the functionality of
> /proc/*/maps.

Thanks!

I applied youyr two first patches with a couple of changes:

- Formatting: Setting of curly braces in class and method defintions,
  a lot of missing spaces, "int *foo" instead of "int* foo", stuff
  like that.  Please compare what I checked in against your patch. 
  That doesn't mean I always get it right myself, but basically that's
  how it should be.

- NT_MAX_PATH is the maximum size of a path including the trailing \0,
  so a buffer size of NT_MAX_PATH + 1 isn't required.

- Please always use sys_wcstombs rather than wcstombs for filenames.

- I replaced the call to GetMappedFileNameEx with a call to
  NtQueryVirtualMemory (MemorySectionName).  This avoids to add another
  dependency to psapi.  I intend to get rid of them entirely, if
  possible.

> NOTE 1: I do not attempt to identify PEB, TEB, or thread stacks. The
> first could be done easily enough, but the second and third require
> venturing into undocumented/private Windows APIs.

Go ahead!  We certainly don't shy away from calls to
NtQueryInformationProcess or NtQueryInformationThread.

> NOTE 2: If desired, we could easily extend format_process_maps
> further to report section names of mapped images (linux does this
> for .so files), [...]

Sorry if I'm dense, but isn't that exactly what GetMappedFileNameEx or
NtQueryVirtualMemory (MemorySectionName) do?  I also don't see any extra
information for .so files in the Linux maps output.  What detail am I
missing?


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11  5:28 Extending /proc/*/maps Ryan Johnson
  2011-05-11 10:32 ` Corinna Vinschen
@ 2011-05-11 11:15 ` Corinna Vinschen
  2011-05-11 17:46   ` Ryan Johnson
  1 sibling, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-11 11:15 UTC (permalink / raw)
  To: cygwin-patches

On May 11 01:27, Ryan Johnson wrote:
> The second (proc-maps-heaps) adds reporting of Windows heaps (or
> their bases, at least). Unfortunately there doesn't seem to be any
> efficient way to identify all virtual allocations which a heap owns.

There's a call RtlQueryDebugInformation which can fetch detailed heap
information, and which is used by Heap32First/Heap32Last.  Using it
directly is much more efficient than using the Heap32 functions.  The
DEBUG_HEAP_INFORMATION is already in ntdll.h, what's missing is the
layout of the Blocks info.  I found the info by googling:

  typedef struct _HEAP_BLOCK
  {
    PVOID addr;
    ULONG size;
    ULONG flags;
    ULONG unknown;
  } HEAP_BLOCK, *PHEAP_BLOCK;

If this information is searched until the address falls into the just
inspected  block of virtual memory, then we would have the information,
isn't it?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 10:32 ` Corinna Vinschen
@ 2011-05-11 12:53   ` Ryan Johnson
  2011-05-11 13:21     ` Corinna Vinschen
  0 siblings, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-11 12:53 UTC (permalink / raw)
  To: cygwin-patches

On 11/05/2011 6:31 AM, Corinna Vinschen wrote:
> Hi Ryan,
>
> On May 11 01:27, Ryan Johnson wrote:
>> Hi all,
>>
>> Please find attached three patches which extend the functionality of
>> /proc/*/maps.
> Thanks!
>
> I applied youyr two first patches with a couple of changes:
>
> - Formatting: Setting of curly braces in class and method defintions,
>    a lot of missing spaces, "int *foo" instead of "int* foo", stuff
>    like that.  Please compare what I checked in against your patch.
>    That doesn't mean I always get it right myself, but basically that's
>    how it should be.
Sorry... I did my best to match the surrounding code but didn't notice 
the '* ' vs ' *' thing.

> - NT_MAX_PATH is the maximum size of a path including the trailing \0,
>    so a buffer size of NT_MAX_PATH + 1 isn't required.
Good to know. Thanks.

> - I replaced the call to GetMappedFileNameEx with a call to
>    NtQueryVirtualMemory (MemorySectionName).  This avoids to add another
>    dependency to psapi.  I intend to get rid of them entirely, if
>    possible.
Nice. One issue: I tried backporting your change to my local tree, and 
the compiler complains that PMEMORY_SECTION_NAME isn't defined; the 
changelog says you updated ntdll.h to add it, but the online definition 
in w32api was last updated 9 months ago by 'duda.' Did it perhaps slip 
past the commit?

>> NOTE 1: I do not attempt to identify PEB, TEB, or thread stacks. The
>> first could be done easily enough, but the second and third require
>> venturing into undocumented/private Windows APIs.
> Go ahead!  We certainly don't shy away from calls to
> NtQueryInformationProcess or NtQueryInformationThread.
Ah. I didn't realize this sort of thing was encouraged. The MSDN docs 
about them are pretty gloomy.

The other things that discouraged me before were:
- the only obvious way to enumerate the threads in a process is to 
create a snapshot using TH32CS_SNAPTHREAD, which enumerates all threads 
in the system. This sounds expensive.
- it's not clear whether GetThreadContext returns a reasonable stack 
pointer if the target thread is active at the time.

However, assuming the above do not bother folks, they should be pretty 
straightforward to use. I won't have time to code this up in the 
immediate future, though. My real goal was to make fork bearable on my 
machine and that ended up sucking away all the time I had and then some...

>> NOTE 2: If desired, we could easily extend format_process_maps
>> further to report section names of mapped images (linux does this
>> for .so files), [...]
> Sorry if I'm dense, but isn't that exactly what GetMappedFileNameEx or
> NtQueryVirtualMemory (MemorySectionName) do?  I also don't see any extra
> information for .so files in the Linux maps output.  What detail am I
> missing?
Interesting... the machine I used for reference a couple weeks ago was 
running a really old debian, and for each allocation entry of a mapped 
image it gave the corresponding section name (.text, .bss, .rdata, etc):
3463600000-346362c000 r-xp 00000000 08:01 2097238                        
/lib64/libpcre.so.0.0.1 .text
346362c000-346382b000 ---p 0002c000 08:01 2097238                        
/lib64/libpcre.so.0.0.1
346382b000-346382c000 rw-p 0002b000 08:01 2097238                        
/lib64/libpcre.so.0.0.1 .bss

However, the machine was upgraded to a newer kernel this week and the 
extra information no longer appears.

In any case, should somebody want to report section names within a 
mapped image, that information can be had easily enough using the pefile 
struct from my fork patches.

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 12:53   ` Ryan Johnson
@ 2011-05-11 13:21     ` Corinna Vinschen
  2011-05-11 14:15       ` Ryan Johnson
  0 siblings, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-11 13:21 UTC (permalink / raw)
  To: cygwin-patches

On May 11 08:53, Ryan Johnson wrote:
> On 11/05/2011 6:31 AM, Corinna Vinschen wrote:
> >- I replaced the call to GetMappedFileNameEx with a call to
> >   NtQueryVirtualMemory (MemorySectionName).  This avoids to add another
> >   dependency to psapi.  I intend to get rid of them entirely, if
> >   possible.
> Nice. One issue: I tried backporting your change to my local tree,
> and the compiler complains that PMEMORY_SECTION_NAME isn't defined;
> the changelog says you updated ntdll.h to add it, but the online
> definition in w32api was last updated 9 months ago by 'duda.' Did it
> perhaps slip past the commit?

The compiler shouldn't complain because we only use the definitions
from ntdll.h and nothing else from w32api/include/ddk (except in rare
cases, see fhandler_tape.cc, fhandler_serial.cc).

Sometimes I apply new stuff to w32api as well on an "as it comes along"
base, but usually it's not the Cygwin project which maintains these
files.

> >>NOTE 1: I do not attempt to identify PEB, TEB, or thread stacks. The
> >>first could be done easily enough, but the second and third require
> >>venturing into undocumented/private Windows APIs.
> >Go ahead!  We certainly don't shy away from calls to
> >NtQueryInformationProcess or NtQueryInformationThread.
> Ah. I didn't realize this sort of thing was encouraged. The MSDN
> docs about them are pretty gloomy.

Gary Nebbett, "Windows NT/2000 Native API Reference".  It's a bit
rusty as far as new stuff is concerned, but it contains the important
stuff we can rely upon.

> The other things that discouraged me before were:
> - the only obvious way to enumerate the threads in a process is to
> create a snapshot using TH32CS_SNAPTHREAD, which enumerates all
> threads in the system. This sounds expensive.
> - it's not clear whether GetThreadContext returns a reasonable stack
> pointer if the target thread is active at the time.

Per MSDN the thread context is only valid if the thread is not running
at the time.   However, we could carefully inspect some of the values
and see how stable they are.

> However, assuming the above do not bother folks, they should be
> pretty straightforward to use. I won't have time to code this up in
> the immediate future, though. My real goal was to make fork bearable
> on my machine and that ended up sucking away all the time I had and
> then some...

Well, it's not high enough on my agenda to dive into it.  Feel free
to do that whenever you find a free time slot.

> >Sorry if I'm dense, but isn't that exactly what GetMappedFileNameEx or
> >NtQueryVirtualMemory (MemorySectionName) do?  I also don't see any extra
> >information for .so files in the Linux maps output.  What detail am I
> >missing?
> Interesting... the machine I used for reference a couple weeks ago
> was running a really old debian, and for each allocation entry of a
> mapped image it gave the corresponding section name (.text, .bss,
> .rdata, etc):
> 3463600000-346362c000 r-xp 00000000 08:01 2097238
> /lib64/libpcre.so.0.0.1 .text
> 346362c000-346382b000 ---p 0002c000 08:01 2097238
> /lib64/libpcre.so.0.0.1
> 346382b000-346382c000 rw-p 0002b000 08:01 2097238
> /lib64/libpcre.so.0.0.1 .bss
> 
> However, the machine was upgraded to a newer kernel this week and
> the extra information no longer appears.

Indeed, this information is not available on my 2.6.35 kernel.

> In any case, should somebody want to report section names within a
> mapped image, that information can be had easily enough using the
> pefile struct from my fork patches.

Right.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 13:21     ` Corinna Vinschen
@ 2011-05-11 14:15       ` Ryan Johnson
  0 siblings, 0 replies; 25+ messages in thread
From: Ryan Johnson @ 2011-05-11 14:15 UTC (permalink / raw)
  To: cygwin-patches

On 11/05/2011 9:20 AM, Corinna Vinschen wrote:
> On May 11 08:53, Ryan Johnson wrote:
>> On 11/05/2011 6:31 AM, Corinna Vinschen wrote:
>>> - I replaced the call to GetMappedFileNameEx with a call to
>>>    NtQueryVirtualMemory (MemorySectionName).  This avoids to add another
>>>    dependency to psapi.  I intend to get rid of them entirely, if
>>>    possible.
>> Nice. One issue: I tried backporting your change to my local tree,
>> and the compiler complains that PMEMORY_SECTION_NAME isn't defined;
>> the changelog says you updated ntdll.h to add it, but the online
>> definition in w32api was last updated 9 months ago by 'duda.' Did it
>> perhaps slip past the commit?
> The compiler shouldn't complain because we only use the definitions
> from ntdll.h and nothing else from w32api/include/ddk (except in rare
> cases, see fhandler_tape.cc, fhandler_serial.cc).
>
> Sometimes I apply new stuff to w32api as well on an "as it comes along"
> base, but usually it's not the Cygwin project which maintains these
> files.
Ah... there are two ntdll.h, and I looked at the one in w32api/include 
instead of cygwin. All is well.

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 11:15 ` Corinna Vinschen
@ 2011-05-11 17:46   ` Ryan Johnson
  2011-05-11 19:32     ` Corinna Vinschen
  0 siblings, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-11 17:46 UTC (permalink / raw)
  To: cygwin-patches

On 11/05/2011 7:14 AM, Corinna Vinschen wrote:
> On May 11 01:27, Ryan Johnson wrote:
>> The second (proc-maps-heaps) adds reporting of Windows heaps (or
>> their bases, at least). Unfortunately there doesn't seem to be any
>> efficient way to identify all virtual allocations which a heap owns.
> There's a call RtlQueryDebugInformation which can fetch detailed heap
> information, and which is used by Heap32First/Heap32Last.  Using it
> directly is much more efficient than using the Heap32 functions.  The
> DEBUG_HEAP_INFORMATION is already in ntdll.h, what's missing is the
> layout of the Blocks info.  I found the info by googling:
>
>    typedef struct _HEAP_BLOCK
>    {
>      PVOID addr;
>      ULONG size;
>      ULONG flags;
>      ULONG unknown;
>    } HEAP_BLOCK, *PHEAP_BLOCK;
>
> If this information is searched until the address falls into the just
> inspected  block of virtual memory, then we would have the information,
> isn't it?
The problem is that the reported heap blocks corresponded to individual 
malloc() calls when I tested it (unordered list, most sizes < 100B). I 
really would have preferred a function that returns the list of memory 
regions owned by the heap. They must track that information -- I highly 
HeapDestroy uses these heap blocks to decide which memory regions to 
VirtualFree, but that info seems to live in the kernel...

Given that Heap32* has already been reverse-engineered by others, the 
main challenge would involve sorting the set of heap block addresses and 
distilling them down to a set of allocation bases. We don't want to do 
repeated linear searches over 50k+ heap blocks.

Also, the cygheap isn't a normal windows heap, is it? I thought it was 
essentially a statically-allocated array (.cygheap) that gets managed as 
a heap. I guess since it's part of cygwin1.dll we already do sort of 
report it...

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 17:46   ` Ryan Johnson
@ 2011-05-11 19:32     ` Corinna Vinschen
  2011-05-12 12:10       ` Corinna Vinschen
  2011-05-12 16:37       ` Ryan Johnson
  0 siblings, 2 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-11 19:32 UTC (permalink / raw)
  To: cygwin-patches

On May 11 13:46, Ryan Johnson wrote:
> On 11/05/2011 7:14 AM, Corinna Vinschen wrote:
> >On May 11 01:27, Ryan Johnson wrote:
> >>The second (proc-maps-heaps) adds reporting of Windows heaps (or
> >>their bases, at least). Unfortunately there doesn't seem to be any
> >>efficient way to identify all virtual allocations which a heap owns.
> >There's a call RtlQueryDebugInformation which can fetch detailed heap
> >information, and which is used by Heap32First/Heap32Last.  Using it
> >directly is much more efficient than using the Heap32 functions.  The
> >DEBUG_HEAP_INFORMATION is already in ntdll.h, what's missing is the
> >layout of the Blocks info.  I found the info by googling:
> >
> >   typedef struct _HEAP_BLOCK
> >   {
> >     PVOID addr;
> >     ULONG size;
> >     ULONG flags;
> >     ULONG unknown;
> >   } HEAP_BLOCK, *PHEAP_BLOCK;
> >
> >If this information is searched until the address falls into the just
> >inspected  block of virtual memory, then we would have the information,
> >isn't it?
> The problem is that the reported heap blocks corresponded to
> individual malloc() calls when I tested it (unordered list, most
> sizes < 100B). I really would have preferred a function that returns
> the list of memory regions owned by the heap. They must track that
> information -- I highly HeapDestroy uses these heap blocks to decide
> which memory regions to VirtualFree, but that info seems to live in
> the kernel...

No, it doesn't.  The heap allocation functions are implemented entirely
in user space.  Only virtual memory and shared sections are maintained
by the kernel.

Looking into the MSDN man page for HeapCreate, I'm wondering if this is
really complicated.  HeapCreate just allocates a block of memory using
VirtualAlloc.  Either the Heap is fixed size, then all allocations are
within the given heap memory area, or the heap can grow, then subsequent
(or big-block) allocations can result in another VirtualAlloc.

Having said that, I don't know where this information is stored exactly,
but it must be available in the DEBUG_HEAP_INFORMATION block.  I guess
a bit of experimenting is asked for.

> Given that Heap32* has already been reverse-engineered by others,
> the main challenge would involve sorting the set of heap block
> addresses and distilling them down to a set of allocation bases. We
> don't want to do repeated linear searches over 50k+ heap blocks.

While the base address of the heap is available in
DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
Assuming just that, you could scan the list of blocks once and drop
those within the orignal heap allocation.  The remaining blocks are big
blocks which have been allocated by additional calls to VirtualAlloc.

However, there's a good chance that the entire information about
blocks allocated with VirtualAlloc is kept in the heap's own
datastructures which, again, are undocumented.  I would assume the
start of that heap info is at the heaps base address, but without
more information about the internal structure...

> Also, the cygheap isn't a normal windows heap, is it? I thought it
> was essentially a statically-allocated array (.cygheap) that gets
> managed as a heap. I guess since it's part of cygwin1.dll we already
> do sort of report it...

The cygheap is the last section in the DLL and gets allocated by the
Windows loader.  The memory management is entirely in Cygwin (cygheap.cc).
Cygwin can raise the size of the cygheap, but only if the blocks right
after the existing cygheap are not already allocated.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 19:32     ` Corinna Vinschen
@ 2011-05-12 12:10       ` Corinna Vinschen
  2011-05-12 15:10         ` Corinna Vinschen
  2011-05-12 16:37       ` Ryan Johnson
  1 sibling, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 12:10 UTC (permalink / raw)
  To: cygwin-patches

On May 11 21:31, Corinna Vinschen wrote:
> On May 11 13:46, Ryan Johnson wrote:
> > Given that Heap32* has already been reverse-engineered by others,
> > the main challenge would involve sorting the set of heap block
> > addresses and distilling them down to a set of allocation bases. We
> > don't want to do repeated linear searches over 50k+ heap blocks.
> 
> While the base address of the heap is available in
> DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
> the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
> Assuming just that, you could scan the list of blocks once and drop
> those within the orignal heap allocation.  The remaining blocks are big
> blocks which have been allocated by additional calls to VirtualAlloc.

After some debugging, I now have the solution.  If you fetch the heap
and heap block information using the RtlQueryProcessDebugInformation
function, you get the list of blocks.  The first block in the list of
blocks has a flag value of 2.  Let's call it a "start block".  This
block contains the information about the address and size of the virtual
memory block reserved for this heap using VirtualAlloc.

Subsequent blocks do not contain address values, but only size values.
The start of the block is counted relative to the start of the previous
block.  All these blocks are "in-use", up to the last block which has
a flag value of 0x100.  This is the free block at the end of the heap.

For fixed-size heaps, that's all.  Growable heaps can have multiple
memory slots reserved with VirtualAlloc.  For these, if there are still
more blocks, the next block in the list after the free block is a start
block with a flag value of 2 again.  Here's the algorithm which prints
only the virtual memory blocks of all heaps:

  typedef struct _HEAPS
  {
    ULONG count;
    DEBUG_HEAP_INFORMATION dhi[1];
  } HEAPS, *PHEAPS;

  typedef struct _HEAP_BLOCK
  {
    ULONG size;
    ULONG flags;
    ULONG unknown;
    ULONG addr;
  } HEAP_BLOCK, *PHEAP_BLOCK;

  PDEBUG_BUFFER buf;
  NTSTATUS status;

  buf = RtlCreateQueryDebugBuffer (0, FALSE);
  if (!buf)
    {
      fprintf (stderr, "RtlCreateQueryDebugBuffer returned NULL\n");
      return 1;
    }
  status = RtlQueryProcessDebugInformation (GetCurrentProcessId (),
					    PDI_HEAPS | PDI_HEAP_BLOCKS,
					    buf);
  if (!NT_SUCCESS (status))
    [...]
  PHEAPS heaps = (PHEAPS) buf->HeapInformation;
  if (!heaps)
    [...]
  for (int heap_id = 0; heap_id < heaps->count; ++heap_id)
    {
      PHEAP_BLOCK blocks = (PHEAP_BLOCK) heaps->dhi[heap_id].Blocks;
      if (!blocks)
	[...]
      uintptr_t addr = 0;
      for (int block_num = 0; block_num < heaps->dhi[heap_id].BlockCount;
	   ++block_num)
	if (blocks[block_num].flags & 2)
	  printf ("addr 0x%08lx, size %8lu"
		  blocks[block_num].addr,
		    blocks[block_num].size);
    }
  RtlDestroyQueryDebugBuffer (buf);

As you can imagine, only the blocks I called the start blocks are
of interest for your struct heap_info.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 12:10       ` Corinna Vinschen
@ 2011-05-12 15:10         ` Corinna Vinschen
  2011-05-12 16:31           ` Ryan Johnson
  0 siblings, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 15:10 UTC (permalink / raw)
  To: cygwin-patches

On May 12 14:10, Corinna Vinschen wrote:
> On May 11 21:31, Corinna Vinschen wrote:
> > On May 11 13:46, Ryan Johnson wrote:
> > > Given that Heap32* has already been reverse-engineered by others,
> > > the main challenge would involve sorting the set of heap block
> > > addresses and distilling them down to a set of allocation bases. We
> > > don't want to do repeated linear searches over 50k+ heap blocks.
> > 
> > While the base address of the heap is available in
> > DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
> > the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
> > Assuming just that, you could scan the list of blocks once and drop
> > those within the orignal heap allocation.  The remaining blocks are big
> > blocks which have been allocated by additional calls to VirtualAlloc.
> 
> After some debugging, I now have the solution. [...]

Here's a prelimiary patch to fhandler_process.cc which takes everything
into account I have learned in the meantime.  For instance, there are
actually heaps marked as shareable.  Please have a look.  What's missing
is the flag for low-fragmentation heaps, but I'm just hunting for it.


Corinna


Index: fhandler_process.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_process.cc,v
retrieving revision 1.96
diff -u -p -r1.96 fhandler_process.cc
--- fhandler_process.cc	11 May 2011 10:31:22 -0000	1.96
+++ fhandler_process.cc	12 May 2011 15:08:03 -0000
@@ -609,39 +609,78 @@ struct dos_drive_mappings
   }
 };
 
+/* Known heap flags */
+#define HEAP_FLAG_NOSERIALIZE	0x1
+#define HEAP_FLAG_GROWABLE	0x2
+#define HEAP_FLAG_EXCEPTIONS	0x4
+#define HEAP_FLAG_NONDEFAULT	0x1000
+#define HEAP_FLAG_SHAREABLE	0x8000
+#define HEAP_FLAG_EXECUTABLE	0x40000
+
 struct heap_info
 {
   struct heap
   {
     heap *next;
-    void *base;
+    unsigned heap_id;
+    uintptr_t base;
+    uintptr_t end;
+    unsigned long flags;
   };
   heap *heaps;
 
   heap_info (DWORD pid)
     : heaps (0)
   {
-    HANDLE hHeapSnap = CreateToolhelp32Snapshot (TH32CS_SNAPHEAPLIST, pid);
-    HEAPLIST32 hl;
-    hl.dwSize = sizeof(hl);
-
-    if (hHeapSnap != INVALID_HANDLE_VALUE && Heap32ListFirst (hHeapSnap, &hl))
-      do
-	{
-	  heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
-	  *h = (heap) {heaps, (void*) hl.th32HeapID};
-	  heaps = h;
-	} while (Heap32ListNext (hHeapSnap, &hl));
-    CloseHandle (hHeapSnap);
+    PDEBUG_BUFFER buf;
+    NTSTATUS status;
+    PDEBUG_HEAP_ARRAY harray;
+
+    buf = RtlCreateQueryDebugBuffer (0, FALSE);
+    if (!buf)
+      return;
+    status = RtlQueryProcessDebugInformation (pid, PDI_HEAPS | PDI_HEAP_BLOCKS,
+					      buf);
+    if (NT_SUCCESS (status)
+	&& (harray = (PDEBUG_HEAP_ARRAY) buf->HeapInformation) != NULL)
+      for (ULONG hcnt = 0; hcnt < harray->Count; ++hcnt)
+      	{
+	  PDEBUG_HEAP_BLOCK barray = (PDEBUG_HEAP_BLOCK)
+				     harray->Heaps[hcnt].Blocks;
+	  if (!barray)
+	    continue;
+	  for (ULONG bcnt = 0; bcnt < harray->Heaps[hcnt].BlockCount; ++bcnt)
+	    if (barray[bcnt].Flags & 2)
+	      {
+		heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
+		*h = (heap) {heaps, hcnt, barray[bcnt].Address,
+			     barray[bcnt].Address + barray[bcnt].Size,
+			     harray->Heaps[hcnt].Flags};
+		heaps = h;
+	      }
+	}
+    RtlDestroyQueryDebugBuffer (buf);
   }
   
-  char *fill_if_match (void *base, char *dest )
+  char *fill_if_match (void *base, ULONG type, char *dest )
   {
-    long count = 0;
-    for (heap *h = heaps; h && ++count; h = h->next)
-      if (base == h->base)
+    for (heap *h = heaps; h; h = h->next)
+      if ((uintptr_t) base >= h->base && (uintptr_t) base < h->end)
 	{
-	  __small_sprintf (dest, "[heap %ld]", count);
+	  char *p;
+	  __small_sprintf (dest, "[heap %ld", h->heap_id);
+	  p = strchr (dest, '\0');
+	  if (!(h->flags & HEAP_FLAG_NONDEFAULT))
+	    p = stpcpy (p, " default");
+	  if ((h->flags & HEAP_FLAG_SHAREABLE) && (type & MEM_MAPPED))
+	    p = stpcpy (p, " share");
+	  if (h->flags & HEAP_FLAG_EXECUTABLE)
+	    p = stpcpy (p, " exec");
+	  if (h->flags & HEAP_FLAG_GROWABLE)
+	    p = stpcpy (p, " grow");
+	  if (h->flags & HEAP_FLAG_NOSERIALIZE)
+	    p = stpcpy (p, " noserial");
+	  stpcpy (p, "]");
 	  return dest;
 	}
     return 0;
@@ -777,11 +816,13 @@ format_process_maps (void *data, char *&
 		    sys_wcstombs (posix_modname, NT_MAX_PATH, dosname);
 		  stat64 (posix_modname, &st);
 		}
-	      else if (mb.Type & MEM_MAPPED)
-		strcpy (posix_modname, "[shareable]");
-	      else if (!(mb.Type & MEM_PRIVATE
-			 && heaps.fill_if_match (cur.abase, posix_modname)))
-		posix_modname[0] = 0;
+	      else if (!heaps.fill_if_match (cur.abase, mb.Type, posix_modname))
+		{
+		  if (mb.Type & MEM_MAPPED)
+		    strcpy (posix_modname, "[shareable]");
+		  else
+		    posix_modname[0] = 0;
+		}
 	    }
 	}
     }
Index: ntdll.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/ntdll.h,v
retrieving revision 1.117
diff -u -p -r1.117 ntdll.h
--- ntdll.h	11 May 2011 13:25:27 -0000	1.117
+++ ntdll.h	12 May 2011 15:08:03 -0000
@@ -63,6 +63,7 @@
 
 #define PDI_MODULES 0x01
 #define PDI_HEAPS 0x04
+#define PDI_HEAP_BLOCKS 0x10
 #define LDRP_IMAGE_DLL 0x00000004
 #define WSLE_PAGE_READONLY 0x001
 #define WSLE_PAGE_EXECUTE 0x002
@@ -525,6 +526,20 @@ typedef struct _DEBUG_HEAP_INFORMATION
   PVOID Blocks;
 } DEBUG_HEAP_INFORMATION, *PDEBUG_HEAP_INFORMATION;
 
+typedef struct _DEBUG_HEAP_ARRAY
+{
+  ULONG Count;
+  DEBUG_HEAP_INFORMATION Heaps[1];
+} DEBUG_HEAP_ARRAY, *PDEBUG_HEAP_ARRAY;
+
+typedef struct _DEBUG_HEAP_BLOCK
+{
+  ULONG Size;
+  ULONG Flags;
+  ULONG Unknown;
+  ULONG Address;
+} DEBUG_HEAP_BLOCK, *PDEBUG_HEAP_BLOCK;
+
 typedef struct _DEBUG_MODULE_INFORMATION
 {
   ULONG Reserved[2];


-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 15:10         ` Corinna Vinschen
@ 2011-05-12 16:31           ` Ryan Johnson
  2011-05-12 16:56             ` Corinna Vinschen
  0 siblings, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-12 16:31 UTC (permalink / raw)
  To: cygwin-patches

On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
> On May 12 14:10, Corinna Vinschen wrote:
>> On May 11 21:31, Corinna Vinschen wrote:
>>> On May 11 13:46, Ryan Johnson wrote:
>>>> Given that Heap32* has already been reverse-engineered by others,
>>>> the main challenge would involve sorting the set of heap block
>>>> addresses and distilling them down to a set of allocation bases. We
>>>> don't want to do repeated linear searches over 50k+ heap blocks.
>>> While the base address of the heap is available in
>>> DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
>>> the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
>>> Assuming just that, you could scan the list of blocks once and drop
>>> those within the orignal heap allocation.  The remaining blocks are big
>>> blocks which have been allocated by additional calls to VirtualAlloc.
>> After some debugging, I now have the solution. [...]
> Here's a prelimiary patch to fhandler_process.cc which takes everything
> into account I have learned in the meantime.  For instance, there are
> actually heaps marked as shareable.  Please have a look.  What's missing
> is the flag for low-fragmentation heaps, but I'm just hunting for it.
I like it. Detailed comments below.

> +/* Known heap flags */
> +#define HEAP_FLAG_NOSERIALIZE	0x1
> +#define HEAP_FLAG_GROWABLE	0x2
> +#define HEAP_FLAG_EXCEPTIONS	0x4
> +#define HEAP_FLAG_NONDEFAULT	0x1000
> +#define HEAP_FLAG_SHAREABLE	0x8000
> +#define HEAP_FLAG_EXECUTABLE	0x40000
Would it make sense to put those in ntdll.h along with the heap structs 
that use them?

>     struct heap
>     {
>       heap *next;
> -    void *base;
> +    unsigned heap_id;
> +    uintptr_t base;
> +    uintptr_t end;
> +    unsigned long flags;
>     };
We don't actually need the end pointer: we're trying to match an unknown 
allocation base against heap region bases. The code which traverses VM 
allocations should query heap_info at most once per allocation (for 
example, it only looks up the file name of cygwin1.dll once even though 
the latter has 12 entries in /proc/*/maps).

>     heap *heaps;
This is a misnomer now -- it's really a list of heap regions/blocks.

> +		heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
> +		*h = (heap) {heaps, hcnt, barray[bcnt].Address,
> +			     barray[bcnt].Address + barray[bcnt].Size,
> +			     harray->Heaps[hcnt].Flags};
> +		heaps = h;
Given that the number of heap blocks is potentially large, I think it 
makes sense to build a sorted list. That way, each query examines only 
one heap block (deleting it unless it was above the queried address). I 
have ready-but-unsent a patch which does this to the checked-in version 
of the code. Shall I send it?

> -  char *fill_if_match (void *base, char *dest )
> +  char *fill_if_match (void *base, ULONG type, char *dest )
>     {
> -    long count = 0;
> -    for (heap *h = heaps; h&&  ++count; h = h->next)
> -      if (base == h->base)
> +    for (heap *h = heaps; h; h = h->next)
> +      if ((uintptr_t) base>= h->base&&  (uintptr_t) base<  h->end)
>   	{
> -	  __small_sprintf (dest, "[heap %ld]", count);
> +	  char *p;
> +	  __small_sprintf (dest, "[heap %ld", h->heap_id);
> +	  p = strchr (dest, '\0');
> +	  if (!(h->flags&  HEAP_FLAG_NONDEFAULT))
> +	    p = stpcpy (p, " default");
> +	  if ((h->flags&  HEAP_FLAG_SHAREABLE)&&  (type&  MEM_MAPPED))
> +	    p = stpcpy (p, " share");
> +	  if (h->flags&  HEAP_FLAG_EXECUTABLE)
> +	    p = stpcpy (p, " exec");
> +	  if (h->flags&  HEAP_FLAG_GROWABLE)
> +	    p = stpcpy (p, " grow");
> +	  if (h->flags&  HEAP_FLAG_NOSERIALIZE)
> +	    p = stpcpy (p, " noserial");
> +	  stpcpy (p, "]");
>   	  return dest;
>   	}
>       return 0;
Do you actually encounter requests which fall inside a heap region 
rather than at its start? I have not seen this in my experiments, and if 
you have it is almost certainly a bug in format_process_maps' allocation 
traversal.

Also, are there ever shareable-but-not-mem_mapped segments? If not, we 
could probably remove 'type.'

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-11 19:32     ` Corinna Vinschen
  2011-05-12 12:10       ` Corinna Vinschen
@ 2011-05-12 16:37       ` Ryan Johnson
  2011-05-12 16:57         ` Corinna Vinschen
  1 sibling, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-12 16:37 UTC (permalink / raw)
  To: cygwin-patches

On 11/05/2011 3:31 PM, Corinna Vinschen wrote:
> On May 11 13:46, Ryan Johnson wrote:
>> Also, the cygheap isn't a normal windows heap, is it? I thought it
>> was essentially a statically-allocated array (.cygheap) that gets
>> managed as a heap. I guess since it's part of cygwin1.dll we already
>> do sort of report it...
> The cygheap is the last section in the DLL and gets allocated by the
> Windows loader.  The memory management is entirely in Cygwin (cygheap.cc).
> Cygwin can raise the size of the cygheap, but only if the blocks right
> after the existing cygheap are not already allocated.
Would it make sense to give that section, and the one(s) which 
immediately follow it, the tag "[cygheap]" rather than 
"/usr/bin/cygwin1.dll" and nothing? It would require struct pefile to 
identify the section, plus some trickery in format_process_maps to treat 
the cygwin dll and adjacent succeeding allocation(s) specially.

Ryan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 16:31           ` Ryan Johnson
@ 2011-05-12 16:56             ` Corinna Vinschen
  2011-05-12 17:12               ` Corinna Vinschen
  2011-05-12 17:53               ` Ryan Johnson
  0 siblings, 2 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 16:56 UTC (permalink / raw)
  To: cygwin-patches

On May 12 12:31, Ryan Johnson wrote:
> On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
> >On May 12 14:10, Corinna Vinschen wrote:
> >>On May 11 21:31, Corinna Vinschen wrote:
> >>>On May 11 13:46, Ryan Johnson wrote:
> >>>>Given that Heap32* has already been reverse-engineered by others,
> >>>>the main challenge would involve sorting the set of heap block
> >>>>addresses and distilling them down to a set of allocation bases. We
> >>>>don't want to do repeated linear searches over 50k+ heap blocks.
> >>>While the base address of the heap is available in
> >>>DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
> >>>the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
> >>>Assuming just that, you could scan the list of blocks once and drop
> >>>those within the orignal heap allocation.  The remaining blocks are big
> >>>blocks which have been allocated by additional calls to VirtualAlloc.
> >>After some debugging, I now have the solution. [...]
> >Here's a prelimiary patch to fhandler_process.cc which takes everything
> >into account I have learned in the meantime.  For instance, there are
> >actually heaps marked as shareable.  Please have a look.  What's missing
> >is the flag for low-fragmentation heaps, but I'm just hunting for it.
> I like it. Detailed comments below.
> 
> >+/* Known heap flags */
> >+#define HEAP_FLAG_NOSERIALIZE	0x1
> >+#define HEAP_FLAG_GROWABLE	0x2
> >+#define HEAP_FLAG_EXCEPTIONS	0x4
> >+#define HEAP_FLAG_NONDEFAULT	0x1000
> >+#define HEAP_FLAG_SHAREABLE	0x8000
> >+#define HEAP_FLAG_EXECUTABLE	0x40000
> Would it make sense to put those in ntdll.h along with the heap
> structs that use them?

Sure.

> >    struct heap
> >    {
> >      heap *next;
> >-    void *base;
> >+    unsigned heap_id;
> >+    uintptr_t base;
> >+    uintptr_t end;
> >+    unsigned long flags;
> >    };
> We don't actually need the end pointer: we're trying to match an

No, we need it.  The heaps consist of reserved and committed memory
blocks, as well as of shareable and non-shareable blocks.  Thus you
get multiple VirtualQuery calls per heap, thus you have to check for
the address within the entire heap(*).

> >    heap *heaps;
> This is a misnomer now -- it's really a list of heap regions/blocks.

I don't think so.  The loop stores only the blocks which constitute
the original VirtualAlloc'ed memory regions.  They are not the real
heap blocks returned by Heap32First/Heap32Next.  These are filtered
out by checking for flags & 2 (**).

> >+		heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
> >+		*h = (heap) {heaps, hcnt, barray[bcnt].Address,
> >+			     barray[bcnt].Address + barray[bcnt].Size,
> >+			     harray->Heaps[hcnt].Flags};
> >+		heaps = h;
> Given that the number of heap blocks is potentially large, I think

Not really.  See (**).  3 heaps -> 3 blocks, or only slightly more
if a growable heap got expanded.

> Do you actually encounter requests which fall inside a heap region
> rather than at its start?

Yes, see (*).  And have a look into the output of my code in
contrast to what's printed by the currently version from CVS.

>  I have not seen this in my experiments,
> and if you have it is almost certainly a bug in format_process_maps'
> allocation traversal.

No, see (*).

> Also, are there ever shareable-but-not-mem_mapped segments?

Yes, there are.  2 of the 3 default heaps are marked as shareable, but
one of them is only partially shared.  Of course I don't understand the
reason behind this and how this is accomplished, given that the user
code can't even set a flag "shareable" at HeapCreate time.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 16:37       ` Ryan Johnson
@ 2011-05-12 16:57         ` Corinna Vinschen
  0 siblings, 0 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 16:57 UTC (permalink / raw)
  To: cygwin-patches

On May 12 12:37, Ryan Johnson wrote:
> On 11/05/2011 3:31 PM, Corinna Vinschen wrote:
> >On May 11 13:46, Ryan Johnson wrote:
> >>Also, the cygheap isn't a normal windows heap, is it? I thought it
> >>was essentially a statically-allocated array (.cygheap) that gets
> >>managed as a heap. I guess since it's part of cygwin1.dll we already
> >>do sort of report it...
> >The cygheap is the last section in the DLL and gets allocated by the
> >Windows loader.  The memory management is entirely in Cygwin (cygheap.cc).
> >Cygwin can raise the size of the cygheap, but only if the blocks right
> >after the existing cygheap are not already allocated.
> Would it make sense to give that section, and the one(s) which
> immediately follow it, the tag "[cygheap]" rather than
> "/usr/bin/cygwin1.dll" and nothing? It would require struct pefile
> to identify the section, plus some trickery in format_process_maps
> to treat the cygwin dll and adjacent succeeding allocation(s)
> specially.

I wouldn't do that.  Just because there's a allocated block right
after the cygheap doesn't mean it's part of the cygheap.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 16:56             ` Corinna Vinschen
@ 2011-05-12 17:12               ` Corinna Vinschen
  2011-05-12 17:54                 ` Ryan Johnson
  2011-05-12 17:53               ` Ryan Johnson
  1 sibling, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 17:12 UTC (permalink / raw)
  To: cygwin-patches

On May 12 18:55, Corinna Vinschen wrote:
> On May 12 12:31, Ryan Johnson wrote:
> > On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
> > >-    void *base;
> > >+    unsigned heap_id;
> > >+    uintptr_t base;
> > >+    uintptr_t end;
> > >+    unsigned long flags;
> > >    };
> > We don't actually need the end pointer: we're trying to match an
> 
> No, we need it.  The heaps consist of reserved and committed memory
> blocks, as well as of shareable and non-shareable blocks.  Thus you
> get multiple VirtualQuery calls per heap, thus you have to check for
> the address within the entire heap(*).

Btw., here's a good example.  There are three default heaps, One of them,
heap 0, is the heap you get with GetProcessHeap ().  I don't know the
task of heap 1 yet, but heap 2 is ... something, as well as the stack of
the first thread in the process.  It looks like this:

  base 0x00020000, flags 0x00008000, granularity     8, unknown     0
  allocated     1448, committed    65536, block count 3
  Block 0: addr 0x00020000, size  2150400, flags 0x00000002, unknown 0x00010000

However, the various calls to VirtualQuery result in this output with
my patch:

  00020000-00030000 rw-p 00000000 0000:0000 0      [heap 2 default share]
  00030000-00212000 ---p 00000000 0000:0000 0      [heap 2 default]
  00212000-00213000 rw-s 001E2000 0000:0000 0      [heap 2 default]
  00213000-00230000 rw-p 001E3000 0000:0000 0      [heap 2 default]

The "something" is the sharable area from 0x20000 up to 0x30000.  The
stack is from 0x30000 up to 0x230000.  The first reagion is only
reserved, then the guard page, then the committed and used  tack area.


Corinna


-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 16:56             ` Corinna Vinschen
  2011-05-12 17:12               ` Corinna Vinschen
@ 2011-05-12 17:53               ` Ryan Johnson
  2011-05-12 18:43                 ` Corinna Vinschen
  1 sibling, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-12 17:53 UTC (permalink / raw)
  To: cygwin-patches

On 12/05/2011 12:55 PM, Corinna Vinschen wrote:
>>>     struct heap
>>>     {
>>>       heap *next;
>>> -    void *base;
>>> +    unsigned heap_id;
>>> +    uintptr_t base;
>>> +    uintptr_t end;
>>> +    unsigned long flags;
>>>     };
>> We don't actually need the end pointer: we're trying to match an
> No, we need it.  The heaps consist of reserved and committed memory
> blocks, as well as of shareable and non-shareable blocks.  Thus you
> get multiple VirtualQuery calls per heap, thus you have to check for
> the address within the entire heap(*).
The code which queries heap_info already deals with allocations that 
take multiple VirtualQuery calls to traverse, and only calls into 
heap_info for the base of any given allocation. These bases are 
identified because they have the same value for both mb.AllocationBase 
and mb.BaseAddress (except with unallocated regions, for which 
AllocationBase is undefined). This is how I compute the reported 
offsets. Further, adjacent regions having the same base and whose 
permissions resolve to the same rwxp bits are coalesced and reported as 
a single region. For example, several parts of a rebased dll will have 
copy-on-write regions (those where no rewrites took place), but these 
are collapsed because rwxp can't express copy-on-write (dlls can have up 
to 2x more VirtualQuery calls than is reflected in the output of 
/proc/*/maps).

In other words, it shouldn't matter how the heap has sliced up a given 
region, as long as the base address of each region is properly 
identified by some heap block having non-zero flags&2 (see caveat 
below). If no such block exists, then the code in 
heap_info::fill_if_match would need to test whether [heap_base,heap_end) 
overlaps [alloc_base,alloc_end) because alloc_base will never fall 
inside any block on the list.

CAVEAT: According to MSDN's documentation for HeapWalk, "a heap consists 
of one or more regions of virtual memory, each with a unique region 
index." During heap walking, wFlags is set to PROCESS_HEAP_REGION 
whenever "the heap element is located at the beginning of a region of 
contiguous virtual memory in use by the heap." However, "large block 
regions" (those allocated directly via VirtualAlloc) do not set the 
PROCESS_HEAP_REGION flag. If this PROCESS_HEAP_REGION flag corresponds 
to your (flags & 2), then we won't report any large blocks (however, 
it's documented to have the value 0x0001, with 0x0002 being 
PROCESS_HEAP_UNCOMMITTED_RANGE).

>>>     heap *heaps;
>> This is a misnomer now -- it's really a list of heap regions/blocks.
> I don't think so.  The loop stores only the blocks which constitute
> the original VirtualAlloc'ed memory regions.  They are not the real
> heap blocks returned by Heap32First/Heap32Next.  These are filtered
> out by checking for flags&  2 (**).
Sorry, I cut too much context out of the diff. That's heap_info::heaps, 
which indeed refers to heap regions which we identified by checking 
flags&2 (that's why it needs the heap_id inside it now, where it didn't 
before) (++)

>>> +		heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
>>> +		*h = (heap) {heaps, hcnt, barray[bcnt].Address,
>>> +			     barray[bcnt].Address + barray[bcnt].Size,
>>> +			     harray->Heaps[hcnt].Flags};
>>> +		heaps = h;
>> Given that the number of heap blocks is potentially large, I think
> Not really.  See (**).  3 heaps ->  3 blocks, or only slightly more
> if a growable heap got expanded.
See (++). When I point my essentially-identical version of the code at 
emacs, it reports 8 heaps, each with 2-4 regions. The list traversed by 
fill_on_match has ~20 entries.

>> Do you actually encounter requests which fall inside a heap region
>> rather than at its start?
> Yes, see (*).  And have a look into the output of my code in
> contrast to what's printed by the currently version from CVS.
I'll have to test out the patch -- my local version which checks only 
allocation bases reports additional regions (those not part of the 
original heap allocation), but if there are yet other regions which I'm 
missing and yours finds then I'll stand corrected about (++).

>>   I have not seen this in my experiments,
>> and if you have it is almost certainly a bug in format_process_maps'
>> allocation traversal.
> No, see (*).
>
>> Also, are there ever shareable-but-not-mem_mapped segments?
> Yes, there are.  2 of the 3 default heaps are marked as shareable, but
> one of them is only partially shared.  Of course I don't understand the
> reason behind this and how this is accomplished, given that the user
> code can't even set a flag "shareable" at HeapCreate time.
Hmm. Weird. I'll respond further after testing your patch and comparing 
its output against my local version of the same code.

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 17:12               ` Corinna Vinschen
@ 2011-05-12 17:54                 ` Ryan Johnson
  2011-05-12 18:48                   ` Corinna Vinschen
  0 siblings, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-12 17:54 UTC (permalink / raw)
  To: cygwin-patches

On 12/05/2011 1:11 PM, Corinna Vinschen wrote:
> On May 12 18:55, Corinna Vinschen wrote:
>> On May 12 12:31, Ryan Johnson wrote:
>>> On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
>>>> -    void *base;
>>>> +    unsigned heap_id;
>>>> +    uintptr_t base;
>>>> +    uintptr_t end;
>>>> +    unsigned long flags;
>>>>     };
>>> We don't actually need the end pointer: we're trying to match an
>> No, we need it.  The heaps consist of reserved and committed memory
>> blocks, as well as of shareable and non-shareable blocks.  Thus you
>> get multiple VirtualQuery calls per heap, thus you have to check for
>> the address within the entire heap(*).
> Btw., here's a good example.  There are three default heaps, One of them,
> heap 0, is the heap you get with GetProcessHeap ().  I don't know the
> task of heap 1 yet, but heap 2 is ... something, as well as the stack of
> the first thread in the process.  It looks like this:
>
>    base 0x00020000, flags 0x00008000, granularity     8, unknown     0
>    allocated     1448, committed    65536, block count 3
>    Block 0: addr 0x00020000, size  2150400, flags 0x00000002, unknown 0x00010000
>
> However, the various calls to VirtualQuery result in this output with
> my patch:
>
>    00020000-00030000 rw-p 00000000 0000:0000 0      [heap 2 default share]
>    00030000-00212000 ---p 00000000 0000:0000 0      [heap 2 default]
>    00212000-00213000 rw-s 001E2000 0000:0000 0      [heap 2 default]
>    00213000-00230000 rw-p 001E3000 0000:0000 0      [heap 2 default]
>
> The "something" is the sharable area from 0x20000 up to 0x30000.  The
> stack is from 0x30000 up to 0x230000.  The first reagion is only
> reserved, then the guard page, then the committed and used  tack area.
Hmm. It looks like heap 2 was allocated by mapping the pagefile rather 
than using VirtualAlloc, and the thread's stack was allocated from heap 
2, which treated the request as a large block and returned the result of 
a call to VirtualAlloc.

Are the other two heap bases not "default share" then?

In any case, coming back to the allocation base issue, heap_info only 
needs to track 0x20000 and 0x30000, because they are the ones with 
offset zero that would trigger a call to heap_info::fill_on_match. I 
argue that heap walking found exactly two flags&2 blocks, with exactly 
those base addresses, making the range check in fill_on_match unecessary.

Again, I'll try running your patch instead of my own when I get a 
chance, and see if yours finds regions mine fails to label. However, if 
0x30000 above really is a large block region, then at least my worries 
about flags&2 were unfounded, which is great news.

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 17:53               ` Ryan Johnson
@ 2011-05-12 18:43                 ` Corinna Vinschen
  2011-05-12 18:51                   ` Corinna Vinschen
                                     ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 18:43 UTC (permalink / raw)
  To: cygwin-patches

On May 12 13:53, Ryan Johnson wrote:
> On 12/05/2011 12:55 PM, Corinna Vinschen wrote:
> >>>    struct heap
> >>>    {
> >>>      heap *next;
> >>>-    void *base;
> >>>+    unsigned heap_id;
> >>>+    uintptr_t base;
> >>>+    uintptr_t end;
> >>>+    unsigned long flags;
> >>>    };
> >>We don't actually need the end pointer: we're trying to match an
> >No, we need it.  The heaps consist of reserved and committed memory
> >blocks, as well as of shareable and non-shareable blocks.  Thus you
> >get multiple VirtualQuery calls per heap, thus you have to check for
> >the address within the entire heap(*).
> The code which queries heap_info already deals with allocations that
> take multiple VirtualQuery calls to traverse, and only calls into
> heap_info for the base of any given allocation. 

However, at least heap 2 consists of multiple allocated memory areas,
which are separately VirtualAlloc'ed or NtMapViewOfSection'ed.  Therefore
the first VirtualQuery returns AllocationBase = BaseAddress = 0x20000
and the next VirtualQuery returns AllocationBase = BaseAddress = 0x30000.
However, all the memory from 0x20000 up to 0x230000 constitutes a single
start block in heap 2!

> CAVEAT: According to MSDN's documentation for HeapWalk, "a heap
> consists of one or more regions of virtual memory, each with a
> unique region index." During heap walking, wFlags is set to
> PROCESS_HEAP_REGION whenever "the heap element is located at the
> beginning of a region of contiguous virtual memory in use by the
> heap." However, "large block regions" (those allocated directly via
> VirtualAlloc) do not set the PROCESS_HEAP_REGION flag. If this
> PROCESS_HEAP_REGION flag corresponds to your (flags & 2), then we
> won't report any large blocks (however, it's documented to have the
> value 0x0001, with 0x0002 being PROCESS_HEAP_UNCOMMITTED_RANGE).

I created a test application with several heaps and I create large
blocks in two of them.  They also have the flags&2 value set.  The
problem for heap walk is to identify blocks with a valid address.  Keep
in mind that all subsequent blocks within an allocated heap area do
*not* have an addres, but only a size, with address == 0.  You only get
their address by iterating from the start block to the block you're
looking for and adding up all sizes of the blocks up to there.

Consequentially a start block of another VirtualAlloc'ed area needs a
marker that the address is valid.  That marker is the flags value 2.
Which is kind of weird, actually, since the existence of a non-0 address
in the block should have been enough of a hint that this is a start
block...

As for the big blocks, they are apparently identified by the value in
the "Unknown" member of the DEBUG_HEAP_BLOCK structure.  Here's what I
figured out so far as far as "Unknown" is concerned:

   0x1000  All normal heaps
   0x3000  The process default heap (heap 0)
   0xc000  A low-frag heap
  0x10000  Heap 2, perhaps the meaning is "stack"?
  0x32000  Subsequently allocated block of a growable heap
 0x1e9000  Large block

I don't claim to understand the values, especially the reason for
setting several bits.

> >>>    heap *heaps;
> >>This is a misnomer now -- it's really a list of heap regions/blocks.
> >I don't think so.  The loop stores only the blocks which constitute
> >the original VirtualAlloc'ed memory regions.  They are not the real
> >heap blocks returned by Heap32First/Heap32Next.  These are filtered
> >out by checking for flags&  2 (**).
> Sorry, I cut too much context out of the diff. That's
> heap_info::heaps, which indeed refers to heap regions which we
> identified by checking flags&2 (that's why it needs the heap_id
> inside it now, where it didn't before) (++)

So you think something like heap_chunks is better?

> >>>+		heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
> >>>+		*h = (heap) {heaps, hcnt, barray[bcnt].Address,
> >>>+			     barray[bcnt].Address + barray[bcnt].Size,
> >>>+			     harray->Heaps[hcnt].Flags};
> >>>+		heaps = h;
> >>Given that the number of heap blocks is potentially large, I think
> >Not really.  See (**).  3 heaps ->  3 blocks, or only slightly more
> >if a growable heap got expanded.
> See (++). When I point my essentially-identical version of the code
> at emacs, it reports 8 heaps, each with 2-4 regions. The list
> traversed by fill_on_match has ~20 entries.

Oh, ok.  From my POV 20 is not a large number.  Ordering might take
more time than scanning.  I don't think it's worth the effort.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 17:54                 ` Ryan Johnson
@ 2011-05-12 18:48                   ` Corinna Vinschen
  2011-05-12 19:19                     ` Ryan Johnson
  0 siblings, 1 reply; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 18:48 UTC (permalink / raw)
  To: cygwin-patches

On May 12 13:53, Ryan Johnson wrote:
> On 12/05/2011 1:11 PM, Corinna Vinschen wrote:
> >On May 12 18:55, Corinna Vinschen wrote:
> >>On May 12 12:31, Ryan Johnson wrote:
> >>>On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
> >>>>-    void *base;
> >>>>+    unsigned heap_id;
> >>>>+    uintptr_t base;
> >>>>+    uintptr_t end;
> >>>>+    unsigned long flags;
> >>>>    };
> >>>We don't actually need the end pointer: we're trying to match an
> >>No, we need it.  The heaps consist of reserved and committed memory
> >>blocks, as well as of shareable and non-shareable blocks.  Thus you
> >>get multiple VirtualQuery calls per heap, thus you have to check for
> >>the address within the entire heap(*).
> >Btw., here's a good example.  There are three default heaps, One of them,
> >heap 0, is the heap you get with GetProcessHeap ().  I don't know the
> >task of heap 1 yet, but heap 2 is ... something, as well as the stack of
> >the first thread in the process.  It looks like this:
> >
> >   base 0x00020000, flags 0x00008000, granularity     8, unknown     0
> >   allocated     1448, committed    65536, block count 3
> >   Block 0: addr 0x00020000, size  2150400, flags 0x00000002, unknown 0x00010000
> >
> >However, the various calls to VirtualQuery result in this output with
> >my patch:
> >
> >   00020000-00030000 rw-p 00000000 0000:0000 0      [heap 2 default share]
> >   00030000-00212000 ---p 00000000 0000:0000 0      [heap 2 default]
> >   00212000-00213000 rw-s 001E2000 0000:0000 0      [heap 2 default]
> >   00213000-00230000 rw-p 001E3000 0000:0000 0      [heap 2 default]
> >
> >The "something" is the sharable area from 0x20000 up to 0x30000.  The
> >stack is from 0x30000 up to 0x230000.  The first reagion is only
> >reserved, then the guard page, then the committed and used  tack area.
> Hmm. It looks like heap 2 was allocated by mapping the pagefile
> rather than using VirtualAlloc, and the thread's stack was allocated
> from heap 2, which treated the request as a large block and returned
> the result of a call to VirtualAlloc.
> 
> Are the other two heap bases not "default share" then?

Here's what I see for instance in tcsh:

00010000-00020000 rw-p 00000000 0000:0000 0       [heap 1 default share]

00020000-00030000 rw-p 00000000 0000:0000 0       [heap 2 default share]
00030000-00212000 ---p 00000000 0000:0000 0       [heap 2 default]
00212000-00213000 rw-s 001E2000 0000:0000 0       [heap 2 default]
00213000-00230000 rw-p 001E3000 0000:0000 0       [heap 2 default]

002E0000-00310000 rw-p 00000000 0000:0000 0       [heap 0 default grow]
00310000-003E0000 ---p 00030000 0000:0000 0       [heap 0 default grow]

> In any case, coming back to the allocation base issue, heap_info
> only needs to track 0x20000 and 0x30000, because they are the ones
> with offset zero that would trigger a call to
> heap_info::fill_on_match. I argue that heap walking found exactly
> two flags&2 blocks, with exactly those base addresses, making the
> range check in fill_on_match unecessary.

Nope.  As I wrote in my previoous mail and as you can still see in my
quote above, the two virtual memory areas from 0x20000 to 0x30000 and
from 0x30000 to 0x230000 together constitute a single start block in
heap 2.  The OS is a great faker in terms of information returned to
the application, apparently.

> Again, I'll try running your patch instead of my own when I get a
> chance, and see if yours finds regions mine fails to label. However,
> if 0x30000 above really is a large block region, then at least my
> worries about flags&2 were unfounded, which is great news.

0x30000 is not a large block region.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 18:43                 ` Corinna Vinschen
@ 2011-05-12 18:51                   ` Corinna Vinschen
  2011-05-12 19:30                   ` Ryan Johnson
  2011-05-12 21:13                   ` Corinna Vinschen
  2 siblings, 0 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 18:51 UTC (permalink / raw)
  To: cygwin-patches

On May 12 20:42, Corinna Vinschen wrote:
> I created a test application with several heaps [...]

Btw., here's my test app.  It's ugly but it's just for testing the
results anyway.


Corinna

#include <stdio.h>
#define _WIN32_WINNT 0x0601
#include <windows.h>
#include <ddk/ntddk.h>
#include <ddk/ntapi.h>

typedef struct _MODULES
{
  ULONG count;
  DEBUG_MODULE_INFORMATION dmi[1];
} MODULES, *PMODULES;

typedef struct _HEAPS
{
  ULONG count;
  DEBUG_HEAP_INFORMATION dhi[1];
} HEAPS, *PHEAPS;

typedef struct _HEAP_BLOCK
{
  ULONG size;
  ULONG flags;
  ULONG unknown;
  ULONG addr;
} HEAP_BLOCK, *PHEAP_BLOCK;

#define HEAP_CREATE_ENABLE_EXECUTE 0x40000
int
main ()
{
  PDEBUG_BUFFER buf;
  NTSTATUS status;
  int i;
  HMODULE modules[100];
  DWORD needed = sizeof modules;

  HANDLE h0 = HeapCreate (0, 0, 0);
  ULONG lowfrag = 2;
  if (!HeapSetInformation (h0, HeapCompatibilityInformation,
			   &lowfrag, sizeof lowfrag))
    fprintf (stderr, "HeapSet: %lu\n", GetLastError ());
  printf ("alloc h0: %p %p\n", h0, HeapAlloc (h0, 0, 32));
  HANDLE h1 = HeapCreate (0, 0, 0);
  printf ("alloc h1: %p %p\n", h1, HeapAlloc (h1, 0, 32));
  HANDLE h1b = HeapCreate (0, 0, 65536);
  printf ("alloc h1b: %p %p\n", h1b, HeapAlloc (h1b, 0, 32));
  HANDLE h2 = HeapCreate (HEAP_NO_SERIALIZE, 4096, 4 * 65536);
  printf ("alloc h2: %p %p\n", h2, HeapAlloc (h2, 0, 32));
  HANDLE h3 = HeapCreate (HEAP_GENERATE_EXCEPTIONS, 4096, 8 * 65536);
  printf ("alloc h3: %p %p\n", h3, HeapAlloc (h3, 0, 32));
  HANDLE h4 = HeapCreate (HEAP_CREATE_ENABLE_EXECUTE, 4096, 0);
  printf ("alloc h4: %p %p\n", h4, HeapAlloc (h4, 0, 200000));
  HANDLE h5 = HeapCreate (0, 4096, 0);
  printf ("alloc h5: %p %p\n", h5, HeapAlloc (h5, 0, 2000000));
  buf = RtlCreateQueryDebugBuffer (0, FALSE);
  if (!buf)
    {
      fprintf (stderr, "RtlCreateQueryDebugBuffer returned NULL\n");
      return 1;
    }
  status = RtlQueryProcessDebugInformation (GetCurrentProcessId (),
					    PDI_MODULES | PDI_HEAPS
					    | PDI_HEAP_BLOCKS, buf);
  if (!NT_SUCCESS (status))
    {
      fprintf (stderr, "RtlQueryProcessDebugInformation returned 0x%08lx\n", status);
      return 1;
    }
#if 0
  PMODULES mods = (PMODULES) buf->ModuleInformation;
  if (!mods)
    fprintf (stderr, "mods is NULL\n");
  else
    {
      for (i = 0; i < mods->count; ++i)
      	printf ("%40s Base 0x%08lx, Size %8lu U 0x%08lx\n",
		mods->dmi[i].ImageName,
		mods->dmi[i].Base,
		mods->dmi[i].Size,
		mods->dmi[i].Unknown);
    }
#endif
  PHEAPS heaps = (PHEAPS) buf->HeapInformation;
  if (!heaps)
    fprintf (stderr, "heaps is NULL\n");
  else
    {
      for (i = 0; i < heaps->count; ++i)
	{
	  int r;

#if 1
	  printf ("%3d: base 0x%08lx, flags 0x%08lx, granularity %5hu, unknown %5hu\n"
		  "     allocated %8lu, committed %8lu, block count %lu\n"
		  "     reserved",
		  i,
		  heaps->dhi[i].Base,
		  heaps->dhi[i].Flags,
		  heaps->dhi[i].Granularity,
		  heaps->dhi[i].Unknown,
		  heaps->dhi[i].Allocated,
		  heaps->dhi[i].Committed,
		  heaps->dhi[i].BlockCount);
	  for (r = 0; r < 7; ++r)
	    printf (" %lu",
		    heaps->dhi[i].Reserved[r],
		    heaps->dhi[i].Reserved[r]);
	  puts ("");
#else
	  printf ("%3d: base 0x%08lx, flags 0x%08lx\n",
		  i,
		  heaps->dhi[i].Base,
		  heaps->dhi[i].Flags);
#endif
	  PHEAP_BLOCK blocks = (PHEAP_BLOCK) heaps->dhi[i].Blocks;
	  if (!blocks)
	    fprintf (stderr, "blocks is NULL\n");
	  else
	    {
	      uintptr_t addr = 0;
	      char flags[32];
	      printf ("     Blocks:\n");
	      for (r = 0; r < heaps->dhi[i].BlockCount; ++r)
		{
		  flags[0] = '\0';
		  if (blocks[r].flags & 2)
		    {
		      addr = blocks[r].addr;
		      strcpy (flags, "start");
		    }
		  else if (blocks[r].flags & 0x2f1) 
		    strcpy (flags, "fixed");
		  else if (blocks[r].flags & 0x20)
		    strcpy (flags, "moveable");
		  else if (blocks[r].flags & 0x100)
		    strcpy (flags, "free");
		  if (blocks[r].flags & 2)
		    printf ("     %3d: addr 0x%08lx, size %8lu, flags 0x%08lx, "
			    "unknown 0x%08lx\n",
			    r,
			    addr,
			    blocks[r].size,
			    blocks[r].flags,
			    blocks[r].unknown);
#if 0
		  else
		    printf ("     %3d: addr 0x%08lx, size %8lu, "
			    "flags 0x%08x %8s\n",
			    r,
			    addr,
			    blocks[r].size,
			    blocks[r].flags,
			    flags);
#endif
		  if (blocks[r].flags & 2)
		    addr += heaps->dhi[i].Granularity;
		  else
		    addr += blocks[r].size;
		}
	    }
	}
    }
  RtlDestroyQueryDebugBuffer (buf);
  getchar ();
  return 0;
}

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 18:48                   ` Corinna Vinschen
@ 2011-05-12 19:19                     ` Ryan Johnson
  2011-05-12 19:31                       ` Corinna Vinschen
  0 siblings, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-12 19:19 UTC (permalink / raw)
  To: cygwin-patches

On 12/05/2011 2:48 PM, Corinna Vinschen wrote:
> On May 12 13:53, Ryan Johnson wrote:
>> On 12/05/2011 1:11 PM, Corinna Vinschen wrote:
>>> On May 12 18:55, Corinna Vinschen wrote:
>>>> On May 12 12:31, Ryan Johnson wrote:
>>>>> On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
>>>>>> -    void *base;
>>>>>> +    unsigned heap_id;
>>>>>> +    uintptr_t base;
>>>>>> +    uintptr_t end;
>>>>>> +    unsigned long flags;
>>>>>>     };
>>>>> We don't actually need the end pointer: we're trying to match an
>>>> No, we need it.  The heaps consist of reserved and committed memory
>>>> blocks, as well as of shareable and non-shareable blocks.  Thus you
>>>> get multiple VirtualQuery calls per heap, thus you have to check for
>>>> the address within the entire heap(*).
>>> Btw., here's a good example.  There are three default heaps, One of them,
>>> heap 0, is the heap you get with GetProcessHeap ().  I don't know the
>>> task of heap 1 yet, but heap 2 is ... something, as well as the stack of
>>> the first thread in the process.  It looks like this:
>>>
>>>    base 0x00020000, flags 0x00008000, granularity     8, unknown     0
>>>    allocated     1448, committed    65536, block count 3
>>>    Block 0: addr 0x00020000, size  2150400, flags 0x00000002, unknown 0x00010000
>>>
>>> However, the various calls to VirtualQuery result in this output with
>>> my patch:
>>>
>>>    00020000-00030000 rw-p 00000000 0000:0000 0      [heap 2 default share]
>>>    00030000-00212000 ---p 00000000 0000:0000 0      [heap 2 default]
>>>    00212000-00213000 rw-s 001E2000 0000:0000 0      [heap 2 default]
>>>    00213000-00230000 rw-p 001E3000 0000:0000 0      [heap 2 default]
>>>
>>> The "something" is the sharable area from 0x20000 up to 0x30000.  The
>>> stack is from 0x30000 up to 0x230000.  The first reagion is only
>>> reserved, then the guard page, then the committed and used  tack area.
>> Hmm. It looks like heap 2 was allocated by mapping the pagefile
>> rather than using VirtualAlloc, and the thread's stack was allocated
>> from heap 2, which treated the request as a large block and returned
>> the result of a call to VirtualAlloc.
>>
>> Are the other two heap bases not "default share" then?
> Here's what I see for instance in tcsh:
>
> 00010000-00020000 rw-p 00000000 0000:0000 0       [heap 1 default share]
>
> 00020000-00030000 rw-p 00000000 0000:0000 0       [heap 2 default share]
> 00030000-00212000 ---p 00000000 0000:0000 0       [heap 2 default]
> 00212000-00213000 rw-s 001E2000 0000:0000 0       [heap 2 default]
> 00213000-00230000 rw-p 001E3000 0000:0000 0       [heap 2 default]
>
> 002E0000-00310000 rw-p 00000000 0000:0000 0       [heap 0 default grow]
> 00310000-003E0000 ---p 00030000 0000:0000 0       [heap 0 default grow]
>
>> In any case, coming back to the allocation base issue, heap_info
>> only needs to track 0x20000 and 0x30000, because they are the ones
>> with offset zero that would trigger a call to
>> heap_info::fill_on_match. I argue that heap walking found exactly
>> two flags&2 blocks, with exactly those base addresses, making the
>> range check in fill_on_match unecessary.
> Nope.  As I wrote in my previoous mail and as you can still see in my
> quote above, the two virtual memory areas from 0x20000 to 0x30000 and
> from 0x30000 to 0x230000 together constitute a single start block in
> heap 2.  The OS is a great faker in terms of information returned to
> the application, apparently.
OK. I finally understand now. I was assuming the heap would not report 
multiple allocations as a single heap region.

Windows is weird.

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 18:43                 ` Corinna Vinschen
  2011-05-12 18:51                   ` Corinna Vinschen
@ 2011-05-12 19:30                   ` Ryan Johnson
  2011-05-13  6:52                     ` Corinna Vinschen
  2011-05-12 21:13                   ` Corinna Vinschen
  2 siblings, 1 reply; 25+ messages in thread
From: Ryan Johnson @ 2011-05-12 19:30 UTC (permalink / raw)
  To: cygwin-patches

On 12/05/2011 2:42 PM, Corinna Vinschen wrote:
> On May 12 13:53, Ryan Johnson wrote:
>> On 12/05/2011 12:55 PM, Corinna Vinschen wrote:
>>>>>     heap *heaps;
>>>> This is a misnomer now -- it's really a list of heap regions/blocks.
>>> I don't think so.  The loop stores only the blocks which constitute
>>> the original VirtualAlloc'ed memory regions.  They are not the real
>>> heap blocks returned by Heap32First/Heap32Next.  These are filtered
>>> out by checking for flags&   2 (**).
>> Sorry, I cut too much context out of the diff. That's
>> heap_info::heaps, which indeed refers to heap regions which we
>> identified by checking flags&2 (that's why it needs the heap_id
>> inside it now, where it didn't before) (++)
> So you think something like heap_chunks is better?
Maybe. I actually only noticed because the code snippet you originally 
sent also used 'heaps' as an identifier and the two clashed when I 
pasted it in ;)

>>>>> +		heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
>>>>> +		*h = (heap) {heaps, hcnt, barray[bcnt].Address,
>>>>> +			     barray[bcnt].Address + barray[bcnt].Size,
>>>>> +			     harray->Heaps[hcnt].Flags};
>>>>> +		heaps = h;
>>>> Given that the number of heap blocks is potentially large, I think
>>> Not really.  See (**).  3 heaps ->   3 blocks, or only slightly more
>>> if a growable heap got expanded.
>> See (++). When I point my essentially-identical version of the code
>> at emacs, it reports 8 heaps, each with 2-4 regions. The list
>> traversed by fill_on_match has ~20 entries.
> Oh, ok.  From my POV 20 is not a large number.  Ordering might take
> more time than scanning.  I don't think it's worth the effort.
You might be right. I just threw my test case at Firefox, and even that 
memory hog generates < 50 entries. I guess it does some sort of manual 
memory management as well, because windbg reports waaaay more allocated 
address space regions than that...

Besides, you've shown that we potentially need each heap block multiple 
times, so delete-after-use isn't a good idea.

Ryan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 19:19                     ` Ryan Johnson
@ 2011-05-12 19:31                       ` Corinna Vinschen
  0 siblings, 0 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 19:31 UTC (permalink / raw)
  To: cygwin-patches

On May 12 15:19, Ryan Johnson wrote:
> On 12/05/2011 2:48 PM, Corinna Vinschen wrote:
> >Nope.  As I wrote in my previoous mail and as you can still see in my
> >quote above, the two virtual memory areas from 0x20000 to 0x30000 and
> >from 0x30000 to 0x230000 together constitute a single start block in
> >heap 2.  The OS is a great faker in terms of information returned to
> >the application, apparently.
> OK. I finally understand now. I was assuming the heap would not
> report multiple allocations as a single heap region.
> 
> Windows is weird.

Exaclty.  Funnily, in the end it always boils down to that. 


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 18:43                 ` Corinna Vinschen
  2011-05-12 18:51                   ` Corinna Vinschen
  2011-05-12 19:30                   ` Ryan Johnson
@ 2011-05-12 21:13                   ` Corinna Vinschen
  2 siblings, 0 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-12 21:13 UTC (permalink / raw)
  To: cygwin-patches

On May 12 20:42, Corinna Vinschen wrote:
> As for the big blocks, they are apparently identified by the value in
> the "Unknown" member of the DEBUG_HEAP_BLOCK structure.  Here's what I
> figured out so far as far as "Unknown" is concerned:

Scratch that.  I finally *really* figured out what the unknown field
contains.  It's just the number of committed bytes in the block.  So
there's no special identification for subsequent blocks.  They are
simply another start block in the heap block list.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Extending /proc/*/maps
  2011-05-12 19:30                   ` Ryan Johnson
@ 2011-05-13  6:52                     ` Corinna Vinschen
  0 siblings, 0 replies; 25+ messages in thread
From: Corinna Vinschen @ 2011-05-13  6:52 UTC (permalink / raw)
  To: cygwin-patches

On May 12 15:30, Ryan Johnson wrote:
> On 12/05/2011 2:42 PM, Corinna Vinschen wrote:
> >On May 12 13:53, Ryan Johnson wrote:
> >>On 12/05/2011 12:55 PM, Corinna Vinschen wrote:
> >>>>>    heap *heaps;
> >>>>This is a misnomer now -- it's really a list of heap regions/blocks.
> >>>I don't think so.  The loop stores only the blocks which constitute
> >>>the original VirtualAlloc'ed memory regions.  They are not the real
> >>>heap blocks returned by Heap32First/Heap32Next.  These are filtered
> >>>out by checking for flags&   2 (**).
> >>Sorry, I cut too much context out of the diff. That's
> >>heap_info::heaps, which indeed refers to heap regions which we
> >>identified by checking flags&2 (that's why it needs the heap_id
> >>inside it now, where it didn't before) (++)
> >So you think something like heap_chunks is better?
> Maybe. I actually only noticed because the code snippet you
> originally sent also used 'heaps' as an identifier and the two
> clashed when I pasted it in ;)

I've renamed it to heap_vm_chunks and checked my patch in.

I'm going to follow up on the cygwin-developers list.  Now that we have
your excellent maps output, it's pretty easy to see where the potential
problems for fork are.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-05-13  6:52 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-11  5:28 Extending /proc/*/maps Ryan Johnson
2011-05-11 10:32 ` Corinna Vinschen
2011-05-11 12:53   ` Ryan Johnson
2011-05-11 13:21     ` Corinna Vinschen
2011-05-11 14:15       ` Ryan Johnson
2011-05-11 11:15 ` Corinna Vinschen
2011-05-11 17:46   ` Ryan Johnson
2011-05-11 19:32     ` Corinna Vinschen
2011-05-12 12:10       ` Corinna Vinschen
2011-05-12 15:10         ` Corinna Vinschen
2011-05-12 16:31           ` Ryan Johnson
2011-05-12 16:56             ` Corinna Vinschen
2011-05-12 17:12               ` Corinna Vinschen
2011-05-12 17:54                 ` Ryan Johnson
2011-05-12 18:48                   ` Corinna Vinschen
2011-05-12 19:19                     ` Ryan Johnson
2011-05-12 19:31                       ` Corinna Vinschen
2011-05-12 17:53               ` Ryan Johnson
2011-05-12 18:43                 ` Corinna Vinschen
2011-05-12 18:51                   ` Corinna Vinschen
2011-05-12 19:30                   ` Ryan Johnson
2011-05-13  6:52                     ` Corinna Vinschen
2011-05-12 21:13                   ` Corinna Vinschen
2011-05-12 16:37       ` Ryan Johnson
2011-05-12 16:57         ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).