[PATCH 0/5] Improve dumper megion region selection

[PATCH 0/5] Improve dumper megion region selection

[Jon Turney]

Improve how dumper determines if a memory region should be dumped:

Currently we open and read the PE file for each module, and exclude regions
corresponding to sections marked 'DEBUGGING' or 'CODE'.

This doesn't work correctly if the DLL has been loaded to an address other
than the ImageBase recorded in the PE header.  It fails to produce a useful
dump if there's a collision in excluded region addresses (which will always
occur on x86_64, as kernel32.dll has an ImageBase which collides with the
cygwin1.dll)

This probably also doesn't produce correct dumps if the protection on memory
regions corresponding to 'CODE' sections is manipulated using VirtualProtect().

Instead, dump memory region based on their type, protection and sharability:

- state is MEM_COMMIT (i.e. is not MEM_RESERVE or MEM_FREE), and
-- type is MEM_PRIVATE and protection allows reads (i.e. not a guardpage), or
-- type is MEM_IMAGE and attribute is non-sharable (i.e. it was WC, got 
   written to, and is now a RW copy)

Jon Turney (5):
  Cygwin: Show details of all memory regions details in dumper debug
    output
  Cygwin: Remove reading of PE for section flags from dumper
  Cygwin: Drop excluded regions list from dumper
  Cygwin: Don't dump non-writable image regions
  Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper

 winsup/doc/utils.xml     |   8 +-
 winsup/utils/Makefile.in |   8 +-
 winsup/utils/dumper.cc   | 214 +++++++++++++++++++++++++++------------
 winsup/utils/dumper.h    |  19 ----
 winsup/utils/parse_pe.cc | 107 --------------------
 5 files changed, 155 insertions(+), 201 deletions(-)
 delete mode 100644 winsup/utils/parse_pe.cc

-- 
2.27.0

[PATCH 1/5] Cygwin: Show details of all memory regions details in dumper debug output

[Jon Turney]

---
 winsup/utils/dumper.cc | 101 ++++++++++++++++++++++++++++++++---------
 1 file changed, 80 insertions(+), 21 deletions(-)

diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index ccc4bd12f..46e4b0692 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -289,6 +289,25 @@ dumper::add_module (LPVOID base_address)
 
 #define PAGE_BUFFER_SIZE 4096
 
