[PATCH 0/7] Add gdb support for Cygwin x86_64 core dumps

[PATCH 0/7] Add gdb support for Cygwin x86_64 core dumps

[Jon Turney]

As far as I know, the only way to generate these "core dumps" is to use
Cygwin's 'dumper' tool, which requires some fixes on x86_64 [1].

[1] https://cygwin.com/pipermail/cygwin-patches/2020q3/010313.html

Jon Turney (7):
  Read tid from correct offset in win32pstatus NOTE_INFO_THREAD
  Don't apply size constraint to all win32pstatus ELF notes.
  Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note
  Add sniffer for Cygwin x86_64 core dumps
  Add amd64_windows_gregset_reg_offset
  Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str
  Add handling for 64-bit module addresses in Cygwin core dumps

 bfd/ChangeLog            |  20 ++++++++
 bfd/elf.c                |  25 +++++----
 gdb/ChangeLog            |  23 +++++++++
 gdb/amd64-windows-tdep.c | 100 ++++++++++++++++++++++++++++++++++++
 gdb/i386-windows-tdep.c  | 100 +-----------------------------------
 gdb/windows-tdep.c       | 108 +++++++++++++++++++++++++++++++++++++++
 gdb/windows-tdep.h       |   8 +++
 7 files changed, 276 insertions(+), 108 deletions(-)

-- 
2.27.0

[PATCH 1/7] Read tid from correct offset in win32pstatus NOTE_INFO_THREAD

[Jon Turney]

Fix the offset used to read the tid from a win32pstatus ELF note.

This probably meant that registers were only being correctly recovered
from the core dump for the current thread.

Also improve comment.

bfd/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* elf.c (elfcore_grok_win32pstatus): Fix the offset used to read
	the tid from a win32pstatus ELF note.
---
 bfd/ChangeLog | 5 +++++
 bfd/elf.c     | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 9ca42e10d8e..5a184d14c66 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10157,9 +10157,10 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case 2 /* NOTE_INFO_THREAD */:
-      /* Make a ".reg/999" section.  */
+      /* Make a ".reg/<tid>" section containing the Win32 API thread CONTEXT
+         structure. */
       /* thread_info.tid */
-      sprintf (buf, ".reg/%ld", (long) bfd_get_32 (abfd, note->descdata + 8));
+      sprintf (buf, ".reg/%ld", (long) bfd_get_32 (abfd, note->descdata + 4));
 
       len = strlen (buf) + 1;
       name = (char *) bfd_alloc (abfd, len);
-- 
2.27.0

[PATCH 2/7] Don't apply size constraint to all win32pstatus ELF notes.

[Jon Turney]

Don't reject any win32pstatus notes smaller than minimum size for a
NOTE_INFO_THREAD.

This only happens to work because the Cygwin dumper tool currently
writes all these notes as the largest size of the union, (which wastes
lots of space in the core dump).

bfd/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* elf.c (elfcore_grok_win32pstatus): Don't apply size constraint
	for NOTE_INFO_THREAD to all win32pstatus ELF notes.
---
 bfd/ChangeLog | 5 +++++
 bfd/elf.c     | 3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 5a184d14c66..a7790a4eec4 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10138,9 +10138,6 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   int is_active_thread;
   bfd_vma base_addr;
 
-  if (note->descsz < 728)
-    return TRUE;
-
   if (! CONST_STRNEQ (note->namedata, "win32"))
     return TRUE;
 
-- 
2.27.0

[PATCH 3/7] Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note

[Jon Turney]

Don't hardcode the size of the Win32 API thread CONTEXT type read from a
NOTE_INFO_THREAD win32pstatus note (since it's different on different
architectures).

bfd/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* elf.c (elfcore_grok_win32pstatus): Don't hardcode the size of
	the Win32 API thread CONTEXT type read from a NOTE_INFO_THREAD
	win32pstatus note.
---
 bfd/ChangeLog | 6 ++++++
 bfd/elf.c     | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index a7790a4eec4..a61e2b7dd1d 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10171,7 +10171,7 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
 	return FALSE;
 
       /* sizeof (thread_info.thread_context) */
-      sect->size = 716;
+      sect->size = note->descsz - 12;
       /* offsetof (thread_info.thread_context) */
       sect->filepos = note->descpos + 12;
       sect->alignment_power = 2;
-- 
2.27.0

Re: [PATCH 3/7] Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note

[Christian Biesinger]

On Wed, Jul 1, 2020 at 4:33 PM Jon Turney <jon.turney@dronecode.org.uk> wrote:
>
> Don't hardcode the size of the Win32 API thread CONTEXT type read from a
> NOTE_INFO_THREAD win32pstatus note (since it's different on different
> architectures).
>
> bfd/ChangeLog:
>
> 2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>
>
>         * elf.c (elfcore_grok_win32pstatus): Don't hardcode the size of
>         the Win32 API thread CONTEXT type read from a NOTE_INFO_THREAD
>         win32pstatus note.
> ---
>  bfd/ChangeLog | 6 ++++++
>  bfd/elf.c     | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index a7790a4eec4..a61e2b7dd1d 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -10171,7 +10171,7 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
>         return FALSE;
>
>        /* sizeof (thread_info.thread_context) */
> -      sect->size = 716;
> +      sect->size = note->descsz - 12;

I guess the "sizeof" comment is now wrong -- where does the "12" come
from though?

