[PATCH] Cygwin: Use documented QueryWorkingSetEx() in dumper

[PATCH] Cygwin: Use documented QueryWorkingSetEx() in dumper

[Jon Turney]

In dumper, use the documented QueryWorkingSetEx(), rather than the
undocumented NtQueryVirtualMemory() with MemoryWorkingSetExInformation.
---

Notes:
    As it's been pointed out to me that there is a documented interface to
    get this information, so use that, rather than relying an undocmented one.

 winsup/utils/dumper.cc | 36 +++++++-----------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index 5f8121aa3..19a279662 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -34,6 +34,7 @@
 #include <unistd.h>
 #include <sys/param.h>
 #include <windows.h>
+#include <psapi.h>
 
 #include "dumper.h"
 
@@ -267,43 +268,20 @@ 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 NTAPI
-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)
+#define PSWSEI_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);
+  PSAPI_WORKING_SET_EX_INFORMATION pswsei = { address };
 
-  if (!status)
+  if (QueryWorkingSetEx(hProcess, &pswsei, sizeof(pswsei)))
     {
-      attribs = mwsei.Long;
+      attribs = pswsei.VirtualAttributes.Flags;
       return TRUE;
     }
 
-  deb_printf("MemoryWorkingSetExInformation failed status %08x\n", status);
+  deb_printf("QueryWorkingSetEx failed status %08x\n", GetLastError());
   return FALSE;
 }
 
@@ -338,7 +316,7 @@ dumper::collect_memory_sections ()
 	  DWORD attribs = 0;
 	  if (getRegionAttributes(hProcess, current_page_address, attribs))
 	    {
-	      if (attribs & MWSEI_ATTRIB_SHARED)
+	      if (attribs & PSWSEI_ATTRIB_SHARED)
 		{
 		  skip_region_p = 1;
 		  disposition = "skipped due to shared MEM_IMAGE";
-- 
2.27.0

Re: [PATCH] Cygwin: Use documented QueryWorkingSetEx() in dumper

[Jon Turney]

On 31/07/2020 19:40, Lavrentiev, Anton (NIH/NLM/NCBI) [C] wrote:
> Hi,
> 
> Reading this MS documentation
> 
> https://docs.microsoft.com/en-us/windows/win32/api/psapi/nf-psapi-queryworkingsetex
> 
> ... the remarks section goes:
> 
> Programs that must run on earlier versions of Windows as well as Windows 7 and later versions should always call this function as QueryWorkingSetEx. To ensure correct resolution of symbols, add Psapi.lib to the TARGETLIBS macro and compile the program with "-DPSAPI_VERSION=1". To use run-time dynamic linking, load Psapi.dll.
> 
> So if I'm reading correctly and if Vista is still being supported in Cygwin, then I guess PSAPI_VERSION=1 must be used (either Makefile or source)...

This is correct.

The MSDN documentation is unhelpfully silent on the default behaviour if 
PSAPI_VERSION is not specified.

For the w32api headers, at least, the default depends on the targetted 
version set via the NTDDI_VERSION macro (or historical alternatives to 
that), and unless Win7 or later is targetted, it defaults to 
PSAPI_VERSION=1.

Note that dumper already uses various psapi functions 
(EnumProcessModule, GetModuleInformation, GetModuleFileNameExA), so this 
patch doesn't introduce this potential ambiguity as a new issue.