+void protect_dump(DWORD protect, char *buf)
+{
+  const char *pt[10];
+  pt[0] = (protect & PAGE_READONLY) ? "RO " : "";
+  pt[1] = (protect & PAGE_READWRITE) ? "RW " : "";
+  pt[2] = (protect & PAGE_WRITECOPY) ? "WC " : "";
+  pt[3] = (protect & PAGE_EXECUTE) ? "EX " : "";
+  pt[4] = (protect & PAGE_EXECUTE_READ) ? "EXRO " : "";
+  pt[5] = (protect & PAGE_EXECUTE_READWRITE) ? "EXRW " : "";
+  pt[6] = (protect & PAGE_EXECUTE_WRITECOPY) ? "EXWC " : "";
+  pt[7] = (protect & PAGE_GUARD) ? "GRD " : "";
+  pt[8] = (protect & PAGE_NOACCESS) ? "NA " : "";
+  pt[9] = (protect & PAGE_NOCACHE) ? "NC " : "";
+
+  buf[0] = '\0';
+  for (int i = 0; i < 10; i++)
+    strcat (buf, pt[i]);
+}
+
 int
 dumper::collect_memory_sections ()
 {
@@ -313,10 +332,65 @@ dumper::collect_memory_sections ()
 	break;
 
       int skip_region_p = 0;
+      const char *disposition = "dumped";
 
-      if (mbi.Protect & (PAGE_NOACCESS | PAGE_GUARD) ||
-	  mbi.State != MEM_COMMIT)
-	skip_region_p = 1;
+      if (mbi.Protect & PAGE_NOACCESS)
+	{
+	  skip_region_p = 1;
+	  disposition = "skipped due to noaccess";
+	}
+
+      if (mbi.Protect & PAGE_GUARD)
+	{
+	  skip_region_p = 1;
+	  disposition = "skipped due to guardpage";
+	}
+
+      if (mbi.State != MEM_COMMIT)
+	{
+	  skip_region_p = 1;
+	  disposition = "skipped due to uncommited";
+	}
+
+      {
+	char buf[10 * 6];
+	protect_dump(mbi.Protect, buf);
+
+	const char *state = "";
+	const char *type = "";
+
+	if (mbi.State & MEM_COMMIT)
+	  {
+	    state = "COMMIT";
+	  }
+	else if (mbi.State & MEM_FREE)
+	  {
+	    state = "FREE";
+	    type = "FREE";
+	  }
+	else if (mbi.State & MEM_RESERVE)
+	  {
+	    state = "RESERVE";
+	  }
+
+	if (mbi.Type & MEM_IMAGE)
+	  {
+	    type = "IMAGE";
+	  }
+	else if (mbi.Type & MEM_MAPPED)
+	  {
+	    type = "MAPPED";
+	  }
+	else if (mbi.Type & MEM_PRIVATE)
+	  {
+	    type = "PRIVATE";
+	  }
+
+	deb_printf ("region 0x%016lx-0x%016lx (protect = %-8s, state = %-7s, type = %-7s, %s)\n",
+		    current_page_address,
+		    current_page_address + mbi.RegionSize,
+		    buf, state, type, disposition);
+      }
 
       if (!skip_region_p)
 	{
@@ -326,26 +400,11 @@ dumper::collect_memory_sections ()
 	  if (!ReadProcessMemory (hProcess, current_page_address, mem_buf, sizeof (mem_buf), &done))
 	    {
 	      DWORD err = GetLastError ();
-	      const char *pt[10];
-	      pt[0] = (mbi.Protect & PAGE_READONLY) ? "RO " : "";
-	      pt[1] = (mbi.Protect & PAGE_READWRITE) ? "RW " : "";
-	      pt[2] = (mbi.Protect & PAGE_WRITECOPY) ? "WC " : "";
-	      pt[3] = (mbi.Protect & PAGE_EXECUTE) ? "EX " : "";
-	      pt[4] = (mbi.Protect & PAGE_EXECUTE_READ) ? "EXRO " : "";
-	      pt[5] = (mbi.Protect & PAGE_EXECUTE_READWRITE) ? "EXRW " : "";
-	      pt[6] = (mbi.Protect & PAGE_EXECUTE_WRITECOPY) ? "EXWC " : "";
-	      pt[7] = (mbi.Protect & PAGE_GUARD) ? "GRD " : "";
-	      pt[8] = (mbi.Protect & PAGE_NOACCESS) ? "NA " : "";
-	      pt[9] = (mbi.Protect & PAGE_NOCACHE) ? "NC " : "";
-	      char buf[10 * 6];
-	      buf[0] = '\0';
-	      for (int i = 0; i < 10; i++)
-		strcat (buf, pt[i]);
-
-	      deb_printf ("warning: failed to read memory at %p-%p (protect = %s), error %ld.\n",
+
+	      deb_printf ("warning: failed to read memory at %p-%p, error %ld.\n",
 			  current_page_address,
 			  current_page_address + mbi.RegionSize,
-			  buf, err);
+			  err);
 	      skip_region_p = 1;
 	    }
 	}
-- 
2.27.0

[PATCH 2/5] Cygwin: Remove reading of PE for section flags from dumper

[Jon Turney]

---
 winsup/utils/Makefile.in |   6 +--
 winsup/utils/dumper.cc   |   2 -
 winsup/utils/dumper.h    |   2 -
 winsup/utils/parse_pe.cc | 107 ---------------------------------------
 4 files changed, 3 insertions(+), 114 deletions(-)
 delete mode 100644 winsup/utils/parse_pe.cc

diff --git a/winsup/utils/Makefile.in b/winsup/utils/Makefile.in
index 5bb62bc6f..f9892fa1d 100644
--- a/winsup/utils/Makefile.in
+++ b/winsup/utils/Makefile.in
@@ -113,9 +113,9 @@ build_dumper := $(shell test -r "$(libbfd)" && echo 1)
 
 ifdef build_dumper
 CYGWIN_BINS += dumper.exe
-dumper.o module_info.o parse_pe.o: CXXFLAGS += -I$(top_srcdir)/include
-dumper.o parse_pe.o: dumper.h
-dumper.exe: module_info.o parse_pe.o
+dumper.o module_info.o: CXXFLAGS += -I$(top_srcdir)/include
+dumper.o: dumper.h
+dumper.exe: module_info.o
 dumper.exe: CYGWIN_LDFLAGS += -lpsapi -lbfd -lintl -liconv -liberty ${ZLIB}
 else
 all: warn_dumper
diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index 46e4b0692..4577d2a3f 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -281,8 +281,6 @@ dumper::add_module (LPVOID base_address)
   new_entity->u.module.base_address = base_address;
   new_entity->u.module.name = module_name;
 
-  parse_pe (module_name, excl_list);
-
   deb_printf ("added module %p %s\n", base_address, module_name);
   return 1;
 }
