diff mbox

ARM: Fix R_ARM_IRELATIVE RELA relocations

Message ID 20140430162520.7d7eb209@octopus
State Committed
Headers

Commit Message

Julian Brown April 30, 2014, 3:25 p.m. UTC 
  Hi,

This patch fixes what I believe to be a bug in the handling of
R_ARM_IRELATIVE RELA relocations. At present, these are handled the
same as REL relocations: i.e. the addend is loaded from the relocation
address. Most of the time this isn't a problem because RELA relocations
aren't used on ARM (GNU/Linux at least) anyway, but it causes problems
with prelink, which uses RELA on all targets for its conflict table.
(Support for ifunc prelinking requires a prelink patch, not yet posted.)

Anyway, this patch works, though I'm not 100% sure if it is correct: I
notice that this code path received attention last year:

https://sourceware.org/ml/libc-ports/2013-07/msg00000.html

I'm not sure under what circumstances that patch would have had an
effect, nor if my patch conflicts with that case.

No regressions using Mentor's usual glibc cross-testing infrastructure.
OK to apply?

(Strangely, this appears to be my first glibc patch, so I'm not
entirely certain if I have write access!)

Thanks,

Julian

ChangeLog

    * sysdeps/arm/dl-machine.h (elf_machine_rela): Fix R_ARM_IRELATIVE
    handling.

Comments

Joseph Myers April 30, 2014, 3:44 p.m. UTC | #1 
On Wed, 30 Apr 2014, Julian Brown wrote:

> Hi,
> 
> This patch fixes what I believe to be a bug in the handling of
> R_ARM_IRELATIVE RELA relocations. At present, these are handled the

Please file a bug in glibc Bugzilla for this bug (glibc policy is that if 
fixing a bug that's user-visible in a release, it should first be filed in 
Bugzilla), then I'll commit the patch and close the bug.
Will Newton April 30, 2014, 3:50 p.m. UTC | #2 
On 30 April 2014 16:25, Julian Brown <julian@codesourcery.com> wrote:
> Hi,
>
> This patch fixes what I believe to be a bug in the handling of
> R_ARM_IRELATIVE RELA relocations. At present, these are handled the
> same as REL relocations: i.e. the addend is loaded from the relocation
> address. Most of the time this isn't a problem because RELA relocations
> aren't used on ARM (GNU/Linux at least) anyway, but it causes problems
> with prelink, which uses RELA on all targets for its conflict table.
> (Support for ifunc prelinking requires a prelink patch, not yet posted.)
>
> Anyway, this patch works, though I'm not 100% sure if it is correct: I
> notice that this code path received attention last year:
>
> https://sourceware.org/ml/libc-ports/2013-07/msg00000.html
>
> I'm not sure under what circumstances that patch would have had an
> effect, nor if my patch conflicts with that case.

I can't really explain what's happened there, I must have found that
issue by reading the code rather than running it. Very odd. Your patch
looks correct however.
Julian Brown April 30, 2014, 4:09 p.m. UTC | #3 
On Wed, 30 Apr 2014 15:44:23 +0000
"Joseph S. Myers" <joseph@codesourcery.com> wrote:

> On Wed, 30 Apr 2014, Julian Brown wrote:
> 
> > Hi,
> > 
> > This patch fixes what I believe to be a bug in the handling of
> > R_ARM_IRELATIVE RELA relocations. At present, these are handled the
> 
> Please file a bug in glibc Bugzilla for this bug (glibc policy is
> that if fixing a bug that's user-visible in a release, it should
> first be filed in Bugzilla), then I'll commit the patch and close the
> bug.

I created:

https://sourceware.org/bugzilla/show_bug.cgi?id=16888

Thanks,

Julian
diff mbox

Patch

diff --git a/sysdeps/arm/dl-machine.h b/sysdeps/arm/dl-machine.h
index 02d1a5e..899b256 100644
--- a/sysdeps/arm/dl-machine.h
+++ b/sysdeps/arm/dl-machine.h
@@ -594,7 +594,7 @@  elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
 	    }
 	  break;
 	case R_ARM_IRELATIVE:
-	  value = map->l_addr + *reloc_addr;
+	  value = map->l_addr + reloc->r_addend;
 	  value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
 	  *reloc_addr = value;
 	  break;