[PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[Jon Turney]

As far as I know, the only way to generate these "core dumps" is to use
Cygwin's 'dumper' tool.

Changes since v1:
- Keep _initialize function last in windows-tdpe.c
- Fix indentation in amd64_windows_gregset_reg_offset
- Use symbolic constants for note types in core_process_module_section()

Jon Turney (4):
  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

 gdb/ChangeLog            |  23 ++++++++
 gdb/amd64-windows-tdep.c | 100 ++++++++++++++++++++++++++++++++++
 gdb/i386-windows-tdep.c  | 100 +---------------------------------
 gdb/windows-tdep.c       | 114 +++++++++++++++++++++++++++++++++++++++
 gdb/windows-tdep.h       |   8 +++
 5 files changed, 246 insertions(+), 99 deletions(-)

-- 
2.28.0

[PATCH 1/4] 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.28.0

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

[Simon Marchi]

On 2020-08-12 3:18 p.m., Jon Turney wrote:
> 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.

Hi Jon,

I am trying to test this using these steps:

1. Write a dummy C program that just sleeps in Cygwin, compile it (gcc test.c -g3 -O0) and run it
2. Run "dumper" on it
3. Load it in GDB (back on my amd64 Linux host)

I get this in step 3:

$ ./gdb --data-directory=data-directory -nx -q ~/a.exe
Reading symbols from /home/simark/a.exe...
(gdb) core ~/core.core
BFD: BFD (GNU Binutils) 2.35.50.20200820 internal error, aborting at /home/simark/src/binutils-gdb/bfd/bfd.c:1311 in _bfd_doprnt_scan

BFD: Please report this bug.

This happens while BFD encounters an error and tries to print this format string:

  "%pB: win32pstatus NOTE_INFO_MODULE of size %lu is too small to contain a name of size %zu"

_bfd_doprnt_scan aborts because it doesn't like the z length modifier:

(top-gdb) p ptr[-1]
$2 = 122 'z'

So there are two issues here:

1. The actual error, the note being too small for the name.  If I go up in
   elfcore_grok_win32pstatus, the name_size doesn't make sense:

   (top-gdb) p name_size
   $9 = 4194304

   whereas the note size makes sense:

   (top-gdb) p note->descsz
   $10 = 1276

2. The internal error from trying to print that error message

I realize this probably isn't caused by your patch at all but... I thought
you'd like to know about it anyway.  I can provide more info for either if
needed.

Simon

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

[Jon Turney]

On 20/08/2020 23:20, Simon Marchi wrote:
> On 2020-08-12 3:18 p.m., Jon Turney wrote:
>> 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.
> 
> Hi Jon,
> 
> I am trying to test this using these steps:

Wow.  Thanks for testing.

> 1. Write a dummy C program that just sleeps in Cygwin, compile it (gcc test.c -g3 -O0) and run it
> 2. Run "dumper" on it

The "dumper" currently shipping in x86_64 Cygwin is utterly broken. (See 
[1] et seq.).

If you use "dumper" from the latest snapshot [2], you may have more success

[1] 
https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=7dd1b08836e8a7bb37d330995096540afce152a0;hp=38f88601469f4a6ab7cf42e1f076775c99eb17f2
[2] https://cygwin.com/snapshots/

> 3. Load it in GDB (back on my amd64 Linux host)
> 
> I get this in step 3:
> 
> $ ./gdb --data-directory=data-directory -nx -q ~/a.exe
> Reading symbols from /home/simark/a.exe...
> (gdb) core ~/core.core
> BFD: BFD (GNU Binutils) 2.35.50.20200820 internal error, aborting at /home/simark/src/binutils-gdb/bfd/bfd.c:1311 in _bfd_doprnt_scan
> 
> BFD: Please report this bug.
> 
> This happens while BFD encounters an error and tries to print this format string:
> 
>    "%pB: win32pstatus NOTE_INFO_MODULE of size %lu is too small to contain a name of size %zu"
> 
> _bfd_doprnt_scan aborts because it doesn't like the z length modifier:
> 
> (top-gdb) p ptr[-1]
> $2 = 122 'z'
> 
> So there are two issues here:
> 
> 1. The actual error, the note being too small for the name.  If I go up in
>     elfcore_grok_win32pstatus, the name_size doesn't make sense:
> 
>     (top-gdb) p name_size
>     $9 = 4194304
> 
>     whereas the note size makes sense:
> 
>     (top-gdb) p note->descsz
>     $10 = 1276

This should be resolved by using a fixed "dumper".

> 2. The internal error from trying to print that error message

Clearly, I shouldn't be using '%zu' here, so I'll make a patch to fix 
that (once I work out what I should be using :) )

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