>        /* offsetof (thread_info.thread_context) */
>        sect->filepos = note->descpos + 12;
>        sect->alignment_power = 2;
> --
> 2.27.0
>

[PATCH 4/7] Add sniffer for Cygwin x86_64 core dumps

[Jon Turney]

Similarly to existing i386_cygwin_core_osabi_sniffer()

gdb/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* amd64-windows-tdep.c (amd64_cygwin_core_osabi_sniffer): New.
	(_initialize_amd64_windows_tdep): Register amd64_cygwin_core_osabi_sniffer.
---
 gdb/ChangeLog            |  5 +++++
 gdb/amd64-windows-tdep.c | 25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 487dfd45fc7..e55d021b6c0 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -42,6 +42,8 @@ static int amd64_windows_dummy_call_integer_regs[] =
   AMD64_R9_REGNUM            /* %r9 */
 };
 
+#define AMD64_WINDOWS_SIZEOF_GREGSET 1232
+
 /* Return nonzero if an argument of type TYPE should be passed
    via one of the integer registers.  */
 
@@ -1276,6 +1278,24 @@ amd64_windows_osabi_sniffer (bfd *abfd)
   return GDB_OSABI_WINDOWS;
 }
 
+static enum gdb_osabi
+amd64_cygwin_core_osabi_sniffer (bfd *abfd)
+{
+  const char *target_name = bfd_get_target (abfd);
+
+  /* Cygwin uses elf core dumps.  Do not claim all ELF executables,
+     check whether there is a .reg section of proper size.  */
+  if (strcmp (target_name, "elf64-x86-64") == 0)
+    {
+      asection *section = bfd_get_section_by_name (abfd, ".reg");
+      if (section != nullptr
+	  && bfd_section_size (section) == AMD64_WINDOWS_SIZEOF_GREGSET)
+	return GDB_OSABI_CYGWIN;
+    }
+
+  return GDB_OSABI_UNKNOWN;
+}
+
 void _initialize_amd64_windows_tdep ();
 void
 _initialize_amd64_windows_tdep ()
@@ -1287,4 +1307,9 @@ _initialize_amd64_windows_tdep ()
 
   gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_coff_flavour,
 				  amd64_windows_osabi_sniffer);
+
+  /* Cygwin uses elf core dumps.  */
+  gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_elf_flavour,
+				  amd64_cygwin_core_osabi_sniffer);
+
 }
-- 
2.27.0

Re: [PATCH 4/7] Add sniffer for Cygwin x86_64 core dumps

[Simon Marchi]

> @@ -1276,6 +1278,24 @@ amd64_windows_osabi_sniffer (bfd *abfd)
>    return GDB_OSABI_WINDOWS;
>  }
>  
> +static enum gdb_osabi
> +amd64_cygwin_core_osabi_sniffer (bfd *abfd)
> +{
> +  const char *target_name = bfd_get_target (abfd);
> +
> +  /* Cygwin uses elf core dumps.  Do not claim all ELF executables,
> +     check whether there is a .reg section of proper size.  */
> +  if (strcmp (target_name, "elf64-x86-64") == 0)
> +    {
> +      asection *section = bfd_get_section_by_name (abfd, ".reg");
> +      if (section != nullptr
> +	  && bfd_section_size (section) == AMD64_WINDOWS_SIZEOF_GREGSET)
> +	return GDB_OSABI_CYGWIN;
> +    }
> +
> +  return GDB_OSABI_UNKNOWN;

The obvious question here is, what happens if we are loading the core for
another architecture, and it happens by bad luck that the .reg section is
of that size, even though it's not a Cygwin core.  Will this give a false
positive?

I presume that since this is copied on the i386, that discussion already
happened in the past for i386, and it was concluded that there was no better
way to identify a Cygwin core.  But I thought I'd ask just to be sure.

Simon

Re: [PATCH 4/7] Add sniffer for Cygwin x86_64 core dumps

[Jon Turney]

On 03/07/2020 00:59, Simon Marchi wrote:
>> @@ -1276,6 +1278,24 @@ amd64_windows_osabi_sniffer (bfd *abfd)
>>     return GDB_OSABI_WINDOWS;
>>   }
>>   
>> +static enum gdb_osabi
>> +amd64_cygwin_core_osabi_sniffer (bfd *abfd)
>> +{
>> +  const char *target_name = bfd_get_target (abfd);
>> +
>> +  /* Cygwin uses elf core dumps.  Do not claim all ELF executables,
>> +     check whether there is a .reg section of proper size.  */
>> +  if (strcmp (target_name, "elf64-x86-64") == 0)
>> +    {
>> +      asection *section = bfd_get_section_by_name (abfd, ".reg");
>> +      if (section != nullptr
>> +	  && bfd_section_size (section) == AMD64_WINDOWS_SIZEOF_GREGSET)
>> +	return GDB_OSABI_CYGWIN;
>> +    }
>> +
>> +  return GDB_OSABI_UNKNOWN;
> 
> The obvious question here is, what happens if we are loading the core for
> another architecture, and it happens by bad luck that the .reg section is
> of that size, even though it's not a Cygwin core.  Will this give a false
> positive?

It would seem to.

> I presume that since this is copied on the i386, that discussion already
> happened in the past for i386, and it was concluded that there was no better
> way to identify a Cygwin core.  But I thought I'd ask just to be sure.

idk.  The x86 Cygwin core dump support was added in 2000 (see commit 
16e9c715dffb96efda481dc20cdb5bb6fbde0dff), when I was blissfully 
ignorant of all this nonsense :).

