public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH 0/3] Stackdump improvements
@ 2022-10-28 15:05 Jon Turney
  2022-10-28 15:05 ` [PATCH 1/3] Cygwin: Tidy up formatting of stackdump Jon Turney
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jon Turney @ 2022-10-28 15:05 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

Jon Turney (3):
  Cygwin: Tidy up formatting of stackdump
  Cygwin: Add addresses as module offsets in .stackdump file
  Cygwin: Add loaded module base address list to stackdump

 winsup/cygwin/exceptions.cc | 46 +++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

-- 
2.38.1


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

* [PATCH 1/3] Cygwin: Tidy up formatting of stackdump
  2022-10-28 15:05 [PATCH 0/3] Stackdump improvements Jon Turney
@ 2022-10-28 15:05 ` Jon Turney
  2022-10-28 15:05 ` [PATCH 2/3] Cygwin: Add addresses as module offsets in .stackdump file Jon Turney
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jon Turney @ 2022-10-28 15:05 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

Resize stackdump headers for b9e97f58
Consistently use \r\n line endings
---
 winsup/cygwin/exceptions.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index e6f868511..a15bc16c5 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -342,7 +342,7 @@ cygwin_exception::dumpstack ()
       int i;
 
       thestack.init (framep, 1, ctx);	/* Initialize from the input CONTEXT */
-      small_printf ("Stack trace:\r\nFrame        Function    Args\r\n");
+      small_printf ("Stack trace:\r\nFrame         Function      Args\r\n");
       for (i = 0; i < DUMPSTACK_FRAME_LIMIT && thestack++; i++)
 	{
 	  small_printf ("%012X  %012X", thestack.sf.AddrFrame.Offset,
@@ -352,9 +352,9 @@ cygwin_exception::dumpstack ()
 			  thestack.sf.Params[j]);
 	  small_printf (")\r\n");
 	}
-      small_printf ("End of stack trace%s\n",
+      small_printf ("End of stack trace%s\r\n",
 		    i == DUMPSTACK_FRAME_LIMIT ?
-		        " (more stack frames may be present)" : "");
+		    " (more stack frames may be present)" : "");
       if (h)
 	NtClose (h);
     }
-- 
2.38.1


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

* [PATCH 2/3] Cygwin: Add addresses as module offsets in .stackdump file
  2022-10-28 15:05 [PATCH 0/3] Stackdump improvements Jon Turney
  2022-10-28 15:05 ` [PATCH 1/3] Cygwin: Tidy up formatting of stackdump Jon Turney