diff --git a/winsup/utils/dumper.h b/winsup/utils/dumper.h
index 9367587bf..78592b61e 100644
--- a/winsup/utils/dumper.h
+++ b/winsup/utils/dumper.h
@@ -133,8 +133,6 @@ extern int deb_printf ( const char* format, ... );
 
 extern char* psapi_get_module_name ( HANDLE hProcess, LPVOID BaseAddress );
 
-extern int parse_pe ( const char* file_name, exclusion* excl_list );
-
 extern BOOL verbose;
 
 #endif
diff --git a/winsup/utils/parse_pe.cc b/winsup/utils/parse_pe.cc
deleted file mode 100644
index 90b5c0b0d..000000000
--- a/winsup/utils/parse_pe.cc
+++ /dev/null
@@ -1,107 +0,0 @@
-/* parse_pe.cc
-
-   Written by Egor Duda <deo@logos-m.ru>
-
-   This file is part of Cygwin.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License (file COPYING.dumper) for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program; if not, write to the Free Software
-   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.  */
-
-#define PACKAGE
-#include <bfd.h>
-#include <stdio.h>
-#include <stdlib.h>
-
-#include "dumper.h"
-
-#ifndef bfd_get_section_size
-#define bfd_get_section_size(sect) bfd_section_size(sect)
-#endif
-
-int
-exclusion::add (LPBYTE mem_base, SIZE_T mem_size)
-{
-  while (last >= size)
-    size += step;
-  region = (process_mem_region *) realloc (region, size * sizeof (process_mem_region));
-  if (region == NULL)
-    return 0;
-  region[last].base = mem_base;
-  region[last].size = mem_size;
-  last++;
-  return 1;
-};
-
-int
-cmp_regions (const void *r1, const void *r2)
-{
-  if (((process_mem_region *) r1)->base < ((process_mem_region *) r2)->base)
-    return -1;
-  if (((process_mem_region *) r1)->base > ((process_mem_region *) r2)->base)
-    return 1;
-  return 0;
-}
-
-int
-exclusion::sort_and_check ()
-{
-  qsort (region, last, sizeof (process_mem_region), &cmp_regions);
-  for (process_mem_region * p = region; p < region + last - 1; p++)
-    {
-      process_mem_region *q = p + 1;
-      if (q == p + 1)
-	continue;
-      if (p->base + size > q->base)
-	{
-	  fprintf (stderr, "region error @ (%p + %zd) > %p\n", p->base, size, q->base);
-	  return 0;
-	}
-    }
-  return 1;
-}
-
-static void
-select_data_section (bfd * abfd, asection * sect, PTR obj)
-{
-  exclusion *excl_list = (exclusion *) obj;
-
-  if ((sect->flags & (SEC_CODE | SEC_DEBUGGING)) &&
-      sect->vma && bfd_get_section_size (sect))
-    {
-      excl_list->add ((LPBYTE) sect->vma, (SIZE_T) bfd_get_section_size (sect));
-      deb_printf ("excluding section: %20s %08lx\n", sect->name,
-		  bfd_get_section_size (sect));
-    }
-}
-
-int
-parse_pe (const char *file_name, exclusion * excl_list)
-{
-  if (file_name == NULL || excl_list == NULL)
-    return 0;
-
-  bfd *abfd = bfd_openr (file_name, "pei-i386");
-  if (abfd == NULL)
-    {
-      bfd_perror ("failed to open file");
-      return 0;
-    }
-
-  bfd_check_format (abfd, bfd_object);
-  bfd_map_over_sections (abfd, &select_data_section, (PTR) excl_list);
-  excl_list->sort_and_check ();
-
-  bfd_close (abfd);
-  return 1;
-}
-- 
2.27.0

