public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH 0/5] Improve dumper megion region selection
@ 2020-07-18 15:00 Jon Turney
  2020-07-18 15:00 ` [PATCH 1/5] Cygwin: Show details of all memory regions details in dumper debug output Jon Turney
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-18 15:00 UTC (permalink / raw)
  To: cygwin-patches; +Cc: 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


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

* [PATCH 1/5] Cygwin: Show details of all memory regions details in dumper debug output
  2020-07-18 15:00 [PATCH 0/5] Improve dumper megion region selection Jon Turney
@ 2020-07-18 15:00 ` Jon Turney
  2020-07-18 15:00 ` [PATCH 2/5] Cygwin: Remove reading of PE for section flags from dumper Jon Turney
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-18 15:00 UTC (permalink / raw)
  To: cygwin-patches; +Cc: 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


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

* [PATCH 2/5] Cygwin: Remove reading of PE for section flags from dumper
  2020-07-18 15:00 [PATCH 0/5] Improve dumper megion region selection Jon Turney
  2020-07-18 15:00 ` [PATCH 1/5] Cygwin: Show details of all memory regions details in dumper debug output Jon Turney
@ 2020-07-18 15:00 ` Jon Turney
  2020-07-18 15:00 ` [PATCH 3/5] Cygwin: Drop excluded regions list " Jon Turney
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-18 15:00 UTC (permalink / raw)
  To: cygwin-patches; +Cc: 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


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

* [PATCH 3/5] Cygwin: Drop excluded regions list from dumper
  2020-07-18 15:00 [PATCH 0/5] Improve dumper megion region selection Jon Turney
  2020-07-18 15:00 ` [PATCH 1/5] Cygwin: Show details of all memory regions details in dumper debug output Jon Turney
  2020-07-18 15:00 ` [PATCH 2/5] Cygwin: Remove reading of PE for section flags from dumper Jon Turney
@ 2020-07-18 15:00 ` Jon Turney
  2020-07-18 15:00 ` [PATCH 4/5] Cygwin: Don't dump non-writable image regions Jon Turney
  2020-07-18 15:00 ` [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper Jon Turney
  4 siblings, 0 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-18 15:00 UTC (permalink / raw)
  To: cygwin-patches; +Cc: 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


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

* [PATCH 4/5] Cygwin: Don't dump non-writable image regions
  2020-07-18 15:00 [PATCH 0/5] Improve dumper megion region selection Jon Turney
                   ` (2 preceding siblings ...)
  2020-07-18 15:00 ` [PATCH 3/5] Cygwin: Drop excluded regions list " Jon Turney
@ 2020-07-18 15:00 ` Jon Turney
  2020-07-18 15:00 ` [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper Jon Turney
  4 siblings, 0 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-18 15:00 UTC (permalink / raw)
  To: cygwin-patches; +Cc: 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


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

* [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper
  2020-07-18 15:00 [PATCH 0/5] Improve dumper megion region selection Jon Turney
                   ` (3 preceding siblings ...)
  2020-07-18 15:00 ` [PATCH 4/5] Cygwin: Don't dump non-writable image regions Jon Turney
@ 2020-07-18 15:00 ` Jon Turney
  2020-07-20  8:15   ` Corinna Vinschen
  2020-07-28  2:20   ` Takashi Yano
  4 siblings, 2 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-18 15:00 UTC (permalink / raw)
  To: cygwin-patches; +Cc: 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


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

* Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper
  2020-07-18 15:00 ` [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper Jon Turney
@ 2020-07-20  8:15   ` Corinna Vinschen
  2020-07-28  2:20   ` Takashi Yano
  1 sibling, 0 replies; 9+ messages in thread
From: Corinna Vinschen @ 2020-07-20  8:15 UTC (permalink / raw)
  To: cygwin-patches

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

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

* Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper
  2020-07-18 15:00 ` [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper Jon Turney
  2020-07-20  8:15   ` Corinna Vinschen
@ 2020-07-28  2:20   ` Takashi Yano
  2020-07-28 21:04     ` Jon Turney
  1 sibling, 1 reply; 9+ messages in thread
From: Takashi Yano @ 2020-07-28  2:20 UTC (permalink / raw)
  To: cygwin-patches

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>

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

* Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper
  2020-07-28  2:20   ` Takashi Yano
@ 2020-07-28 21:04     ` Jon Turney
  0 siblings, 0 replies; 9+ messages in thread
From: Jon Turney @ 2020-07-28 21:04 UTC (permalink / raw)
  To: Cygwin Patches

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,
> 

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

end of thread, other threads:[~2020-07-28 21:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-18 15:00 [PATCH 0/5] Improve dumper megion region selection Jon Turney
2020-07-18 15:00 ` [PATCH 1/5] Cygwin: Show details of all memory regions details in dumper debug output Jon Turney
2020-07-18 15:00 ` [PATCH 2/5] Cygwin: Remove reading of PE for section flags from dumper Jon Turney
2020-07-18 15:00 ` [PATCH 3/5] Cygwin: Drop excluded regions list " Jon Turney
2020-07-18 15:00 ` [PATCH 4/5] Cygwin: Don't dump non-writable image regions Jon Turney
2020-07-18 15:00 ` [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper Jon Turney
2020-07-20  8:15   ` Corinna Vinschen
2020-07-28  2:20   ` Takashi Yano
2020-07-28 21:04     ` Jon Turney

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).