@ 2022-10-28 15:05 ` Jon Turney
  2022-10-28 15:05 ` [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump Jon Turney
  2022-10-29  8:34 ` [PATCH 0/3] Stackdump improvements Corinna Vinschen
  3 siblings, 0 replies; 8+ messages in thread
From: Jon Turney @ 2022-10-28 15:05 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

This adds an additional column to the stack trace in a .stackdump file,
which gives the stack frame return address as a module name+offset. This
makes it a possible to convert the address to a function name without
having to guess what module the address belongs to.

> Stack trace:
> Frame         Function     Args
> 0007FFFFCC30  0001004010E9 (000180048055, 000180046FA0, 000000000002, 00018031E160) segv-test.exe+0x10E9
> 0007FFFFCD30  0001800480C1 (000000000000, 000000000000, 000000000000, 000000000000) cygwin1.dll+0x80C1
> 0007FFFFFFF0  000180045C86 (000000000000, 000000000000, 000000000000, 000000000000) cygwin1.dll+0x5C86
> 0007FFFFFFF0  000180045D34 (000000000000, 000000000000, 000000000000, 000000000000) cygwin1.dll+0x5D34
> End of stack trace

Loosely based on this patch [1] by Brian Dessent.

[1] https://cygwin.com/pipermail/cygwin-patches/2008q1/006306.html
---
 winsup/cygwin/exceptions.cc | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index a15bc16c5..1e9ea26bf 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -324,6 +324,34 @@ stack_info::walk ()
   return 1;
 }
 
+/*
+  Walk the list of modules in the current process to find the one containing
+   'func_va'.
+
+   This implementation requires no allocation of memory and minimal system
+   calls, so it should be safe in the context of an exception handler.
+*/
+static char *
+prettyprint_va (PVOID func_va)
+{
+  static char buf[256];
+  buf[0] = '\0';
+
+  PLIST_ENTRY head = &NtCurrentTeb()->Peb->Ldr->InMemoryOrderModuleList;
+  for (PLIST_ENTRY x = head->Flink; x != head; x = x->Flink)
+    {
+      PLDR_DATA_TABLE_ENTRY mod = CONTAINING_RECORD (x, LDR_DATA_TABLE_ENTRY,
+						     InMemoryOrderLinks);
+      if (mod->DllBase > func_va)
+	continue;
+
+      __small_sprintf (buf, "%S+0x%x", &mod->BaseDllName,
+		       (DWORD_PTR)func_va - (DWORD_PTR)mod->DllBase);
+    }
+
+  return buf;
+}
+
 void
 cygwin_exception::dumpstack ()
 {
@@ -350,7 +378,7 @@ cygwin_exception::dumpstack ()
 	  for (unsigned j = 0; j < NPARAMS; j++)
 	    small_printf ("%s%012X", j == 0 ? " (" : ", ",
 			  thestack.sf.Params[j]);
-	  small_printf (")\r\n");
+	  small_printf (") %s\r\n", prettyprint_va((PVOID)thestack.sf.AddrPC.Offset));
 	}
       small_printf ("End of stack trace%s\r\n",
 		    i == DUMPSTACK_FRAME_LIMIT ?
-- 
2.38.1


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

* [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump
  2022-10-28 15:05 [PATCH 0/3] Stackdump improvements Jon Turney
  2022-10-28 15:05 ` [PATCH 1/3] Cygwin: Tidy up formatting of stackdump Jon Turney
  2022-10-28 15:05 ` [PATCH 2/3] Cygwin: Add addresses as module offsets in .stackdump file Jon Turney
@ 2022-10-28 15:05 ` Jon Turney
  2022-10-29  8:32   ` Corinna Vinschen
  2022-10-29  8:34 ` [PATCH 0/3] Stackdump improvements Corinna Vinschen
  3 siblings, 1 reply; 8+ messages in thread
From: Jon Turney @ 2022-10-28 15:05 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon Turney

This adds an extra section to the stackdump, which lists the loaded
modules and their base address.  This is perhaps useful as it makes it
immediately clear if RandomCrashInjectedDll.dll is loaded...

XXX: It seems like the 'InMemoryOrder' part of 'InMemoryOrderModuleList' is a lie?

> Loaded modules
> 000100400000 segv-test.exe
> 7FFF2AC30000 ntdll.dll
> 7FFF29050000 KERNEL32.DLL
> 7FFF28800000 KERNELBASE.dll
> 000180040000 cygwin1.dll
> 7FFF28FA0000 advapi32.dll
> 7FFF29F20000 msvcrt.dll
> 7FFF299E0000 sechost.dll
> 7FFF29B30000 RPCRT4.dll
> 7FFF27C10000 CRYPTBASE.DLL
> 7FFF28770000 bcryptPrimitives.dll
---
 winsup/cygwin/exceptions.cc | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 1e9ea26bf..7dde44140 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -383,6 +383,16 @@ cygwin_exception::dumpstack ()
       small_printf ("End of stack trace%s\r\n",
 		    i == DUMPSTACK_FRAME_LIMIT ?
 		    " (more stack frames may be present)" : "");
+
+      small_printf ("Loaded modules\r\n");
+      PLIST_ENTRY head = &NtCurrentTeb()->Peb->Ldr->InMemoryOrderModuleList;
+      for (PLIST_ENTRY x = head->Flink; x != head; x = x->Flink)
+	{
+	  PLDR_DATA_TABLE_ENTRY mod = CONTAINING_RECORD (x, LDR_DATA_TABLE_ENTRY,
+							 InMemoryOrderLinks);
+	  small_printf ("%012X %S\r\n", mod->DllBase, &mod->BaseDllName);
+	}
+
       if (h)
 	NtClose (h);
     }
-- 
2.38.1


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

