[PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Julian Brown]

[-- Attachment #1: Type: text/plain, Size: 2926 bytes --]

Hi,

This patch fixes two problems: the type of mapping symbols is made 
STT_NOTYPE (from STT_FUNC or STT_OBJECT from ARM/Thumb or data 
respectively), and the alignment is corrected to not have the low bit 
set for Thumb mapping symbols, in accordance with the latest version of 
the ARM AAELF spec.

Unfortunately altering the symbol type to STT_NOTYPE for mapping symbols 
affects the output of disassembly from objdump. $a and $t were 
previously treated as functions. When objdump tried to find the closest 
symbol for possibly-symbol-relative offsets previously, it found those 
mapping symbols since compare_symbols places functions first in a sorted 
version of the symbol table. They were then rejected by the 
symbol_is_valid hook (arm_symbol_is_valid) from the disassemble_info 
struct, and the section name was printed instead.

When the type of mapping symbols is changed to STT_NOTYPE, they are no 
longer placed first in the sorted symbol list, so another symbol is used 
instead. This causes test regressions, e.g. a symbol-offset address 
which was previously output as:

   bl      8224 <.text-0xc>

was output instead as:

   bl      8224 <_start-0xc>

In the interest of preserving existing behaviour, I have fixed this by 
adding a new disassemble_info hook which allows the symbol table used 
for disassembly to have a target-specific hook for altering each symbol 
(or remove it) in the remove_useless_symbols function. This sets the 
BSF_FUNCTION flag for mapping symbols, which makes things behave the way 
they used to, and possibly provides useful functionality for other 
platforms as well.

It may be better to just change the expected test output instead. Thoughts?

Tested on arm-none-eabi, arm-none-symbianelf, and i686-pc-linux-gnu with 
all targets.

OK to apply?

ChangeLog:

   * bfd/elf32-arm.c (arm_elf_find_function): include STT_NOTYPE in test
   for mapping symbols.
   * binutils/objdump.c (remove_useless_symbols): Add disassemble_info
   parameter, and call new hook to filter symbols in a target-specific
   way.
   (disassemble_data): Initialise sorted symbols after disassemble_info
   struct, and call remove_useless_symbols before sorting.
   * gas/config/tc-arm.c (mapping_state): Update documentation in
   function comment. Change type of all mapping symbols to BSF_NO_FLAGS.
   (is_mapping_symbol_name): New function.
   (arm_adjust_symtab): Don't adjust type of mapping symbols here.
   * gas/testsuite/gas/arm/mapping.d: Change expected output for untyped
   symbols.
   * include/dis-asm.h (disassemble_info): Add filter_symbol hook.
   (arm_filter_symbol): Add prototype.
   * opcodes/arm-dis.c (arm_filter_symbol): New function to add
   BSF_FUNCTION for code mapping symbols.
   * opcodes/dis-init.c (init_disassemble_info): Initialise filter_symbol
   hook (NULL).
   * opcodes/disassemble.c (disassemble_init_for_target): Initialise
   filter_symbol hook for ARM.


[-- Attachment #2: patch-3 --]
[-- Type: text/plain, Size: 18522 bytes --]

? bfd/doc/bfd.info
? bfd/doc/bfd.info-1
? gas/doc/as.info
? gas/doc/as.info-1
? gprof/gprof.info
? gprof/gprof.info-1
Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.26
diff -c -p -r1.26 elf32-arm.c
*** bfd/elf32-arm.c	22 Mar 2005 15:39:32 -0000	1.26
--- bfd/elf32-arm.c	27 Mar 2005 19:47:21 -0000
*************** arm_elf_find_function (bfd *         abf
*** 4261,4272 ****
  	  break;
  	case STT_FUNC:
  	case STT_ARM_TFUNC:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
  	      && is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
- 	case STT_NOTYPE:
  	  if (bfd_get_section (&q->symbol) == section
  	      && q->symbol.value >= low_func
  	      && q->symbol.value <= offset)
--- 4261,4272 ----
  	  break;
  	case STT_FUNC:
  	case STT_ARM_TFUNC:
+ 	case STT_NOTYPE:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
  	      && is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
  	  if (bfd_get_section (&q->symbol) == section
  	      && q->symbol.value >= low_func
  	      && q->symbol.value <= offset)
Index: binutils/objdump.c
===================================================================
RCS file: /cvs/src/src/binutils/objdump.c,v
retrieving revision 1.102
diff -c -p -r1.102 objdump.c
*** binutils/objdump.c	18 Mar 2005 16:28:13 -0000	1.102
--- binutils/objdump.c	27 Mar 2005 19:47:22 -0000
*************** slurp_dynamic_symtab (bfd *abfd)
*** 454,460 ****
     Return the number of useful symbols.  */
  
  static long
! remove_useless_symbols (asymbol **symbols, long count)
  {
    asymbol **in_ptr = symbols, **out_ptr = symbols;
  
--- 454,460 ----
     Return the number of useful symbols.  */
  
  static long
! remove_useless_symbols (asymbol **symbols, long count, disassemble_info *info)
  {
    asymbol **in_ptr = symbols, **out_ptr = symbols;
  
*************** remove_useless_symbols (asymbol **symbol
*** 469,474 ****
--- 469,477 ----
        if (bfd_is_und_section (sym->section)
  	  || bfd_is_com_section (sym->section))
  	continue;
+       if (info->filter_symbol
+ 	  && ! info->filter_symbol (sym))
+ 	continue;
  
        *out_ptr++ = sym;
      }
*************** disassemble_data (bfd *abfd)
*** 1844,1867 ****
    prev_functionname = NULL;
    prev_line = -1;
  
-   /* We make a copy of syms to sort.  We don't want to sort syms
-      because that will screw up the relocs.  */
-   sorted_symcount = symcount ? symcount : dynsymcount;
-   sorted_syms = xmalloc ((sorted_symcount + synthcount) * sizeof (asymbol *));
-   memcpy (sorted_syms, symcount ? syms : dynsyms,
- 	  sorted_symcount * sizeof (asymbol *));
- 
-   sorted_symcount = remove_useless_symbols (sorted_syms, sorted_symcount);
- 
-   for (i = 0; i < synthcount; ++i)
-     {
-       sorted_syms[sorted_symcount] = synthsyms + i;
-       ++sorted_symcount;
-     }
- 
-   /* Sort the symbols into section and symbol order.  */
-   qsort (sorted_syms, sorted_symcount, sizeof (asymbol *), compare_symbols);
- 
    init_disassemble_info (&disasm_info, stdout, (fprintf_ftype) fprintf);
  
    disasm_info.application_data = (void *) &aux;
--- 1847,1852 ----
*************** disassemble_data (bfd *abfd)
*** 1926,1931 ****
--- 1911,1937 ----
    /* Allow the target to customize the info structure.  */
    disassemble_init_for_target (& disasm_info);
  
+   /* We make a copy of syms to sort.  We don't want to sort syms
+      because that will screw up the relocs.  */
+   sorted_symcount = symcount ? symcount : dynsymcount;
+   sorted_syms = xmalloc ((sorted_symcount + synthcount) * sizeof (asymbol *));
+   memcpy (sorted_syms, symcount ? syms : dynsyms,
+ 	  sorted_symcount * sizeof (asymbol *));
+ 
+   /* Remove useless symbols before sorting (remove_useless_symbols may also
+      mutate some symbols to affect sort order).  */
+   sorted_symcount = remove_useless_symbols (sorted_syms, sorted_symcount,
+                                             &disasm_info);
+ 
+   for (i = 0; i < synthcount; ++i)
+     {
+       sorted_syms[sorted_symcount] = synthsyms + i;
+       ++sorted_symcount;
+     }
+ 
+   /* Sort the symbols into section and symbol order.  */
+   qsort (sorted_syms, sorted_symcount, sizeof (asymbol *), compare_symbols);
+ 
    /* Pre-load the dynamic relocs if we are going
       to be dumping them along with the disassembly.  */
    if (dump_dynamic_reloc_info)
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.197
diff -c -p -r1.197 tc-arm.c
*** gas/config/tc-arm.c	23 Mar 2005 15:49:02 -0000	1.197
--- gas/config/tc-arm.c	27 Mar 2005 19:47:22 -0000
*************** validate_offset_imm (unsigned int val, i
*** 1238,1311 ****
  \f
  #ifdef OBJ_ELF
  /* This code is to handle mapping symbols as defined in the ARM ELF spec.
!    (This text is taken from version B-02 of the spec):
  
!       4.4.7 Mapping and tagging symbols
  
!       A section of an ARM ELF file can contain a mixture of ARM code,
!       Thumb code, and data.  There are inline transitions between code
!       and data at literal pool boundaries. There can also be inline
!       transitions between ARM code and Thumb code, for example in
!       ARM-Thumb inter-working veneers.  Linkers, machine-level
!       debuggers, profiling tools, and disassembly tools need to map
!       images accurately. For example, setting an ARM breakpoint on a
!       Thumb location, or in a literal pool, can crash the program
!       being debugged, ruining the debugging session.
! 
!       ARM ELF entities are mapped (see section 4.4.7.1 below) and
!       tagged (see section 4.4.7.2 below) using local symbols (with
!       binding STB_LOCAL).  To assist consumers, mapping and tagging
!       symbols should be collated first in the symbol table, before
!       other symbols with binding STB_LOCAL.
! 
!       To allow properly collated mapping and tagging symbols to be
!       skipped by consumers that have no interest in them, the first
!       such symbol should have the name $m and its st_value field equal
!       to the total number of mapping and tagging symbols (including
!       the $m) in the symbol table.
! 
!       4.4.7.1 Mapping symbols
! 
!       $a    Labels the first byte of a sequence of ARM instructions.
!             Its type is STT_FUNC.
! 
!       $d    Labels the first byte of a sequence of data items.
!             Its type is STT_OBJECT.
! 
!       $t    Labels the first byte of a sequence of Thumb instructions.
!             Its type is STT_FUNC.
! 
!       This list of mapping symbols may be extended in the future.
! 
!       Section-relative mapping symbols
! 
!       Mapping symbols defined in a section define a sequence of
!       half-open address intervals that cover the address range of the
!       section. Each interval starts at the address defined by a
!       mapping symbol, and continues up to, but not including, the
!       address defined by the next (in address order) mapping symbol or
!       the end of the section. A corollary is that there must be a
!       mapping symbol defined at the beginning of each section.
!       Consumers can ignore the size of a section-relative mapping
!       symbol. Producers can set it to 0.
! 
!       Absolute mapping symbols
! 
!       Because of the need to crystallize a Thumb address with the
!       Thumb-bit set, absolute symbol of type STT_FUNC (symbols of type
!       STT_FUNC defined in section SHN_ABS) need to be mapped with $a
!       or $t.
! 
!       The extent of a mapping symbol defined in SHN_ABS is [st_value,
!       st_value + st_size), or [st_value, st_value + 1) if st_size = 0,
!       where [x, y) denotes the half-open address range from x,
!       inclusive, to y, exclusive.
! 
!       In the absence of a mapping symbol, a consumer can interpret a
!       function symbol with an odd value as the Thumb code address
!       obtained by clearing the least significant bit of the
!       value. This interpretation is deprecated, and it may not work in
!       the future.
  
     Note - the Tagging symbols ($b, $f, $p $m) have been dropped from
     the EABI (which is still under development), so they are not
--- 1238,1303 ----
  \f
  #ifdef OBJ_ELF
  /* This code is to handle mapping symbols as defined in the ARM ELF spec.
!    (This text is taken from version 0.62 RC 2 of the spec):
  
!       4.5.7 Mapping symbols
  
!       A section of an ELF file can contain a mixture of ARM code, Thumb code
!       and data.
! 
!       There are inline transitions between code and data at literal pool
!       boundaries.  There can also be inline transitions between ARM code and
!       Thumb code, for example in ARM-Thumb inter-working veneers.
! 
!       Linkers, and potentially other tools, need to map images correctly (for
!       example, to support byte swapping to produce a BE-8 image from a BE-32
!       object file).  To support this, a number of symbols, termed mapping
!       symbols appear in the symbol table to denote the start of a sequence of
!       bytes of the appropriate type.  All mapping symbols have type STT_NOTYPE
!       and binding STB_LOCAL.  The st_size field is unused and must be zero.
! 
!       The mapping symbols are defined in Table 4-6, Mapping symbols.  It is an
!       error for a relocation to reference a mapping symbol.  Two forms of
!       mapping symbol are supported:
! 
!         * a short form, that uses a dollar character and a single letter
! 	  denoting the class.  This form can be used when an object producer
! 	  creates mapping symbols automatically, and minimizes symbol table
! 	  space
! 
!         * a longer form, where the short form is extended with a period and
! 	  then any sequence of characters that are legal for a symbol.  This
! 	  form can be used when assembler files have to be annotated manually
! 	  and the assembler does not support multiple definitions of symbols. 
! 
!       Table 4-6, Mapping symbols
! 
! 	Name         Meaning
! 
! 	$a           Start of a sequence of ARM instructions
! 	$a.<any...>
! 
! 	$d           Start of a sequence of data items (for example, a literal
! 	$d.<any...>  pool)
! 
! 	$t           Start of a sequence of Thumb instructions
! 	$t.<any...>
! 
!       4.5.7.1 Section-relative mapping symbols
!  
!       Mapping symbols defined in a section define a sequence of half-open
!       address intervals that cover the address range of the section.  Each
!       interval starts at the address defined by the mapping symbol, and
!       continues up to, but not including, the address defined by the next (in
!       address order) mapping symbol or the end of the section.  A section must
!       have a mapping symbol defined at the beginning of the section; however,
!       if the section contains only data then the mapping symbol may be omitted.
! 
!       4.5.7.2 Absolute mapping symbols
! 
!       Mapping symbols are no-longer required for the absolute section.  The
!       equivalent information is now conveyed by the type of the absolute
!       symbol.
  
     Note - the Tagging symbols ($b, $f, $p $m) have been dropped from
     the EABI (which is still under development), so they are not
*************** mapping_state (enum mstate state)
*** 1331,1345 ****
      {
      case MAP_DATA:
        symname = "$d";
!       type = BSF_OBJECT;
        break;
      case MAP_ARM:
        symname = "$a";
!       type = BSF_FUNCTION;
        break;
      case MAP_THUMB:
        symname = "$t";
!       type = BSF_FUNCTION;
        break;
      case MAP_UNDEFINED:
        return;
--- 1323,1337 ----
      {
      case MAP_DATA:
        symname = "$d";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_ARM:
        symname = "$a";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_THUMB:
        symname = "$t";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_UNDEFINED:
        return;
*************** arm_frob_label (symbolS * sym)
*** 13645,13650 ****
--- 13637,13652 ----
      }
  }
  
+ /* Recognize short-form or long-form mapping symbol names.  */
+ 
+ static bfd_boolean
+ is_arm_mapping_symbol_name (const char* name)
+ {
+   return name && name[0] == '$'
+ 	 && (name[1] == 'a' || name[1] == 'd' || name[1] == 't')
+ 	 && (name[2] == '\0' || name[2] == '.');
+ }
+ 
  /* Adjust the symbol table.  This marks Thumb symbols as distinct from
     ARM ones.  */
  
*************** arm_adjust_symtab (void)
*** 13705,13718 ****
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  /* If it's a .thumb_func, declare it as so,
! 	     otherwise tag label as .code 16.  */
! 	  if (THUMB_IS_FUNC (sym))
! 	    elf_sym->internal_elf_sym.st_info =
! 	      ELF_ST_INFO (bind, STT_ARM_TFUNC);
! 	  else
! 	    elf_sym->internal_elf_sym.st_info =
! 	      ELF_ST_INFO (bind, STT_ARM_16BIT);
  	}
      }
  #endif
--- 13707,13723 ----
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  if (!is_arm_mapping_symbol_name (elf_sym->symbol.name))
! 	    {
! 	      /* If it's a .thumb_func, declare it as so,
! 		 otherwise tag label as .code 16.  */
! 	      if (THUMB_IS_FUNC (sym))
! 		elf_sym->internal_elf_sym.st_info =
! 		  ELF_ST_INFO (bind, STT_ARM_TFUNC);
! 	      else
! 		elf_sym->internal_elf_sym.st_info =
! 		  ELF_ST_INFO (bind, STT_ARM_16BIT);
! 	    }
  	}
      }
  #endif
Index: gas/testsuite/gas/arm/mapping.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/mapping.d,v
retrieving revision 1.4
diff -c -p -r1.4 mapping.d
*** gas/testsuite/gas/arm/mapping.d	11 Dec 2004 04:32:36 -0000	1.4
--- gas/testsuite/gas/arm/mapping.d	27 Mar 2005 19:47:24 -0000
*************** SYMBOL TABLE:
*** 9,18 ****
  0+00 l    d  .text	0+0 (|.text)
  0+00 l    d  .data	0+0 (|.data)
  0+00 l    d  .bss	0+0 (|.bss)
! 0+00 l     F .text	0+0 \$a
! 0+08 l     F .text	0+0 \$t
! 0+00 l     O .data	0+0 \$d
  0+00 l    d  foo	0+0 (|foo)
! 0+00 l     F foo	0+0 \$t
  0+00 g       .text	0+0 mapping
  0+08 g     F .text	0+0 thumb_mapping
--- 9,18 ----
  0+00 l    d  .text	0+0 (|.text)
  0+00 l    d  .data	0+0 (|.data)
  0+00 l    d  .bss	0+0 (|.bss)
! 0+00 l       .text	0+0 \$a
! 0+08 l       .text	0+0 \$t
! 0+00 l       .data	0+0 \$d
  0+00 l    d  foo	0+0 (|foo)
! 0+00 l       foo	0+0 \$t
  0+00 g       .text	0+0 mapping
  0+08 g     F .text	0+0 thumb_mapping
Index: include/dis-asm.h
===================================================================
RCS file: /cvs/src/src/include/dis-asm.h,v
retrieving revision 1.50
diff -c -p -r1.50 dis-asm.h
*** include/dis-asm.h	3 Mar 2005 11:58:01 -0000	1.50
--- include/dis-asm.h	27 Mar 2005 19:47:27 -0000
*************** typedef struct disassemble_info {
*** 136,142 ****
       displaying debugging outout.  */
    bfd_boolean (* symbol_is_valid)
      (asymbol *, struct disassemble_info * info);
!     
    /* These are for buffer_read_memory.  */
    bfd_byte *buffer;
    bfd_vma buffer_vma;
--- 136,148 ----
       displaying debugging outout.  */
    bfd_boolean (* symbol_is_valid)
      (asymbol *, struct disassemble_info * info);
!  
!   /* Filter symbols used in disassembly, in a target-dependent fashion.
!      Return TRUE if symbol should be retained, or FALSE to remove it. May also
!      modify symbols in-place.  */
!   bfd_boolean (* filter_symbol)
!     (asymbol *);
! 
    /* These are for buffer_read_memory.  */
    bfd_byte *buffer;
    bfd_vma buffer_vma;
*************** extern int get_arm_regname_num_options (
*** 275,280 ****
--- 281,287 ----
  extern int set_arm_regname_option (int);
  extern int get_arm_regnames (int, const char **, const char **, const char ***);
  extern bfd_boolean arm_symbol_is_valid (asymbol *, struct disassemble_info *);
+ extern bfd_boolean arm_filter_symbol (asymbol *);
  
  /* Fetch the disassembler for a given BFD, if that support is available.  */
  extern disassembler_ftype disassembler (bfd *);
Index: opcodes/arm-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/arm-dis.c,v
retrieving revision 1.44
diff -c -p -r1.44 arm-dis.c
*** opcodes/arm-dis.c	12 Mar 2005 18:25:47 -0000	1.44
--- opcodes/arm-dis.c	27 Mar 2005 19:47:30 -0000
*************** arm_symbol_is_valid (asymbol * sym,
*** 1937,1942 ****
--- 1937,1964 ----
    return (name && *name != '$');
  }
  
+ /* Filter symbols used for disassembly (alter mapping symbols).  */
+ 
+ bfd_boolean
+ arm_filter_symbol (asymbol *sym)
+ {
+   const char *name;
+ 
+   if (sym == NULL)
+     return TRUE;
+ 
+   name = bfd_asymbol_name (sym);
+ 
+   /* ARM/Thumb mapping symbols used to be treated as functions when sorting
+      symbols before disassembly. This makes them behave as if they still
+      were.  */
+   if (name && name[0] == '$'
+       && (name[1] == 'a' || name[1] == 't'))
+     sym->flags |= BSF_FUNCTION;
+ 
+   return TRUE;
+ }
+ 
  /* Parse an individual disassembler option.  */
  
  void
Index: opcodes/dis-init.c
===================================================================
RCS file: /cvs/src/src/opcodes/dis-init.c,v
retrieving revision 1.2
diff -c -p -r1.2 dis-init.c
*** opcodes/dis-init.c	14 Nov 2003 15:12:44 -0000	1.2
--- opcodes/dis-init.c	27 Mar 2005 19:47:30 -0000
*************** init_disassemble_info (struct disassembl
*** 38,43 ****
--- 38,44 ----
    info->print_address_func = generic_print_address;
    info->symbol_at_address_func = generic_symbol_at_address;
    info->symbol_is_valid = generic_symbol_is_valid;
+   info->filter_symbol = NULL;
    info->display_endian = BFD_ENDIAN_UNKNOWN;
  }
  
Index: opcodes/disassemble.c
===================================================================
RCS file: /cvs/src/src/opcodes/disassemble.c,v
retrieving revision 1.50
diff -c -p -r1.50 disassemble.c
*** opcodes/disassemble.c	3 Mar 2005 11:49:48 -0000	1.50
--- opcodes/disassemble.c	27 Mar 2005 19:47:30 -0000
*************** disassemble_init_for_target (struct disa
*** 422,427 ****
--- 422,428 ----
  #ifdef ARCH_arm
      case bfd_arch_arm:
        info->symbol_is_valid = arm_symbol_is_valid;
+       info->filter_symbol = arm_filter_symbol;
        break;
  #endif
  #ifdef ARCH_ia64

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Daniel Jacobowitz]

On Sun, Mar 27, 2005 at 09:25:49PM +0100, Julian Brown wrote:
> In the interest of preserving existing behaviour, I have fixed this by 
> adding a new disassemble_info hook which allows the symbol table used 
> for disassembly to have a target-specific hook for altering each symbol 
> (or remove it) in the remove_useless_symbols function. This sets the 
> BSF_FUNCTION flag for mapping symbols, which makes things behave the way 
> they used to, and possibly provides useful functionality for other 
> platforms as well.

How about checking bfd_is_target_special_symbol in remove_useless_syms?


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Julian Brown]

Daniel Jacobowitz wrote:
> On Sun, Mar 27, 2005 at 09:25:49PM +0100, Julian Brown wrote:
> 
>>In the interest of preserving existing behaviour, I have fixed this by 
>>adding a new disassemble_info hook which allows the symbol table used 
>>for disassembly to have a target-specific hook for altering each symbol 
>>(or remove it) in the remove_useless_symbols function. This sets the 
>>BSF_FUNCTION flag for mapping symbols, which makes things behave the way 
>>they used to, and possibly provides useful functionality for other 
>>platforms as well.
> 
> How about checking bfd_is_target_special_symbol in remove_useless_syms?

To remove the mapping symbols altogether? That would make sense, but the 
output of objdump would still be altered then -- my patch doesn't 
actually remove the mapping symbols in remove_useless_symbols, just 
munges their type. This was the best way I could come up with to 
preserve existing behaviour without affecting other platforms, but I 
suspect it might be trying too hard (it is "only" disassembled output 
which is affected, presumably meant for human consumption).

Julian

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Richard Earnshaw]

On Sun, 2005-03-27 at 21:25, Julian Brown wrote:
> Hi,
> 
> This patch fixes two problems: the type of mapping symbols is made 
> STT_NOTYPE (from STT_FUNC or STT_OBJECT from ARM/Thumb or data 
> respectively), and the alignment is corrected to not have the low bit 
> set for Thumb mapping symbols, in accordance with the latest version of 
> the ARM AAELF spec.
> 
> Unfortunately altering the symbol type to STT_NOTYPE for mapping symbols 
> affects the output of disassembly from objdump. $a and $t were 
> previously treated as functions. When objdump tried to find the closest 
> symbol for possibly-symbol-relative offsets previously, it found those 
> mapping symbols since compare_symbols places functions first in a sorted 
> version of the symbol table. They were then rejected by the 
> symbol_is_valid hook (arm_symbol_is_valid) from the disassemble_info 
> struct, and the section name was printed instead.
> 
> When the type of mapping symbols is changed to STT_NOTYPE, they are no 
> longer placed first in the sorted symbol list, so another symbol is used 
> instead. This causes test regressions, e.g. a symbol-offset address 
> which was previously output as:
> 
>    bl      8224 <.text-0xc>
> 
> was output instead as:
> 
>    bl      8224 <_start-0xc>
> 
> In the interest of preserving existing behaviour, I have fixed this by 
> adding a new disassemble_info hook which allows the symbol table used 
> for disassembly to have a target-specific hook for altering each symbol 
> (or remove it) in the remove_useless_symbols function. This sets the 
> BSF_FUNCTION flag for mapping symbols, which makes things behave the way 
> they used to, and possibly provides useful functionality for other 
> platforms as well.
> 
> It may be better to just change the expected test output instead. Thoughts?
> 

I think the tests should be fixed.  Other targets (those that don't have
mapping symbols) would almost certainly output _start in the example
above, so the existing behaviour should be considered undesirable at
best and probably just buggy.

That should allow you to simplify your patch so that you don't need to
touch MI parts.

R.

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Daniel Jacobowitz]

On Tue, Mar 29, 2005 at 04:44:07PM +0100, Richard Earnshaw wrote:
> I think the tests should be fixed.  Other targets (those that don't have
> mapping symbols) would almost certainly output _start in the example
> above, so the existing behaviour should be considered undesirable at
> best and probably just buggy.
> 
> That should allow you to simplify your patch so that you don't need to
> touch MI parts.

I agree.  Sorry, Julian, this conflicts with what I said before -
that's what I get for not reading your examples properly!

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Julian Brown]

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Richard Earnshaw wrote:
> On Sun, 2005-03-27 at 21:25, Julian Brown wrote:
> 
>>Hi,
>>
>>This patch fixes two problems: the type of mapping symbols is made 
>>STT_NOTYPE (from STT_FUNC or STT_OBJECT from ARM/Thumb or data 
>>respectively), and the alignment is corrected to not have the low bit 
>>set for Thumb mapping symbols, in accordance with the latest version of 
>>the ARM AAELF spec.
>>
>>Unfortunately altering the symbol type to STT_NOTYPE for mapping symbols 
>>affects the output of disassembly from objdump. $a and $t were 
>>previously treated as functions. When objdump tried to find the closest 
>>symbol for possibly-symbol-relative offsets previously, it found those 
>>mapping symbols since compare_symbols places functions first in a sorted 
>>version of the symbol table. They were then rejected by the 
>>symbol_is_valid hook (arm_symbol_is_valid) from the disassemble_info 
>>struct, and the section name was printed instead.
>>
>>When the type of mapping symbols is changed to STT_NOTYPE, they are no 
>>longer placed first in the sorted symbol list, so another symbol is used 
>>instead. This causes test regressions, e.g. a symbol-offset address 
>>which was previously output as:
>>
>>   bl      8224 <.text-0xc>
>>
>>was output instead as:
>>
>>   bl      8224 <_start-0xc>
>>
>>In the interest of preserving existing behaviour, I have fixed this by 
>>adding a new disassemble_info hook which allows the symbol table used 
>>for disassembly to have a target-specific hook for altering each symbol 
>>(or remove it) in the remove_useless_symbols function. This sets the 
>>BSF_FUNCTION flag for mapping symbols, which makes things behave the way 
>>they used to, and possibly provides useful functionality for other 
>>platforms as well.
>>
>>It may be better to just change the expected test output instead. Thoughts?
>>
> 
> 
> I think the tests should be fixed.  Other targets (those that don't have
> mapping symbols) would almost certainly output _start in the example
> above, so the existing behaviour should be considered undesirable at
> best and probably just buggy.
> 
> That should allow you to simplify your patch so that you don't need to
> touch MI parts.