[Simon Marchi]

On 2020-08-21 11:25 a.m., Jon Turney wrote:
> The "dumper" currently shipping in x86_64 Cygwin is utterly broken. (See [1] et seq.).
> 
> If you use "dumper" from the latest snapshot [2], you may have more success
> 
> [1] https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=7dd1b08836e8a7bb37d330995096540afce152a0;hp=38f88601469f4a6ab7cf42e1f076775c99eb17f2
> [2] https://cygwin.com/snapshots/

Ok, thanks for the tip.  I'll give it a try with a snapshot.

Do I just extract cygwin-20200821.tar.xz into my current Cygwin installation to overwrite
the corresponding files?  If I want to revert to the stable versions, how can I do that?

Simon

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

[Jon Turney]

On 21/08/2020 17:22, Simon Marchi wrote:
> On 2020-08-21 11:25 a.m., Jon Turney wrote:
>> The "dumper" currently shipping in x86_64 Cygwin is utterly broken. (See [1] et seq.).
>>
>> If you use "dumper" from the latest snapshot [2], you may have more success
>>
>> [1] https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=7dd1b08836e8a7bb37d330995096540afce152a0;hp=38f88601469f4a6ab7cf42e1f076775c99eb17f2
>> [2] https://cygwin.com/snapshots/
> 
> Ok, thanks for the tip.  I'll give it a try with a snapshot.
> 
> Do I just extract cygwin-20200821.tar.xz into my current Cygwin installation to overwrite
> the corresponding files?  If I want to revert to the stable versions, how can I do that?

You should be able to just extract the dumper.exe from that archive, and 
use it.  That's what I would suggest.

If you were to extract it over an existing Cygwin installation, to 
revert you could use the cygwin setup program to re-install all cygwin* 
packages.

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

[Simon Marchi]

On 2020-08-21 12:28 p.m., Jon Turney wrote:
> On 21/08/2020 17:22, Simon Marchi wrote:
>> On 2020-08-21 11:25 a.m., Jon Turney wrote:
>>> The "dumper" currently shipping in x86_64 Cygwin is utterly broken. (See [1] et seq.).
>>>
>>> If you use "dumper" from the latest snapshot [2], you may have more success
>>>
>>> [1] https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=7dd1b08836e8a7bb37d330995096540afce152a0;hp=38f88601469f4a6ab7cf42e1f076775c99eb17f2
>>> [2] https://cygwin.com/snapshots/
>>
>> Ok, thanks for the tip.  I'll give it a try with a snapshot.
>>
>> Do I just extract cygwin-20200821.tar.xz into my current Cygwin installation to overwrite
>> the corresponding files?  If I want to revert to the stable versions, how can I do that?
> 
> You should be able to just extract the dumper.exe from that archive, and 
> use it.  That's what I would suggest.

Hmm, just tried that, now running "dumper" does nothing:

Baube@DESKTOP-42816MD ~
$ dumper

Baube@DESKTOP-42816MD ~
$ echo $?
127

One thing that's weird is that in my installation, binaries are in C:\cygwin64\bin,
whereas in the snapshot, they are in usr/bin.  Did the filesystem layout change
recently?

So I am not sure how to proceed to try the snapshot.

> If you were to extract it over an existing Cygwin installation, to 
> revert you could use the cygwin setup program to re-install all cygwin* 
> packages.

I successfully repaired my broken dumper by re-installing the cygwin* packages.

Simon

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

[Jon Turney]