* Re: [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump
  2022-10-28 15:05 ` [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump Jon Turney
@ 2022-10-29  8:32   ` Corinna Vinschen
  2022-11-03 17:02     ` Jon Turney
  0 siblings, 1 reply; 8+ messages in thread
From: Corinna Vinschen @ 2022-10-29  8:32 UTC (permalink / raw)
  To: Jon Turney; +Cc: cygwin-patches

On Oct 28 16:05, Jon Turney wrote:
> This adds an extra section to the stackdump, which lists the loaded
> modules and their base address.  This is perhaps useful as it makes it
> immediately clear if RandomCrashInjectedDll.dll is loaded...
> 
> XXX: It seems like the 'InMemoryOrder' part of 'InMemoryOrderModuleList' is a lie?

Probably just an alternative fact...

> 
> > Loaded modules
> > 000100400000 segv-test.exe
> > 7FFF2AC30000 ntdll.dll
> > 7FFF29050000 KERNEL32.DLL
> > 7FFF28800000 KERNELBASE.dll
> > 000180040000 cygwin1.dll
> > 7FFF28FA0000 advapi32.dll
> > 7FFF29F20000 msvcrt.dll
> > 7FFF299E0000 sechost.dll
> > 7FFF29B30000 RPCRT4.dll
> > 7FFF27C10000 CRYPTBASE.DLL
> > 7FFF28770000 bcryptPrimitives.dll
> ---
>  winsup/cygwin/exceptions.cc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
> index 1e9ea26bf..7dde44140 100644
> --- a/winsup/cygwin/exceptions.cc
> +++ b/winsup/cygwin/exceptions.cc
> @@ -383,6 +383,16 @@ cygwin_exception::dumpstack ()
>        small_printf ("End of stack trace%s\r\n",
>  		    i == DUMPSTACK_FRAME_LIMIT ?
>  		    " (more stack frames may be present)" : "");
> +
> +      small_printf ("Loaded modules\r\n");
> +      PLIST_ENTRY head = &NtCurrentTeb()->Peb->Ldr->InMemoryOrderModuleList;
> +      for (PLIST_ENTRY x = head->Flink; x != head; x = x->Flink)
> +	{
> +	  PLDR_DATA_TABLE_ENTRY mod = CONTAINING_RECORD (x, LDR_DATA_TABLE_ENTRY,
> +							 InMemoryOrderLinks);
> +	  small_printf ("%012X %S\r\n", mod->DllBase, &mod->BaseDllName);
> +	}
> +
>        if (h)
>  	NtClose (h);
>      }
> -- 
> 2.38.1

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

* Re: [PATCH 0/3] Stackdump improvements
  2022-10-28 15:05 [PATCH 0/3] Stackdump improvements Jon Turney
                   ` (2 preceding siblings ...)
  2022-10-28 15:05 ` [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump Jon Turney
@ 2022-10-29  8:34 ` Corinna Vinschen
  3 siblings, 0 replies; 8+ messages in thread
From: Corinna Vinschen @ 2022-10-29  8:34 UTC (permalink / raw)
  To: Jon Turney; +Cc: cygwin-patches

On Oct 28 16:05, Jon Turney wrote:
> Jon Turney (3):
>   Cygwin: Tidy up formatting of stackdump
>   Cygwin: Add addresses as module offsets in .stackdump file
>   Cygwin: Add loaded module base address list to stackdump
> 
>  winsup/cygwin/exceptions.cc | 46 +++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 4 deletions(-)

Looks really good to me and will be quite helpful as soon as
ASLR is used.  Please push.


Thanks,
Corinna

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

* Re: [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump
  2022-10-29  8:32   ` Corinna Vinschen
@ 2022-11-03 17:02     ` Jon Turney
  2022-11-04 10:34       ` Corinna Vinschen
  0 siblings, 1 reply; 8+ messages in thread
From: Jon Turney @ 2022-11-03 17:02 UTC (permalink / raw)
  To: Cygwin Patches

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

On 29/10/2022 09:32, Corinna Vinschen wrote:
> On Oct 28 16:05, Jon Turney wrote:
>> This adds an extra section to the stackdump, which lists the loaded
>> modules and their base address.  This is perhaps useful as it makes it
>> immediately clear if RandomCrashInjectedDll.dll is loaded...
>>
>> XXX: It seems like the 'InMemoryOrder' part of 'InMemoryOrderModuleList' is a lie?
> 
> Probably just an alternative fact...

Yeah.  I did stared a bit at the code wondering if the structure layouts 
were incorrect so we were somehow traversing one of the other module 
lists with a different ordering, but everything looks correct.

The attached might be a good idea, then, to ensure that module+offset is 
calculated correctly.

[-- Attachment #2: 0001-Cygwin-Handle-out-of-order-modules-for-module-offset.patch --]
[-- Type: text/plain, Size: 1150 bytes --]

From ea47826047e8bb175b1b0e0286d7d7b8cf15c7fe Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Tue, 1 Nov 2022 14:01:08 +0000
Subject: [PATCH] Cygwin: Handle out of order modules for module offsets in
 stackdump

Improve address to module+offset conversion, to work correctly in the
presence of out-of-order elements in InMemoryOrderModuleList.

Fixes: d59651d4
---
 winsup/cygwin/exceptions.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 8cc454c90..c3433ab94 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -342,11 +342,13 @@ prettyprint_va (PVOID func_va)
     {
       PLDR_DATA_TABLE_ENTRY mod = CONTAINING_RECORD (x, LDR_DATA_TABLE_ENTRY,
 						     InMemoryOrderLinks);
-      if (mod->DllBase > func_va)
+      if ((func_va < mod->DllBase) ||
+	  (func_va > (PVOID)((DWORD_PTR)mod->DllBase + mod->SizeOfImage)))
 	continue;
 
       __small_sprintf (buf, "%S+0x%x", &mod->BaseDllName,
 		       (DWORD_PTR)func_va - (DWORD_PTR)mod->DllBase);
+      break;
     }
 
   return buf;
-- 
2.38.1


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

* Re: [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump
  2022-11-03 17:02     ` Jon Turney
@ 2022-11-04 10:34       ` Corinna Vinschen
  0 siblings, 0 replies; 8+ messages in thread
From: Corinna Vinschen @ 2022-11-04 10:34 UTC (permalink / raw)
  To: cygwin-patches

On Nov  3 17:02, Jon Turney wrote:
> On 29/10/2022 09:32, Corinna Vinschen wrote:
> > On Oct 28 16:05, Jon Turney wrote:
> > > This adds an extra section to the stackdump, which lists the loaded
> > > modules and their base address.  This is perhaps useful as it makes it
> > > immediately clear if RandomCrashInjectedDll.dll is loaded...
> > > 
> > > XXX: It seems like the 'InMemoryOrder' part of 'InMemoryOrderModuleList' is a lie?
> > 
> > Probably just an alternative fact...
> 
> Yeah.  I did stared a bit at the code wondering if the structure layouts
> were incorrect so we were somehow traversing one of the other module lists
> with a different ordering, but everything looks correct.
> 
> The attached might be a good idea, then, to ensure that module+offset is
> calculated correctly.

Good idea, please push.


Corinna

> From ea47826047e8bb175b1b0e0286d7d7b8cf15c7fe Mon Sep 17 00:00:00 2001
> From: Jon Turney <jon.turney@dronecode.org.uk>
> Date: Tue, 1 Nov 2022 14:01:08 +0000
> Subject: [PATCH] Cygwin: Handle out of order modules for module offsets in
>  stackdump
> 
> Improve address to module+offset conversion, to work correctly in the
> presence of out-of-order elements in InMemoryOrderModuleList.
> 
> Fixes: d59651d4
> ---
>  winsup/cygwin/exceptions.cc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
> index 8cc454c90..c3433ab94 100644
> --- a/winsup/cygwin/exceptions.cc
> +++ b/winsup/cygwin/exceptions.cc
> @@ -342,11 +342,13 @@ prettyprint_va (PVOID func_va)
>      {
>        PLDR_DATA_TABLE_ENTRY mod = CONTAINING_RECORD (x, LDR_DATA_TABLE_ENTRY,
>  						     InMemoryOrderLinks);
> -      if (mod->DllBase > func_va)
> +      if ((func_va < mod->DllBase) ||
> +	  (func_va > (PVOID)((DWORD_PTR)mod->DllBase + mod->SizeOfImage)))
>  	continue;
>  
>        __small_sprintf (buf, "%S+0x%x", &mod->BaseDllName,
>  		       (DWORD_PTR)func_va - (DWORD_PTR)mod->DllBase);
> +      break;
>      }
>  
>    return buf;
> -- 
> 2.38.1
> 


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

end of thread, other threads:[~2022-11-04 11:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-28 15:05 [PATCH 0/3] Stackdump improvements Jon Turney
2022-10-28 15:05 ` [PATCH 1/3] Cygwin: Tidy up formatting of stackdump Jon Turney
2022-10-28 15:05 ` [PATCH 2/3] Cygwin: Add addresses as module offsets in .stackdump file Jon Turney
2022-10-28 15:05 ` [PATCH 3/3] Cygwin: Add loaded module base address list to stackdump Jon Turney
2022-10-29  8:32   ` Corinna Vinschen
2022-11-03 17:02     ` Jon Turney
2022-11-04 10:34       ` Corinna Vinschen
2022-10-29  8:34 ` [PATCH 0/3] Stackdump improvements Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).