OK, this is a simpler version of the patch which alters the expected 
test output instead (only ARM-specific code is now touched).

Tested with cross to arm-none-eabi and arm-none-symbianelf, and with all 
targets on i686-pc-linux-gnu. OK to apply?

ChangeLog:

   * bfd/elf32-arm.c (arm_elf_find_function): Include STT_NOTYPE in test
   for mapping symbols.
   * gas/config/tc-arm.c (mapping_state): Update documentation in
   function comment. Change type of mapping symbols to BSF_NO_TYPE.
   (is_arm_mapping_symbol_name): New function.
   (arm_adjust_symtab): Don't change type of mapping symbols here.
   * gas/testsuite/gas/arm/mapping.d: Update expected output.
   * ld/testsuite/ld-arm/arm-app-abs32.d: Likewise.
   * ld/testsuite/ld-arm/arm-app.d: Likewise.
   * ld/testsuite/ld-arm/mixed-app.d: Likewise.

[-- Attachment #2: patch-4 --]
[-- Type: text/plain, Size: 15332 bytes --]

? bfd/doc/bfd.info
? bfd/doc/bfd.info-1
? gas/doc/as.info
? gas/doc/as.info-1
Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.28
diff -c -p -r1.28 elf32-arm.c
*** bfd/elf32-arm.c	29 Mar 2005 16:54:19 -0000	1.28
--- bfd/elf32-arm.c	29 Mar 2005 21:13:42 -0000
*************** arm_elf_find_function (bfd *         abf
*** 4754,4765 ****
  	  break;
  	case STT_FUNC:
  	case STT_ARM_TFUNC:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
  	      && is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
- 	case STT_NOTYPE:
  	  if (bfd_get_section (&q->symbol) == section
  	      && q->symbol.value >= low_func
  	      && q->symbol.value <= offset)
--- 4754,4765 ----
  	  break;
  	case STT_FUNC:
  	case STT_ARM_TFUNC:
+ 	case STT_NOTYPE:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
  	      && is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
  	  if (bfd_get_section (&q->symbol) == section
  	      && q->symbol.value >= low_func
  	      && q->symbol.value <= offset)
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.199
diff -c -p -r1.199 tc-arm.c
*** gas/config/tc-arm.c	29 Mar 2005 16:54:21 -0000	1.199
--- gas/config/tc-arm.c	29 Mar 2005 21:13:47 -0000
*************** validate_offset_imm (unsigned int val, i
*** 1243,1316 ****
  \f
  #ifdef OBJ_ELF
  /* This code is to handle mapping symbols as defined in the ARM ELF spec.
!    (This text is taken from version B-02 of the spec):
  
!       4.4.7 Mapping and tagging symbols
  
!       A section of an ARM ELF file can contain a mixture of ARM code,
!       Thumb code, and data.  There are inline transitions between code
!       and data at literal pool boundaries. There can also be inline
!       transitions between ARM code and Thumb code, for example in
!       ARM-Thumb inter-working veneers.  Linkers, machine-level
!       debuggers, profiling tools, and disassembly tools need to map
!       images accurately. For example, setting an ARM breakpoint on a
!       Thumb location, or in a literal pool, can crash the program
!       being debugged, ruining the debugging session.
! 
!       ARM ELF entities are mapped (see section 4.4.7.1 below) and
!       tagged (see section 4.4.7.2 below) using local symbols (with
!       binding STB_LOCAL).  To assist consumers, mapping and tagging
!       symbols should be collated first in the symbol table, before
!       other symbols with binding STB_LOCAL.
! 
!       To allow properly collated mapping and tagging symbols to be
!       skipped by consumers that have no interest in them, the first
!       such symbol should have the name $m and its st_value field equal
!       to the total number of mapping and tagging symbols (including
!       the $m) in the symbol table.
! 
!       4.4.7.1 Mapping symbols
! 
!       $a    Labels the first byte of a sequence of ARM instructions.
!             Its type is STT_FUNC.
! 
!       $d    Labels the first byte of a sequence of data items.
!             Its type is STT_OBJECT.
! 
!       $t    Labels the first byte of a sequence of Thumb instructions.
!             Its type is STT_FUNC.
! 
!       This list of mapping symbols may be extended in the future.
! 
!       Section-relative mapping symbols
! 
!       Mapping symbols defined in a section define a sequence of
!       half-open address intervals that cover the address range of the
!       section. Each interval starts at the address defined by a
!       mapping symbol, and continues up to, but not including, the
!       address defined by the next (in address order) mapping symbol or
!       the end of the section. A corollary is that there must be a
!       mapping symbol defined at the beginning of each section.
!       Consumers can ignore the size of a section-relative mapping
!       symbol. Producers can set it to 0.
! 
!       Absolute mapping symbols
! 
!       Because of the need to crystallize a Thumb address with the
!       Thumb-bit set, absolute symbol of type STT_FUNC (symbols of type
!       STT_FUNC defined in section SHN_ABS) need to be mapped with $a
!       or $t.
! 
!       The extent of a mapping symbol defined in SHN_ABS is [st_value,
!       st_value + st_size), or [st_value, st_value + 1) if st_size = 0,
!       where [x, y) denotes the half-open address range from x,
!       inclusive, to y, exclusive.
! 
!       In the absence of a mapping symbol, a consumer can interpret a
!       function symbol with an odd value as the Thumb code address
!       obtained by clearing the least significant bit of the
!       value. This interpretation is deprecated, and it may not work in
!       the future.
  
     Note - the Tagging symbols ($b, $f, $p $m) have been dropped from
     the EABI (which is still under development), so they are not
--- 1243,1308 ----
  \f
  #ifdef OBJ_ELF
  /* This code is to handle mapping symbols as defined in the ARM ELF spec.
!    (This text is taken from version 0.62 RC 2 of the spec):
  
!       4.5.7 Mapping symbols
  
!       A section of an ELF file can contain a mixture of ARM code, Thumb code
!       and data.
! 
!       There are inline transitions between code and data at literal pool
!       boundaries.  There can also be inline transitions between ARM code and
!       Thumb code, for example in ARM-Thumb inter-working veneers.
! 
!       Linkers, and potentially other tools, need to map images correctly (for
!       example, to support byte swapping to produce a BE-8 image from a BE-32
!       object file).  To support this, a number of symbols, termed mapping
!       symbols appear in the symbol table to denote the start of a sequence of
!       bytes of the appropriate type.  All mapping symbols have type STT_NOTYPE
!       and binding STB_LOCAL.  The st_size field is unused and must be zero.
! 
!       The mapping symbols are defined in Table 4-6, Mapping symbols.  It is an
!       error for a relocation to reference a mapping symbol.  Two forms of
!       mapping symbol are supported:
! 
!         * a short form, that uses a dollar character and a single letter
!         denoting the class.  This form can be used when an object producer
!         creates mapping symbols automatically, and minimizes symbol table
!         space
! 
!         * a longer form, where the short form is extended with a period and
!         then any sequence of characters that are legal for a symbol.  This
!         form can be used when assembler files have to be annotated manually
!         and the assembler does not support multiple definitions of symbols.
! 
!       Table 4-6, Mapping symbols
! 
!       Name         Meaning
! 
!       $a           Start of a sequence of ARM instructions
!       $a.<any...>
! 
!       $d           Start of a sequence of data items (for example, a literal
!       $d.<any...>  pool)
! 
!       $t           Start of a sequence of Thumb instructions
!       $t.<any...>
! 
!       4.5.7.1 Section-relative mapping symbols
! 
!       Mapping symbols defined in a section define a sequence of half-open
!       address intervals that cover the address range of the section.  Each
!       interval starts at the address defined by the mapping symbol, and
!       continues up to, but not including, the address defined by the next (in
!       address order) mapping symbol or the end of the section.  A section must
!       have a mapping symbol defined at the beginning of the section; however,
!       if the section contains only data then the mapping symbol may be omitted.
! 
!       4.5.7.2 Absolute mapping symbols
! 
!       Mapping symbols are no-longer required for the absolute section.  The
!       equivalent information is now conveyed by the type of the absolute
!       symbol.
  
     Note - the Tagging symbols ($b, $f, $p $m) have been dropped from
     the EABI (which is still under development), so they are not
*************** mapping_state (enum mstate state)
*** 1336,1350 ****
      {
      case MAP_DATA:
        symname = "$d";
!       type = BSF_OBJECT;
        break;
      case MAP_ARM:
        symname = "$a";
!       type = BSF_FUNCTION;
        break;
      case MAP_THUMB:
        symname = "$t";
!       type = BSF_FUNCTION;
        break;
      case MAP_UNDEFINED:
        return;
--- 1328,1342 ----
      {
      case MAP_DATA:
        symname = "$d";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_ARM:
        symname = "$a";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_THUMB:
        symname = "$t";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_UNDEFINED:
        return;
*************** arm_frob_label (symbolS * sym)
*** 13678,13683 ****
--- 13670,13685 ----
      }
  }
  
+ /* Recognize short-form or long-form mapping symbol names.  */
+ 
+ static bfd_boolean
+ is_arm_mapping_symbol_name (const char* name)
+ {
+   return name && name[0] == '$'
+          && (name[1] == 'a' || name[1] == 'd' || name[1] == 't')
+          && (name[2] == '\0' || name[2] == '.');
+ }
+ 
  /* Adjust the symbol table.  This marks Thumb symbols as distinct from
     ARM ones.  */
  
*************** arm_adjust_symtab (void)
*** 13738,13751 ****
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  /* If it's a .thumb_func, declare it as so,
! 	     otherwise tag label as .code 16.  */
! 	  if (THUMB_IS_FUNC (sym))
! 	    elf_sym->internal_elf_sym.st_info =
! 	      ELF_ST_INFO (bind, STT_ARM_TFUNC);
! 	  else
! 	    elf_sym->internal_elf_sym.st_info =
! 	      ELF_ST_INFO (bind, STT_ARM_16BIT);
  	}
      }
  #endif
--- 13740,13756 ----
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  if (!is_arm_mapping_symbol_name (elf_sym->symbol.name))
! 	    { 
! 	      /* If it's a .thumb_func, declare it as so,
! 		 otherwise tag label as .code 16.  */
! 	      if (THUMB_IS_FUNC (sym))
! 		elf_sym->internal_elf_sym.st_info =
! 		  ELF_ST_INFO (bind, STT_ARM_TFUNC);
! 	      else
! 		elf_sym->internal_elf_sym.st_info =
! 		  ELF_ST_INFO (bind, STT_ARM_16BIT);
! 	    }
  	}
      }
  #endif
