This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Various SH fixes; make R_SH_REL32 partial_inplace etc.
- To: <kkojima at rr dot iij4u dot or dot jp>, <binutils at sources dot redhat dot com>
- Subject: Various SH fixes; make R_SH_REL32 partial_inplace etc.
- From: Hans-Peter Nilsson <hp at bitrange dot com>
- Date: Mon, 17 Sep 2001 02:57:15 -0400 (EDT)
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