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

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[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

Reply | Threaded
Open this post in threaded view
|

[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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by Jon TURNEY
Similarly to existing i386_cygwin_core_osabi_sniffer()

gdb/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

[PATCH 5/7] Add amd64_windows_gregset_reg_offset

Jon TURNEY
In reply to this post by 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

[PATCH 6/7] Promote windows_core_xfer_shared_libraries and windows_core_pid_to_str

Jon TURNEY
In reply to this post by 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by Jon TURNEY
bfd/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * elf.c (elfcore_grok_win32pstatus): Handle NOTE_INFO_MODULE64.

gdb/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-2
In reply to this post by Jon TURNEY
>>>>> "Jon" == Jon Turney <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
In reply to this post by Jon TURNEY
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
Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
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
Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
In reply to this post by Jon TURNEY
> @@ -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
Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
In reply to this post by Jon TURNEY
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
Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by Simon Marchi-4
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.
Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by Simon Marchi-4
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.
Reply | Threaded
Open this post in threaded view
|

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

Jon TURNEY
In reply to this post by Tom Tromey-2
On 02/07/2020 22:17, Tom Tromey wrote:

>>>>>> "Jon" == Jon Turney <[hidden email]> 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!
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5/7] Add amd64_windows_gregset_reg_offset

Pedro Alves-2
In reply to this post by Jon TURNEY
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.
Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
In reply to this post by Jon TURNEY
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
Reply | Threaded
Open this post in threaded view
|

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())
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
In reply to this post by Jon TURNEY
On Wed, Jul 1, 2020 at 4:33 PM Jon Turney <[hidden email]> 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  <[hidden email]>
>
>         * 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
>
12