Index: gas/testsuite/gas/arm/mapping.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/mapping.d,v
retrieving revision 1.4
diff -c -p -r1.4 mapping.d
*** gas/testsuite/gas/arm/mapping.d	11 Dec 2004 04:32:36 -0000	1.4
--- gas/testsuite/gas/arm/mapping.d	29 Mar 2005 21:13:47 -0000
*************** SYMBOL TABLE:
*** 9,18 ****
  0+00 l    d  .text	0+0 (|.text)
  0+00 l    d  .data	0+0 (|.data)
  0+00 l    d  .bss	0+0 (|.bss)
! 0+00 l     F .text	0+0 \$a
! 0+08 l     F .text	0+0 \$t
! 0+00 l     O .data	0+0 \$d
  0+00 l    d  foo	0+0 (|foo)
! 0+00 l     F foo	0+0 \$t
  0+00 g       .text	0+0 mapping
  0+08 g     F .text	0+0 thumb_mapping
--- 9,18 ----
  0+00 l    d  .text	0+0 (|.text)
  0+00 l    d  .data	0+0 (|.data)
  0+00 l    d  .bss	0+0 (|.bss)
! 0+00 l       .text	0+0 \$a
! 0+08 l       .text	0+0 \$t
! 0+00 l       .data	0+0 \$d
  0+00 l    d  foo	0+0 (|foo)