[PATCH 3/5] Cygwin: Drop excluded regions list from dumper

[Jon Turney]

Drop excluded regions, now it's always empty
---
 winsup/utils/dumper.cc | 48 ++++--------------------------------------
 winsup/utils/dumper.h  | 17 ---------------
 2 files changed, 4 insertions(+), 61 deletions(-)

diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index 4577d2a3f..2a0c66002 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -83,7 +83,6 @@ dumper::dumper (DWORD pid, DWORD tid, const char *file_name)
   this->pid = pid;
   this->tid = tid;
   core_bfd = NULL;
-  excl_list = new exclusion (20);
 
   list = last = NULL;
 
@@ -125,19 +124,16 @@ dumper::close ()
 {
   if (core_bfd)
     bfd_close (core_bfd);
-  if (excl_list)
-    delete excl_list;
   if (hProcess)
     CloseHandle (hProcess);
   core_bfd = NULL;
   hProcess = NULL;
-  excl_list = NULL;
 }
 
 int
 dumper::sane ()
 {
-  if (hProcess == NULL || core_bfd == NULL || excl_list == NULL)
+  if (hProcess == NULL || core_bfd == NULL)
     return 0;
   return 1;
 }
@@ -226,42 +222,6 @@ dumper::add_mem_region (LPBYTE base, SIZE_T size)
   return 1;
 }
 
-/* split_add_mem_region scans list of regions to be excluded from dumping process
-   (excl_list) and removes all "excluded" parts from given region. */
-int
-dumper::split_add_mem_region (LPBYTE base, SIZE_T size)
-{
-  if (!sane ())
-    return 0;
-
-  if (base == NULL || size == 0)
-    return 1;			// just ignore empty regions
-
-  LPBYTE last_base = base;
-
-  for (process_mem_region * p = excl_list->region;
-       p < excl_list->region + excl_list->last;
-       p++)
-    {
-      if (p->base >= base + size || p->base + p->size <= base)
-	continue;
-
-      if (p->base <= base)
-	{
-	  last_base = p->base + p->size;
-	  continue;
-	}
-
-      add_mem_region (last_base, p->base - last_base);
-      last_base = p->base + p->size;
-    }
-
-  if (last_base < base + size)
-    add_mem_region (last_base, base + size - last_base);
-
-  return 1;
-}
-
 int
 dumper::add_module (LPVOID base_address)
 {
@@ -413,14 +373,14 @@ dumper::collect_memory_sections ()
 	    last_size += mbi.RegionSize;
 	  else
 	    {
-	      split_add_mem_region (last_base, last_size);
+	      add_mem_region (last_base, last_size);
 	      last_base = (LPBYTE) mbi.BaseAddress;
 	      last_size = mbi.RegionSize;
 	    }
 	}
       else
 	{
-	  split_add_mem_region (last_base, last_size);
+	  add_mem_region (last_base, last_size);
 	  last_base = NULL;
 	  last_size = 0;
 	}