It certainly would be possible to improve this by checking if the ELF 
file contains a note of type NT_WIN32PSTATUS, but I'm not sure how we 
might do that here.

Re: [PATCH 4/7] Add sniffer for Cygwin x86_64 core dumps

[Simon Marchi]

On 2020-07-03 9:30 a.m., Jon Turney wrote:
> It certainly would be possible to improve this by checking if the ELF file contains a note of type NT_WIN32PSTATUS, but I'm not sure how we might do that here.

You have access to the `bfd *`.  I'm not familiar with the BFD API, but
surely it can give you access to that?

Simon

Re: [PATCH 4/7] Add sniffer for Cygwin x86_64 core dumps

[Jon Turney]

On 03/07/2020 15:17, Simon Marchi wrote:
> On 2020-07-03 9:30 a.m., Jon Turney wrote:
>> It certainly would be possible to improve this by checking if the ELF file contains a note of type NT_WIN32PSTATUS, but I'm not sure how we might do that here.
> 
> You have access to the `bfd *`.  I'm not familiar with the BFD API, but
> surely it can give you access to that?

"notes" don't appear to be part of the data model used by the BFD API 
(which I guess is why they are turned into pseudo-sections by the 
various functions called by elfcore_grok_note())

[PATCH 5/7] Add amd64_windows_gregset_reg_offset

[Jon Turney]

Register a gregset_reg_offset array for Cygwin x86_64 core dump parsing
(this causes the generic i386_iterate_over_regset_sections() '.reg'
section iterator get installed by i386_gdbarch_init()).

gdb/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* amd64-windows-tdep.c(amd64_windows_gregset_reg_offset): Add.
	(amd64_windows_init_abi_common): ... and register.
---
 gdb/ChangeLog            |  5 +++
 gdb/amd64-windows-tdep.c | 70 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index e55d021b6c0..f1526283f2f 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -42,6 +42,69 @@ static int amd64_windows_dummy_call_integer_regs[] =
   AMD64_R9_REGNUM            /* %r9 */
 };
 
+/* This vector maps GDB's idea of a register's number into an offset into
+   the Windows API CONTEXT structure.  */
+static int amd64_windows_gregset_reg_offset[] =
+{
+ 120, /* Rax */
+ 144, /* Rbx */
+ 128, /* Rcx */
+ 136, /* Rdx */
+ 168, /* Rsi */
+ 176, /* Rdi */
+ 160, /* Rbp */
+ 152, /* Rsp */
+ 184, /* R8 */
+ 192, /* R9 */
+ 200, /* R10 */
+ 208, /* R11 */
+ 216, /* R12 */
+ 224, /* R13 */
+ 232, /* R14 */
+ 240, /* R15 */
+ 248, /* Rip */
+ 68,  /* EFlags */
+ 56,  /* SegCs */
+ 66,  /* SegSs */
+ 58,  /* SegDs */
+ 60,  /* SegEs */
+ 62,  /* SegFs */
+ 64,  /* SegGs */
+ 288, /* FloatSave.FloatRegisters[0] */
+ 304, /* FloatSave.FloatRegisters[1] */
+ 320, /* FloatSave.FloatRegisters[2] */
+ 336, /* FloatSave.FloatRegisters[3] */
+ 352, /* FloatSave.FloatRegisters[4] */
+ 368, /* FloatSave.FloatRegisters[5] */
+ 384, /* FloatSave.FloatRegisters[6] */
+ 400, /* FloatSave.FloatRegisters[7] */
+ 256, /* FloatSave.ControlWord */
+ 258, /* FloatSave.StatusWord */
+ 260, /* FloatSave.TagWord */
+ 268, /* FloatSave.ErrorSelector */
+ 264, /* FloatSave.ErrorOffset */
+ 276, /* FloatSave.DataSelector */
+ 272, /* FloatSave.DataOffset */
+ 268, /* FloatSave.ErrorSelector */
+ 416, /* Xmm0 */
+ 432, /* Xmm1 */
+ 448, /* Xmm2 */
+ 464, /* Xmm3 */
+ 480, /* Xmm4 */
+ 496, /* Xmm5 */
+ 512, /* Xmm6 */
+ 528, /* Xmm7 */
+ 544, /* Xmm8 */
+ 560, /* Xmm9 */
+ 576, /* Xmm10 */
+ 592, /* Xmm11 */
+ 608, /* Xmm12 */
+ 624, /* Xmm13 */
+ 640, /* Xmm14 */
+ 656, /* Xmm15 */
+ 280, /* FloatSave.MxCsr */
+};
+
 #define AMD64_WINDOWS_SIZEOF_GREGSET 1232
 
 /* Return nonzero if an argument of type TYPE should be passed
@@ -1215,6 +1278,8 @@ amd64_windows_auto_wide_charset (void)
 static void
 amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   /* The dwarf2 unwinder (appended very early by i386_gdbarch_init) is
      preferred over the SEH one.  The reasons are:
      - binaries without SEH but with dwarf2 debug info are correctly handled
@@ -1240,6 +1305,11 @@ amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_skip_prologue (gdbarch, amd64_windows_skip_prologue);
 
+  tdep->gregset_reg_offset = amd64_windows_gregset_reg_offset;
+  tdep->gregset_num_regs = ARRAY_SIZE (amd64_windows_gregset_reg_offset);
+  tdep->sizeof_gregset = AMD64_WINDOWS_SIZEOF_GREGSET;
+  tdep->sizeof_fpregset = 0;
+
   set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
 }
 
-- 
2.27.0

Re: [PATCH 5/7] Add amd64_windows_gregset_reg_offset

[Pedro Alves]

On 7/1/20 10:32 PM, Jon Turney wrote:
> +/* This vector maps GDB's idea of a register's number into an offset into
> +   the Windows API CONTEXT structure.  */
> +static int amd64_windows_gregset_reg_offset[] =
> +{
> + 120, /* Rax */
> + 144, /* Rbx */

Super important nit (just kidding) -- array elements
should be indented by two spaces instead of just one.

[PATCH 6/7] Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str

[Jon Turney]

Move windows_core_xfer_shared_libraries() and windows_core_pid_to_str()
to windows-tdep, use in amd64-windows-tdep.c to handle Cygwin x86_64
core dumps.

gdb/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* windows-tdep.h: Add prototypes.
	* i386-windows-tdep.c(windows_core_xfer_shared_libraries): Move.
	(i386_windows_core_pid_to_str): Move and rename ...
	* windows-tdep.c (windows_core_xfer_shared_libraries): ... to here
	(windows_core_pid_to_str): ... and here.
	* amd64-windows-tdep.c (amd64_windows_init_abi_common): Register here.
---
 gdb/ChangeLog            |   8 ++++
 gdb/amd64-windows-tdep.c |   5 ++
 gdb/i386-windows-tdep.c  | 100 +--------------------------------------
 gdb/windows-tdep.c       |  98 ++++++++++++++++++++++++++++++++++++++
 gdb/windows-tdep.h       |   8 ++++
 5 files changed, 120 insertions(+), 99 deletions(-)

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index f1526283f2f..89b3de15bce 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -1310,6 +1310,11 @@ amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch)
   tdep->sizeof_gregset = AMD64_WINDOWS_SIZEOF_GREGSET;
   tdep->sizeof_fpregset = 0;
 
