Re: Various SH fixes; make R_SH_REL32 partial_inplace etc.

[kaz Kojima <kkojima at rr dot iij4u dot or dot jp>]

Hans-Peter Nilsson <hp@bitrange.com> wrote:
> 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.

Now I'm building binutils with your patch. I'll report the result
of tests on glibc and X in which the relocation problem was found,
ASAP.

> 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.

Your change is the Right Thing though the current dynamic linker is
using r_addend for these relocations. I think that we can make ld.so
to handle old/new such relocations at the same time and avoid the binary
compatibility problem in sh-linux. I'll try it.

> 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.

I'd like to look these failures in detail.
Thank you for your great work.

Regards,
	kaz