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]

Various SH fixes; make R_SH_REL32 partial_inplace etc.


After some investigation, it seems your original patches at
<URL:http://sources.redhat.com/ml/binutils/2001-01/msg00071.html> are
mostly correct.  The BFD_RELOC_32_PCREL/R_SH_REL32 reloc is emitted
by gas as a partial_inplace reloc: I can't see how the rela.r_addend for
that relocation can ever be emitted non-zero, and the addend is always
emitted in the data.  Therefore, it should be treated in the linker as a
partial_inplace reloc, as with your patch.  For compatibility, this seems
the best route; "ld -r" on BFD_RELOC_32_PCREL/R_SH_REL32 with non-zero
addends just doesn't work, so there doesn't seem to be a compatibility
issue with existing (non-sh-linux) objects treated with "ld -r".

The gas pcrel adjustment should be done in md_pcrel_from_section; I can't
see correctness in that patch.  The i386 reference does not apply; the
same change just accidentally works, and I'm not sure it works for all
cases.

Can I ask you to please test the patch below on an unpatched CVS binutils
checkout (or a snapshot), on glibc and dynamically linked programs you've
tested before with your previous patch?  I intend to check it in within a
week if no flaws are found.  I've composed a few test-cases from the
bug-reports as well.

Handling of R_SH_DIR32 and R_SH_REL32 for -shared looked strange and
wrong; rel->r_addend was used, which should always be zero.  I changed
that to dig out the in-place addends using bfd_get_32.  Those relocs are
probably rare enough in shlibs for the problem not to be experienced very
often.  Have test-case, will commit.

Does anybody successfully use relaxation and objects treated
with "ld -r"?  It looks like that wouldn't work.

For plain sh-elf, the gcc testsuite with sources as of a few days ago,
patched with your patch at
<URL:http://gcc.gnu.org/ml/gcc-patches/2001-09/msg00009.html>) have less
failures with the g++ test-suite (51 vs. 59).  There are a few regressions
though: g++.jason/template31.C, g++.mike/p658.C and g++.robertl/eb73.C.  I
think all of the differences are spurious, since the high number of
failures without the patch indicate a failure elsewhere.

gas:
2001-09-17  Hans-Peter Nilsson  <hp@bitrange.com>

	* config/tc-sh.c (md_pcrel_from_section): Transformed from
	md_pcrel_from.  Handle pc-relativeness against link-time
	symbol.  Handle relativeness to elsewhere than the fixup.

bfd:
2001-09-17  kaz Kojima  <kkojima@rr.iij4u.or.jp>
            Hans-Peter Nilsson  <hp@bitrange.com>

	* elf32-sh.c (sh_elf_howto_table, R_SH_REL32): Make
	partial_inplace, matching assembler output.  Set src_mask to
	ones.
	(sh_elf_relocate_section): Delete misplaced comment.
	For relocatable linking against section symbol, call
	_bfd_relocate_contents for partial_inplace relocs and adjust
	rel->r_addend for others.
	<case R_SH_DIR32, R_SH_REL32>: Fetch partial_inplace addend with
	bfd_get_32, not at rel->r_addend.

Index: tc-sh.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-sh.c,v
retrieving revision 1.41
diff -p -c -r1.41 tc-sh.c
*** gas/config/tc-sh.c	2001/09/15 14:49:54	1.41
--- gas/config/tc-sh.c	2001/09/17 02:20:42
*************** md_number_to_chars (ptr, use, nbytes)
*** 3110,3118 ****
  }

  long
! md_pcrel_from (fixP)
       fixS *fixP;
  {
    return fixP->fx_size + fixP->fx_where + fixP->fx_frag->fr_address + 2;
  }

--- 3110,3132 ----
  }

  long
! md_pcrel_from_section (fixP, sec)
       fixS *fixP;
+      segT sec;
  {
+   if (fixP->fx_addsy != (symbolS *) NULL
+       && (! S_IS_DEFINED (fixP->fx_addsy)
+ 	  || S_IS_EXTERN (fixP->fx_addsy)
+ 	  || S_IS_WEAK (fixP->fx_addsy)
+ 	  || S_GET_SEGMENT (fixP->fx_addsy) != sec))
+     {
+       /* The symbol is undefined (or is defined but not in this section,
+ 	 or we're not sure about it being the final definition).  Let the
+ 	 linker figure it out.  We need to adjust the subtraction of a
+ 	 symbol to the position of the relocated data, though.  */
+       return fixP->fx_subsy ? fixP->fx_where + fixP->fx_frag->fr_address : 0;
+     }
+
    return fixP->fx_size + fixP->fx_where + fixP->fx_frag->fr_address + 2;
  }

Index: tc-sh.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-sh.h,v
retrieving revision 1.13
diff -p -c -r1.13 tc-sh.h
*** gas/config/tc-sh.h	2001/09/15 14:49:54	1.13
--- gas/config/tc-sh.h	2001/09/17 02:20:42
*************** extern boolean sh_fix_adjustable PARAMS
*** 76,81 ****
--- 76,84 ----
  #define TC_FIX_ADJUSTABLE(fixP) obj_fix_adjustable (fixP)
  #endif

+ #define MD_PCREL_FROM_SECTION(FIXP, SEC) md_pcrel_from_section (FIXP, SEC)
+ extern long md_pcrel_from_section PARAMS ((struct fix *, segT));
+
  #define IGNORE_NONSTANDARD_ESCAPES

  #define LISTING_HEADER (shl ? "Hitachi Super-H GAS Little Endian" : "Hitachi Super-H GAS Big Endian")
