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] objdump relocation fixes for ARM disassembly

From: Nick Clifton <nickc at redhat dot com>
To: Stig Petter Olsrød <stigpo at users dot sourceforge dot net>
Cc: binutils at sources dot redhat dot com
Date: Tue, 01 Mar 2005 15:28:21 +0000
Subject: Re: [PATCH] objdump relocation fixes for ARM disassembly
References: <421B86D8.8080604@users.sourceforge.net>

Hi Stig,

2005-02-22 Stig Petter Olsroed <stigpo@users.sourceforge.net>

    * objdump.c (disassemble_bytes): Fixed relocation check for
    DISASSEMBLER_NEEDS_RELOCS platforms to properly trigger the
    INSN_HAS_RELOC flag.  Set the current relocation entry in
    objdump_disasm_info to allow printing the proper symbol.
    (objdump_print_addr): Use the relocation entry in
    objdump_disasm_info to lookup the correct symbol for
    DISASSEMBLER_NEEDS_RELOCS platforms.

Well this is a good idea - but it does have a couple of problems:

1. The new 'relocs' field of the aux structure is not initialised in disassemble_data().

2. The revised test inside DISASSEMBLER_NEEDS_INFO will not work with relocs whose address is part way through an instruction. eg on VLIW architectures.

3. The patch does not adjust the GAS testsuite files to take account of the new behaviour of the disassembler.

So I have amended your patch (as attached) and checked it in with the following ChangeLog entries.

Cheers
  Nick

binutils/ChangeLog
2005-03-01  Stig Petter Olsroed  <stigpo@users.sourceforge.net>
	    Nick Clifton  <nickc@redhat.com>

	* objdump.c: Fix coding for DISASSEMBLER_NEEDS_RELOC:
	(struct objdump_disasm_info): Add 'reloc' field.
	(disassemble_bytes): Fix check for when an insn has a reloc
	associated with it.  Improve comment explaining why the use of
	octets is wrong.  Set the 'reloc' field in objdump_disasm_info
	structure.
	(objdump_print_addr): Use new 'reloc' field to lookup the correct
	address for the symbol associated with the current instruction's
	relocation.
	(disassemble_info): Initialise 'reloc' field.


gas/testsuite/ChangeLog
2005-03-01  Stig Petter Olsroed  <stigpo@users.sourceforge.net>
	    Nick Clifton  <nickc@redhat.com>

	* gas/arm/inst.d: Allow for ARM ports which decode the reloc
	associated with branches and so show the exact symbolic destination
	address rather than an offset from the start of the section.
	* gas/arm/pic.d: Likewise.

Index: binutils/objdump.c
===================================================================
RCS file: /cvs/src/src/binutils/objdump.c,v
retrieving revision 1.100
diff -c -3 -p -r1.100 objdump.c
*** binutils/objdump.c	23 Feb 2005 12:25:57 -0000	1.100
--- binutils/objdump.c	1 Mar 2005 14:52:56 -0000
*************** struct objdump_disasm_info
*** 128,133 ****
--- 128,136 ----
    arelent **         dynrelbuf;
    long               dynrelcount;
    disassembler_ftype disassemble_fn;
+ #ifdef DISASSEMBLER_NEEDS_RELOCS
+   arelent *          reloc;
+ #endif
  };
  
  /* Architecture to disassemble for, or default if NULL.  */
*************** objdump_print_addr (bfd_vma vma,
*** 852,857 ****
--- 855,863 ----
  {
    struct objdump_disasm_info *aux;
    asymbol *sym;
+ #ifdef DISASSEMBLER_NEEDS_RELOCS
+   bfd_boolean skip_find = FALSE;
+ #endif
  
    if (sorted_symcount < 1)
      {
*************** objdump_print_addr (bfd_vma vma,
*** 861,867 ****
      }
  
    aux = (struct objdump_disasm_info *) info->application_data;
!   sym = find_symbol_for_address (vma, info, NULL);
    objdump_print_addr_with_sym (aux->abfd, aux->sec, sym, vma, info,
  			       skip_zeroes);
  }
--- 867,891 ----
      }
  
    aux = (struct objdump_disasm_info *) info->application_data;
! 
! #ifdef DISASSEMBLER_NEEDS_RELOCS
!   if (aux->reloc != NULL
!       && aux->reloc->sym_ptr_ptr != NULL
!       && * aux->reloc->sym_ptr_ptr != NULL)
!     {
!       sym = * aux->reloc->sym_ptr_ptr;
! 
!       /* Adjust the vma to the reloc.  */
!       vma += bfd_asymbol_value (sym);
! 
!       if (bfd_is_und_section (bfd_get_section (sym)))
! 	skip_find = TRUE;
!     }
! 
!   if (!skip_find)
! #endif
!     sym = find_symbol_for_address (vma, info, NULL);
! 
    objdump_print_addr_with_sym (aux->abfd, aux->sec, sym, vma, info,
  			       skip_zeroes);
  }
