elf32-arm.c corrections

elf32-arm.c corrections

[Peter S. Mazinger]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 534 bytes --]

Hello!

add_dynamic_entry: changes !info->shared to info->executable (PIE)
corrects typo, and syncs up with other archs (some others could do the 
same). For !relocs the hole part would be omitted, probably some speed gain.

Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
ppc32)?

Why is ELIMINATE_COPY_RELOCS not used for arm?

Thanks, Peter

Please cc to me, 

-- 
Peter S. Mazinger <ps dot m at gmx dot net>           ID: 0xA5F059F2
Key fingerprint = 92A4 31E1 56BC 3D5A 2D08  BB6E C389 975E A5F0 59F2

[-- Attachment #2: Type: TEXT/PLAIN, Size: 724 bytes --]

--- bfd/elf32-arm.c.mps	2005-03-19 22:57:32 +0100
+++ bfd/elf32-arm.c	2005-03-19 23:05:34 +0100
@@ -4658,6 +4658,19 @@
       if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
 	  && h->root.type == bfd_link_hash_undefweak)
 	eh->relocs_copied = NULL;
+
+      /* Make sure undefined weak symbols are output as a dynamic symbol
+	 in PIEs.  */
+      if (info->pie
+	  && eh->relocs_copied != NULL
+	  && h->dynindx == -1
+	  && h->root.type == bfd_link_hash_undefweak
+	  && !h->forced_local)
+	{
+	  if (! bfd_elf_link_record_dynamic_symbol (info, h))
+	    return FALSE;
+	}
+
       else if (htab->root.is_relocatable_executable && h->dynindx == -1
 	       && h->root.type == bfd_link_hash_new)
 	{

[-- Attachment #3: Type: TEXT/PLAIN, Size: 1338 bytes --]

--- bfd/elf32-arm.c.mps	2005-03-19 22:57:32 +0100
+++ bfd/elf32-arm.c	2005-03-19 23:05:34 +0100
@@ -4926,7 +4926,7 @@
 #define add_dynamic_entry(TAG, VAL) \
   _bfd_elf_add_dynamic_entry (info, TAG, VAL)
 
-      if (!info->shared)
+      if (info->executable)
 	{
 	  if (!add_dynamic_entry (DT_DEBUG, 0))
 	    return FALSE;
@@ -4947,22 +4947,21 @@
 	      || !add_dynamic_entry (DT_RELSZ, 0)
 	      || !add_dynamic_entry (DT_RELENT, sizeof (Elf32_External_Rel)))
 	    return FALSE;
-	}
 
-      /* If any dynamic relocs apply to a read-only section,
-	 then we need a DT_TEXTREL entry.  */
-      if ((info->flags & DF_TEXTREL) == 0)
-	elf_link_hash_traverse (&htab->root, elf32_arm_readonly_dynrelocs,
+	  /* If any dynamic relocs apply to a read-only section,
+	     then we need a DT_TEXTREL entry.  */
+	  if ((info->flags & DF_TEXTREL) == 0)
+	    elf_link_hash_traverse (&htab->root, elf32_arm_readonly_dynrelocs,
 				(PTR) info);
 
-      if ((info->flags & DF_TEXTREL) != 0)
-	{
-	  if (!add_dynamic_entry (DT_TEXTREL, 0))
-	    return FALSE;
-	  info->flags |= DF_TEXTREL;
+	  if ((info->flags & DF_TEXTREL) != 0)
+	    {
+	      if (!add_dynamic_entry (DT_TEXTREL, 0))
+		return FALSE;
+	    }
 	}
     }
-#undef add_synamic_entry
+#undef add_dynamic_entry
 
   return TRUE;
 }

Re: elf32-arm.c corrections

[Daniel Jacobowitz]

On Sun, Mar 20, 2005 at 02:15:24AM +0100, Peter S. Mazinger wrote:
> Hello!
> 
> add_dynamic_entry: changes !info->shared to info->executable (PIE)
> corrects typo, and syncs up with other archs (some others could do the 
> same). For !relocs the hole part would be omitted, probably some speed gain.

Um, why are disabling the setting of DT_TEXTREL for shared libraries?

> Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
> ppc32)?

Probably.

> Why is ELIMINATE_COPY_RELOCS not used for arm?

Because no one implemented it.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: elf32-arm.c corrections

[Peter S. Mazinger]