Index: elf32-sh.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-sh.c,v
retrieving revision 1.31
diff -p -c -r1.31 elf32-sh.c
*** bfd/elf32-sh.c	2001/08/26 18:03:19	1.31
--- bfd/elf32-sh.c	2001/09/17 06:10:09
*************** static reloc_howto_type sh_elf_howto_tab
*** 134,141 ****
  	 complain_overflow_signed, /* complain_on_overflow */
  	 sh_elf_ignore_reloc,	/* special_function */
  	 "R_SH_REL32",		/* name */
! 	 false,			/* partial_inplace */
! 	 0,			/* src_mask */
  	 0xffffffff,		/* dst_mask */
  	 true),			/* pcrel_offset */

--- 134,141 ----
  	 complain_overflow_signed, /* complain_on_overflow */
  	 sh_elf_ignore_reloc,	/* special_function */
  	 "R_SH_REL32",		/* name */
! 	 true,			/* partial_inplace */
! 	 0xffffffff,		/* src_mask */
  	 0xffffffff,		/* dst_mask */
  	 true),			/* pcrel_offset */

*************** sh_elf_relocate_section (output_bfd, inf
*** 3019,3026 ****
  	}

        howto = sh_elf_howto_table + r_type;

-       /* This is a final link.  */
        h = NULL;
        sym = NULL;
        sec = NULL;
--- 3019,3030 ----
  	}

        howto = sh_elf_howto_table + r_type;
+
+       /* For relocs that aren't partial_inplace, we get the addend from
+          the relocation.  */
+       if (! howto->partial_inplace)
+ 	addend = rel->r_addend;

        h = NULL;
        sym = NULL;
        sec = NULL;
*************** sh_elf_relocate_section (output_bfd, inf
*** 3040,3046 ****
  		 section symbol winds up in the output section.  */
  	      sym = local_syms + r_symndx;
  	      if (ELF_ST_TYPE (sym->st_info) == STT_SECTION)
! 		goto final_link_relocate;

  	      continue;
  	    }
--- 3044,3075 ----
  		 section symbol winds up in the output section.  */
  	      sym = local_syms + r_symndx;
  	      if (ELF_ST_TYPE (sym->st_info) == STT_SECTION)
! 		{
! 		  if (! howto->partial_inplace)
! 		    {
! 		      /* For relocations with the addend in the
! 			 relocation, we need just to update the addend.
! 			 All real relocs are of type partial_inplace; this
! 			 code is mostly for completeness.  */
! 		      rel->r_addend += sec->output_offset + sym->st_value;
!
! 		      continue;
! 		    }
!
! 		  /* Relocs of type partial_inplace need to pick up the
! 		     contents in the contents and add the offset resulting
! 		     from the changed location of the section symbol.
! 		     Using _bfd_final_link_relocate (e.g. goto
! 		     final_link_relocate) here would be wrong, because
! 		     relocations marked pc_relative would get the current
! 		     location subtracted, and we must only do that at the
! 		     final link.  */
! 		  r = _bfd_relocate_contents (howto, input_bfd,
! 					      sec->output_offset
! 					      + sym->st_value,
! 					      contents + rel->r_offset);
! 		  goto relocation_done;
! 		}

  	      continue;
  	    }
*************** sh_elf_relocate_section (output_bfd, inf
*** 3245,3251 ****
  		  BFD_ASSERT (h != NULL && h->dynindx != -1);
  		  relocate = false;
  		  outrel.r_info = ELF32_R_INFO (h->dynindx, R_SH_REL32);
! 		  outrel.r_addend = rel->r_addend;
  		}
  	      else
  		{
--- 3274,3281 ----
  		  BFD_ASSERT (h != NULL && h->dynindx != -1);
  		  relocate = false;
  		  outrel.r_info = ELF32_R_INFO (h->dynindx, R_SH_REL32);
! 		  outrel.r_addend
! 		    = bfd_get_32 (input_bfd, contents + rel->r_offset);
  		}
  	      else
  		{
*************** sh_elf_relocate_section (output_bfd, inf
*** 3258,3271 ****
  		    {
  		      relocate = true;
  		      outrel.r_info = ELF32_R_INFO (0, R_SH_RELATIVE);
! 		      outrel.r_addend = relocation + rel->r_addend;
  		    }
  		  else
  		    {
  		      BFD_ASSERT (h->dynindx != -1);
  		      relocate = false;
  		      outrel.r_info = ELF32_R_INFO (h->dynindx, R_SH_DIR32);
! 		      outrel.r_addend = relocation + rel->r_addend;
  		    }
  		}

--- 3288,3305 ----
  		    {
  		      relocate = true;
  		      outrel.r_info = ELF32_R_INFO (0, R_SH_RELATIVE);
! 		      outrel.r_addend
! 			= relocation + bfd_get_32 (input_bfd,
! 						   contents + rel->r_offset);
  		    }
  		  else
  		    {
  		      BFD_ASSERT (h->dynindx != -1);
  		      relocate = false;
  		      outrel.r_info = ELF32_R_INFO (h->dynindx, R_SH_DIR32);
! 		      outrel.r_addend
! 			= relocation + bfd_get_32 (input_bfd,
! 						   contents + rel->r_offset);
  		    }
  		}

*************** sh_elf_relocate_section (output_bfd, inf
*** 3282,3289 ****
  	      if (! relocate)
  		continue;
  	    }
- 	  else if (r_type == R_SH_DIR32)
- 	    addend = rel->r_addend;
  	  goto final_link_relocate;

  	case R_SH_GOT32:
--- 3316,3321 ----
*************** sh_elf_relocate_section (output_bfd, inf
*** 3463,3468 ****
--- 3495,3501 ----
  	  }
  	}

+     relocation_done:
        if (r != bfd_reloc_ok)
  	{
  	  switch (r)

brgds, H-P


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