+  /* Core file support.  */
+  set_gdbarch_core_xfer_shared_libraries
+    (gdbarch, windows_core_xfer_shared_libraries);
+  set_gdbarch_core_pid_to_str (gdbarch, windows_core_pid_to_str);
+
   set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
 }
 
diff --git a/gdb/i386-windows-tdep.c b/gdb/i386-windows-tdep.c
index ae61ed8291c..1477e54b4d5 100644
--- a/gdb/i386-windows-tdep.c
+++ b/gdb/i386-windows-tdep.c
@@ -89,104 +89,6 @@ static int i386_windows_gregset_reg_offset[] =
 
 #define I386_WINDOWS_SIZEOF_GREGSET 716
 
-struct cpms_data
-{
-  struct gdbarch *gdbarch;
-  struct obstack *obstack;
-  int module_count;
-};
-
-static void
-core_process_module_section (bfd *abfd, asection *sect, void *obj)
-{
-  struct cpms_data *data = (struct cpms_data *) obj;
-  enum bfd_endian byte_order = gdbarch_byte_order (data->gdbarch);
-
-  char *module_name;
-  size_t module_name_size;
-  CORE_ADDR base_addr;
-
-  gdb_byte *buf = NULL;
-
-  if (!startswith (sect->name, ".module"))
-    return;
-
-  buf = (gdb_byte *) xmalloc (bfd_section_size (sect) + 1);
-  if (!buf)
-    {
-      printf_unfiltered ("memory allocation failed for %s\n", sect->name);
-      goto out;
-    }
-  if (!bfd_get_section_contents (abfd, sect,
-				 buf, 0, bfd_section_size (sect)))
-    goto out;
-
-
-
-  /* A DWORD (data_type) followed by struct windows_core_module_info.  */
-
-  base_addr =
-    extract_unsigned_integer (buf + 4, 4, byte_order);
-
-  module_name_size =
-    extract_unsigned_integer (buf + 8, 4, byte_order);
-
-  if (12 + module_name_size > bfd_section_size (sect))
-    goto out;
-  module_name = (char *) buf + 12;
-
-  /* The first module is the .exe itself.  */
-  if (data->module_count != 0)
-    windows_xfer_shared_library (module_name, base_addr,
-				 NULL, data->gdbarch, data->obstack);
-  data->module_count++;
-
-out:
-  xfree (buf);
-  return;
-}
-
-static ULONGEST
-windows_core_xfer_shared_libraries (struct gdbarch *gdbarch,
-				  gdb_byte *readbuf,
-				  ULONGEST offset, ULONGEST len)
-{
-  struct obstack obstack;
-  const char *buf;
-  ULONGEST len_avail;
-  struct cpms_data data = { gdbarch, &obstack, 0 };
-
-  obstack_init (&obstack);
-  obstack_grow_str (&obstack, "<library-list>\n");
-  bfd_map_over_sections (core_bfd,
-			 core_process_module_section,
-			 &data);
-  obstack_grow_str0 (&obstack, "</library-list>\n");
-
-  buf = (const char *) obstack_finish (&obstack);
-  len_avail = strlen (buf);
-  if (offset >= len_avail)
-    return 0;
-
-  if (len > len_avail - offset)
-    len = len_avail - offset;
-  memcpy (readbuf, buf + offset, len);
-
-  obstack_free (&obstack, NULL);
-  return len;
-}
-
-/* This is how we want PTIDs from core files to be printed.  */
-
-static std::string
-i386_windows_core_pid_to_str (struct gdbarch *gdbarch, ptid_t ptid)
-{
-  if (ptid.lwp () != 0)
-    return string_printf ("Thread 0x%lx", ptid.lwp ());
-
-  return normal_pid_to_str (ptid);
-}
-
 static CORE_ADDR
 i386_windows_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
 {
@@ -251,7 +153,7 @@ i386_windows_init_abi_common (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Core file support.  */
   set_gdbarch_core_xfer_shared_libraries
     (gdbarch, windows_core_xfer_shared_libraries);
-  set_gdbarch_core_pid_to_str (gdbarch, i386_windows_core_pid_to_str);
+  set_gdbarch_core_pid_to_str (gdbarch, windows_core_pid_to_str);
 
   set_gdbarch_auto_wide_charset (gdbarch, i386_windows_auto_wide_charset);
 }
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index aa0adeba99b..9dae287554c 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -1120,3 +1120,101 @@ even if their meaning is unknown."),
      isn't another convenience variable of the same name.  */
   create_internalvar_type_lazy ("_tlb", &tlb_funcs, NULL);
 }
