[incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
[Retry to send this message without the attachment that probably blocked the earlier attempt.] I've attached a patch that'll get objdump to report exception tables correctly for ARM and SH4 which, according to MSDN, have a different format for storing the pdata segment. Here's part of the output, used on a test executable : dannypc: {642} tail ~/tmp/arm/test/exception/syntax.od 603c The Function Table (interpreted .pdata section contents) vma: BeginProlog Function FlagsException EH Address Length Length 32b exc Handler Data 00014000 00011008 0002 000f 1 1 000110f8 (_eh_handler) 00014008 000110a4 0002 0015 1 1 00011058 (handler) private flags = 820: [APCS-32] [floats passed in integer registers] [absolute position] [interworking not supported] dannypc: {643} I have three uncertainties with this patch : 1. I've copied code from elsewhere in binutils. Just want to point to the fact that I did this; I don't think this one is worth exporting as a public API. 2. Cleanup in my_symbol_for_address. There's none now. Should there be? 3. Which macros should be used for the conditional compilation ? I've used #if defined(ARM_WINCE) || defined(SH4) in my code. This happens to work for me because our build chain defines ARM_WINCE, but I don't suppose this is right. Attached are : - a sample ChangeLog entry - the patch in its current state - a gzipped sample file on which this can be used to produce the output as above. I've renamed it slightly, to avoid virus scanners. Please advise on how to proceed. Danny -- Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info 2007-08-05 Danny Backx <[EMAIL PROTECTED]> * bfd/peXXigen.c (pe_print_pdata) : Add code to support ARM and SH4 compressed .pdata segments as described on MSDN. * bfd/peXXigen.c (slurp_symtab, my_symbol_for_address) : New private functions, copied from/based on other binutils source files. Index: bfd/peXXigen.c === --- bfd/peXXigen.c (revision 1020) +++ bfd/peXXigen.c (working copy) @@ -1561,16 +1561,63 @@ return TRUE; } +static int symcount=0; +static asymbol ** +slurp_symtab (bfd *abfd) +{ + asymbol **sy = NULL; + long storage; + + if (!(bfd_get_file_flags (abfd) & HAS_SYMS)) +{ + symcount = 0; + return NULL; +} + + storage = bfd_get_symtab_upper_bound (abfd); + if (storage < 0) +return NULL; + if (storage) +sy = bfd_malloc (storage); + + symcount = bfd_canonicalize_symtab (abfd, sy); + if (symcount < 0) +return NULL; + return sy; +} + +static const char * +my_symbol_for_address(bfd *abfd, bfd_vma func) +{ + static asymbol **syms = 0; + int i; + + if (syms == 0) + syms = slurp_symtab (abfd); + for (i=0; isection->vma + syms[i]->value == func) + return syms[i]->name; + } + return NULL; +} + /* This really is architecture dependent. On IA-64, a .pdata entry consists of three dwords containing relative virtual addresses that specify the start and end address of the code range the entry - covers and the address of the corresponding unwind info data. */ + covers and the address of the corresponding unwind info data. + On ARM and SH-4, a compressed PDATA structure is used : + _IMAGE_CE_RUNTIME_FUNCTION_ENTRY, whereas MIPS is documented to use + _IMAGE_ALPHA_RUNTIME_FUNCTION_ENTRY. + See http://msdn2.microsoft.com/en-us/library/ms253988(VS.80).aspx . + */ static bfd_boolean pe_print_pdata (bfd * abfd, void * vfile) { #if defined(COFF_WITH_pep) && !defined(COFF_WITH_pex64) # define PDATA_ROW_SIZE (3 * 8) +#elif defined(ARM_WINCE) || defined(SH4) +# define PDATA_ROW_SIZE (2 * 4) #else # define PDATA_ROW_SIZE (5 * 4) #endif @@ -1598,6 +1645,10 @@ #if defined(COFF_WITH_pep) && !defined(COFF_WITH_pex64) fprintf (file, _(" vma:\t\t\tBegin AddressEnd Address Unwind Info\n")); +#elif defined(ARM_WINCE) || defined(SH4) + fprintf (file, _("\ + vma:\t\tBeginProlog Function FlagsException EH\n\ + \t\tAddress Length Length 32b exc Handler Data\n")); #else fprintf (file, _("\ vma:\t\tBeginEnd EH EH PrologEnd Exception\n\ @@ -1620,16 +1671,75 @@ for (i = start; i < stop; i += onaline) { bfd_vma begin_addr; +#if defined(ARM_WINCE) || defined(SH4) + bfd_vma other_data; + bfd_vma prolog_length, function_length; + int flag32bit, exception_flag; + bfd_byte *tdata = 0; + asection *tsection; +#else bfd_vma end_addr; bfd_vma eh_handler; bfd_vma eh_data; bfd_vma prolog_end_addr; int em_data; +#endif if (i + PDATA_ROW_SIZE > stop) break; +#if
Re: [incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
This has been on my todo-list for far too long, apologies. I've tried to tackle the conditional compilation part, and would like your renewed input on it, I may have missed some points. Naming for instance. Diffs in the attachment are relative to the cegcc svn, I've checked and they patch cleanly in a binutils 2.18 tree. So, please tell me what I'm missing and I'll change my patch. Danny On Mon, 2007-08-06 at 16:22 +0100, Nick Clifton wrote: > Hi Danny, > > > > 2. Cleanup in my_symbol_for_address. There's none now. Should there be? > > Yes. :-) > > Also it looks like my_symbol_for_address is going to quite slow for large > symbol tables. You might want to consider ensuring that the symbol time is > sorted by address and then performing some kind of binary search on it. (Or > do > you know that my_symbol_for_address will always be called with incrementally > increasing addresses, in which case you could just cache the last returned > value and start your search there). > > > 3. Which macros should be used for the conditional compilation ? I've > > used > > #if defined(ARM_WINCE) || defined(SH4) > > in my code. This happens to work for me because our build chain defines > > ARM_WINCE, but I don't suppose this is right. > > The correct way to handle this is to add a new function pointer field to the > bfd_coff_backend_data structure. Arrange for this field to be initialized by > the various backends that need it (IA64, ARM, MIPS, SH) pointing at static > functions defined in the appropriate target specific source file (pe-arm.c > etc), and otherwise for it to be empty(*). Then in > _bfd_XX_print_private_bfd_data_common test the field and call it if it is not > empty. > > Cheers >Nick > > (*) Or maybe to point at a default pdata display function which does not try > to > provide any target specific interpretation. Index: pe-arm-wince.c === --- pe-arm-wince.c (revision 1151) +++ pe-arm-wince.c (working copy) @@ -35,4 +35,179 @@ #define LOCAL_LABEL_PREFIX "." +#include "sysdep.h" +#include "bfd.h" + +#undef bfd_pe_print_pdata +#define bfd_pe_print_pdata pe_print_compressed_pdata +extern bfd_boolean pe_print_compressed_pdata (bfd * abfd, void * vfile); + #include "pe-arm.c" + +static int symcount=0; +static asymbol ** +slurp_symtab (bfd *abfd) +{ + asymbol **sy = NULL; + long storage; + + if (!(bfd_get_file_flags (abfd) & HAS_SYMS)) +{ + symcount = 0; + return NULL; +} + + storage = bfd_get_symtab_upper_bound (abfd); + if (storage < 0) +return NULL; + if (storage) +sy = bfd_malloc (storage); + + symcount = bfd_canonicalize_symtab (abfd, sy); + if (symcount < 0) +return NULL; + return sy; +} + +static const char * +my_symbol_for_address(bfd *abfd, bfd_vma func) +{ + static asymbol **syms = 0; + int i; + + if (syms == 0) + syms = slurp_symtab (abfd); + for (i=0; isection->vma + syms[i]->value == func) + return syms[i]->name; + } + return NULL; +} + +/* Copied from peXXigen.c , then modified for compressed pdata. + + This really is architecture dependent. On IA-64, a .pdata entry + consists of three dwords containing relative virtual addresses that + specify the start and end address of the code range the entry + covers and the address of the corresponding unwind info data. + + On ARM and SH-4, a compressed PDATA structure is used : + _IMAGE_CE_RUNTIME_FUNCTION_ENTRY, whereas MIPS is documented to use + _IMAGE_ALPHA_RUNTIME_FUNCTION_ENTRY. + See http://msdn2.microsoft.com/en-us/library/ms253988(VS.80).aspx . + */ + +/* This is the version for "compressed" pdata. */ +bfd_boolean +pe_print_compressed_pdata (bfd * abfd, void * vfile) +{ +# define PDATA_ROW_SIZE (2 * 4) + FILE *file = (FILE *) vfile; + bfd_byte *data = 0; + asection *section = bfd_get_section_by_name (abfd, ".pdata"); + bfd_size_type datasize = 0; + bfd_size_type i; + bfd_size_type start, stop; + int onaline = PDATA_ROW_SIZE; + + if (section == NULL + || coff_section_data (abfd, section) == NULL + || pei_section_data (abfd, section) == NULL) +return TRUE; + + stop = pei_section_data (abfd, section)->virt_size; + if ((stop % onaline) != 0) +fprintf (file, + _("Warning, .pdata section size (%ld) is not a multiple of %d\n"), + (long) stop, onaline); + + fprintf (file, + _("\nThe Function Table (interpreted .pdata section contents)\n")); + + fprintf (file, _("\ + vma:\t\tBeginProlog Function FlagsException EH\n\ + \t\tAddress Length Length 32b exc Handler Data\n")); + + datasize = section->size; + if (datasize == 0) +return TRUE; + + if (! bfd_malloc_and_get_section (abfd, section, &data)) +{ + if (data != NULL) + free (data); + return FALSE; +} + + start = 0; + + for (i = start; i < stop; i += onaline) +{ + bfd_vma begin_addr; + bfd_vma other_data; + bfd_vma prolog_length, f
Re: [incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
On Wed, 2008-04-02 at 16:47 +0100, Nick Clifton wrote: > > I've tried to tackle the conditional compilation part, and would like > > your renewed input on it, I may have missed some points. Naming for > > instance. > > > Naming was mostly OK. There were two problems. > pe_print_compressed_pdata() should really be called > pe_print_ce_compressed_pdata() since it assumes > _IMAGE_CE_RUNTIME_FUNCTION_ENTRY formatted data right ? One day someone > might want to write a MIPS version of this function, so we will need a Looks like you got interrupted while writing this sentence. Not sure what you mean. > Secondly you did not provide a default definition of bfd_pe_print_pdata, > so the PE ports which do not use this new feature will not build. Try > configuring a binutils build with "--enable-targets=all" to see this > happening. Didn't know about that. I've used it, and added one or more lines to 30 other .c files. In most cases this is just #define bfd_pe_print_pdata NULL That addresses the build problems with "--enable-targets=all". This is what you wanted me to do, right ? > Here are two other things however which I think you should also fix: I'll address the other points in your message too. One more question though. I moved my pe_print_ce_compressed_pdata function into pe-arm-wince.c , it was in peXXigen.c in my initial patch. The peXXigen.c doesn't seem right for this because this function is not to be treated with the XX replacement stuff. However, pe-arm-wince.c doesn't seem right either because the pe_print_ce_compressed_pdata function is to be used both for ARM and for SH. So where should I put it (and its helper functions) ? Thanks for your help ! Danny -- Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info ___ bug-binutils mailing list bug-binutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-binutils
Re: [incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
On Fri, 2008-04-04 at 08:10 +0100, Nick Clifton wrote: > > Didn't know about that. I've used it, and added one or more lines to 30 > > other .c files. In most cases this is just > > > > #define bfd_pe_print_pdata NULL > > Shouldn't that be: > >#define bfd_pe_print_pdata pe_print_data > > ie use the default print_pdata routine if compressed pdata is not > supported by the target. One of the earlier suggestions was for the default print_pdata to be used when the new field is NULL. This is how I implemented it. > > One more question though. I moved my pe_print_ce_compressed_pdata > > function into pe-arm-wince.c , it was in peXXigen.c in my initial patch. > > The peXXigen.c doesn't seem right for this because this function is not > > to be treated with the XX replacement stuff. However, pe-arm-wince.c > > doesn't seem right either because the pe_print_ce_compressed_pdata > > function is to be used both for ARM and for SH. > > > > So where should I put it (and its helper functions) ? > > I would actually suggest leaving it in peXXigen.c. The current > pe_print_pdata function lives there and it does not make use of the XX > replacement stuff. Make sure that it is static though, since peXXigen.c > can be included multiple times in a single build. I don't think I understand. If it's a static function in peXXigen.c then I can't use it in e.g. pe-arm-wince.c to initialise the structure. Also right now there are two functions. The original one, and one to handle the ARM and SH4 compressed pdata cases. Putting them together in one file is fine by me, but putting them in a source file that can be included more than once seems wrong. This would, I think, apply to both the function I'm adding and to the default one. Should they not be in another source file (one that doesn't get the XX treatment) so they get included just once ? So I still think I need a source file that doesn't get the XX treatment to put both functions in as non-static functions. But I am probably overlooking something, I'm new to the binutils source. Danny signature.asc Description: This is a digitally signed message part ___ bug-binutils mailing list bug-binutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-binutils
Re: [incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
I was waiting for a more informative reply but that didn't happen, and I lost track of this whole thing until someone reminded me recently. Which source file should I put my stuff into ? An existing one, a new one ? I'd like to get this thing over with ... Danny On Mon, 2008-04-14 at 15:39 +0100, Nick Clifton wrote: > Hi Danny, > >(Sorry about the delay in replying, I am a bit snowed under at the > moment). > > > One of the earlier suggestions was for the default print_pdata to be > > used when the new field is NULL. This is how I implemented it. > > Doh, yes I should have realized that. > > > > I don't think I understand. If it's a static function in peXXigen.c then > > I can't use it in e.g. pe-arm-wince.c to initialise the structure. > > > Also right now there are two functions. The original one, and one to > > handle the ARM and SH4 compressed pdata cases. Putting them together in > > one file is fine by me, but putting them in a source file that can be > > included more than once seems wrong. This would, I think, apply to both > > the function I'm adding and to the default one. Should they not be in > > another source file (one that doesn't get the XX treatment) so they get > > included just once ? > > Yes of course. (Sorry my brain was not firing on all cylinders that day). > > > So I still think I need a source file that doesn't get the XX treatment > > to put both functions in as non-static functions. But I am probably > > overlooking something, I'm new to the binutils source. > > No, I think that you are right. > > Cheers >Nick > > -- Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info ___ bug-binutils mailing list bug-binutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-binutils
Re: [incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
On Mon, 2008-07-14 at 10:26 +0100, Nick Clifton wrote: > Hi Danny, > > > I was waiting for a more informative reply but that didn't happen, > > Sorry - I had meant to say that I now agreed with your version of the > patch and that I was not expecting any more changes. > > > > Which source file should I put my stuff into ? An existing one, a new > > one ? > > An existing one. > > Cheers >Nick Ok, patch attached. Suggested ChangeLog entry in attachment "cl". Danny -- Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info Index: bfd/pe-arm-wince.c === --- bfd/pe-arm-wince.c (revision 1130) +++ bfd/pe-arm-wince.c (working copy) @@ -35,4 +35,178 @@ #define LOCAL_LABEL_PREFIX "." +#include "sysdep.h" +#include "bfd.h" + +#undef bfd_pe_print_pdata +#define bfd_pe_print_pdata pe_print_ce_compressed_pdata +extern bfd_boolean pe_print_ce_compressed_pdata (bfd * abfd, void * vfile); + #include "pe-arm.c" + +typedef struct sym_cache { + int symcount; + asymbol **syms; +} sym_cache; + +static asymbol ** +slurp_symtab (bfd *abfd, sym_cache *psc) +{ + asymbol **sy = NULL; + long storage; + + if (!(bfd_get_file_flags (abfd) & HAS_SYMS)) +{ + psc->symcount = 0; + return NULL; +} + + storage = bfd_get_symtab_upper_bound (abfd); + if (storage < 0) +return NULL; + if (storage) +sy = bfd_malloc (storage); + + psc->symcount = bfd_canonicalize_symtab (abfd, sy); + if (psc->symcount < 0) +return NULL; + return sy; +} + +static const char * +my_symbol_for_address(bfd *abfd, bfd_vma func, sym_cache *psc) +{ + int i; + + if (psc->syms == 0) + psc->syms = slurp_symtab (abfd, psc); + for (i=0; isymcount; i++) { + if (psc->syms[i]->section->vma + psc->syms[i]->value == func) + return psc->syms[i]->name; + } + return NULL; +} + +static void cleanup_syms(sym_cache *psc) +{ + psc->symcount = 0; + free(psc->syms); + psc->syms = (asymbol **)0; +} + +/* This is the version for "compressed" pdata. */ +bfd_boolean +pe_print_ce_compressed_pdata (bfd * abfd, void * vfile) +{ +# define PDATA_ROW_SIZE (2 * 4) + FILE *file = (FILE *) vfile; + bfd_byte *data = 0; + asection *section = bfd_get_section_by_name (abfd, ".pdata"); + bfd_size_type datasize = 0; + bfd_size_type i; + bfd_size_type start, stop; + int onaline = PDATA_ROW_SIZE; + struct sym_cache sym_cache = {0, 0} ; + + if (section == NULL + || coff_section_data (abfd, section) == NULL + || pei_section_data (abfd, section) == NULL) +return TRUE; + + stop = pei_section_data (abfd, section)->virt_size; + if ((stop % onaline) != 0) +fprintf (file, + _("Warning, .pdata section size (%ld) is not a multiple of %d\n"), + (long) stop, onaline); + + fprintf (file, + _("\nThe Function Table (interpreted .pdata section contents)\n")); + + fprintf (file, _("\ + vma:\t\tBeginProlog Function FlagsException EH\n\ + \t\tAddress Length Length 32b exc Handler Data\n")); + + datasize = section->size; + if (datasize == 0) +return TRUE; + + if (! bfd_malloc_and_get_section (abfd, section, &data)) +{ + if (data != NULL) + free (data); + return FALSE; +} + + start = 0; + + for (i = start; i < stop; i += onaline) +{ + bfd_vma begin_addr; + bfd_vma other_data; + bfd_vma prolog_length, function_length; + int flag32bit, exception_flag; + bfd_byte *tdata = 0; + asection *tsection; + + if (i + PDATA_ROW_SIZE > stop) + break; + + begin_addr = GET_PDATA_ENTRY (abfd, data + i ); + other_data = GET_PDATA_ENTRY (abfd, data + i + 4); + + if (begin_addr == 0 && other_data == 0) + /* We are probably into the padding of the section now. */ + break; + + prolog_length = (other_data & 0x00FF); + function_length = (other_data & 0x3F00) >> 8; + flag32bit = (int)((other_data & 0x4000) >> 30); + exception_flag = (int)((other_data & 0x8000) >> 31); + + fputc (' ', file); + fprintf_vma (file, i + section->vma); fputc ('\t', file); + fprintf_vma (file, begin_addr); fputc (' ', file); + fprintf_vma (file, prolog_length); fputc (' ', file); + fprintf_vma (file, function_length); fputc (' ', file); + fprintf (file, "%2d %2d ", flag32bit, exception_flag); + + /* Get the exception handler's address and the data passed from the + * .text section. This is really the data that belongs with the .pdata + * but got "compressed" out for the ARM and SH4 architectures. */ + tsection = bfd_get_section_b