On Sat, 19 Mar 2005, Daniel Jacobowitz wrote:

> On Sun, Mar 20, 2005 at 02:15:24AM +0100, Peter S. Mazinger wrote:
> > Hello!
> > 
> > add_dynamic_entry: changes !info->shared to info->executable (PIE)
> > corrects typo, and syncs up with other archs (some others could do the 
> > same). For !relocs the hole part would be omitted, probably some speed gain.
> 
> Um, why are disabling the setting of DT_TEXTREL for shared libraries?

The 1 line removal is because none of the archs has that.
The i386 implementation has everything within if (relocs), the other 
archs have it outside, but DT_TEXTREL is only valid for if (relocs), so we 
would omit that part gaining some speed

> 
> > Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
> > ppc32)?
> 
> Probably.

I am asking it, because no other arch has that, only ppc32, so there must 
be some other solution to that as well.

> 
> > Why is ELIMINATE_COPY_RELOCS not used for arm?
> 
> Because no one implemented it.

Would the implementation make the binaries smaller?

Peter

-- 
Peter S. Mazinger <ps dot m at gmx dot net>           ID: 0xA5F059F2
Key fingerprint = 92A4 31E1 56BC 3D5A 2D08  BB6E C389 975E A5F0 59F2

Re: elf32-arm.c corrections

[Daniel Jacobowitz]

On Sun, Mar 20, 2005 at 10:26:12AM +0100, Peter S. Mazinger wrote:
> On Sat, 19 Mar 2005, Daniel Jacobowitz wrote:
> 
> > On Sun, Mar 20, 2005 at 02:15:24AM +0100, Peter S. Mazinger wrote:
> > > Hello!
> > > 
> > > add_dynamic_entry: changes !info->shared to info->executable (PIE)
> > > corrects typo, and syncs up with other archs (some others could do the 
> > > same). For !relocs the hole part would be omitted, probably some speed gain.
> > 
> > Um, why are disabling the setting of DT_TEXTREL for shared libraries?
> 
> The 1 line removal is because none of the archs has that.
> The i386 implementation has everything within if (relocs), the other 
> archs have it outside, but DT_TEXTREL is only valid for if (relocs), so we 
> would omit that part gaining some speed

Ah, not enough context in the diff.  Makes sense.

> > > Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
> > > ppc32)?
> > 
> > Probably.
> 
> I am asking it, because no other arch has that, only ppc32, so there must 
> be some other solution to that as well.
> 
> > 
> > > Why is ELIMINATE_COPY_RELOCS not used for arm?
> > 
> > Because no one implemented it.
> 
> Would the implementation make the binaries smaller?

Not appreciably.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: elf32-arm.c corrections

[Peter S. Mazinger]

On Sun, 20 Mar 2005, Daniel Jacobowitz wrote:

> On Sun, Mar 20, 2005 at 10:26:12AM +0100, Peter S. Mazinger wrote:
> > On Sat, 19 Mar 2005, Daniel Jacobowitz wrote:
> > 
> > > On Sun, Mar 20, 2005 at 02:15:24AM +0100, Peter S. Mazinger wrote:
> > > > Hello!
> > > > 
> > > > add_dynamic_entry: changes !info->shared to info->executable (PIE)
> > > > corrects typo, and syncs up with other archs (some others could do the 
> > > > same). For !relocs the hole part would be omitted, probably some speed gain.
> > > 
> > > Um, why are disabling the setting of DT_TEXTREL for shared libraries?
> > 
> > The 1 line removal is because none of the archs has that.
> > The i386 implementation has everything within if (relocs), the other 
> > archs have it outside, but DT_TEXTREL is only valid for if (relocs), so we 
> > would omit that part gaining some speed
> 
> Ah, not enough context in the diff.  Makes sense.

Please apply then, I have signed aggreement, no cvs access


Thanks, Peter

> 
> > > > Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
> > > > ppc32)?
> > > 
> > > Probably.
> > 
> > I am asking it, because no other arch has that, only ppc32, so there must 
> > be some other solution to that as well.
> > 
> > > 
> > > > Why is ELIMINATE_COPY_RELOCS not used for arm?
> > > 
> > > Because no one implemented it.
> > 
> > Would the implementation make the binaries smaller?
> 
> Not appreciably.
> 
> 