On 22/08/2020 20:41, Simon Marchi wrote:
> On 2020-08-21 12:28 p.m., Jon Turney wrote:
>> On 21/08/2020 17:22, Simon Marchi wrote:
>>> On 2020-08-21 11:25 a.m., Jon Turney wrote:
>>>> The "dumper" currently shipping in x86_64 Cygwin is utterly broken. (See [1] et seq.).
>>>>
>>>> If you use "dumper" from the latest snapshot [2], you may have more success
>>>>
>>>> [1] https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=7dd1b08836e8a7bb37d330995096540afce152a0;hp=38f88601469f4a6ab7cf42e1f076775c99eb17f2
>>>> [2] https://cygwin.com/snapshots/
>>>
>>> Ok, thanks for the tip.  I'll give it a try with a snapshot.
>>>
>>> Do I just extract cygwin-20200821.tar.xz into my current Cygwin installation to overwrite
>>> the corresponding files?  If I want to revert to the stable versions, how can I do that?
>>
>> You should be able to just extract the dumper.exe from that archive, and
>> use it.  That's what I would suggest.
> 
> Hmm, just tried that, now running "dumper" does nothing:
> 
> Baube@DESKTOP-42816MD ~
> $ dumper
> 
> Baube@DESKTOP-42816MD ~
> $ echo $?
> 127

Hmmm.  This often means the executable can't be started due to problems 
loading DLLs.

If you want to pursue this further, running under cygwin's strace (e.g. 
'strace dumper') might shed more light.

> One thing that's weird is that in my installation, binaries are in C:\cygwin64\bin,
> whereas in the snapshot, they are in usr/bin.  Did the filesystem layout change
> recently?

This is expected.

If you examine the output of 'mount', you'll see that C:\cygwin64\bin is 
mounted at /usr/bin

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

[Simon Marchi]

On 2020-08-22 3:52 p.m., Jon Turney wrote:
> On 22/08/2020 20:41, Simon Marchi wrote:
>> On 2020-08-21 12:28 p.m., Jon Turney wrote:
>>> On 21/08/2020 17:22, Simon Marchi wrote:
>>>> On 2020-08-21 11:25 a.m., Jon Turney wrote:
>>>>> The "dumper" currently shipping in x86_64 Cygwin is utterly broken. (See [1] et seq.).
>>>>>
>>>>> If you use "dumper" from the latest snapshot [2], you may have more success
>>>>>
>>>>> [1] https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=7dd1b08836e8a7bb37d330995096540afce152a0;hp=38f88601469f4a6ab7cf42e1f076775c99eb17f2
>>>>> [2] https://cygwin.com/snapshots/
>>>>
>>>> Ok, thanks for the tip.  I'll give it a try with a snapshot.
>>>>
>>>> Do I just extract cygwin-20200821.tar.xz into my current Cygwin installation to overwrite
>>>> the corresponding files?  If I want to revert to the stable versions, how can I do that?
>>>
>>> You should be able to just extract the dumper.exe from that archive, and
>>> use it.  That's what I would suggest.
>>
>> Hmm, just tried that, now running "dumper" does nothing:
>>
>> Baube@DESKTOP-42816MD ~
>> $ dumper
>>
>> Baube@DESKTOP-42816MD ~
>> $ echo $?
>> 127
> 
> Hmmm.  This often means the executable can't be started due to problems 
> loading DLLs.
> 
> If you want to pursue this further, running under cygwin's strace (e.g. 
> 'strace dumper') might shed more light.

Ahh, I downloaded the x86 version of the snapshot instead of the x86-64!

When using dumper.exe from the x86-64, it works.  I that dumper.exe from the
snapshot also runs much faster than the original dumper.exe, don't know if that's
expected.

I now get this:

$ ./gdb -q --data-directory=data-directory ~/a.exe -ex "core ~/core.core"
Reading symbols from /home/simark/a.exe...
warning: core file may not match specified executable file.
[New process 6408]
[New process 6504]
[New process 904]
[New process 5068]
[New process 4892]
#0  0x00007ff92bba01b4 in ?? ()
[Current thread is 1 (process 6408)]
(gdb) bt
#0  0x00007ff92bba01b4 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Since I just copied my binary and core out of Cygwin, and the program was stopped somewhere
under sleep(), it's perhaps expected that GDB has trouble unwinding from system libraries
it doesn't have access to.