+
+struct cpms_data
+{
+  struct gdbarch *gdbarch;
+  struct obstack *obstack;
+  int module_count;
+};
+
+static void
+core_process_module_section (bfd *abfd, asection *sect, void *obj)
+{
+  struct cpms_data *data = (struct cpms_data *) obj;
+  enum bfd_endian byte_order = gdbarch_byte_order (data->gdbarch);
+
+  char *module_name;
+  size_t module_name_size;
+  CORE_ADDR base_addr;
+
+  gdb_byte *buf = NULL;
+
+  if (!startswith (sect->name, ".module"))
+    return;
+
+  buf = (gdb_byte *) xmalloc (bfd_section_size (sect) + 1);
+  if (!buf)
+    {
+      printf_unfiltered ("memory allocation failed for %s\n", sect->name);
+      goto out;
+    }
+  if (!bfd_get_section_contents (abfd, sect,
+				 buf, 0, bfd_section_size (sect)))
+    goto out;
+
+
+
+  /* A DWORD (data_type) followed by struct windows_core_module_info.  */
+
+  base_addr =
+    extract_unsigned_integer (buf + 4, 4, byte_order);
+
+  module_name_size =
+    extract_unsigned_integer (buf + 8, 4, byte_order);
+
+  if (12 + module_name_size > bfd_section_size (sect))
+    goto out;
+  module_name = (char *) buf + 12;
+
+  /* The first module is the .exe itself.  */
+  if (data->module_count != 0)
+    windows_xfer_shared_library (module_name, base_addr,
+				 NULL, data->gdbarch, data->obstack);
+  data->module_count++;
+
+out:
+  xfree (buf);
+  return;
+}
+
+ULONGEST
+windows_core_xfer_shared_libraries (struct gdbarch *gdbarch,
+				  gdb_byte *readbuf,
+				  ULONGEST offset, ULONGEST len)
+{
+  struct obstack obstack;
+  const char *buf;
+  ULONGEST len_avail;
+  struct cpms_data data = { gdbarch, &obstack, 0 };
+
+  obstack_init (&obstack);
+  obstack_grow_str (&obstack, "<library-list>\n");
+  bfd_map_over_sections (core_bfd,
+			 core_process_module_section,
+			 &data);
+  obstack_grow_str0 (&obstack, "</library-list>\n");
+
+  buf = (const char *) obstack_finish (&obstack);
+  len_avail = strlen (buf);
+  if (offset >= len_avail)
+    return 0;
+
+  if (len > len_avail - offset)
+    len = len_avail - offset;
+  memcpy (readbuf, buf + offset, len);
+
+  obstack_free (&obstack, NULL);
+  return len;
+}
+
+/* This is how we want PTIDs from core files to be printed.  */
+
+std::string
+windows_core_pid_to_str (struct gdbarch *gdbarch, ptid_t ptid)
+{
+  if (ptid.lwp () != 0)
+    return string_printf ("Thread 0x%lx", ptid.lwp ());
+
+  return normal_pid_to_str (ptid);
+}
diff --git a/gdb/windows-tdep.h b/gdb/windows-tdep.h
index cd7717bd917..ec677cbdd46 100644
--- a/gdb/windows-tdep.h
+++ b/gdb/windows-tdep.h
@@ -31,6 +31,14 @@ extern void windows_xfer_shared_library (const char* so_name,
 					 struct gdbarch *gdbarch,
 					 struct obstack *obstack);
 
+extern ULONGEST windows_core_xfer_shared_libraries (struct gdbarch *gdbarch,
+						    gdb_byte *readbuf,
+						    ULONGEST offset,
+						    ULONGEST len);
+
+extern std::string windows_core_pid_to_str (struct gdbarch *gdbarch,
+					    ptid_t ptid);
+
 /* To be called from the various GDB_OSABI_WINDOWS handlers for the
    various Windows architectures and machine types.  */
 
