Patch to allow ARM dynamic linker to fix up PC24 relocs...

Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Scott Bambrough]

The attached patch adds a cache flush just after the readonly protection
is restored to .text.  I've done this in generic code, because the cache
flush is an extremely expensive operation on the ARM.  We don't want to
do this after every fixup, and only if we do fixups.  The patch as
written should only affect the ARM (or anyone else who defines
CLEAR_CACHE).  For all other targets it will disappear after
pre-processing.  This patch has the fix for the problem Phil Blundell
pointed out.  I've tested it and it seems to work.

1999-12-21  Scott Bambrough  <scottb@netwinder.org>

        * sysdeps/arm/dl-machine.h (CLEAR_CACHE): New macro to force a
        cache flush.
        * elf/dl-reloc.c (_dl_relocate_object): Add call to flush cache
        after .text segment fixups.

Scott

-- 
Scott Bambrough - Software Engineer
REBEL.COM    http://www.rebel.com
NetWinder    http://www.netwinder.org
diff -upr glibc-2.1.2.orig/elf/dl-reloc.c glibc-2.1.2/elf/dl-reloc.c
--- glibc-2.1.2.orig/elf/dl-reloc.c	Sat Feb 20 09:45:53 1999
+++ glibc-2.1.2/elf/dl-reloc.c	Fri Dec 10 10:36:33 1999
@@ -126,6 +126,10 @@ _dl_relocate_object (struct link_map *l,
 	    if (__mprotect (mapstart, mapend - mapstart, prot) < 0)
 	      _dl_signal_error (errno, l->l_name,
 				"can't restore segment prot after reloc");
+
+#ifdef CLEAR_CACHE
+	    CLEAR_CACHE (mapstart, mapend);
+#endif 
 	  }
     }
 }
diff -upr glibc-2.1.2.orig/sysdeps/arm/dl-machine.h glibc-2.1.2/sysdeps/arm/dl-machine.h
--- glibc-2.1.2.orig/sysdeps/arm/dl-machine.h	Tue Aug 10 01:13:22 1999
+++ glibc-2.1.2/sysdeps/arm/dl-machine.h	Fri Dec 10 10:36:57 1999
@@ -35,6 +35,14 @@
   VALID_ELF_ABIVERSION(hdr[EI_ABIVERSION]) \
 )
 
+#define CLEAR_CACHE(BEG,END)						\
+{									\
+  register unsigned long _beg __asm ("a1") = (unsigned long)(BEG);	\
+  register unsigned long _end __asm ("a2") = (unsigned long)(END);	\
+  register unsigned long _flg __asm ("a3") = 0;			\
+  __asm __volatile ("swi 0x9f0002");					\
+}
+
 /* Return nonzero iff E_MACHINE is compatible with the running host.  */
 static inline int __attribute__ ((unused))
 elf_machine_matches_host (Elf32_Half e_machine)

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Ulrich Drepper]

Scott Bambrough <scottb@netwinder.org> writes:

> The attached patch adds a cache flush just after the readonly protection
> is restored to .text.  I've done this in generic code, because the cache
> flush is an extremely expensive operation on the ARM.  We don't want to
> do this after every fixup, and only if we do fixups.  The patch as
> written should only affect the ARM (or anyone else who defines
> CLEAR_CACHE).  For all other targets it will disappear after
> pre-processing.  This patch has the fix for the problem Phil Blundell
> pointed out.  I've tested it and it seems to work.

I've already added this code to 2.1.3.  Could you update your tree?

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Scott Bambrough]

Ulrich Drepper wrote:
 
> I've already added this code to 2.1.3.  Could you update your tree?

You never posted a message saying you did.  I assumed you would wait
after Phil pointed out the error in the patch.  The macro CLEAR_CACHE in
dl-machine.h is incorrect.   This patch corrects it.  It's a simple fix.

Thanks 

Scott

-- 
Scott Bambrough - Software Engineer
REBEL.COM    http://www.rebel.com
NetWinder    http://www.netwinder.org

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Scott Bambrough]

Philip Blundell wrote:
> 
> >+  register unsigned long _beg __asm ("a1") = (unsigned long)(BEG);    \
> >+  register unsigned long _end __asm ("a2") = (unsigned long)((END) - (BEG));\
> >+  register unsigned long _flg __asm ("a3") = 0;
> 
> Did you check that this works properly?  I just inspected the kernel code
> again and it still looks to me like R1 is supposed to actually be the end
> address (and the comment in traps.c is wrong).

Yes, I checked.  It works with my test cases, and Mozilla.  I posted a
question on the ARM Linux list about the second parameter and got two
replys.  One from Russell, saying it was a length, the other from you
saying it was the end address.  The comment in traps.c led me to believe
it was a length.  It was 2-1 in favour of the length.

It's a good thing you are persistent.  I just went and checked the
actual SA110 assembler code for clear_cache_area.  The comment there
says it is the end address.  Inspection of the code verifies it is
correct.  My code was working however, because start - length > 32K. 
This triggers a call to flush_cache_all instead of just those cache
lines.  I'll fix it up and resubmit the patch.

Shouldn't have been so lazy, and checked the source before I guess. 
Thanks Phil.

Cheers,
 
Scott

-- 
Scott Bambrough - Software Engineer
REBEL.COM    http://www.rebel.com
NetWinder    http://www.netwinder.org

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Scott Bambrough]

> >This will only happen during program startup.  The code in
> >question is not in the Icache yet.
> 
> Yes; the danger is that on a system with a writeback Dcache and/or a store
> buffer, the modified data might not get flushed out to main memory before
> the Icache fill happens.

