public inbox for libc-ports@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
@ 2013-08-28  4:23 Carlos O'Donell
  2013-08-28 16:14 ` Joseph S. Myers
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos O'Donell @ 2013-08-28  4:23 UTC (permalink / raw)
  To: libc-ports, Joseph S. Myers; +Cc: Kyle McMartin

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  <kmcmarti@redhat.com>
	    Carlos O'Donell  <carlos@redhat.com>

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-28  4:23 [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs Carlos O'Donell
@ 2013-08-28 16:14 ` Joseph S. Myers
  2013-08-29  4:17   ` Carlos O'Donell
  0 siblings, 1 reply; 8+ messages in thread
From: Joseph S. Myers @ 2013-08-28 16:14 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: libc-ports, Kyle McMartin

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

> 2013-08-28  Kyle McMartin  <kmcmarti@redhat.com>
> 	    Carlos O'Donell  <carlos@redhat.com>
> 
> 	* 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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-28 16:14 ` Joseph S. Myers
@ 2013-08-29  4:17   ` Carlos O'Donell
  2013-08-29 10:00     ` Will Newton
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Carlos O'Donell @ 2013-08-29  4:17 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: libc-ports, Kyle McMartin

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  <kmcmarti@redhat.com>
>> 	    Carlos O'Donell  <carlos@redhat.com>
>>
>> 	* 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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-29  4:17   ` Carlos O'Donell
@ 2013-08-29 10:00     ` Will Newton
  2013-08-29 12:25       ` Joseph S. Myers
  2013-08-29 12:24     ` Joseph S. Myers
  2013-08-29 19:27     ` Joseph S. Myers
  2 siblings, 1 reply; 8+ messages in thread
From: Will Newton @ 2013-08-29 10:00 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: Joseph S. Myers, libc-ports, Kyle McMartin

On 29 August 2013 05:16, Carlos O'Donell <carlos@redhat.com> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-29  4:17   ` Carlos O'Donell
  2013-08-29 10:00     ` Will Newton
@ 2013-08-29 12:24     ` Joseph S. Myers
  2013-08-29 19:27     ` Joseph S. Myers
  2 siblings, 0 replies; 8+ messages in thread
From: Joseph S. Myers @ 2013-08-29 12:24 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: libc-ports, Kyle McMartin

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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-29 10:00     ` Will Newton
@ 2013-08-29 12:25       ` Joseph S. Myers
  2013-08-29 19:18         ` Will Newton
  0 siblings, 1 reply; 8+ messages in thread
From: Joseph S. Myers @ 2013-08-29 12:25 UTC (permalink / raw)
  To: Will Newton; +Cc: Carlos O'Donell, libc-ports, Kyle McMartin

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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-29 12:25       ` Joseph S. Myers
@ 2013-08-29 19:18         ` Will Newton
  0 siblings, 0 replies; 8+ messages in thread
From: Will Newton @ 2013-08-29 19:18 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Carlos O'Donell, libc-ports, Kyle McMartin

On 29 August 2013 13:25, Joseph S. Myers <joseph@codesourcery.com> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs.
  2013-08-29  4:17   ` Carlos O'Donell
  2013-08-29 10:00     ` Will Newton
  2013-08-29 12:24     ` Joseph S. Myers
@ 2013-08-29 19:27     ` Joseph S. Myers
  2 siblings, 0 replies; 8+ messages in thread
From: Joseph S. Myers @ 2013-08-29 19:27 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: libc-ports, Kyle McMartin

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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-08-29 19:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28  4:23 [PATCH] arm: Fix R_ARM_IRELATIVE for REL relocs Carlos O'Donell
2013-08-28 16:14 ` Joseph S. Myers
2013-08-29  4:17   ` Carlos O'Donell
2013-08-29 10:00     ` Will Newton
2013-08-29 12:25       ` Joseph S. Myers
2013-08-29 19:18         ` Will Newton
2013-08-29 12:24     ` Joseph S. Myers
2013-08-29 19:27     ` Joseph S. Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).