-- 
2.27.0

Re: [PATCH 6/7] Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str

[Simon Marchi]

On 2020-07-01 5:32 p.m., Jon Turney wrote:
> Move windows_core_xfer_shared_libraries() and windows_core_pid_to_str()
> to windows-tdep, use in amd64-windows-tdep.c to handle Cygwin x86_64
> core dumps.

As a convention, the _initialize function is always at the bottom of the file,
so I'd put the new functions in windows-tdep.c just above it.

Were the functions copied as-is, or did you need to make changes to support 64-bits?

Simon

Re: [PATCH 6/7] Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str

[Simon Marchi]

On 2020-07-02 7:53 p.m., Simon Marchi wrote:
> On 2020-07-01 5:32 p.m., Jon Turney wrote:
>> Move windows_core_xfer_shared_libraries() and windows_core_pid_to_str()
>> to windows-tdep, use in amd64-windows-tdep.c to handle Cygwin x86_64
>> core dumps.
> 
> As a convention, the _initialize function is always at the bottom of the file,
> so I'd put the new functions in windows-tdep.c just above it.
> 
> Were the functions copied as-is, or did you need to make changes to support 64-bits?
> 
> Simon
> 

Well, right after writing this I saw patch 7, "Add handling for 64-bit module addresses
in Cygwin core dumps".  Now you know that I read series in a very linear fashion :).

So, I suppose that in this patch, the functions are copied as-is, and are not suitable
yet for the 64-bit cores?

Simon

Re: [PATCH 6/7] Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str

[Jon Turney]

On 03/07/2020 00:56, Simon Marchi wrote:
> On 2020-07-02 7:53 p.m., Simon Marchi wrote:
>> On 2020-07-01 5:32 p.m., Jon Turney wrote:
>>> Move windows_core_xfer_shared_libraries() and windows_core_pid_to_str()
>>> to windows-tdep, use in amd64-windows-tdep.c to handle Cygwin x86_64
>>> core dumps.
>>
>> As a convention, the _initialize function is always at the bottom of the file,
>> so I'd put the new functions in windows-tdep.c just above it.
>>
>> Were the functions copied as-is, or did you need to make changes to support 64-bits?
> 
> Well, right after writing this I saw patch 7, "Add handling for 64-bit module addresses
> in Cygwin core dumps".  Now you know that I read series in a very linear fashion :).
> 
> So, I suppose that in this patch, the functions are copied as-is, and are not suitable
> yet for the 64-bit cores?

Yes, this patch is just moves the code, but makes no changes.

[PATCH 7/7] Add handling for 64-bit module addresses in Cygwin core dumps

[Jon Turney]

bfd/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* elf.c (elfcore_grok_win32pstatus): Handle NOTE_INFO_MODULE64.

gdb/ChangeLog:

2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>

	* windows-tdep.c (core_process_module_section): Handle
	NOTE_INFO_MODULE64.
---
 bfd/ChangeLog      |  4 ++++
 bfd/elf.c          | 15 ++++++++++++---
 gdb/ChangeLog      |  5 +++++
 gdb/windows-tdep.c | 24 +++++++++++++++++-------
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index a61e2b7dd1d..00858e16fd3 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10185,10 +10185,19 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case 3 /* NOTE_INFO_MODULE */:
-      /* Make a ".module/xxxxxxxx" section.  */
+    case 4 /* NOTE_INFO_MODULE64 */:
+      /* Make a ".module/<base_address>" section.  */
       /* module_info.base_address */
-      base_addr = bfd_get_32 (abfd, note->descdata + 4);
-      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+      if (type == 3)
+        {
+          base_addr = bfd_get_32 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+        }
+      else
+        {
+          base_addr = bfd_get_64 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%016lx", (unsigned long) base_addr);
+        }
 
       len = strlen (buf) + 1;
       name = (char *) bfd_alloc (abfd, len);
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index 9dae287554c..7dffad00240 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -1134,8 +1134,10 @@ core_process_module_section (bfd *abfd, asection *sect, void *obj)
   struct cpms_data *data = (struct cpms_data *) obj;
   enum bfd_endian byte_order = gdbarch_byte_order (data->gdbarch);
 
+  unsigned int data_type;
   char *module_name;
   size_t module_name_size;
+  size_t module_name_offset;
   CORE_ADDR base_addr;
 
   gdb_byte *buf = NULL;
@@ -1156,16 +1158,24 @@ core_process_module_section (bfd *abfd, asection *sect, void *obj)
 
 
   /* A DWORD (data_type) followed by struct windows_core_module_info.  */
+  data_type = extract_unsigned_integer (buf, 4, byte_order);
 
-  base_addr =
-    extract_unsigned_integer (buf + 4, 4, byte_order);
-
-  module_name_size =
-    extract_unsigned_integer (buf + 8, 4, byte_order);
+  if (data_type == 3) /* NOTE_INFO_MODULE */
+    {
+      base_addr = extract_unsigned_integer (buf + 4, 4, byte_order);
+      module_name_size = extract_unsigned_integer (buf + 8, 4, byte_order);
+      module_name_offset = 12;
+    }
+  else /* NOTE_INFO_MODULE64 */
+    {
+      base_addr = extract_unsigned_integer (buf + 4, 8, byte_order);
+      module_name_size = extract_unsigned_integer (buf + 12, 4, byte_order);
+      module_name_offset = 16;
+    }
 