Simon

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

[Jon Turney]

On 24/08/2020 16:47, Simon Marchi wrote:
> On 2020-08-22 3:52 p.m., Jon Turney wrote:
>>
>> Hmmm.  This often means the executable can't be started due to problems
>> loading DLLs.
>>
>> If you want to pursue this further, running under cygwin's strace (e.g.
>> 'strace dumper') might shed more light.
> 
> Ahh, I downloaded the x86 version of the snapshot instead of the x86-64!

:)

> When using dumper.exe from the x86-64, it works.  I that dumper.exe from the
> snapshot also runs much faster than the original dumper.exe, don't know if that's
> expected.

Yeah, I fixed some bogosity which was making it slow.

> I now get this:
> 
> $ ./gdb -q --data-directory=data-directory ~/a.exe -ex "core ~/core.core"
> Reading symbols from /home/simark/a.exe...
> warning: core file may not match specified executable file.
> [New process 6408]
> [New process 6504]
> [New process 904]
> [New process 5068]
> [New process 4892]
> #0  0x00007ff92bba01b4 in ?? ()
> [Current thread is 1 (process 6408)]
> (gdb) bt
> #0  0x00007ff92bba01b4 in ?? ()
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> Since I just copied my binary and core out of Cygwin, and the program was stopped somewhere
> under sleep(), it's perhaps expected that GDB has trouble unwinding from system libraries
> it doesn't have access to.

Yeah, I don't think unwinding is going to be very successful in that 
case. Thanks for trying it out.

The "testing" I have been doing is mainly:

> $ cat dumper-test.c
> 
> void fault()
> {
>   // deref null pointer
>   *(int *)0 = 1;
> }
> 
> int main()
> {
>   fault();
> }
> 
> $ gcc -g -O0 dumper-test.c -o dumper-test.exe
> 
> $ CYGWIN='error_start=dumper' ./dumper-test
> 
> $ gdb dumper-test.exe dumper-test.exe.core
> [...]
> (gdb) bt
> #0  0x00007ffb4498fc20 in ntdll!KiUserExceptionDispatcher () from C:/WINDOWS/SYSTEM32/ntdll.dll
> #1  0x0000000100401089 in fault () at dumper-test.c:5
> #2  0x00000001004010a4 in main () at dumper-test.c:10

[PATCH 2/4] 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 to 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..7bb98bd2bd2 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.28.0

Re: [PATCH 2/4] Add amd64_windows_gregset_reg_offset

[Simon Marchi]

On 2020-08-12 3:18 p.m., Jon Turney wrote:
> 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 to 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.

That looks reasonable.  I think i386_iterate_over_regset_sections should
be made static, if you want to make another (obvious I would say) patch.

Simon

Re: [PATCH 2/4] Add amd64_windows_gregset_reg_offset

[Jon Turney]

On 20/08/2020 23:08, Simon Marchi wrote:
> On 2020-08-12 3:18 p.m., Jon Turney wrote:
>> 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 to get installed by i386_gdbarch_init()).

One thing I notice is that, after this change, we now have (for both x86 
and x86_64), a mapping between regno and CONTEXT offset both as a 
gregset_reg_offset mapping (used when interpreting coredumps) and a 
similar mapping fed into windows_set_context_register_offsets() (used on 
an inferior process).

There's probably some scope for de-duplication and genericization there.

>> 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.
> 
> That looks reasonable.  I think i386_iterate_over_regset_sections should
> be made static, if you want to make another (obvious I would say) patch.

[PATCH 3/4] 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, and use in amd64-windows-tdep.c to handle Cygwin x86_64
core dumps.

v2:
Keep _initialize function at the bottom of the file.

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 7bb98bd2bd2..e427c205389 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..037958a4bf5 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -1088,6 +1088,104 @@ range [0x%" BFD_VMA_FMT "x, 0x%" BFD_VMA_FMT "x[."),
   return false;
 }
 
+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);
+}
+
 void _initialize_windows_tdep ();
 void
 _initialize_windows_tdep ()
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.28.0

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

[Jon Turney]

gdb/ChangeLog:

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

	* windows-tdep.c (NOTE_INFO_MODULE, NOTE_INFO_MODULE64): Define.
	(core_process_module_section): Handle NOTE_INFO_MODULE64.
