This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH-ping] VAX: Forced decoding of function entry masks (eg.for disassembling ROM images)


Hi Jan-Benedict,

I cannot do that on my own. AFAICT, I've only done all the FSF
paperwork, but that doesn't come along with write privilege (and
I'm not the VAX port maintainer).

OK - well I was going through the process of applying your patch when I discovered several more, err not bugs, but features, which I thought needed to be addressed. Specifically:


* The new feature of the disassembler should be mentioned in the binutils NEWS file.

* Parsing the disassembler options string every time print_insn_vax() is called seems to be hugely wasteful. Surely it is better to parse it just once.

* When free_private_data() is called it does not reset the entry_array and num_entry_fields to zero, so that the next time init_private_data is called it will try to realloc freed memory.

* What is the purpose of the private structure anyway ? It seems simpler to just a couple of static variables here.

Rather than just complain however, I have produced an alternative version of your patch which addresses most of these issues (attached). What do you think ? (Also, if you do like it, please could you test it as I have now way of doing so myself).

Cheers
  Nick


opcodes/
2005-03-21  Jan-Benedict Glaw  <jbglaw@lug-owl.de>

	* vax-dis.c: (entry_addr): New varible:  An array of user supplied
	function entry mask addresses.
	(num_entry_addr): New variable: The number of elements in entry_addr.
	(parse_disassembler_options): New function.  Fills in the entry_addr
	array.
	(is_function_entry): Check if a given address is a function's
	start address by looking at supplied entry mask addresses and
	symbol information, if available.
	(print_insn_vax): Use parse_disassembler_options and is_function_entry.

binutils
2005-03-21  Jan-Benedict Glaw  <jbglaw@lug-owl.de>

	* doc/binutils.texi: Document new VAX disassembler-specific option
	-M entry:0xfooba8.
	* NEWS: Mention the new option.

Index: opcodes/vax-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/vax-dis.c,v
retrieving revision 1.9
diff -c -3 -p -r1.9 vax-dis.c
*** opcodes/vax-dis.c	14 Mar 2005 09:35:26 -0000	1.9
--- opcodes/vax-dis.c	24 Mar 2005 14:45:57 -0000
***************
*** 17,22 ****
--- 17,24 ----
     along with this program; if not, write to the Free Software
     Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
  
+ #include <setjmp.h>
+ #include <string.h>
  #include "sysdep.h"
  #include "opcode/vax.h"
  #include "dis-asm.h"
*************** static char *entry_mask_bit[] =
*** 77,91 ****
  /* Maximum length of an instruction.  */
  #define MAXLEN 25
  
- #include <setjmp.h>
- 
  struct private
  {
    /* Points to first byte not fetched.  */
!   bfd_byte *max_fetched;
!   bfd_byte the_buffer[MAXLEN];
!   bfd_vma insn_start;
!   jmp_buf bailout;
  };
  
  /* Make sure that bytes from INFO->PRIVATE_DATA->BUFFER (inclusive)
--- 79,91 ----
  /* Maximum length of an instruction.  */
  #define MAXLEN 25
  
  struct private
  {
    /* Points to first byte not fetched.  */
!   bfd_byte * max_fetched;
!   bfd_byte   the_buffer[MAXLEN];
!   bfd_vma    insn_start;
!   jmp_buf    bailout;
  };
  
  /* Make sure that bytes from INFO->PRIVATE_DATA->BUFFER (inclusive)
*************** fetch_data (info, addr)
*** 119,124 ****
--- 119,202 ----
    return 1;
  }
  
+ /* Entry mask handling.  */
+ static unsigned int  num_entry_addr = 0;
+ static bfd_vma *     entry_addr = NULL;
+ 
+ /* Parse the VAX specific disassembler options.  These contain functions
+    entry addresses, which can be useful to disassemble ROM images, since
+    there's no symbol table.  Returns TRUE upon success, FALSE otherwise.  */
+ 
+ static bfd_boolean
+ parse_disassembler_options (char * options)
+ {
+   const char * entry_switch = "entry:";
+ 
+   while ((options = strstr (options, entry_switch)))
+     {
+       options += strlen (entry_switch);
+ 
+       /* FIXME: This is not a very efficient way of extending the table.  */
+       entry_addr = realloc (entry_addr, sizeof (bfd_vma)
+ 			    * (num_entry_addr + 1));
+       if (entry_addr == NULL)
+ 	return FALSE;
+       entry_addr[num_entry_addr ++] = bfd_scan_vma (options, NULL, 0);
+     }
+ 
+   return TRUE;
+ }
+ 
+ #if 0 /* FIXME:  Ideally the disassembler should have target specific
+ 	 initialisation and termination function pointers.  Then
+ 	 parse_disassembler_options could be the init function and
+ 	 free_entry_array (below) could be the termination routine.
+ 	 Until then there is no way for the disassembler to tell us
+ 	 that it has finished and that we no longer need the entry
+ 	 array, so this routine is suprpess for now.  It does mean
+ 	 that we leak memory, but only to the extent that we do not
+ 	 free it just before the disassembler is about to terminate
+ 	 anyway.  */
+ 
+ /* Free memory allocated to our entry array.  */
+ 
+ static void
+ free_entry_array (struct private *priv)
+ {
+   if (entry_addr)
+     {
+       free (entry_addr);
+       entry_addr = NULL;
+       num_entry_addr = 0;
+     }
+ }
+ #endif
+ /* Check if the given address is a known function entry. Either there must
+    be a symbol of function type at this address, or the address must be
+    a forced entry point.  The later helps in disassembling ROM images, because
+    there's no symbol table at all.  Forced entry points can be given by
+    supplying several -M options to objdump: -M entry:0xffbb7730.  */
+ 
+ static bfd_boolean
+ is_function_entry (struct disassemble_info *info, bfd_vma addr)
+ {
+   unsigned int i;
+ 
+   /* Check if there's a BSF_FUNCTION symbol at our address.  */
+   if (info->symbols
+       && info->symbols[0]
+       && (info->symbols[0]->flags & BSF_FUNCTION)
+       && addr == bfd_asymbol_value (info->symbols[0]))
+     return TRUE;
+ 
+   /* Check for forced function entry address.  */
+   for (i = 0; i < num_entry_addr; i++)
+     if (entry_addr[i] == addr)
+       return TRUE;
+ 
+   return FALSE;
+ }
+ 
  /* Print the vax instruction at address MEMADDR in debugged memory,
     on INFO->STREAM.  Returns length of the instruction, in bytes.  */
  