-- 
Peter S. Mazinger <ps dot m at gmx dot net>           ID: 0xA5F059F2
Key fingerprint = 92A4 31E1 56BC 3D5A 2D08  BB6E C389 975E A5F0 59F2

Re: elf32-arm.c corrections

[Daniel Jacobowitz]

On Sat, Mar 26, 2005 at 06:27:08PM +0100, Peter S. Mazinger wrote:
> On Sun, 20 Mar 2005, Daniel Jacobowitz wrote:
> 
> > On Sun, Mar 20, 2005 at 10:26:12AM +0100, Peter S. Mazinger wrote:
> > > On Sat, 19 Mar 2005, Daniel Jacobowitz wrote:
> > > 
> > > > On Sun, Mar 20, 2005 at 02:15:24AM +0100, Peter S. Mazinger wrote:
> > > > > Hello!
> > > > > 
> > > > > add_dynamic_entry: changes !info->shared to info->executable (PIE)
> > > > > corrects typo, and syncs up with other archs (some others could do the 
> > > > > same). For !relocs the hole part would be omitted, probably some speed gain.
> > > > 
> > > > Um, why are disabling the setting of DT_TEXTREL for shared libraries?
> > > 
> > > The 1 line removal is because none of the archs has that.
> > > The i386 implementation has everything within if (relocs), the other 
> > > archs have it outside, but DT_TEXTREL is only valid for if (relocs), so we 
> > > would omit that part gaining some speed
> > 
> > Ah, not enough context in the diff.  Makes sense.
> 
> Please apply then, I have signed aggreement, no cvs access

I don't approve ARM patches, sorry.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: elf32-arm.c corrections

[Peter S. Mazinger]

On Sat, 26 Mar 2005, Daniel Jacobowitz wrote:

> On Sat, Mar 26, 2005 at 06:27:08PM +0100, Peter S. Mazinger wrote:
> > On Sun, 20 Mar 2005, Daniel Jacobowitz wrote:
> > 
> > > On Sun, Mar 20, 2005 at 10:26:12AM +0100, Peter S. Mazinger wrote:
> > > > On Sat, 19 Mar 2005, Daniel Jacobowitz wrote:
> > > > 
> > > > > On Sun, Mar 20, 2005 at 02:15:24AM +0100, Peter S. Mazinger wrote:
> > > > > > Hello!
> > > > > > 
> > > > > > add_dynamic_entry: changes !info->shared to info->executable (PIE)
> > > > > > corrects typo, and syncs up with other archs (some others could do the 
> > > > > > same). For !relocs the hole part would be omitted, probably some speed gain.
> > > > > 
> > > > > Um, why are disabling the setting of DT_TEXTREL for shared libraries?
> > > > 
> > > > The 1 line removal is because none of the archs has that.
> > > > The i386 implementation has everything within if (relocs), the other 
> > > > archs have it outside, but DT_TEXTREL is only valid for if (relocs), so we 
> > > > would omit that part gaining some speed
> > > 
> > > Ah, not enough context in the diff.  Makes sense.
> > 
> > Please apply then, I have signed aggreement, no cvs access
> 
> I don't approve ARM patches, sorry.

Ok, sorry, don't know how the procedure is, who than?

Peter

-- 
Peter S. Mazinger <ps dot m at gmx dot net>           ID: 0xA5F059F2
Key fingerprint = 92A4 31E1 56BC 3D5A 2D08  BB6E C389 975E A5F0 59F2

Re: elf32-arm.c corrections

[Nick Clifton]

Hi Peter,

> Ok, sorry, don't know how the procedure is, who than?

Richard Earnshaw is the primary maintainer for the ARM port of the 
binutils project.  If you ping him and send a link to the patch I am 
sure that he will review it.  If you have any problems though you can 
also ping me and I will review it.

Cheers
   Nick

Re: elf32-arm.c corrections

[Richard Earnshaw]

On Sun, 2005-03-20 at 01:15, Peter S. Mazinger wrote:
> Hello!
> 
> add_dynamic_entry: changes !info->shared to info->executable (PIE)
> corrects typo, and syncs up with other archs (some others could do the 
> same). For !relocs the hole part would be omitted, probably some speed gain.
> 
> Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
> ppc32)?
> 
> Why is ELIMINATE_COPY_RELOCS not used for arm?
> 
> Thanks, Peter
> 
> Please cc to me, 

