[PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Carlos O'Donell-6
Joseph,

While running glibc on 32-bit ARM hardware with multiarch enabled,
VFP ABI, but no NEON, almost the entire testsuite fails with
SIGILL.

Debugging shows glibc trying to execute the NEON optimized
routines although no NEON is present and the kernel has indicated
that via the HWCAP.

This is because ARM's dl-machine.h fails to pass dl_hwcap to the
IFUNC resolver function for REL relocs in elf_machine_rel.

The RELA case was fixed by Will Newton here:
http://sourceware.org/ml/libc-ports/2013-07/msg00000.html

Verified by building on ARM with no regressions.

OK to checkin?

ports/Changelog.arm

2013-08-28  Kyle McMartin  <[hidden email]>
            Carlos O'Donell  <[hidden email]>

        * sysdeps/arm/dl-machine [!RTLD_BOOTSTRAP] (elf_machine_rel):
        Pass GLRO(dl_hwcap) to the IFUNC resolver.

diff --git a/ports/sysdeps/arm/dl-machine.h b/ports/sysdeps/arm/dl-machine.h
index d251527..85dba67 100644
--- a/ports/sysdeps/arm/dl-machine.h
+++ b/ports/sysdeps/arm/dl-machine.h
@@ -503,7 +503,7 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
          break;
        case R_ARM_IRELATIVE:
          value = map->l_addr + *reloc_addr;
-         value = ((Elf32_Addr (*) (void)) value) ();
+         value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
          *reloc_addr = value;
          break;
 #endif
---

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Joseph Myers
On Wed, 28 Aug 2013, Carlos O'Donell wrote:

> 2013-08-28  Kyle McMartin  <[hidden email]>
>    Carlos O'Donell  <[hidden email]>
>
> * sysdeps/arm/dl-machine [!RTLD_BOOTSTRAP] (elf_machine_rel):
> Pass GLRO(dl_hwcap) to the IFUNC resolver.

OK, given a bug filed in Bugzilla and a corresponding [BZ #N] notation and
entry in NEWS for the fixed bug.  Have you verified that all ARM instances
of such code now consistently pass the HWCAP value?

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Carlos O'Donell-6
On 08/28/2013 12:14 PM, Joseph S. Myers wrote:

> On Wed, 28 Aug 2013, Carlos O'Donell wrote:
>
>> 2013-08-28  Kyle McMartin  <[hidden email]>
>>    Carlos O'Donell  <[hidden email]>
>>
>> * sysdeps/arm/dl-machine [!RTLD_BOOTSTRAP] (elf_machine_rel):
>> Pass GLRO(dl_hwcap) to the IFUNC resolver.
>
> OK, given a bug filed in Bugzilla and a corresponding [BZ #N] notation and
> entry in NEWS for the fixed bug.  

I created BZ# 15905.

Sorry for forgetting, and thanks for reminding me.

I've checked in the patch which fixes the failures we're
seeing in Fedora on this hardware.

> Have you verified that all ARM instances
> of such code now consistently pass the HWCAP value?

Yes.

There are 4 places that require dl_hwcap to be used:
* ifunc-impl-list.c (__libc_ifunc_impl_list) - Already uses dl_hwcap.

* dl-irel.h (elf_ifunc_invoke) - Already uses dl_hwcap. Was fixed by
  Richard Henderson in commit 73da6bacf (along with other instances
  in dl-machine.h).

* dl-machine.h (elf_machine_rel) - Fixed by this patch.

* dl-machine.h (elf_machine_rela) - Fixed by Will Newton's patch.

These are the only place that I know about that require using
dl_hwcap and all of them are now fixed.

The uses are inconsistent in their use of `unsigned long int'
vs. 'int' for the dl_hwcap parameter, but changing that could
be a future cleanup.

I will note that the 32-bit ARM testsuite on this hardware is
not clean e.g.
 
make[2]: *** [/home/codonell/build/math/test-fenv.out] Error 1
make[1]: *** [math/tests] Error 2
make[2]: *** [/home/codonell/build/stdio-common/bug22.out] Error 1
make[1]: *** [stdio-common/tests] Error 2
make[2]: [/home/codonell/build/posix/annexc.out] Error 1 (ignored)
make[2]: *** [/home/codonell/build/nptl/tst-cleanup2.out] Error 1
make[2]: *** [/home/codonell/build/nptl/tst-cleanupx2.out] Error 1
make[1]: *** [nptl/tests] Error 2
make[2]: [/home/codonell/build/conform/run-conformtest.out] Error 1 (ignored)
make[2]: *** [/home/codonell/build/elf/ifuncmain5picstatic.out] Error 139
make[1]: *** [elf/tests] Error 2
make[1]: *** [/home/codonell/build/check-local-headers.out] Error 1
make: *** [check] Error 2

In particular ifuncmain5picstatic.out looks troublesome, and
I need to look at it to see if something is further wrong with
the IFUNC support.

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Will Newton
On 29 August 2013 05:16, Carlos O'Donell <[hidden email]> wrote:

> I will note that the 32-bit ARM testsuite on this hardware is
> not clean e.g.
>
> make[2]: *** [/home/codonell/build/math/test-fenv.out] Error 1

This always seems to have failed, not sure if anyone is actively
looking at fixing it.

> make[1]: *** [math/tests] Error 2
> make[2]: *** [/home/codonell/build/stdio-common/bug22.out] Error 1
> make[1]: *** [stdio-common/tests] Error 2
> make[2]: [/home/codonell/build/posix/annexc.out] Error 1 (ignored)
> make[2]: *** [/home/codonell/build/nptl/tst-cleanup2.out] Error 1
> make[2]: *** [/home/codonell/build/nptl/tst-cleanupx2.out] Error 1
> make[1]: *** [nptl/tests] Error 2
> make[2]: [/home/codonell/build/conform/run-conformtest.out] Error 1 (ignored)

I don't see these failures on Ubuntu raring, but I do see some others
that you haven't listed here:

make[2]: *** [/home/linaro/glibc-build/nptl/tst-align2.out] Error 1
make[2]: *** [/home/linaro/glibc-build/nptl/tst-getpid1.out] Error 1
make[2]: *** [/home/linaro/glibc-build/nptl/tst-getpid2.out] Error 1

I haven't had time to investigate these unfortunately.

> make[2]: *** [/home/codonell/build/elf/ifuncmain5picstatic.out] Error 139
> make[1]: *** [elf/tests] Error 2
> make[1]: *** [/home/codonell/build/check-local-headers.out] Error 1
> make: *** [check] Error 2
>
> In particular ifuncmain5picstatic.out looks troublesome, and
> I need to look at it to see if something is further wrong with
> the IFUNC support.

It's a binutils bug that is fixed in binutils trunk and the 2.23
branch, but not yet in any releases.

https://sourceware.org/git/?p=binutils.git;a=commitdiff;h=f8d29fea418654937fc6f22d817a4eb97159528d

--
Will Newton
Toolchain Working Group, Linaro
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Joseph Myers
In reply to this post by Carlos O'Donell-6
On Thu, 29 Aug 2013, Carlos O'Donell wrote:

> make[2]: *** [/home/codonell/build/math/test-fenv.out] Error 1

See wiki <https://sourceware.org/glibc/wiki/Release/2.18> - I presume this
is a system with VFPv3/VFPv4.

> make[2]: *** [/home/codonell/build/stdio-common/bug22.out] Error 1

See wiki - I presume this system has too little memory for this test (bug
14231).

> make[2]: [/home/codonell/build/posix/annexc.out] Error 1 (ignored)

System-independent.

> make[2]: *** [/home/codonell/build/nptl/tst-cleanup2.out] Error 1
> make[2]: *** [/home/codonell/build/nptl/tst-cleanupx2.out] Error 1

I haven't seen these.

> make[2]: [/home/codonell/build/conform/run-conformtest.out] Error 1 (ignored)

System-independent.

> make[2]: *** [/home/codonell/build/elf/ifuncmain5picstatic.out] Error 139
> make[1]: *** [/home/codonell/build/check-local-headers.out] Error 1

I haven't seen these.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Joseph Myers
In reply to this post by Will Newton
On Thu, 29 Aug 2013, Will Newton wrote:

> make[2]: *** [/home/linaro/glibc-build/nptl/tst-align2.out] Error 1
> make[2]: *** [/home/linaro/glibc-build/nptl/tst-getpid1.out] Error 1
> make[2]: *** [/home/linaro/glibc-build/nptl/tst-getpid2.out] Error 1

I haven't seen these failures.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Will Newton
On 29 August 2013 13:25, Joseph S. Myers <[hidden email]> wrote:
> On Thu, 29 Aug 2013, Will Newton wrote:
>
>> make[2]: *** [/home/linaro/glibc-build/nptl/tst-align2.out] Error 1
>> make[2]: *** [/home/linaro/glibc-build/nptl/tst-getpid1.out] Error 1
>> make[2]: *** [/home/linaro/glibc-build/nptl/tst-getpid2.out] Error 1
>
> I haven't seen these failures.

I had a look at these and it looks like there is a problem with clone
when it's built as Thumb. I've submitted a patch.

--
Will Newton
Toolchain Working Group, Linaro
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.

Joseph Myers
In reply to this post by Carlos O'Donell-6
On Thu, 29 Aug 2013, Carlos O'Donell wrote:

> I created BZ# 15905.
>
> Sorry for forgetting, and thanks for reminding me.
>
> I've checked in the patch which fixes the failures we're
> seeing in Fedora on this hardware.

I think this fix is also appropriate for 2.18 branch.

--
Joseph S. Myers
[hidden email]