@@ -429,7 +389,7 @@ dumper::collect_memory_sections ()
     }
 
   /* dump last sections, if any */
-  split_add_mem_region (last_base, last_size);
+  add_mem_region (last_base, last_size);
   return 1;
 };
 
diff --git a/winsup/utils/dumper.h b/winsup/utils/dumper.h
index 78592b61e..6e624a983 100644
--- a/winsup/utils/dumper.h
+++ b/winsup/utils/dumper.h
@@ -62,22 +62,6 @@ typedef struct _process_entity
   struct _process_entity* next;
 } process_entity;
 
-class exclusion
-{
-public:
-  size_t last;
-  size_t size;
-  size_t step;
-  process_mem_region* region;
-
-  exclusion ( size_t step ) { last = size = 0;
-			      this->step = step;
-			      region = NULL; }
-  ~exclusion () { free ( region ); }
-  int add ( LPBYTE mem_base, SIZE_T mem_size );
-  int sort_and_check ();
-};
-
 #define PAGE_BUFFER_SIZE 4096
 
 class dumper
@@ -87,7 +71,6 @@ class dumper
   HANDLE hProcess;
   process_entity* list;
   process_entity* last;
-  exclusion* excl_list;
 
   char* file_name;
   bfd* core_bfd;
-- 
2.27.0

[PATCH 4/5] Cygwin: Don't dump non-writable image regions

[Jon Turney]

After this, we will end up dumping memory regions where:

- state is MEM_COMMIT (i.e. is not MEM_RESERVE or MEM_FREE), and
-- type is MEM_PRIVATE and protection allows reads (i.e. not a guardpage), or
-- type is MEM_IMAGE and protection allows writes

Making this decision based on the current protection isn't 100% correct,
because it may have been changed using VirtualProtect().  But we don't
know how to determine if a region is shareable.

(As a practical matter, anything which gets us the stack (MEM_PRIVATE)
and .data/.bss (RW MEM_IMAGE) is going to be enough for 99% of cases)
---
 winsup/utils/dumper.cc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index 2a0c66002..b96ee54cc 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -292,6 +292,12 @@ dumper::collect_memory_sections ()
       int skip_region_p = 0;
       const char *disposition = "dumped";
 