Sorry for the delay replying, I've been snowed under with other work of
late.

I think this is probably OK, but it's hard to be sure because you
haven't followed all the procedures.  When submitting a patch you need
to do all of the following (as a minimum):

1) Explain what you are trying to do.  Is this a bug that you've found?
If so, provide a testcase.  Is it an optimization?  If so, why is the
existing code non-optimal.  Try to avoid diving in with too much detail.
2) Describe the patch (you've done that, though it could be a little
clearer).
3) Provide a ChangeLog entry describing the mechanics of the patch
(functions that have been changed and how).
4) Provide a patch produced with 'diff -p' or 'diff -up' format (the -p
option is essential so that I can see which functions are being changed
from the patch).
5) State which configurations you've tested the patch on.

Please can you supply the additional information.

R.

Re: elf32-arm.c corrections

[Peter S. Mazinger]

On Mon, 4 Apr 2005, Richard Earnshaw wrote:

> On Sun, 2005-03-20 at 01:15, Peter S. Mazinger wrote:
> > Hello!
> > 
> > add_dynamic_entry: changes !info->shared to info->executable (PIE)
> > corrects typo, and syncs up with other archs (some others could do the 
> > same). For !relocs the hole part would be omitted, probably some speed gain.
> > 
> > Should the other patch (*3) for allocate_dynrelocs be applied (as done for 
> > ppc32)?
> > 
> > Why is ELIMINATE_COPY_RELOCS not used for arm?
> > 
> > Thanks, Peter
> > 
> > Please cc to me, 
> 
> Sorry for the delay replying, I've been snowed under with other work of
> late.
> 
> I think this is probably OK, but it's hard to be sure because you
> haven't followed all the procedures.  When submitting a patch you need
> to do all of the following (as a minimum):

it was only sent to discuss it, because I can't be sure that it is ok, I 
am looking since about 1-2 month at differences between arm and other 
archs, because it had a false TEXTREL entry in all shared libs. This is 
solved in the mean time. My intention is to get arm in sync w/ other 
archs, to support PIE as well (currently not supported), see another patch 
from me.
 
> 1) Explain what you are trying to do.  Is this a bug that you've found?
> If so, provide a testcase.  Is it an optimization?  If so, why is the
> existing code non-optimal.  Try to avoid diving in with too much detail.

optimization (shorter exec due to moving it to if (relocs) and typo 
correction.

> 2) Describe the patch (you've done that, though it could be a little
> clearer).
> 3) Provide a ChangeLog entry describing the mechanics of the patch
> (functions that have been changed and how).

Is it approved? I will.

> 4) Provide a patch produced with 'diff -p' or 'diff -up' format (the -p
> option is essential so that I can see which functions are being changed
> from the patch).
didn't know that -p is needed, sorry

> 5) State which configurations you've tested the patch on.

works natively on arm v4 (uclibc based)

Thanks, Peter

-- 
Peter S. Mazinger <ps dot m at gmx dot net>           ID: 0xA5F059F2
Key fingerprint = 92A4 31E1 56BC 3D5A 2D08  BB6E C389 975E A5F0 59F2

Re: elf32-arm.c corrections

[Richard Earnshaw]

On Mon, 2005-04-04 at 15:27, Peter S. Mazinger wrote:
> it was only sent to discuss it, because I can't be sure that it is ok, I 
> am looking since about 1-2 month at differences between arm and other 
> archs, because it had a false TEXTREL entry in all shared libs. This is 
> solved in the mean time. My intention is to get arm in sync w/ other 
> archs, to support PIE as well (currently not supported), see another patch 
> from me.
>  

Asking if it is OK is, in effect, asking me to review it.  If you are
asking for a general 'this is a work-in-progress, but is this the right
approach' comments then please put RFC in the subject line.

> > 2) Describe the patch (you've done that, though it could be a little
> > clearer).
> > 3) Provide a ChangeLog entry describing the mechanics of the patch
> > (functions that have been changed and how).
> 
> Is it approved? I will.

You miss the point.  It can't be approved until that is provided.  It's
needed as part of the review process.

> > 5) State which configurations you've tested the patch on.
> 
> works natively on arm v4 (uclibc based)

You still haven't provided the configuration string that you used to
configure the tools (arm-elf? arm-coff?).

Please don't make me try to guess what you know, you'll end up wasting
both my and your time in the long run.

R.