Seems Phil and Richard are both right.  The attached patch adds a cache
flush just after the readonly protection is restored to .text.  I've
done this in generic code, because the cache flush is an extremely
expensive operation on the ARM.  We don't want to do this after every
fixup, and only if we do fixups.  The patch as written should only
affect the ARM (or anyone else who defines CLEAR_CACHE).  For all other
targets it will disappear after pre-processing.

1999-12-17  Scott Bambrough  <scottb@netwinder.org>

	* sysdeps/arm/dl-machine.h (CLEAR_CACHE): New macro to force a
	cache flush.
	* elf/dl-reloc.c (_dl_relocate_object): Add call to flush cache
	after .text segment fixups.

Scott

-- 
Scott Bambrough - Software Engineer
REBEL.COM    http://www.rebel.com
NetWinder    http://www.netwinder.org
diff -upr glibc-2.1.2.orig/elf/dl-reloc.c glibc-2.1.2/elf/dl-reloc.c
--- glibc-2.1.2.orig/elf/dl-reloc.c	Sat Feb 20 09:45:53 1999
+++ glibc-2.1.2/elf/dl-reloc.c	Fri Dec 10 10:36:33 1999
@@ -126,6 +126,10 @@ _dl_relocate_object (struct link_map *l,
 	    if (__mprotect (mapstart, mapend - mapstart, prot) < 0)
 	      _dl_signal_error (errno, l->l_name,
 				"can't restore segment prot after reloc");
+
+#ifdef CLEAR_CACHE
+	    CLEAR_CACHE (mapstart, mapend);
+#endif 
 	  }
     }
 }
diff -upr glibc-2.1.2.orig/sysdeps/arm/dl-machine.h glibc-2.1.2/sysdeps/arm/dl-machine.h
--- glibc-2.1.2.orig/sysdeps/arm/dl-machine.h	Tue Aug 10 01:13:22 1999
+++ glibc-2.1.2/sysdeps/arm/dl-machine.h	Fri Dec 10 10:36:57 1999
@@ -35,6 +35,14 @@
   VALID_ELF_ABIVERSION(hdr[EI_ABIVERSION]) \
 )
 
+#define CLEAR_CACHE(BEG,END)						\
+{									\
+  register unsigned long _beg __asm ("a1") = (unsigned long)(BEG);	\
+  register unsigned long _end __asm ("a2") = (unsigned long)((END) - (BEG));\
+  register unsigned long _flg __asm ("a3") = 0;				\
+  __asm __volatile ("swi 0x9f0002");					\
+}
+
 /* Return nonzero iff E_MACHINE is compatible with the running host.  */
 static inline int __attribute__ ((unused))
 elf_machine_matches_host (Elf32_Half e_machine)

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Ulrich Drepper]

Scott Bambrough <scottb@netwinder.org> writes:

> The attached patch allows the ARM dynamic linker to fix up PC24 relocs
> in the code segment.

Thanks, I've applied it.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Richard Henderson]

On Tue, Dec 07, 1999 at 08:12:20PM +0000, Philip Blundell wrote:
> This is fine as far as it goes, but you probably want to add code to 
> synchronise the StrongARM caches if we are going to get into modifying the 
> text segment.

This will only happen during program startup.  The code in
question is not in the Icache yet.


r~

Re: Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Philip Blundell]

>	* sysdeps/arm/dl-machine.h (elf_machine_rel): Fixup R_ARM_PC24
> 	  relocs if possible.

This is fine as far as it goes, but you probably want to add code to 
synchronise the StrongARM caches if we are going to get into modifying the 
text segment.

p.

Patch to allow ARM dynamic linker to fix up PC24 relocs...

[Scott Bambrough]

Hi guys,

The attached patch allows the ARM dynamic linker to fix up PC24 relocs
in the code segment.  A bit of background is attached.

The PC24 reloc is generated for ARM branch and branch and link
instructions (b,bl).  The least significant 24 bits are used to for a 26
bit offset (the offset is modulo 4).  The address calculation performed
by the CPU is as follows:

PC = PC + (SignExtend(<24_bit_signed_offset>) << 2;

If the calculated 26 bit offset is out of range, the dynamic linker
displays an error message like the following:

foo_dynamic: error in loading shared libraries: ./foo.so: R_ARM_PC24
relocation out of range

1999-12-07  Scott Bambrough <scottb@netwinder.org>

	* sysdeps/arm/dl-machine.h (elf_machine_rel): Fixup R_ARM_PC24
 	  relocs if possible.

-- 
Scott Bambrough - Software Engineer
REBEL.COM    http://www.rebel.com
NetWinder    http://www.netwinder.org
--- dl-machine.h.orig	Fri Nov 26 08:56:05 1999
+++ dl-machine.h	Tue Dec  7 14:04:13 1999
@@ -446,6 +446,23 @@ elf_machine_rel (struct link_map *map, c
 	    *reloc_addr += value;
 	    break;
 	  }
+	case R_ARM_PC24:
+	  {
+	     signed int addend;
+
+	     addend = *reloc_addr & 0x00ffffff;
+	     if (addend & 0x00800000) addend |= 0xff000000;
+
+	     value = value - (unsigned int)reloc_addr + (addend << 2);
+	     if (value & 0xfc000003)
+	       _dl_signal_error (0, map->l_name,
+			  "R_ARM_PC24 relocation out of range");
+	       
+	     value = value >> 2;
+	     value = (*reloc_addr & 0xff000000) | (value & 0x00ffffff);
+	     *reloc_addr = value;
+	  }
+	break;
 	default:
 	  assert (! "unexpected dynamic reloc type");
 	  break;