---
 gdb/ChangeLog      |  5 +++++
 gdb/windows-tdep.c | 30 +++++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index 037958a4bf5..dd5f69a9acd 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -105,6 +105,10 @@ enum
   CYGWIN_SIGUSR2 = 31,
 };
 
+/* These constants are defined by Cygwin's core_dump.h */
+static constexpr unsigned int NOTE_INFO_MODULE = 3;
+static constexpr unsigned int NOTE_INFO_MODULE64 = 4;
+
 struct cmd_list_element *info_w32_cmdlist;
 
 typedef struct thread_information_block_32
@@ -1101,8 +1105,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;
@@ -1123,16 +1129,26 @@ 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 == 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 if (data_type == 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;
+    }
+  else
+    goto out;
 
-  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.28.0

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

[Simon Marchi]

On 2020-08-12 3:18 p.m., Jon Turney wrote:
> gdb/ChangeLog:
> 
> 2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>
> 
> 	* windows-tdep.c (NOTE_INFO_MODULE, NOTE_INFO_MODULE64): Define.
> 	(core_process_module_section): Handle NOTE_INFO_MODULE64.

Hi Jon,

The series LGTM.  It would be nice however if you could write some commit message
for this patch here, explaining what you are adding support for.

Simon

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

[Jon Turney]

On 24/08/2020 19:34, Simon Marchi wrote:
> On 2020-08-12 3:18 p.m., Jon Turney wrote:
>> gdb/ChangeLog:
>>
>> 2020-07-01  Jon Turney  <jon.turney@dronecode.org.uk>
>>
>> 	* windows-tdep.c (NOTE_INFO_MODULE, NOTE_INFO_MODULE64): Define.
>> 	(core_process_module_section): Handle NOTE_INFO_MODULE64.
> 
> Hi Jon,
> 
> The series LGTM.

I'm hoping this means 'Approved. Please apply.'.

> It would be nice however if you could write some commit message
> for this patch here, explaining what you are adding support for.

I will try to improve the commit message first.

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

[Simon Marchi]

On 2020-09-15 5:44 p.m., Jon Turney wrote:
> I'm hoping this means 'Approved. Please apply.'.

Yes :).

Simon

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

[Christian Biesinger]

For everyone who may not know, LGTM="Looks good to me"

Christian

On Wed, Sep 16, 2020 at 4:20 AM Simon Marchi <simark@simark.ca> wrote:
>
> On 2020-09-15 5:44 p.m., Jon Turney wrote:
> > I'm hoping this means 'Approved. Please apply.'.
>
> Yes :).
>
> Simon