*************** print_insn_vax (memaddr, info)
*** 137,142 ****
--- 215,228 ----
    priv.max_fetched = priv.the_buffer;
    priv.insn_start = memaddr;
  
+   if (info->disassembler_options)
+     {
+       parse_disassembler_options (info->disassembler_options);
+ 
+       /* To avoid repeated parsing of these options, we remove them here.  */
+       info->disassembler_options = NULL;
+     }
+ 
    if (setjmp (priv.bailout) != 0)
      {
        /* Error return.  */
*************** print_insn_vax (memaddr, info)
*** 157,166 ****
      }
  
    /* Decode function entry mask.  */
!   if (info->symbols
!       && info->symbols[0]
!       && (info->symbols[0]->flags & BSF_FUNCTION)
!       && memaddr == bfd_asymbol_value (info->symbols[0]))
      {
        int i = 0;
        int register_mask = buffer[1] << 8 | buffer[0];
--- 243,249 ----
      }
  
    /* Decode function entry mask.  */
!   if (is_function_entry (info, memaddr))
      {
        int i = 0;
        int register_mask = buffer[1] << 8 | buffer[0];
Index: binutils/doc/binutils.texi
===================================================================
RCS file: /cvs/src/src/binutils/doc/binutils.texi,v
retrieving revision 1.69
diff -c -3 -p -r1.69 binutils.texi
*** binutils/doc/binutils.texi	15 Mar 2005 17:45:19 -0000	1.69
--- binutils/doc/binutils.texi	24 Mar 2005 14:45:58 -0000
*************** rather than names, for the selected type
*** 1793,1798 ****
--- 1793,1805 ----
  You can list the available values of @var{ABI} and @var{ARCH} using
  the @option{--help} option.
  
+ For VAX, you can specify function entry addresses with @option{-M
+ entry:0xf00ba}.  You can use this multiple times to properly
+ disassemble VAX binary files that don't contain symbol tables (like
+ ROM dumps).  In these cases, the function entry mask would otherwise
+ be decoded as VAX instructions, which would probably lead the the rest
+ of the function being wrongly disassembled.
+ 
  @item -p
  @itemx --private-headers
  Print information that is specific to the object file format.  The exact
Index: binutils/NEWS
===================================================================
RCS file: /cvs/src/src/binutils/NEWS,v
retrieving revision 1.49
diff -c -3 -p -r1.49 NEWS
*** binutils/NEWS	15 Mar 2005 17:45:18 -0000	1.49
--- binutils/NEWS	24 Mar 2005 14:45:58 -0000
***************
*** 1,5 ****
--- 1,8 ----
  -*- text -*-
  
+ * Add "-M entry:<addr>" switch to objdump to specify a function entry address
+   when disassembling VAX binaries.
+ 
  * Add "--globalize-symbol <name>" and "--globalize-symbols <filename>" switches
    to objcopy to convert local symbols into global symbols.
  

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]