-  if (12 + module_name_size > bfd_section_size (sect))
+  if (module_name_offset + module_name_size > bfd_section_size (sect))
     goto out;
-  module_name = (char *) buf + 12;
+  module_name = (char *) buf + module_name_offset;
 
   /* The first module is the .exe itself.  */
   if (data->module_count != 0)
-- 
2.27.0

Re: [PATCH 7/7] Add handling for 64-bit module addresses in Cygwin core dumps

[Christian Biesinger]

On Wed, Jul 1, 2020 at 4:34 PM Jon Turney <jon.turney@dronecode.org.uk> wrote:
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -10185,10 +10185,19 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
>        break;
>
>      case 3 /* NOTE_INFO_MODULE */:
> -      /* Make a ".module/xxxxxxxx" section.  */
> +    case 4 /* NOTE_INFO_MODULE64 */:

It really seems like these should be actual constants, which would
also make it easier to understand...

> +      /* Make a ".module/<base_address>" section.  */
>        /* module_info.base_address */
> -      base_addr = bfd_get_32 (abfd, note->descdata + 4);
> -      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
> +      if (type == 3)

...this if.

> --- a/gdb/windows-tdep.c
> +++ b/gdb/windows-tdep.c
> +  if (data_type == 3) /* NOTE_INFO_MODULE */

Same here.

Re: [PATCH 7/7] Add handling for 64-bit module addresses in Cygwin core dumps

[Jon Turney]

On 06/07/2020 21:13, Christian Biesinger via Gdb-patches wrote:
> On Wed, Jul 1, 2020 at 4:34 PM Jon Turney <jon.turney@dronecode.org.uk> wrote:
>> --- a/bfd/elf.c
>> +++ b/bfd/elf.c
>> @@ -10185,10 +10185,19 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
>>         break;
>>
>>       case 3 /* NOTE_INFO_MODULE */:
>> -      /* Make a ".module/xxxxxxxx" section.  */
>> +    case 4 /* NOTE_INFO_MODULE64 */:
> 
> It really seems like these should be actual constants, which would
> also make it easier to understand...

Absolutely.

Before [1], this structure was defined by including Cygwin's 
<sys/procfs.h> (so this code didn't work in cross-environments).

I can't find a model of adding note structure definitions (the other 
grok_note functions seem to rely on OS definitions), so I'm not sure 
what kind of change you are asking me to make in this patch.

[1] 
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=4a6636fbb5c051fbe15d18a005a2a505ef652571

> 
>> +      /* Make a ".module/<base_address>" section.  */
>>         /* module_info.base_address */
>> -      base_addr = bfd_get_32 (abfd, note->descdata + 4);
>> -      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
>> +      if (type == 3)
> 
> ...this if.
> 
>> --- a/gdb/windows-tdep.c
>> +++ b/gdb/windows-tdep.c
>> +  if (data_type == 3) /* NOTE_INFO_MODULE */
> 
> Same here.

Re: [PATCH 7/7] Add handling for 64-bit module addresses in Cygwin core dumps

[Christian Biesinger]

On Wed, Jul 8, 2020 at 10:50 AM Jon Turney <jon.turney@dronecode.org.uk> wrote:
>
> On 06/07/2020 21:13, Christian Biesinger via Gdb-patches wrote:
> > On Wed, Jul 1, 2020 at 4:34 PM Jon Turney <jon.turney@dronecode.org.uk> wrote:
> >> --- a/bfd/elf.c
> >> +++ b/bfd/elf.c
> >> @@ -10185,10 +10185,19 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
> >>         break;
> >>
> >>       case 3 /* NOTE_INFO_MODULE */:
> >> -      /* Make a ".module/xxxxxxxx" section.  */
> >> +    case 4 /* NOTE_INFO_MODULE64 */:
> >
> > It really seems like these should be actual constants, which would
> > also make it easier to understand...
>
> Absolutely.
>
> Before [1], this structure was defined by including Cygwin's
> <sys/procfs.h> (so this code didn't work in cross-environments).
>
> I can't find a model of adding note structure definitions (the other
> grok_note functions seem to rely on OS definitions), so I'm not sure
> what kind of change you are asking me to make in this patch.

I was just suggesting adding a `static constexpr int
NOTE_INFO_MODULE64 = 4` in some appropriate place, maybe at the top of
this file.

Er, I guess this is bfd, so make it a #define.

>
> [1]
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=4a6636fbb5c051fbe15d18a005a2a505ef652571
>
> >
> >> +      /* Make a ".module/<base_address>" section.  */
> >>         /* module_info.base_address */
> >> -      base_addr = bfd_get_32 (abfd, note->descdata + 4);
> >> -      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
> >> +      if (type == 3)
> >
> > ...this if.
> >
> >> --- a/gdb/windows-tdep.c
> >> +++ b/gdb/windows-tdep.c
> >> +  if (data_type == 3) /* NOTE_INFO_MODULE */
> >
> > Same here.

Re: [PATCH 7/7] Add handling for 64-bit module addresses in Cygwin core dumps

[Jon Turney]

