This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc 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: mips64 n32 and n64 support in dl-machine.h


Alexandre Oliva <aoliva at redhat dot com> writes:

> This patch adds support for n32 and n64 ABIs in
> sysdeps/mips/dl-machine.h, and gets us rid of an old, broken and
> obsolete mips64-specific dl-machine.h.  Ok?

Getting rid of the mips64-specific file is definitly fine.  I don't
think it ever worked.

> [...]
> Index: sysdeps/mips/dl-machine.h
> [...]
> -/* The MIPS never uses Elfxx_Rela relocations.  */
> +/* No mips dynamic relocations are RELA.  We do have code in this file
> +   is conditional to ! ELF_MACHINE_NO_RELA.  It was never

Shouldn't this be: "We do have code in this file that is..." ?

> +   exercised.  */
>  #define ELF_MACHINE_NO_RELA 1
>  
>  #endif /* !dl_machine_h */
> @@ -472,6 +527,9 @@ _dl_start_user:\n\
>     MAP is the object containing the reloc.  */
>  
>  static inline void
> +#ifdef RTLD_BOOTSTRAP
> +  __attribute__ ((always_inline))
> +#endif
>  elf_machine_rel (struct link_map *map, const ElfW(Rel) *reloc,
>  		 const ElfW(Sym) *sym, const struct r_found_version *version,
>  		 ElfW(Addr) *const reloc_addr)
> @@ -490,9 +548,16 @@ elf_machine_rel (struct link_map *map, c
>  
>    switch (r_type)
>      {
> +#if _MIPS_SIM == _MIPS_SIM_ABI64
> +    case (R_MIPS_64 << 8) | R_MIPS_REL32:
> +#else
>      case R_MIPS_REL32:
> +#endif
>        {
>  	int symidx = ELFW(R_SYM) (reloc->r_info);
> +	ElfW(Addr) reloc_value;
> +
> +	__builtin_memcpy (&reloc_value, reloc_addr, sizeof (reloc_value));

Why is this line needed?  I don't understand the need for it.
And why a memcpy and not just an assignment?  Please add a comment to
explain this line - and the other memcpy line below.

>  	if (symidx)
>  	  {
> @@ -501,10 +566,25 @@ elf_machine_rel (struct link_map *map, c
>  
>  	    if (symidx < gotsym)
>  	      {
> +		/* This wouldn't work for a symbol imported from other
> +		   libraries for which there's no GOT entry, but MIPS
> +		   requires every symbol referenced in a dynamic
> +		   relocation to have a GOT entry in the primary GOT,
> +		   so we only get here for locally-defined symbols.
> +		   For section symbols, we should *NOT* be adding
> +		   sym->st_value (per the definition of the meaning of
> +		   S in reloc expressions in the ELF64 MIPS ABI),
> +		   since it should have already been added to
> +		   reloc_value by the linker, but older versions of
> +		   GNU ld didn't add it, and newer versions don't emit
> +		   useless relocations to section symbols any more, so
> +		   it is safe to keep on adding sym->st_value, even
> +		   though it's not ABI compliant.  Some day we should
> +		   bite the bullet and stop doing this.  */
>  #ifndef RTLD_BOOTSTRAP
>  		if (map != &GL(dl_rtld_map))
>  #endif
> -		  *reloc_addr += sym->st_value + map->l_addr;
> +		  reloc_value += sym->st_value + map->l_addr;
>  	      }
>  	    else
>  	      {
> @@ -515,7 +595,7 @@ elf_machine_rel (struct link_map *map, c
>  		  = (const ElfW(Word))
>  		    map->l_info[DT_MIPS (LOCAL_GOTNO)]->d_un.d_val;
>  
> -		*reloc_addr += got[symidx + local_gotno - gotsym];
> +		reloc_value += got[symidx + local_gotno - gotsym];
>  #endif
>  	      }
>  	  }
> @@ -523,11 +603,26 @@ elf_machine_rel (struct link_map *map, c
>  #ifndef RTLD_BOOTSTRAP
>  	  if (map != &GL(dl_rtld_map))
>  #endif
> -	    *reloc_addr += map->l_addr;
> +	    reloc_value += map->l_addr;
> +
> +	__builtin_memcpy (reloc_addr, &reloc_value, sizeof (reloc_value));
>        }
>        break;
>      case R_MIPS_NONE:		/* Alright, Wilbur.  */
>        break;
> +#if _MIPS_SIM == _MIPS_SIM_ABI64
> +    case R_MIPS_64:
> +      /* For full compliance with the ELF64 ABI, one must precede the
> +	 _REL32/_64 pair of relocations with a _64 relocation, such
> +	 that the in-place addend is read as a 64-bit value.  IRIX
> +	 didn't pick up on this requirement, so we treat the
> +	 _REL32/_64 relocation as a 64-bit relocation even if it's by
> +	 itself.  For ABI compliance, we ignore such _64 dummy
> +	 relocations.  */
> +      if (ELFW(R_SYM) (reloc->r_info) == 0)
> +	break;
> +      /* Fall through.  */
> +#endif
>      default:
>        _dl_reloc_bad_type (map, r_type, 0);
>        break;
> @@ -547,6 +642,104 @@ elf_machine_lazy_rel (struct link_map *m
>  {
>    /* Do nothing.  */
>  }
> +
> +#if ! ELF_MACHINE_NO_RELA

I suggest to leave this out completly since it's not used anywhere.

> +/* Perform the relocation specified by RELOC and SYM (which is fully resolved).
> +   MAP is the object containing the reloc.  */
> +
> +static inline void
> +#ifdef RTLD_BOOTSTRAP
> +  __attribute__ ((always_inline))
> +#endif
> +elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
> +		  const ElfW(Sym) *sym, const struct r_found_version *version,
> +		  ElfW(Addr) *const reloc_addr)
> +{
> +  const unsigned long int r_type = ELFW(R_TYPE) (reloc->r_info);
> +
> +#if !defined RTLD_BOOTSTRAP && !defined SHARED
> +  /* This is defined in rtld.c, but nowhere in the static libc.a;
> +     make the reference weak so static programs can still link.  This
> +     declaration cannot be done when compiling rtld.c (i.e.  #ifdef
> +     RTLD_BOOTSTRAP) because rtld.c contains the common defn for
> +     _dl_rtld_map, which is incompatible with a weak decl in the same
> +     file.  */
> +  weak_extern (GL(dl_rtld_map));
> +#endif
> +
> +  switch (r_type)
> +    {
> +#if _MIPS_SIM == _MIPS_SIM_ABI64
> +    case (R_MIPS_64 << 8) | R_MIPS_REL32:
> +#else
> +    case R_MIPS_REL32:
> +#endif
> +      {
> +	int symidx = ELFW(R_SYM) (reloc->r_info);
> +	ElfW(Addr) reloc_value = reloc->r_addend;
> +
> +	if (symidx)
> +	  {
> +	    const ElfW(Word) gotsym
> +	      = (const ElfW(Word)) map->l_info[DT_MIPS (GOTSYM)]->d_un.d_val;
> +
> +	    if (symidx < gotsym)
> +	      {
> +#ifndef RTLD_BOOTSTRAP
> +		if (map != &GL(dl_rtld_map))
> +#endif
> +		  reloc_value += sym->st_value + map->l_addr;
> +	      }
> +	    else
> +	      {
> +#ifndef RTLD_BOOTSTRAP
> +		const ElfW(Addr) *got
> +		  = (const ElfW(Addr) *) D_PTR (map, l_info[DT_PLTGOT]);
> +		const ElfW(Word) local_gotno
> +		  = (const ElfW(Word))
> +		    map->l_info[DT_MIPS (LOCAL_GOTNO)]->d_un.d_val;
> +
> +		reloc_value += got[symidx + local_gotno - gotsym];
> +#endif
> +	      }
> +	  }
> +	else
> +#ifndef RTLD_BOOTSTRAP
> +	  if (map != &GL(dl_rtld_map))
> +#endif
> +	    reloc_value += map->l_addr;
> +
> +	__builtin_memcpy (reloc_addr, &reloc_value, sizeof (reloc_value));
> +      }
> +      break;
> +    case R_MIPS_NONE:		/* Alright, Wilbur.  */
> +      break;
> +#if _MIPS_SIM == _MIPS_SIM_ABI64
> +    case R_MIPS_64:
> +      /* There's no point in handling the artificial R_MIPS_64
> +	 relocations for RELA relocations: the addend is not in-place
> +	 anyway.  */
> +#endif
> +    default:
> +      _dl_reloc_bad_type (map, r_type, 0);
> +      break;
> +    }
> +}
> +
> +static inline void
> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
> +			   ElfW(Addr) *const reloc_addr)
> +{
> +  /* XXX Nothing to do.  There is no relative relocation, right?  */
> +}
> +
> +static inline void
> +elf_machine_lazy_rela (struct link_map *map,
> +		       ElfW(Addr) l_addr, const ElfW(Rela) *reloc)
> +{
> +  /* Do nothing.  */
> +}
> +#endif /* ! ELF_MACHINE_NO_RELA */


Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj at suse dot de
   private aj at arthur dot inka dot de
    http://www.suse.de/~aj


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