bug-binutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [incomplete patch] Get objdump to report exception tables for ARM/SH


From: Nick Clifton
Subject: Re: [incomplete patch] Get objdump to report exception tables for ARM/SH4 correctly
Date: Wed, 02 Apr 2008 16:47:58 +0100
User-agent: Thunderbird 1.5.0.12 (X11/20080213)

Hi Danny,

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

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.

Here are two other things however which I think you should also fix:

+static int symcount=0;
+static asymbol **
+slurp_symtab (bfd *abfd)

Communicating this information via a static is a bit naughty. It would be cleaner to return a structure containing the symbol table pointer and the symbol count. This matters even more here:

+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);

This means that a symbol table will only be read once, even if we are displaying the pe pdata for more than one file. (Something that objdump can do). You need to cache the bfd pointer for the symbol table stored in 'syms' and check that on entry to my_symbol_for_address. Or a cleaner method would be to have some way to release the loaded symbol table at the end of pe_print_compressed_pdata so that a following call to pe_print_compressed_pdata will load a new symbol table.


Index: libcoff.h
===================================================================
--- libcoff.h   (revision 1151)
+++ libcoff.h   (working copy)
@@ -802,6 +802,8 @@
   bfd_boolean (*_bfd_coff_final_link_postscript)
     (bfd *, struct coff_final_link_info *);
+ bfd_boolean (*_bfd_coff_print_pdata)
+    (bfd *, void *);
 } bfd_coff_backend_data;
#define coff_backend_info(abfd) \
@@ -934,3 +936,7 @@
 #define bfd_coff_final_link_postscript(a,p) \
   ((coff_backend_info (a)->_bfd_coff_final_link_postscript) (a, p))
+#define bfd_coff_have_print_pdata(a) \
+  (coff_backend_info (a)->_bfd_coff_print_pdata)
+#define bfd_coff_print_pdata(a,p) \
+  ((coff_backend_info (a)->_bfd_coff_print_pdata) (a, p))

This is wrong. The libcoff.h file is a generated file. You should patch the source file that is used to generate these parts of libcoff.h (in this case coffcode.h) and then either provide a patch for libcoff.h as well, or else add a small note in the patch submission saying that the constructed files inside the bfd/ source directory need to be regenerated.

Apart from that and some minor formatting issues, the patch looks fine.

Cheers
  Nick





reply via email to

[Prev in Thread] Current Thread [Next in Thread]