! 0+00 l       foo	0+0 \$t
  0+00 g       .text	0+0 mapping
  0+08 g     F .text	0+0 thumb_mapping
Index: ld/testsuite/ld-arm/arm-app-abs32.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/arm-app-abs32.d,v
retrieving revision 1.2
diff -c -p -r1.2 arm-app-abs32.d
*** ld/testsuite/ld-arm/arm-app-abs32.d	17 Nov 2004 17:50:27 -0000	1.2
--- ld/testsuite/ld-arm/arm-app-abs32.d	29 Mar 2005 21:14:01 -0000
*************** Disassembly of section .plt:
*** 8,14 ****
  
  .* <.plt>:
      .*:	e52de004 	str	lr, \[sp, #-4\]!
!     .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <.plt\+0x10>
      .*:	e08fe00e 	add	lr, pc, lr
      .*:	e5bef008 	ldr	pc, \[lr, #8\]!
      .*:	.* 	.*
--- 8,14 ----
  
  .* <.plt>:
      .*:	e52de004 	str	lr, \[sp, #-4\]!
!     .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <_start-0x10>
      .*:	e08fe00e 	add	lr, pc, lr
      .*:	e5bef008 	ldr	pc, \[lr, #8\]!
      .*:	.* 	.*
Index: ld/testsuite/ld-arm/arm-app.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/arm-app.d,v
retrieving revision 1.2
diff -c -p -r1.2 arm-app.d
*** ld/testsuite/ld-arm/arm-app.d	17 Nov 2004 17:50:27 -0000	1.2
--- ld/testsuite/ld-arm/arm-app.d	29 Mar 2005 21:14:01 -0000
*************** Disassembly of section .plt:
*** 8,14 ****
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <.plt\+0x10>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
--- 8,14 ----
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <_start-0x10>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
*************** Disassembly of section .text:
*** 27,33 ****
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebfffff4 	bl	.* <.text-0xc>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
  
--- 27,33 ----
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebfffff4 	bl	.* <_start-0xc>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
  
Index: ld/testsuite/ld-arm/mixed-app.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/mixed-app.d,v
retrieving revision 1.1
diff -c -p -r1.1 mixed-app.d
*** ld/testsuite/ld-arm/mixed-app.d	17 Nov 2004 17:50:27 -0000	1.1
--- ld/testsuite/ld-arm/mixed-app.d	29 Mar 2005 21:14:01 -0000
*************** Disassembly of section .plt:
*** 8,14 ****
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <.plt\+0x10>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
--- 8,14 ----
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <_start-0x2c>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
*************** Disassembly of section .text:
*** 34,40 ****
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebffffe. 	bl	.* <.text-0x..>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
   .*:	e1a00000 	nop			\(mov r0,r0\)
--- 34,40 ----
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebffffe. 	bl	.* <_start-0x..>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
   .*:	e1a00000 	nop			\(mov r0,r0\)
*************** Disassembly of section .text:
*** 49,55 ****
  
  .* <app_tfunc>:
   .*:	b500      	push	{lr}
!  .*:	(ffc.f7ff|f7ffffc.) 	bl	.* <.text-0x..>
   .*:	bd00      	pop	{pc}
   .*:	4770      	bx	lr
   .*:	46c0      	nop			\(mov r8, r8\)
--- 49,55 ----
  
  .* <app_tfunc>:
   .*:	b500      	push	{lr}
!  .*:	(ffc.f7ff|f7ffffc.) 	bl	.* <_start-0x..>
   .*:	bd00      	pop	{pc}
   .*:	4770      	bx	lr
   .*:	46c0      	nop			\(mov r8, r8\)

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Richard Earnshaw]

On Tue, 2005-03-29 at 22:26, Julian Brown wrote:
> OK, this is a simpler version of the patch which alters the expected 
> test output instead (only ARM-specific code is now touched).
> 
> Tested with cross to arm-none-eabi and arm-none-symbianelf, and with all 
> targets on i686-pc-linux-gnu. OK to apply?
> 
> ChangeLog:
> 
>    * bfd/elf32-arm.c (arm_elf_find_function): Include STT_NOTYPE in test
>    for mapping symbols.
>    * gas/config/tc-arm.c (mapping_state): Update documentation in
>    function comment. Change type of mapping symbols to BSF_NO_TYPE.
>    (is_arm_mapping_symbol_name): New function.
>    (arm_adjust_symtab): Don't change type of mapping symbols here.
>    * gas/testsuite/gas/arm/mapping.d: Update expected output.
>    * ld/testsuite/ld-arm/arm-app-abs32.d: Likewise.
>    * ld/testsuite/ld-arm/arm-app.d: Likewise.
>    * ld/testsuite/ld-arm/mixed-app.d: Likewise.
> 
> ______________________________________________________________________
> Index: gas/config/tc-arm.c
> ===================================================================
> RCS file: /cvs/src/src/gas/config/tc-arm.c,v
> retrieving revision 1.199
> diff -c -p -r1.199 tc-arm.c
> *** gas/config/tc-arm.c	29 Mar 2005 16:54:21 -0000	1.199
> --- gas/config/tc-arm.c	29 Mar 2005 21:13:47 -0000
> *************** validate_offset_imm (unsigned int val, i
> *** 1243,1316 ****
> --- 1243,1308 ----
>   \f
>   #ifdef OBJ_ELF
>   /* This code is to handle mapping symbols as defined in the ARM ELF spec.
> !    (This text is taken from version 0.62 RC 2 of the spec):
>   
> !       4.5.7 Mapping symbols
>   
> !       A section of an ELF file can contain a mixture of ARM code, Thumb code

There's no need to quote the entirety of this section in the source
file.  A cross reference is fine.  The correct reference should be to
v1.0 of the spec, where the section number is 4.5.5.

> *************** arm_frob_label (symbolS * sym)
> *** 13678,13683 ****
> --- 13670,13685 ----
>       }
>   }
>   
> + /* Recognize short-form or long-form mapping symbol names.  */
> + 
> + static bfd_boolean
> + is_arm_mapping_symbol_name (const char* name)
> + {
> +   return name && name[0] == '$'
> +          && (name[1] == 'a' || name[1] == 'd' || name[1] == 't')
> +          && (name[2] == '\0' || name[2] == '.');
> + }
> + 
>   /* Adjust the symbol table.  This marks Thumb symbols as distinct from
>      ARM ones.  */
>   

It seems silly to duplicate this function here when it also exists in
libbfd (though also static).  It would make more sense to make the bfd
implementation public and call that.  I would suggest you move the code
to cpu-arm.c.

This is ok with the above changes.

R.

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Paul Brook]

> > !       4.5.7 Mapping symbols
> >
> > !       A section of an ELF file can contain a mixture of ARM code, Thumb
> > code
>
> There's no need to quote the entirety of this section in the source
> file.  A cross reference is fine.  The correct reference should be to
> v1.0 of the spec, where the section number is 4.5.5.

Might also be worth explicitly noting that this is a change from previous 
behaviour.

Paul

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Julian Brown]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

Paul Brook wrote:
>>>!       4.5.7 Mapping symbols
>>>
>>>!       A section of an ELF file can contain a mixture of ARM code, Thumb
>>>code
>>
>>There's no need to quote the entirety of this section in the source
>>file.  A cross reference is fine.  The correct reference should be to
>>v1.0 of the spec, where the section number is 4.5.5.
> 
> 
> Might also be worth explicitly noting that this is a change from previous 
> behaviour.

I'll apply this version on mainline and binutils-csl-arm-2005q1-branch.

ChangeLog:

   * bfd/bfd-in.h (bfd_elf32_is_arm_mapping_symbol_name): Add prototype.
   * bfd/bfd-in2.h: Regenerate.
   * bfd/elf32-arm.c (elf32_arm_is_target_special_symbol): Rename call to
   bfd_elf32_is_arm_mapping_symbol_name.
   (elf32_arm_output_symbol_hook): Likewise.
   (arm_elf_find_function): Likewise, and include STT_NOTYPE in test
   for mapping symbols.
   (is_arm_mapping_symbol_name): Function moved from here...
   * bfd/cpu-arm.c (bfd_elf32_is_arm_mapping_symbol_name): ...to here,
   renamed and made global.
   * gas/config/tc-arm.c (mapping_state): Change documentation in
   function comment to cross-reference spec instead. Change type of
   mapping symbols to BSF_NO_TYPE.
   (arm_adjust_symtab): Don't change type of mapping symbols here.
   * gas/testsuite/gas/arm/mapping.d: Update expected output.
   * ld/testsuite/ld-arm/arm-app-abs32.d: Likewise.
   * ld/testsuite/ld-arm/arm-app.d: Likewise.
   * ld/testsuite/ld-arm/mixed-app.d: Likewise.


[-- Attachment #2: patch-6 --]
[-- Type: text/plain, Size: 14971 bytes --]

Index: bfd/bfd-in.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in.h,v
retrieving revision 1.95
diff -c -p -r1.95 bfd-in.h
*** bfd/bfd-in.h	23 Mar 2005 04:14:42 -0000	1.95
--- bfd/bfd-in.h	30 Mar 2005 15:25:34 -0000
*************** extern bfd_boolean bfd_elf32_arm_get_bfd
*** 824,829 ****
--- 824,833 ----
  extern bfd_boolean bfd_elf32_arm_add_glue_sections_to_bfd
    (bfd *, struct bfd_link_info *);
  
+ /* ELF ARM mapping symbol support */
+ extern bfd_boolean bfd_elf32_is_arm_mapping_symbol_name
+   (const char * name);
+ 
  /* ARM Note section processing.  */
  extern bfd_boolean bfd_arm_merge_machines
    (bfd *, bfd *);
Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.28
diff -c -p -r1.28 elf32-arm.c
*** bfd/elf32-arm.c	29 Mar 2005 16:54:19 -0000	1.28
--- bfd/elf32-arm.c	30 Mar 2005 15:25:35 -0000
*************** elf32_arm_check_relocs (bfd *abfd, struc
*** 4705,4725 ****
    return TRUE;
  }
  
- static bfd_boolean
- is_arm_mapping_symbol_name (const char * name)
- {
-   return (name != NULL)
-     && (name[0] == '$')
-     && ((name[1] == 'a') || (name[1] == 't') || (name[1] == 'd'))
-     && (name[2] == 0);
- }
- 
  /* Treat mapping symbols as special target symbols.  */
  
  static bfd_boolean
  elf32_arm_is_target_special_symbol (bfd * abfd ATTRIBUTE_UNUSED, asymbol * sym)
  {
!   return is_arm_mapping_symbol_name (sym->name);
  }
  
  /* This is a copy of elf_find_function() from elf.c except that
--- 4705,4716 ----
    return TRUE;
  }
  
  /* Treat mapping symbols as special target symbols.  */
  
  static bfd_boolean
  elf32_arm_is_target_special_symbol (bfd * abfd ATTRIBUTE_UNUSED, asymbol * sym)
  {
!   return bfd_elf32_is_arm_mapping_symbol_name (sym->name);
  }
  
  /* This is a copy of elf_find_function() from elf.c except that
*************** arm_elf_find_function (bfd *         abf
*** 4754,4765 ****
  	  break;
  	case STT_FUNC:
  	case STT_ARM_TFUNC:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
! 	      && is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
- 	case STT_NOTYPE:
  	  if (bfd_get_section (&q->symbol) == section
  	      && q->symbol.value >= low_func
  	      && q->symbol.value <= offset)
--- 4745,4756 ----
  	  break;
  	case STT_FUNC:
  	case STT_ARM_TFUNC:
+ 	case STT_NOTYPE:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
! 	      && bfd_elf32_is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
  	  if (bfd_get_section (&q->symbol) == section
  	      && q->symbol.value >= low_func
  	      && q->symbol.value <= offset)
*************** elf32_arm_output_symbol_hook (struct bfd
*** 6086,6092 ****
      return TRUE;
  
    /* We only want mapping symbols.  */
!   if (! is_arm_mapping_symbol_name (name))
      return TRUE;
  
    mapcount = ++(elf32_arm_section_data (input_sec)->mapcount);
--- 6077,6083 ----
      return TRUE;
  
    /* We only want mapping symbols.  */
!   if (! bfd_elf32_is_arm_mapping_symbol_name (name))
      return TRUE;
  
    mapcount = ++(elf32_arm_section_data (input_sec)->mapcount);
Index: bfd/cpu-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/cpu-arm.c,v
retrieving revision 1.15
diff -c -p -r1.15 cpu-arm.c
*** bfd/cpu-arm.c	20 Feb 2005 14:59:06 -0000	1.15
--- bfd/cpu-arm.c	30 Mar 2005 15:25:35 -0000
*************** bfd_arm_get_mach_from_notes (bfd *abfd, 
*** 400,402 ****
--- 400,412 ----
      free (buffer);
    return bfd_mach_arm_unknown;
  }
+ 
+ bfd_boolean
+ bfd_elf32_is_arm_mapping_symbol_name (const char * name)
+ {
+   return (name != NULL)
+     && (name[0] == '$')
+     && ((name[1] == 'a') || (name[1] == 't') || (name[1] == 'd'))
+     && (name[2] == 0);
+ }
+ 
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.199
diff -c -p -r1.199 tc-arm.c
*** gas/config/tc-arm.c	29 Mar 2005 16:54:21 -0000	1.199
--- gas/config/tc-arm.c	30 Mar 2005 15:25:35 -0000
*************** validate_offset_imm (unsigned int val, i
*** 1243,1320 ****
  \f
  #ifdef OBJ_ELF
  /* This code is to handle mapping symbols as defined in the ARM ELF spec.
!    (This text is taken from version B-02 of the spec):
! 
!       4.4.7 Mapping and tagging symbols
! 
!       A section of an ARM ELF file can contain a mixture of ARM code,
!       Thumb code, and data.  There are inline transitions between code
!       and data at literal pool boundaries. There can also be inline
!       transitions between ARM code and Thumb code, for example in
!       ARM-Thumb inter-working veneers.  Linkers, machine-level
!       debuggers, profiling tools, and disassembly tools need to map
!       images accurately. For example, setting an ARM breakpoint on a
!       Thumb location, or in a literal pool, can crash the program
!       being debugged, ruining the debugging session.
! 
!       ARM ELF entities are mapped (see section 4.4.7.1 below) and
!       tagged (see section 4.4.7.2 below) using local symbols (with
!       binding STB_LOCAL).  To assist consumers, mapping and tagging
!       symbols should be collated first in the symbol table, before
!       other symbols with binding STB_LOCAL.
! 
!       To allow properly collated mapping and tagging symbols to be
!       skipped by consumers that have no interest in them, the first
!       such symbol should have the name $m and its st_value field equal
!       to the total number of mapping and tagging symbols (including
!       the $m) in the symbol table.
! 
!       4.4.7.1 Mapping symbols
! 
!       $a    Labels the first byte of a sequence of ARM instructions.
!             Its type is STT_FUNC.
! 
!       $d    Labels the first byte of a sequence of data items.
!             Its type is STT_OBJECT.
! 
!       $t    Labels the first byte of a sequence of Thumb instructions.
!             Its type is STT_FUNC.
! 
!       This list of mapping symbols may be extended in the future.
! 
!       Section-relative mapping symbols
! 
!       Mapping symbols defined in a section define a sequence of
!       half-open address intervals that cover the address range of the
!       section. Each interval starts at the address defined by a
!       mapping symbol, and continues up to, but not including, the
!       address defined by the next (in address order) mapping symbol or
!       the end of the section. A corollary is that there must be a
!       mapping symbol defined at the beginning of each section.
!       Consumers can ignore the size of a section-relative mapping
!       symbol. Producers can set it to 0.
! 
!       Absolute mapping symbols
! 
!       Because of the need to crystallize a Thumb address with the
!       Thumb-bit set, absolute symbol of type STT_FUNC (symbols of type
!       STT_FUNC defined in section SHN_ABS) need to be mapped with $a
!       or $t.
! 
!       The extent of a mapping symbol defined in SHN_ABS is [st_value,
!       st_value + st_size), or [st_value, st_value + 1) if st_size = 0,
!       where [x, y) denotes the half-open address range from x,
!       inclusive, to y, exclusive.
! 
!       In the absence of a mapping symbol, a consumer can interpret a
!       function symbol with an odd value as the Thumb code address
!       obtained by clearing the least significant bit of the
!       value. This interpretation is deprecated, and it may not work in
!       the future.
! 
!    Note - the Tagging symbols ($b, $f, $p $m) have been dropped from
!    the EABI (which is still under development), so they are not
!    implemented here.  */
  
  static enum mstate mapstate = MAP_UNDEFINED;
  
--- 1243,1251 ----
  \f
  #ifdef OBJ_ELF
  /* This code is to handle mapping symbols as defined in the ARM ELF spec.
!    (See "Mapping symbols", section 4.5.5, ARM AAELF version 1.0).
!    Note that previously, $a and $t has type STT_FUNC (BSF_OBJECT flag),
!    and $d has type STT_OBJECT (BSF_OBJECT flag). Now all three are untyped.  */
  
  static enum mstate mapstate = MAP_UNDEFINED;
  
*************** mapping_state (enum mstate state)
*** 1336,1350 ****
      {
      case MAP_DATA:
        symname = "$d";
!       type = BSF_OBJECT;
        break;
      case MAP_ARM:
        symname = "$a";
!       type = BSF_FUNCTION;
        break;
      case MAP_THUMB:
        symname = "$t";
!       type = BSF_FUNCTION;
        break;
      case MAP_UNDEFINED:
        return;
--- 1267,1281 ----
      {
      case MAP_DATA:
        symname = "$d";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_ARM:
        symname = "$a";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_THUMB:
        symname = "$t";
!       type = BSF_NO_FLAGS;
        break;
      case MAP_UNDEFINED:
        return;
*************** arm_adjust_symtab (void)
*** 13738,13751 ****
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  /* If it's a .thumb_func, declare it as so,
! 	     otherwise tag label as .code 16.  */
! 	  if (THUMB_IS_FUNC (sym))
! 	    elf_sym->internal_elf_sym.st_info =
! 	      ELF_ST_INFO (bind, STT_ARM_TFUNC);
! 	  else
! 	    elf_sym->internal_elf_sym.st_info =
! 	      ELF_ST_INFO (bind, STT_ARM_16BIT);
  	}
      }
  #endif
--- 13669,13685 ----
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  if (! bfd_elf32_is_arm_mapping_symbol_name (elf_sym->symbol.name))
! 	    { 
! 	      /* If it's a .thumb_func, declare it as so,
! 		 otherwise tag label as .code 16.  */
! 	      if (THUMB_IS_FUNC (sym))
! 		elf_sym->internal_elf_sym.st_info =
! 		  ELF_ST_INFO (bind, STT_ARM_TFUNC);
! 	      else
! 		elf_sym->internal_elf_sym.st_info =
! 		  ELF_ST_INFO (bind, STT_ARM_16BIT);
! 	    }
  	}
      }
  #endif
Index: gas/testsuite/gas/arm/mapping.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/mapping.d,v
retrieving revision 1.4
diff -c -p -r1.4 mapping.d
*** gas/testsuite/gas/arm/mapping.d	11 Dec 2004 04:32:36 -0000	1.4
--- gas/testsuite/gas/arm/mapping.d	30 Mar 2005 15:25:35 -0000
*************** SYMBOL TABLE:
*** 9,18 ****
  0+00 l    d  .text	0+0 (|.text)
  0+00 l    d  .data	0+0 (|.data)
  0+00 l    d  .bss	0+0 (|.bss)
! 0+00 l     F .text	0+0 \$a
! 0+08 l     F .text	0+0 \$t
! 0+00 l     O .data	0+0 \$d
  0+00 l    d  foo	0+0 (|foo)
! 0+00 l     F foo	0+0 \$t
  0+00 g       .text	0+0 mapping
  0+08 g     F .text	0+0 thumb_mapping
--- 9,18 ----
  0+00 l    d  .text	0+0 (|.text)
  0+00 l    d  .data	0+0 (|.data)
  0+00 l    d  .bss	0+0 (|.bss)
! 0+00 l       .text	0+0 \$a
! 0+08 l       .text	0+0 \$t
! 0+00 l       .data	0+0 \$d
  0+00 l    d  foo	0+0 (|foo)
! 0+00 l       foo	0+0 \$t
  0+00 g       .text	0+0 mapping
  0+08 g     F .text	0+0 thumb_mapping
Index: ld/testsuite/ld-arm/arm-app-abs32.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/arm-app-abs32.d,v
retrieving revision 1.2
diff -c -p -r1.2 arm-app-abs32.d
*** ld/testsuite/ld-arm/arm-app-abs32.d	17 Nov 2004 17:50:27 -0000	1.2
--- ld/testsuite/ld-arm/arm-app-abs32.d	30 Mar 2005 15:25:35 -0000
*************** Disassembly of section .plt:
*** 8,14 ****
  
  .* <.plt>:
      .*:	e52de004 	str	lr, \[sp, #-4\]!
!     .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <.plt\+0x10>
      .*:	e08fe00e 	add	lr, pc, lr
      .*:	e5bef008 	ldr	pc, \[lr, #8\]!
      .*:	.* 	.*
--- 8,14 ----
  
  .* <.plt>:
      .*:	e52de004 	str	lr, \[sp, #-4\]!
!     .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <_start-0x10>
      .*:	e08fe00e 	add	lr, pc, lr
      .*:	e5bef008 	ldr	pc, \[lr, #8\]!
      .*:	.* 	.*
Index: ld/testsuite/ld-arm/arm-app.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/arm-app.d,v
retrieving revision 1.2
diff -c -p -r1.2 arm-app.d
*** ld/testsuite/ld-arm/arm-app.d	17 Nov 2004 17:50:27 -0000	1.2
--- ld/testsuite/ld-arm/arm-app.d	30 Mar 2005 15:25:35 -0000
*************** Disassembly of section .plt:
*** 8,14 ****
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <.plt\+0x10>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
--- 8,14 ----
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <_start-0x10>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
*************** Disassembly of section .text:
*** 27,33 ****
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebfffff4 	bl	.* <.text-0xc>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
  
--- 27,33 ----
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebfffff4 	bl	.* <_start-0xc>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
  
Index: ld/testsuite/ld-arm/mixed-app.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/mixed-app.d,v
retrieving revision 1.1
diff -c -p -r1.1 mixed-app.d
*** ld/testsuite/ld-arm/mixed-app.d	17 Nov 2004 17:50:27 -0000	1.1
--- ld/testsuite/ld-arm/mixed-app.d	30 Mar 2005 15:25:35 -0000
*************** Disassembly of section .plt:
*** 8,14 ****
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <.plt\+0x10>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
--- 8,14 ----
  
  .* <.plt>:
   .*:	e52de004 	str	lr, \[sp, #-4\]!
!  .*:	e59fe004 	ldr	lr, \[pc, #4\]	; .* <_start-0x2c>
   .*:	e08fe00e 	add	lr, pc, lr
   .*:	e5bef008 	ldr	pc, \[lr, #8\]!
   .*:	.*
*************** Disassembly of section .text:
*** 34,40 ****
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebffffe. 	bl	.* <.text-0x..>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
   .*:	e1a00000 	nop			\(mov r0,r0\)
--- 34,40 ----
  .* <app_func>:
   .*:	e1a0c00d 	mov	ip, sp
   .*:	e92dd800 	stmdb	sp!, {fp, ip, lr, pc}
!  .*:	ebffffe. 	bl	.* <_start-0x..>
   .*:	e89d6800 	ldmia	sp, {fp, sp, lr}
   .*:	e12fff1e 	bx	lr
   .*:	e1a00000 	nop			\(mov r0,r0\)
*************** Disassembly of section .text:
*** 49,55 ****
  
  .* <app_tfunc>:
   .*:	b500      	push	{lr}
!  .*:	(ffc.f7ff|f7ffffc.) 	bl	.* <.text-0x..>
   .*:	bd00      	pop	{pc}
   .*:	4770      	bx	lr
   .*:	46c0      	nop			\(mov r8, r8\)
--- 49,55 ----
  
  .* <app_tfunc>:
   .*:	b500      	push	{lr}
!  .*:	(ffc.f7ff|f7ffffc.) 	bl	.* <_start-0x..>
   .*:	bd00      	pop	{pc}
   .*:	4770      	bx	lr
   .*:	46c0      	nop			\(mov r8, r8\)

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Richard Earnshaw]

On Wed, 2005-03-30 at 16:31, Julian Brown wrote:

>    * bfd/cpu-arm.c (bfd_elf32_is_arm_mapping_symbol_name): ...to here,
>    renamed and made global.

This function isn't elf specific, which is one of the reasons I
suggested moving it to cpu-arm.c.  So why have you put elf32 in the
name?

R.

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Julian Brown]

Richard Earnshaw wrote:
> On Wed, 2005-03-30 at 16:31, Julian Brown wrote:
> 
> 
>>   * bfd/cpu-arm.c (bfd_elf32_is_arm_mapping_symbol_name): ...to here,
>>   renamed and made global.
> 
> 
> This function isn't elf specific, which is one of the reasons I
> suggested moving it to cpu-arm.c.  So why have you put elf32 in the
> name?

Doh, sorry. I was confused by the mapping symbols being defined by the 
AAELF spec, and didn't realise they would be used elsewhere too.

Is the original name best, or would bfd_is_arm_mapping_symbol_name be 
better?

Julian

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Richard Earnshaw]

On Wed, 2005-03-30 at 17:34, Julian Brown wrote:
> Richard Earnshaw wrote:
> > On Wed, 2005-03-30 at 16:31, Julian Brown wrote:
> > 
> > 
> >>   * bfd/cpu-arm.c (bfd_elf32_is_arm_mapping_symbol_name): ...to here,
> >>   renamed and made global.
> > 
> > 
> > This function isn't elf specific, which is one of the reasons I
> > suggested moving it to cpu-arm.c.  So why have you put elf32 in the
> > name?
> 
> Doh, sorry. I was confused by the mapping symbols being defined by the 
> AAELF spec, and didn't realise they would be used elsewhere too.
> 
True, but we don't define a coff spec.  However, I suspect that if we
did, we'd probably put mapping symbols in there too.

> Is the original name best, or would bfd_is_arm_mapping_symbol_name be 
> better?
> 

bfd_is_arm_mapping_symbol_name is fine.

R.

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Julian Brown]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Richard Earnshaw wrote:
> On Wed, 2005-03-30 at 17:34, Julian Brown wrote:
>>Richard Earnshaw wrote:
>>>This function isn't elf specific, which is one of the reasons I
>>>suggested moving it to cpu-arm.c.  So why have you put elf32 in the
>>>name?
>>
>>Doh, sorry. I was confused by the mapping symbols being defined by the 
>>AAELF spec, and didn't realise they would be used elsewhere too.
> 
> True, but we don't define a coff spec.  However, I suspect that if we
> did, we'd probably put mapping symbols in there too.

OK, here is a further patch to rename the function.

ChangeLog:

bfd:
   * bfd-in.h (bfd_is_arm_mapping_symbol_name): Rename from
   bfd_elf32_is_arm_mapping_symbol_name.
   * bfd-in2.h: Regenerate.
   * cpu-arm.c (bfd_is_arm_mapping_symbol_name): Rename from
   bfd_elf32_is_arm_mapping_symbol_name.
   * elf32-arm.c (elf32_arm_is_target_special_symbol): Rename
   bfd_elf32_is_arm_mapping_symbol_name to bfd_is_arm_mapping_symbol_name.
   (arm_elf_find_function): Likewise.
   (elf32_arm_output_symbol_hook): Likewise.

gas:
   * config/tc-arm.c (arm_adjust_symtab): Rename
   bfd_elf32_is_arm_mapping_symbol_name to bfd_is_arm_mapping_symbol_name.

[-- Attachment #2: patchpatch-1 --]
[-- Type: text/plain, Size: 4315 bytes --]

Index: bfd/bfd-in.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in.h,v
retrieving revision 1.96
diff -c -p -r1.96 bfd-in.h
*** bfd/bfd-in.h	30 Mar 2005 16:21:58 -0000	1.96
--- bfd/bfd-in.h	30 Mar 2005 17:02:07 -0000
*************** extern bfd_boolean bfd_elf32_arm_add_glu
*** 825,831 ****
    (bfd *, struct bfd_link_info *);
  
  /* ELF ARM mapping symbol support */
! extern bfd_boolean bfd_elf32_is_arm_mapping_symbol_name
    (const char * name);
  
  /* ARM Note section processing.  */
--- 825,831 ----
    (bfd *, struct bfd_link_info *);
  
  /* ELF ARM mapping symbol support */
! extern bfd_boolean bfd_is_arm_mapping_symbol_name
    (const char * name);
  
  /* ARM Note section processing.  */
Index: bfd/cpu-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/cpu-arm.c,v
retrieving revision 1.16
diff -c -p -r1.16 cpu-arm.c
*** bfd/cpu-arm.c	30 Mar 2005 16:21:59 -0000	1.16
--- bfd/cpu-arm.c	30 Mar 2005 17:02:07 -0000
*************** bfd_arm_get_mach_from_notes (bfd *abfd, 
*** 402,408 ****
  }
  
  bfd_boolean
! bfd_elf32_is_arm_mapping_symbol_name (const char * name)
  {
    return (name != NULL)
      && (name[0] == '$')
--- 402,408 ----
  }
  
  bfd_boolean
! bfd_is_arm_mapping_symbol_name (const char * name)
  {
    return (name != NULL)
      && (name[0] == '$')
Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.29
diff -c -p -r1.29 elf32-arm.c
*** bfd/elf32-arm.c	30 Mar 2005 16:21:59 -0000	1.29
--- bfd/elf32-arm.c	30 Mar 2005 17:02:08 -0000
*************** elf32_arm_check_relocs (bfd *abfd, struc
*** 4710,4716 ****
  static bfd_boolean
  elf32_arm_is_target_special_symbol (bfd * abfd ATTRIBUTE_UNUSED, asymbol * sym)
  {
!   return bfd_elf32_is_arm_mapping_symbol_name (sym->name);
  }
  
  /* This is a copy of elf_find_function() from elf.c except that
--- 4710,4716 ----
  static bfd_boolean
  elf32_arm_is_target_special_symbol (bfd * abfd ATTRIBUTE_UNUSED, asymbol * sym)
  {
!   return bfd_is_arm_mapping_symbol_name (sym->name);
  }
  
  /* This is a copy of elf_find_function() from elf.c except that
*************** arm_elf_find_function (bfd *         abf
*** 4748,4754 ****
  	case STT_NOTYPE:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
! 	      && bfd_elf32_is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
  	  if (bfd_get_section (&q->symbol) == section
--- 4748,4754 ----
  	case STT_NOTYPE:
  	  /* Skip $a and $t symbols.  */
  	  if ((q->symbol.flags & BSF_LOCAL)
! 	      && bfd_is_arm_mapping_symbol_name (q->symbol.name))
  	    continue;
  	  /* Fall through.  */
  	  if (bfd_get_section (&q->symbol) == section
*************** elf32_arm_output_symbol_hook (struct bfd
*** 6077,6083 ****
      return TRUE;
  
    /* We only want mapping symbols.  */
!   if (! bfd_elf32_is_arm_mapping_symbol_name (name))
      return TRUE;
  
    mapcount = ++(elf32_arm_section_data (input_sec)->mapcount);
--- 6077,6083 ----
      return TRUE;
  
    /* We only want mapping symbols.  */
!   if (! bfd_is_arm_mapping_symbol_name (name))
      return TRUE;
  
    mapcount = ++(elf32_arm_section_data (input_sec)->mapcount);
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.200
diff -c -p -r1.200 tc-arm.c
*** gas/config/tc-arm.c	30 Mar 2005 16:22:02 -0000	1.200
--- gas/config/tc-arm.c	30 Mar 2005 17:02:08 -0000
*************** arm_adjust_symtab (void)
*** 13669,13675 ****
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  if (! bfd_elf32_is_arm_mapping_symbol_name (elf_sym->symbol.name))
  	    { 
  	      /* If it's a .thumb_func, declare it as so,
  		 otherwise tag label as .code 16.  */
--- 13669,13675 ----
  	  elf_sym = elf_symbol (symbol_get_bfdsym (sym));
  	  bind = ELF_ST_BIND (elf_sym->internal_elf_sym.st_info);
  
! 	  if (! bfd_is_arm_mapping_symbol_name (elf_sym->symbol.name))
  	    { 
  	      /* If it's a .thumb_func, declare it as so,
  		 otherwise tag label as .code 16.  */

Re: [PATCH] Fix type and alignment of ARM/Thumb EABI mapping symbols

[Richard Earnshaw]

On Wed, 2005-03-30 at 18:05, Julian Brown wrote:
> Richard Earnshaw wrote:
> > On Wed, 2005-03-30 at 17:34, Julian Brown wrote:
> >>Richard Earnshaw wrote:
> >>>This function isn't elf specific, which is one of the reasons I
> >>>suggested moving it to cpu-arm.c.  So why have you put elf32 in the
> >>>name?
> >>
> >>Doh, sorry. I was confused by the mapping symbols being defined by the 
> >>AAELF spec, and didn't realise they would be used elsewhere too.
> > 
> > True, but we don't define a coff spec.  However, I suspect that if we
> > did, we'd probably put mapping symbols in there too.
> 
> OK, here is a further patch to rename the function.
> 
> ChangeLog:
> 
> bfd:
>    * bfd-in.h (bfd_is_arm_mapping_symbol_name): Rename from
>    bfd_elf32_is_arm_mapping_symbol_name.
>    * bfd-in2.h: Regenerate.
>    * cpu-arm.c (bfd_is_arm_mapping_symbol_name): Rename from
>    bfd_elf32_is_arm_mapping_symbol_name.
>    * elf32-arm.c (elf32_arm_is_target_special_symbol): Rename
>    bfd_elf32_is_arm_mapping_symbol_name to bfd_is_arm_mapping_symbol_name.
>    (arm_elf_find_function): Likewise.
>    (elf32_arm_output_symbol_hook): Likewise.
> 
> gas:
>    * config/tc-arm.c (arm_adjust_symtab): Rename
>    bfd_elf32_is_arm_mapping_symbol_name to bfd_is_arm_mapping_symbol_name.

This one counts as obvious :-)

R.