*************** disassemble_bytes (struct disassemble_in
*** 1238,1243 ****
--- 1262,1268 ----
    unsigned int opb = info->octets_per_byte;
    unsigned int skip_zeroes = info->skip_zeroes;
    unsigned int skip_zeroes_at_end = info->skip_zeroes_at_end;
+   int octets = opb;
    SFILE sfile;
  
    aux = (struct objdump_disasm_info *) info->application_data;
*************** disassemble_bytes (struct disassemble_in
*** 1282,1289 ****
    while (addr_offset < stop_offset)
      {
        bfd_vma z;
-       int octets = 0;
        bfd_boolean need_nl = FALSE;
  
        /* If we see more than SKIP_ZEROES octets of zeroes, we just
  	 print `...'.  */
--- 1307,1320 ----
    while (addr_offset < stop_offset)
      {
        bfd_vma z;
        bfd_boolean need_nl = FALSE;
+ #ifdef DISASSEMBLER_NEEDS_RELOCS
+       int previous_octets;
+ 
+       /* Remember the length of the previous instruction.  */
+       previous_octets = octets;
+ #endif
+       octets = 0;
  
        /* If we see more than SKIP_ZEROES octets of zeroes, we just
  	 print `...'.  */
*************** disassemble_bytes (struct disassemble_in
*** 1348,1366 ****
  	      info->stream = (FILE *) &sfile;
  	      info->bytes_per_line = 0;
  	      info->bytes_per_chunk = 0;
  
  #ifdef DISASSEMBLER_NEEDS_RELOCS
! 	      /* FIXME: This is wrong.  It tests the number of octets
! 		 in the last instruction, not the current one.  */
! 	      if (*relppp < relppend
! 		  && (**relppp)->address >= rel_offset + addr_offset
! 		  && ((**relppp)->address
! 		      < rel_offset + addr_offset + octets / opb))
! 		info->flags = INSN_HAS_RELOC;
! 	      else
! #endif
! 		info->flags = 0;
  
  	      octets = (*disassemble_fn) (section->vma + addr_offset, info);
  	      info->fprintf_func = (fprintf_ftype) fprintf;
  	      info->stream = stdout;
--- 1379,1418 ----
  	      info->stream = (FILE *) &sfile;
  	      info->bytes_per_line = 0;
  	      info->bytes_per_chunk = 0;
+ 	      info->flags = 0;
  
  #ifdef DISASSEMBLER_NEEDS_RELOCS
! 	      if (*relppp < relppend)
! 		{
! 		  bfd_signed_vma distance_to_rel;
! 
! 		  distance_to_rel = (**relppp)->address
! 		    - (rel_offset + addr_offset);
  
+ 		  /* Check to see if the current reloc is associated with
+ 		     the instruction that we are about to disassemble.  */
+ 		  if (distance_to_rel == 0
+ 		      /* FIXME: This is wrong.  We are trying to catch
+ 			 relocs that are addressed part way through the
+ 			 current instruction, as might happen with a packed
+ 			 VLIW instruction.  Unfortunately we do not know the
+ 			 length of the current instruction since we have not
+ 			 disassembled it yet.  Instead we take a guess based
+ 			 upon the length of the previous instruction.  The
+ 			 proper solution is to have a new target-specific
+ 			 disassembler function which just returns the length
+ 			 of an instruction at a given address without trying
+ 			 to display its disassembly. */
+ 		      || (distance_to_rel > 0
+ 			  && distance_to_rel < (bfd_signed_vma) (previous_octets/ opb)))
+ 		    {
+ 		      info->flags = INSN_HAS_RELOC;
+ 		      aux->reloc = **relppp;
+ 		    }
+ 		  else
+ 		    aux->reloc = NULL;
+ 		}
+ #endif
  	      octets = (*disassemble_fn) (section->vma + addr_offset, info);
  	      info->fprintf_func = (fprintf_ftype) fprintf;
  	      info->stream = stdout;
*************** disassemble_data (bfd *abfd)
*** 1817,1822 ****
--- 1869,1877 ----
    aux.require_sec = FALSE;
    aux.dynrelbuf = NULL;
    aux.dynrelcount = 0;
+ #ifdef DISASSEMBLER_NEEDS_RELOCS
+   aux.reloc = NULL;
+ #endif
  
    disasm_info.print_address_func = objdump_print_address;
    disasm_info.symbol_at_address_func = objdump_symbol_at_address;
Index: gas/testsuite/gas/arm/inst.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/inst.d,v
retrieving revision 1.12
diff -c -3 -p -r1.12 inst.d
*** gas/testsuite/gas/arm/inst.d	14 Aug 2003 17:11:08 -0000	1.12
--- gas/testsuite/gas/arm/inst.d	1 Mar 2005 14:52:56 -0000
*************** Disassembly of section .text:
*** 159,171 ****
  0+254 <[^>]*> e9c40300 ?	stmib	r4, {r8, r9}\^
  0+258 <[^>]*> ef123456 ?	swi	0x00123456
  0+25c <[^>]*> 2f000033 ?	swics	0x00000033
! 0+260 <[^>]*> ebfffffe ?	bl	0+260 <[^>]*>
  [		]*260:.*_wombat.*
! 0+264 <[^>]*> 5bfffffe ?	blpl	0+264 <[^>]*>
  [		]*264:.*ARM.*hohum
! 0+268 <[^>]*> eafffffe ?	b	0+268 <[^>]*>
  [		]*268:.*_wibble.*
! 0+26c <[^>]*> dafffffe ?	ble	0+26c <[^>]*>
  [		]*26c:.*testerfunc.*
  0+270 <[^>]*> e1a01102 ?	mov	r1, r2, lsl #2
  0+274 <[^>]*> e1a01002 ?	mov	r1, r2
--- 159,171 ----
  0+254 <[^>]*> e9c40300 ?	stmib	r4, {r8, r9}\^
  0+258 <[^>]*> ef123456 ?	swi	0x00123456
  0+25c <[^>]*> 2f000033 ?	swics	0x00000033
! 0+260 <[^>]*> ebfffffe ?	bl	0[0123456789abcdef]+ <[^>]*>
  [		]*260:.*_wombat.*
! 0+264 <[^>]*> 5bfffffe ?	blpl	0[0123456789abcdef]+ <[^>]*>
  [		]*264:.*ARM.*hohum
! 0+268 <[^>]*> eafffffe ?	b	0[0123456789abcdef]+ <[^>]*>
  [		]*268:.*_wibble.*
! 0+26c <[^>]*> dafffffe ?	ble	0[0123456789abcdef]+ <[^>]*>
  [		]*26c:.*testerfunc.*
  0+270 <[^>]*> e1a01102 ?	mov	r1, r2, lsl #2
  0+274 <[^>]*> e1a01002 ?	mov	r1, r2
Index: gas/testsuite/gas/arm/pic.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/pic.d,v
retrieving revision 1.8
diff -c -3 -p -r1.8 pic.d
*** gas/testsuite/gas/arm/pic.d	17 Sep 2004 12:35:00 -0000	1.8
--- gas/testsuite/gas/arm/pic.d	1 Mar 2005 14:52:56 -0000
***************
*** 8,14 ****
  Disassembly of section .text:
  00+0 <[^>]*> ebfffffe 	bl	00+0 <[^>]*>
  			0: R_ARM_PC24	foo
! 00+4 <[^>]*> ebfffffe 	bl	00+4 <[^>]*>
  			4: R_ARM_PLT32	foo
  	\.\.\.
  			8: R_ARM_ABS32	sym
--- 8,14 ----
  Disassembly of section .text:
  00+0 <[^>]*> ebfffffe 	bl	00+0 <[^>]*>
  			0: R_ARM_PC24	foo
! 00+4 <[^>]*> ebfffffe 	bl	0[0123456789abcdef]+ <[^>]*>
  			4: R_ARM_PLT32	foo
  	\.\.\.
  			8: R_ARM_ABS32	sym

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