[PATCH] better stackdumps
This patch adds the ability to see functions/symbols in the .stackdump
files generated when there's a fault. It parses the export sections of
each loaded module and finds the closest exported address for each stack
frame address. This of course won't be perfect as it will show the
wrong function if the frame is in the middle of a non-exported function,
but it's better than what we have now.
This also uses a couple of tricks to make the output more sensible. It
can "see through" the sigfe wrappers and print the actual functions
being wrapped. It also has a set of internal symbols that it consults
for symbols in Cygwin. This allows it to get the bottom frame correct
(_dll_crt0_1) even though that function isn't exported. If there are
any other such functions they can be easily added to the 'hints' array.
Also attached is a sample output of an invalid C program and the
resulting stackdump. Note that the frame labeled _sigbe really should
be a frame somewhere inside the main .exe. I pondered trying to extract
the sigbe's return address off the signal stack and using that for the
label but I haven't quite gotten there, since I can't think of a
reliable way to figure out the correct location on the tls stack where
the real return address is stored.
Of course the labeling works for any module/dll, not just cygwin1.dll,
but I didn't have a more elaborate testcase to demonstrate.
Brian2008-03-18 Brian Dessent <[EMAIL PROTECTED]>
* exceptions.cc (maybe_adjust_va_for_sigfe): New function to cope
with signal wrappers.
(prettyprint_va): New function that attempts to find a symbolic
name for a memory location by walking the export sections of all
modules.
(stackdump): Call it.
* gendef: Mark __sigfe as a global so that its address can be
used by the backtrace code.
* ntdll.h (struct _PEB_LDR_DATA): Declare.
(struct _LDR_MODULE): Declare.
(struct _PEB): Use actual LDR_DATA type for LdrData.
(RtlImageDirectoryEntryToData): Declare.
Index: exceptions.cc
===
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.319
diff -u -p -r1.319 exceptions.cc
--- exceptions.cc 12 Mar 2008 12:41:49 - 1.319
+++ exceptions.cc 19 Mar 2008 00:04:13 -
@@ -284,6 +284,158 @@ stack_info::walk ()
return 1;
}
+/* These symbols are used by the below functions to put a prettier face
+ on a stack backtrace. */
+extern u_char etext asm ("etext"); /* End of .text */
+extern u_char _sigfe, _sigbe;
+void dll_crt0_1 (void *);
+
+const struct {
+ DWORD va;
+ const char *label;
+} hints[] = {
+ { (DWORD) &_sigbe, "_sigbe" },
+ { (DWORD) &dll_crt0_1, "_dll_crt0_1" }
+};
+
+/* Helper function to assist with backtraces. This tries to detect if
+ an entrypoint is really a sigfe wrapper and returns the actual address
+ of the function. Here's an example:
+
+ 610ab9f0 <__sigfe_printf>:
+ 610ab9f0: 68 40 a4 10 61 push $0x6110a440
+ 610ab9f5: e9 bf eb ff ff jmp610aa5b9 <__sigfe>
+
+ Suppose that we are passed 0x610ab9f0. We need to recognise the
+ push/jmp combination and return 0x6110a440 <_printf> instead. Note
+ that this is a relative jump. */
+static DWORD
+maybe_adjust_va_for_sigfe (DWORD va)
+{
+ if (va < (DWORD) user_data->hmodule || va > (DWORD) &etext)
+return va;
+
+ unsigned char *x = (unsigned char *) va;
+
+ if (x[0] == 0x68 && x[5] == 0xe9)
+{
+ DWORD jmprel = *(DWORD *)(x + 6);
+
+ if ((unsigned) va + 10 + (unsigned) jmprel == (unsigned) &_sigfe)
+return *(DWORD *)(x + 1);
+}
+ return va;
+}
+
+/* Walk the list of modules in the current process and parse their
+ export tables in order to find the entrypoint closest to but less
+ than 'faultva'. This won't be perfect, such as when 'faultva'
+ actually resides in a non-exported function, but it is still better
+ than nothing. Note that this algorithm could be made much more
+ efficient by both sorting the export tables as well as saving the
+ result between calls. However, this implementation requires no
+ allocation of memory and minimal system calls, so it should be safe
+ in the context of an exception handler. And we're probably about to
+ terminate the process anyway, so performance is not critical. */
+static char *
+prettyprint_va (DWORD faultva)
+{
+ static char buf[256];
+
+ ULONG bestmatch_va = 0;
+
+ PLIST_ENTRY head = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList;
+ for (PLIST_ENTRY x = head->Flink; x != head; x = x->Flink)
+{
+ PLDR_MODULE mod = CONTAINING_RECORD (x, LDR_MODULE,
+ InMemoryOrderModuleList);
+ if ((DWORD) mod->BaseAddress > faultva)
+continue;
+
+ DWORD len;
+ IMAGE_EXPORT_DIRECTORY *edata_va = (IMAGE_EXPORT_DIRECTORY *)
+ R
Re: [PATCH] better stackdumps
Brian Dessent wrote: > Of course the labeling works for any module/dll, not just cygwin1.dll, > but I didn't have a more elaborate testcase to demonstrate. Forgot to mention... The symbols are just tacked on on the right hand side there for now. I wasn't really sure how to handle that. I didn't want to remove display of the actual EIP for each frame as that could be removing useful info, but I wasn't quite sure where to put everything or how to align it... so as it is now it wraps wider than 80 chars which is probably pretty ugly on a default size terminal. Brian
Re: [PATCH] better stackdumps
On Tue, 18 Mar 2008, Brian Dessent wrote: > Brian Dessent wrote: > > > Of course the labeling works for any module/dll, not just cygwin1.dll, > > but I didn't have a more elaborate testcase to demonstrate. > > Forgot to mention... > > The symbols are just tacked on on the right hand side there for now. I > wasn't really sure how to handle that. I didn't want to remove display > of the actual EIP for each frame as that could be removing useful info, > but I wasn't quite sure where to put everything or how to align it... so > as it is now it wraps wider than 80 chars which is probably pretty ugly > on a default size terminal. Would it make sense to force a newline before the function name and to display it with a small indent? That way people who want the old-style stackdump could just feed the new one into "grep -v '^ '" or something... Igor -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_[EMAIL PROTECTED] | [EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_Igor Peshansky, Ph.D. (name changed!) |,4- ) )-,_. ,\ ( `'-' old name: Igor Pechtchanski '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! "That which is hateful to you, do not do to your neighbor. That is the whole Torah; the rest is commentary. Go and study it." -- Rabbi Hillel
Re: [PATCH] better stackdumps
Igor Peshansky wrote: > Would it make sense to force a newline before the function name and to > display it with a small indent? That way people who want the old-style > stackdump could just feed the new one into "grep -v '^ '" or something... Yes, that would be one way. That actually reminds me of another issue that I forgot to mention: glibc has a backtrace API that can be called from user-code at any time, not just at faults. At the moment we are exporting something similar called cygwin_stackdump but we don't declare it in any header. Would it be worthwhile to try to match the glibc API and export it under the same name/output format? Brian
Re: [PATCH] better stackdumps
On Tue, Mar 18, 2008 at 05:24:20PM -0700, Brian Dessent wrote: > >This patch adds the ability to see functions/symbols in the .stackdump >files generated when there's a fault. It parses the export sections of >each loaded module and finds the closest exported address for each stack >frame address. This of course won't be perfect as it will show the >wrong function if the frame is in the middle of a non-exported function, >but it's better than what we have now. > >This also uses a couple of tricks to make the output more sensible. It >can "see through" the sigfe wrappers and print the actual functions >being wrapped. It also has a set of internal symbols that it consults >for symbols in Cygwin. This allows it to get the bottom frame correct >(_dll_crt0_1) even though that function isn't exported. If there are >any other such functions they can be easily added to the 'hints' array. > >Also attached is a sample output of an invalid C program and the >resulting stackdump. Note that the frame labeled _sigbe really should >be a frame somewhere inside the main .exe. I pondered trying to extract >the sigbe's return address off the signal stack and using that for the >label but I haven't quite gotten there, since I can't think of a >reliable way to figure out the correct location on the tls stack where >the real return address is stored. > >Of course the labeling works for any module/dll, not just cygwin1.dll, >but I didn't have a more elaborate testcase to demonstrate. > >Brian >2008-03-18 Brian Dessent <[EMAIL PROTECTED]> > > * exceptions.cc (maybe_adjust_va_for_sigfe): New function to cope > with signal wrappers. > (prettyprint_va): New function that attempts to find a symbolic > name for a memory location by walking the export sections of all > modules. > (stackdump): Call it. > * gendef: Mark __sigfe as a global so that its address can be > used by the backtrace code. > * ntdll.h (struct _PEB_LDR_DATA): Declare. > (struct _LDR_MODULE): Declare. > (struct _PEB): Use actual LDR_DATA type for LdrData. > (RtlImageDirectoryEntryToData): Declare. Sorry, but I don't like this concept. This bloats the cygwin DLL for a condition that would be better served by either using gdb or generating a real coredump. OTOH, adding a list of loaded dlls to a stackdump might not be a bad idea so that some postprocessing program could generate the same output as long as that didn't add too much code to cygwin. cgf