+      if ((mbi.Type & MEM_IMAGE) && !(mbi.Protect & (PAGE_EXECUTE_READWRITE | PAGE_READWRITE)))
+	{
+	  skip_region_p = 1;
+	  disposition = "skipped due to non-writeable MEM_IMAGE";
+	}
+
       if (mbi.Protect & PAGE_NOACCESS)
 	{
 	  skip_region_p = 1;
-- 
2.27.0

[PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper

[Jon Turney]

Use the (undocumented) MEMORY_WORKING_SET_EX_INFORMATION in dumper to
determine if a MEM_IMAGE region is unsharable, and hence has been
modified.
---
 winsup/doc/utils.xml     |  8 ++---
 winsup/utils/Makefile.in |  2 +-
 winsup/utils/dumper.cc   | 63 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/winsup/doc/utils.xml b/winsup/doc/utils.xml
index 5f266bcb1..8b92bfdf1 100644
--- a/winsup/doc/utils.xml
+++ b/winsup/doc/utils.xml
@@ -524,11 +524,11 @@ error_start=x:\path\to\dumper.exe
       <command>dumper</command> exits, the target process is terminated too. </para>
 
     <para> To save space in the core dump, <command>dumper</command> doesn't
-      write those portions of target process' memory space that are loaded from
-      executable and dll files and are unchangeable, such as program code and
-      debug info. Instead, <command>dumper</command> saves paths to files which
+      write those portions of the target process's memory space that are loaded
+      from executable and dll files and are unchanged (e.g. program code).
+      Instead, <command>dumper</command> saves paths to the files which
       contain that data. When a core dump is loaded into gdb, it uses these
-      paths to load appropriate files. That means that if you create a core
+      paths to load the appropriate files. That means that if you create a core
       dump on one machine and try to debug it on another, you'll need to place
       identical copies of the executable and dlls in the same directories as on
       the machine where the core dump was created. </para>
diff --git a/winsup/utils/Makefile.in b/winsup/utils/Makefile.in
index f9892fa1d..6bf4454c5 100644
--- a/winsup/utils/Makefile.in
+++ b/winsup/utils/Makefile.in
@@ -116,7 +116,7 @@ CYGWIN_BINS += dumper.exe
 dumper.o module_info.o: CXXFLAGS += -I$(top_srcdir)/include
 dumper.o: dumper.h
 dumper.exe: module_info.o
-dumper.exe: CYGWIN_LDFLAGS += -lpsapi -lbfd -lintl -liconv -liberty ${ZLIB}
+dumper.exe: CYGWIN_LDFLAGS += -lpsapi -lbfd -lintl -liconv -liberty ${ZLIB} -lntdll
 else
 all: warn_dumper
 endif
diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index b96ee54cc..3af138b9e 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -266,6 +266,46 @@ void protect_dump(DWORD protect, char *buf)
     strcat (buf, pt[i]);
 }
 
+typedef enum _MEMORY_INFORMATION_CLASS
+{
+ MemoryWorkingSetExInformation = 4, // MEMORY_WORKING_SET_EX_INFORMATION
+} MEMORY_INFORMATION_CLASS;
+
+extern "C"
+NTSTATUS
+NtQueryVirtualMemory(HANDLE ProcessHandle,
+		     LPVOID BaseAddress,
+		     MEMORY_INFORMATION_CLASS MemoryInformationClass,
+		     LPVOID MemoryInformation,
+		     SIZE_T MemoryInformationLength,
+		     SIZE_T *ReturnLength);
+
+typedef struct _MEMORY_WORKING_SET_EX_INFORMATION
+{
+  LPVOID VirtualAddress;
+  ULONG_PTR Long;
+} MEMORY_WORKING_SET_EX_INFORMATION;
+
+#define MWSEI_ATTRIB_SHARED (0x1 << 15)
+
+static BOOL
+getRegionAttributes(HANDLE hProcess, LPVOID address, DWORD &attribs)
+{
+  MEMORY_WORKING_SET_EX_INFORMATION mwsei = { address };
+  NTSTATUS status = NtQueryVirtualMemory(hProcess, 0,
+					 MemoryWorkingSetExInformation,
+					 &mwsei, sizeof(mwsei), 0);
+
+  if (!status)
+    {
+      attribs = mwsei.Long;
+      return TRUE;
+    }
+
+  deb_printf("MemoryWorkingSetExInformation failed status %08x\n", status);
+  return FALSE;
+}
+
 int
 dumper::collect_memory_sections ()
 {
@@ -292,10 +332,27 @@ dumper::collect_memory_sections ()
       int skip_region_p = 0;
       const char *disposition = "dumped";
 
-      if ((mbi.Type & MEM_IMAGE) && !(mbi.Protect & (PAGE_EXECUTE_READWRITE | PAGE_READWRITE)))
+      if (mbi.Type & MEM_IMAGE)
 	{
-	  skip_region_p = 1;
-	  disposition = "skipped due to non-writeable MEM_IMAGE";
+	  DWORD attribs = 0;
+	  if (getRegionAttributes(hProcess, current_page_address, attribs))
+	    {
+	      if (attribs & MWSEI_ATTRIB_SHARED)
+		{
+		  skip_region_p = 1;
+		  disposition = "skipped due to shared MEM_IMAGE";
+		}
+	    }
+	  /*
+	    The undocumented MemoryWorkingSetExInformation is allegedly
+	    supported since XP, so should always succeed, but if it fails,
+	    fallback to looking at region protection.
+	   */
+	  else if (!(mbi.Protect & (PAGE_EXECUTE_READWRITE | PAGE_READWRITE)))
+	    {
+	      skip_region_p = 1;
+	      disposition = "skipped due to non-writeable MEM_IMAGE";
+	    }
 	}
 
       if (mbi.Protect & PAGE_NOACCESS)
-- 
2.27.0

Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper

[Corinna Vinschen]

On Jul 18 16:00, Jon Turney wrote:
> Use the (undocumented) MEMORY_WORKING_SET_EX_INFORMATION in dumper to
> determine if a MEM_IMAGE region is unsharable, and hence has been
> modified.

Great!


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper

[Takashi Yano]

Hi Jon,

On Sat, 18 Jul 2020 16:00:28 +0100
Jon Turney wrote:
> Use the (undocumented) MEMORY_WORKING_SET_EX_INFORMATION in dumper to
> determine if a MEM_IMAGE region is unsharable, and hence has been
> modified.
> ---
>  winsup/doc/utils.xml     |  8 ++---
>  winsup/utils/Makefile.in |  2 +-
>  winsup/utils/dumper.cc   | 63 ++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 65 insertions(+), 8 deletions(-)

After this commit, 32bit build failes with:

/usr/lib/gcc/i686-pc-cygwin/9.3.0/../../../../i686-pc-cygwin/bin/ld: dumper.o:/home/yano/newlib-cygwin/i686-pc-cygwin/winsup/utils/../../.././winsup/utils/dumper.cc:295: undefined reference to `NtQueryVirtualMemory'

This seems to be solved with the patch:

diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index 3af138b9e..36dbf9dbb 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -272,7 +272,7 @@ typedef enum _MEMORY_INFORMATION_CLASS
 } MEMORY_INFORMATION_CLASS;

 extern "C"
-NTSTATUS
+NTSTATUS NTAPI
 NtQueryVirtualMemory(HANDLE ProcessHandle,
                     LPVOID BaseAddress,
                     MEMORY_INFORMATION_CLASS MemoryInformationClass,

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper

[Jon Turney]

On 28/07/2020 03:20, Takashi Yano via Cygwin-patches wrote:
> Hi Jon,
> 
> On Sat, 18 Jul 2020 16:00:28 +0100
> Jon Turney wrote:
>> Use the (undocumented) MEMORY_WORKING_SET_EX_INFORMATION in dumper to
>> determine if a MEM_IMAGE region is unsharable, and hence has been
>> modified.
>> ---
>>   winsup/doc/utils.xml     |  8 ++---
>>   winsup/utils/Makefile.in |  2 +-
>>   winsup/utils/dumper.cc   | 63 ++++++++++++++++++++++++++++++++++++++--
>>   3 files changed, 65 insertions(+), 8 deletions(-)
> 
> After this commit, 32bit build failes with:
> 
> /usr/lib/gcc/i686-pc-cygwin/9.3.0/../../../../i686-pc-cygwin/bin/ld: dumper.o:/home/yano/newlib-cygwin/i686-pc-cygwin/winsup/utils/../../.././winsup/utils/dumper.cc:295: undefined reference to `NtQueryVirtualMemory'

Thanks very much for pointing this out.

> This seems to be solved with the patch:

Yes, I guess that it needs that for stdcall.  I applied your patch.

> diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
> index 3af138b9e..36dbf9dbb 100644
> --- a/winsup/utils/dumper.cc
> +++ b/winsup/utils/dumper.cc
> @@ -272,7 +272,7 @@ typedef enum _MEMORY_INFORMATION_CLASS
>   } MEMORY_INFORMATION_CLASS;
> 
>   extern "C"
> -NTSTATUS
> +NTSTATUS NTAPI
>   NtQueryVirtualMemory(HANDLE ProcessHandle,
>                       LPVOID BaseAddress,
>                       MEMORY_INFORMATION_CLASS MemoryInformationClass,
>