Re: [PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[Eli Zaretskii]

> From: Jon Turney <jon.turney@dronecode.org.uk>
> Date: Wed, 12 Aug 2020 20:18:12 +0100
> Cc: Jon Turney <jon.turney@dronecode.org.uk>
> 
> As far as I know, the only way to generate these "core dumps" is to use
> Cygwin's 'dumper' tool.
> 
> Changes since v1:
> - Keep _initialize function last in windows-tdpe.c
> - Fix indentation in amd64_windows_gregset_reg_offset
> - Use symbolic constants for note types in core_process_module_section()
> 
> Jon Turney (4):
>   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
> 
>  gdb/ChangeLog            |  23 ++++++++
>  gdb/amd64-windows-tdep.c | 100 ++++++++++++++++++++++++++++++++++
>  gdb/i386-windows-tdep.c  | 100 +---------------------------------
>  gdb/windows-tdep.c       | 114 +++++++++++++++++++++++++++++++++++++++
>  gdb/windows-tdep.h       |   8 +++
>  5 files changed, 246 insertions(+), 99 deletions(-)

Thanks, but isn't this NEWS-worthy?  And what about some documentation
for the manual?

Re: [PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[Jon Turney]

On 12/08/2020 20:30, Eli Zaretskii wrote:
>> From: Jon Turney <jon.turney@dronecode.org.uk>
>> Date: Wed, 12 Aug 2020 20:18:12 +0100
>> Cc: Jon Turney <jon.turney@dronecode.org.uk>
>>
>> As far as I know, the only way to generate these "core dumps" is to use
>> Cygwin's 'dumper' tool.
>>
>> Changes since v1:
>> - Keep _initialize function last in windows-tdpe.c
>> - Fix indentation in amd64_windows_gregset_reg_offset
>> - Use symbolic constants for note types in core_process_module_section()
[...]
> 
> Thanks, but isn't this NEWS-worthy?

This doesn't add a new feature to gdb, just (belatedly) makes it work on 
x86_64 Cygwin as it does on x86 cygwin.

What would you suggest adding to NEWS?

 > And what about some documentation
 > for the manual?

No changes should be needed to the existing documentation of debugging 
using core files (since there shouldn't be any difference in behaviour 
when using Cygwin core dumps)

What else do you think needs documenting?

Re: [PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>
> From: Jon Turney <jon.turney@dronecode.org.uk>
> Date: Thu, 13 Aug 2020 15:43:41 +0100
> 
> > Thanks, but isn't this NEWS-worthy?
> 
> This doesn't add a new feature to gdb, just (belatedly) makes it work on 
> x86_64 Cygwin as it does on x86 cygwin.
> 
> What would you suggest adding to NEWS?

Something like "GDB now supports core file debugging for Cygwin
programs."

>  > And what about some documentation
>  > for the manual?
> 
> No changes should be needed to the existing documentation of debugging 
> using core files (since there shouldn't be any difference in behaviour 
> when using Cygwin core dumps)

Is there perhaps something special that needs to be done to produce
Cygwin core files?  Or do you use the same commands as on Posix hosts
("ulimit -c" etc.)?

Re: [PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[Jon Turney]

On 13/08/2020 17:48, Eli Zaretskii wrote:
>> Cc: Eli Zaretskii <eliz@gnu.org>
>> From: Jon Turney <jon.turney@dronecode.org.uk>
>> Date: Thu, 13 Aug 2020 15:43:41 +0100
>>
>>> Thanks, but isn't this NEWS-worthy?
>>
>> This doesn't add a new feature to gdb, just (belatedly) makes it work on
>> x86_64 Cygwin as it does on x86 cygwin.
>>
>> What would you suggest adding to NEWS?
> 
> Something like "GDB now supports core file debugging for Cygwin
> programs."

This feature has existed for x86 Cygwin since 2000 (and it's been 
possible to try to use it for x86_64 since that was added, but it just 
fails with an assert)

I'll make a patch to add "GDB now supports core file debugging for 
x86_64 Cygwin programs." to NEWS.

>>   > And what about some documentation
>>   > for the manual?
>>
>> No changes should be needed to the existing documentation of debugging
>> using core files (since there shouldn't be any difference in behaviour
>> when using Cygwin core dumps)
> 
> Is there perhaps something special that needs to be done to produce
> Cygwin core files?  Or do you use the same commands as on Posix hosts
> ("ulimit -c" etc.)?
There are some special steps needed at the moment (more due to 
implementation shortcomings than real difficulties), which are described 
in Cygwin documentation.

I can't find any mention of 'ulimit -c' or how core files might be 
produced on any platform in the current gdb documentation, so I'm not 
sure why Cygwin needs to be a special case here.

Re: [PATCH 0/4] Add gdb support for Cygwin x86_64 core dumps (v2)

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>
> From: Jon Turney <jon.turney@dronecode.org.uk>
> Date: Tue, 18 Aug 2020 16:32:45 +0100
> 
> I'll make a patch to add "GDB now supports core file debugging for 
> x86_64 Cygwin programs." to NEWS.

Thanks, that'll be good, I think.

> > Is there perhaps something special that needs to be done to produce
> > Cygwin core files?  Or do you use the same commands as on Posix hosts
> > ("ulimit -c" etc.)?
> There are some special steps needed at the moment (more due to 
> implementation shortcomings than real difficulties), which are described 
> in Cygwin documentation.
> 
> I can't find any mention of 'ulimit -c' or how core files might be 
> produced on any platform in the current gdb documentation, so I'm not 
> sure why Cygwin needs to be a special case here.

Then I guess NEWS should be enough.

Thanks.