On 08/07/2020 17:11, Christian Biesinger via Gdb-patches wrote:
> On Wed, Jul 8, 2020 at 10:50 AM Jon Turney <jon.turney@dronecode.org.uk> wrote:
>> On 06/07/2020 21:13, Christian Biesinger via Gdb-patches wrote:
>>> On Wed, Jul 1, 2020 at 4:34 PM Jon Turney <jon.turney@dronecode.org.uk> wrote:
>>>> --- a/bfd/elf.c
>>>> +++ b/bfd/elf.c
>>>> @@ -10185,10 +10185,19 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
>>>>          break;
>>>>
>>>>        case 3 /* NOTE_INFO_MODULE */:
>>>> -      /* Make a ".module/xxxxxxxx" section.  */
>>>> +    case 4 /* NOTE_INFO_MODULE64 */:
>>>
>>> It really seems like these should be actual constants, which would
>>> also make it easier to understand...
>>
>> Absolutely.
>>
>> Before [1], this structure was defined by including Cygwin's
>> <sys/procfs.h> (so this code didn't work in cross-environments).
>>
>> I can't find a model of adding note structure definitions (the other
>> grok_note functions seem to rely on OS definitions), so I'm not sure
>> what kind of change you are asking me to make in this patch.
> 
> I was just suggesting adding a `static constexpr int
> NOTE_INFO_MODULE64 = 4` in some appropriate place, maybe at the top of
> this file.
> 
> Er, I guess this is bfd, so make it a #define.

I posted a revised patchset with that change (to the binutils list, 
where it belongs).

Re: [PATCH 0/7] Add gdb support for Cygwin x86_64 core dumps

[Tom Tromey]

>>>>> "Jon" == Jon Turney <jon.turney@dronecode.org.uk> writes:

Jon> As far as I know, the only way to generate these "core dumps" is to use
Jon> Cygwin's 'dumper' tool, which requires some fixes on x86_64 [1].

Jon> [1] https://cygwin.com/pipermail/cygwin-patches/2020q3/010313.html

Jon> Jon Turney (7):
Jon>   Read tid from correct offset in win32pstatus NOTE_INFO_THREAD
Jon>   Don't apply size constraint to all win32pstatus ELF notes.
Jon>   Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note
Jon>   Add sniffer for Cygwin x86_64 core dumps
Jon>   Add amd64_windows_gregset_reg_offset
Jon>   Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str
Jon>   Add handling for 64-bit module addresses in Cygwin core dumps

Jon>  bfd/ChangeLog            |  20 ++++++++
Jon>  bfd/elf.c                |  25 +++++----

The BFD changes should be sent to the binutils list.

Tom

Re: [PATCH 0/7] Add gdb support for Cygwin x86_64 core dumps

[Jon Turney]

On 02/07/2020 22:17, Tom Tromey wrote:
>>>>>> "Jon" == Jon Turney <jon.turney@dronecode.org.uk> writes:
> 
> Jon> As far as I know, the only way to generate these "core dumps" is to use
> Jon> Cygwin's 'dumper' tool, which requires some fixes on x86_64 [1].
> 
> Jon> [1] https://cygwin.com/pipermail/cygwin-patches/2020q3/010313.html
> 
> Jon> Jon Turney (7):
> Jon>   Read tid from correct offset in win32pstatus NOTE_INFO_THREAD
> Jon>   Don't apply size constraint to all win32pstatus ELF notes.
> Jon>   Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note
> Jon>   Add sniffer for Cygwin x86_64 core dumps
> Jon>   Add amd64_windows_gregset_reg_offset
> Jon>   Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str
> Jon>   Add handling for 64-bit module addresses in Cygwin core dumps
> 
> Jon>  bfd/ChangeLog            |  20 ++++++++
> Jon>  bfd/elf.c                |  25 +++++----
> 
> The BFD changes should be sent to the binutils list.

Doh.  I knew that. Sorry!

Re: [PATCH 0/7] Add gdb support for Cygwin x86_64 core dumps

[Simon Marchi]

On 2020-07-01 5:32 p.m., Jon Turney wrote:
> As far as I know, the only way to generate these "core dumps" is to use
> Cygwin's 'dumper' tool, which requires some fixes on x86_64 [1].
> 
> [1] https://cygwin.com/pipermail/cygwin-patches/2020q3/010313.html
> 
> Jon Turney (7):
>   Read tid from correct offset in win32pstatus NOTE_INFO_THREAD
>   Don't apply size constraint to all win32pstatus ELF notes.
>   Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note
>   Add sniffer for Cygwin x86_64 core dumps
>   Add amd64_windows_gregset_reg_offset
>   Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str
>   Add handling for 64-bit module addresses in Cygwin core dumps
> 
>  bfd/ChangeLog            |  20 ++++++++
>  bfd/elf.c                |  25 +++++----
>  gdb/ChangeLog            |  23 +++++++++
>  gdb/amd64-windows-tdep.c | 100 ++++++++++++++++++++++++++++++++++++
>  gdb/i386-windows-tdep.c  | 100 +-----------------------------------
>  gdb/windows-tdep.c       | 108 +++++++++++++++++++++++++++++++++++++++
>  gdb/windows-tdep.h       |   8 +++
>  7 files changed, 276 insertions(+), 108 deletions(-)
> 
> -- 
> 2.27.0
> 

I've sent a few comments, but in general the GDB bits look fine to me.  But
as Tom said, the bfd bits need to be approved by the binutils folks.

Simon