public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: 32-bit PowerPC sdata linker problem
       [not found] <53918356.3040102@embedded-brains.de>
@ 2014-06-06 10:54 ` Alan Modra
  2014-06-06 11:23   ` Sebastian Huber
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2014-06-06 10:54 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: binutils

On Fri, Jun 06, 2014 at 11:01:10AM +0200, Sebastian Huber wrote:
> I performed a git bisect and found this:
> 
> 93d1b056cb396d6468781fe0e40dd769891bed32 is the first bad commit
> commit 93d1b056cb396d6468781fe0e40dd769891bed32
> Author: Alan Modra <amodra@gmail.com>
> Date:   Tue May 20 11:42:42 2014 +0930
> 
>     Rewrite ppc32 backend .sdata and .sdata2 handling

Hmm, I'm surprised that your git bisect found this patch.  Was
_SDA_BASE_ set differently before this?

>                 0x00000000000dfc00                _SDA_BASE_
>                 0x00000000000d7f78                ppc_exc_lock_std

>      4b8:       28 05 00 00     cmplwi  r5,0
>                         4ba: R_PPC_SDAREL16     ppc_exc_lock_std

ppc_exc_lock_std@sdarel will be calculating 0xd7f78 - 0xdfc00
which is 0xf...fff8378, and that falls foul of

commit 86c9573369616e7437481b6e5533aef3a435cdcf
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Mar 8 13:05:06 2014 +1030

    Better overflow checking for powerpc32 relocations

cmplwi has an *unsigned* 16-bit field, and we now check the overflow
properly.

I wonder how many more of these we'll hit, and whether the uproar will
be enough that I'll be forced to relax the checks?

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-06 10:54 ` 32-bit PowerPC sdata linker problem Alan Modra
@ 2014-06-06 11:23   ` Sebastian Huber
  2014-06-06 11:31     ` Sebastian Huber
  2014-06-06 12:17     ` Alan Modra
  0 siblings, 2 replies; 14+ messages in thread
From: Sebastian Huber @ 2014-06-06 11:23 UTC (permalink / raw)
  To: binutils

On 2014-06-06 12:54, Alan Modra wrote:
> On Fri, Jun 06, 2014 at 11:01:10AM +0200, Sebastian Huber wrote:
>> I performed a git bisect and found this:
>>
>> 93d1b056cb396d6468781fe0e40dd769891bed32 is the first bad commit
>> commit 93d1b056cb396d6468781fe0e40dd769891bed32
>> Author: Alan Modra <amodra@gmail.com>
>> Date:   Tue May 20 11:42:42 2014 +0930
>>
>>      Rewrite ppc32 backend .sdata and .sdata2 handling
>
> Hmm, I'm surprised that your git bisect found this patch.  Was
> _SDA_BASE_ set differently before this?

A diff -u map.ok map.error yields:

@@ -6549,10 +6550,11 @@
                  0x0000000000008000                PROVIDE (_SDA_BASE_, 0x8000)
   *(.sdata .sdata.* .gnu.linkonce.s.*)
   .sdata         0x00000000000d7c00        0x0 
/opt/rtems-4.11/lib64/gcc/powerpc-rtems4.11/4.8.3/m603e/ecrti.o
-                0x00000000000d7c00                _SDA_BASE_
                  0x00000000000d7c00                __SDATA_START__
   .sdata         0x00000000000d7c00        0x4 
/opt/rtems-4.11/lib64/gcc/powerpc-rtems4.11/4.8.3/m603e/crtbegin.o
                  0x00000000000d7c00                __dso_handle
+                0x00000000000dfc00                _SDA_BASE_
+ .sdata         0x00000000000d7c04        0x0 
/opt/rtems-4.11/lib64/gcc/powerpc-rtems4.11/4.8.3/m603e/crtbegin.o
   .sdata         0x00000000000d7c04       0x4c b-br_uid/init.o
                  0x00000000000d7c0c                rtems_shell_Initial_aliases
                  0x00000000000d7c14 
_RTEMS_tasks_Initialize_user_tasks_p

>
>>                  0x00000000000dfc00                _SDA_BASE_
>>                  0x00000000000d7f78                ppc_exc_lock_std
>
>>       4b8:       28 05 00 00     cmplwi  r5,0
>>                          4ba: R_PPC_SDAREL16     ppc_exc_lock_std
>
> ppc_exc_lock_std@sdarel will be calculating 0xd7f78 - 0xdfc00
> which is 0xf...fff8378, and that falls foul of
>
> commit 86c9573369616e7437481b6e5533aef3a435cdcf
> Author: Alan Modra <amodra@gmail.com>
> Date:   Sat Mar 8 13:05:06 2014 +1030
>
>      Better overflow checking for powerpc32 relocations
>
> cmplwi has an *unsigned* 16-bit field, and we now check the overflow
> properly.
>
> I wonder how many more of these we'll hit, and whether the uproar will
> be enough that I'll be forced to relax the checks?
>

Ok, so this "cmplwi cr0, rX, ppc_exc_lock_std@sdarel" is illegal, since 
ppc_exc_lock_std@sdarel is signed and the immediate is unsigned 16-bit?  The 
assembler doesn't issue a warning about this.

Exists there a way to rescue this cmplwi hack without relaxing the overflow checks?

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-06 11:23   ` Sebastian Huber
@ 2014-06-06 11:31     ` Sebastian Huber
  2014-06-06 12:15       ` Alan Modra
  2014-06-06 12:17     ` Alan Modra
  1 sibling, 1 reply; 14+ messages in thread
From: Sebastian Huber @ 2014-06-06 11:31 UTC (permalink / raw)
  To: binutils

On 2014-06-06 13:23, Sebastian Huber wrote:
> Ok, so this "cmplwi cr0, rX, ppc_exc_lock_std@sdarel" is illegal, since
> ppc_exc_lock_std@sdarel is signed and the immediate is unsigned 16-bit?  The
> assembler doesn't issue a warning about this.
>
> Exists there a way to rescue this cmplwi hack without relaxing the overflow
> checks?

Hm, sorry, it was surprisingly simple.  This works:

"cmplwi cr0, rX, ppc_exc_lock_std@sdarel@l"

I was not aware that you can add several @ in a row.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-06 11:31     ` Sebastian Huber
@ 2014-06-06 12:15       ` Alan Modra
  2014-06-06 12:49         ` Sebastian Huber
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2014-06-06 12:15 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: binutils

On Fri, Jun 06, 2014 at 01:31:48PM +0200, Sebastian Huber wrote:
> On 2014-06-06 13:23, Sebastian Huber wrote:
> >Ok, so this "cmplwi cr0, rX, ppc_exc_lock_std@sdarel" is illegal, since
> >ppc_exc_lock_std@sdarel is signed and the immediate is unsigned 16-bit?  The
> >assembler doesn't issue a warning about this.
> >
> >Exists there a way to rescue this cmplwi hack without relaxing the overflow
> >checks?
> 
> Hm, sorry, it was surprisingly simple.  This works:
> 
> "cmplwi cr0, rX, ppc_exc_lock_std@sdarel@l"
> 
> I was not aware that you can add several @ in a row.

That is the wrong thing to use here.  sdarel@l translates to a VLE
reloc which applies to a split 16-bit field in VLE insns.

You want
 cmpwi cr0, rX, ppc_exc_lock_std@sdarel
to properly compare a 16-bit signed number from sym@sdarel.

Note that the assembler does error if you write something like
 cmplwi 3,-30000
or
 cmpwi 3,40000
so what the linker is now doing is extending this behaviour to link
time.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-06 11:23   ` Sebastian Huber
  2014-06-06 11:31     ` Sebastian Huber
@ 2014-06-06 12:17     ` Alan Modra
  2014-06-07 12:47       ` Alan Modra
  1 sibling, 1 reply; 14+ messages in thread
From: Alan Modra @ 2014-06-06 12:17 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: binutils

On Fri, Jun 06, 2014 at 01:23:43PM +0200, Sebastian Huber wrote:
> On 2014-06-06 12:54, Alan Modra wrote:
> >Hmm, I'm surprised that your git bisect found this patch.  Was
> >_SDA_BASE_ set differently before this?
> 
> A diff -u map.ok map.error yields:
> 
> @@ -6549,10 +6550,11 @@
>                  0x0000000000008000                PROVIDE (_SDA_BASE_, 0x8000)
>   *(.sdata .sdata.* .gnu.linkonce.s.*)
>   .sdata         0x00000000000d7c00        0x0
> /opt/rtems-4.11/lib64/gcc/powerpc-rtems4.11/4.8.3/m603e/ecrti.o
> -                0x00000000000d7c00                _SDA_BASE_
>                  0x00000000000d7c00                __SDATA_START__
>   .sdata         0x00000000000d7c00        0x4
> /opt/rtems-4.11/lib64/gcc/powerpc-rtems4.11/4.8.3/m603e/crtbegin.o
>                  0x00000000000d7c00                __dso_handle
> +                0x00000000000dfc00                _SDA_BASE_
> + .sdata         0x00000000000d7c04        0x0
> /opt/rtems-4.11/lib64/gcc/powerpc-rtems4.11/4.8.3/m603e/crtbegin.o
>   .sdata         0x00000000000d7c04       0x4c b-br_uid/init.o
>                  0x00000000000d7c0c                rtems_shell_Initial_aliases
>                  0x00000000000d7c14
> _RTEMS_tasks_Initialize_user_tasks_p

Ah, light dawns.  I'm guessing you still have a definition for
_SDA_BASE_ in your linker script, but using PROVIDE.  Due to the way I
implemented the automatic define of _SDA_BASE_, PROVIDE in a linker
script won't override the automatic define.  That's a bug.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-06 12:15       ` Alan Modra
@ 2014-06-06 12:49         ` Sebastian Huber
       [not found]           ` <20140606130559.GK5592@bubble.grove.modra.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Sebastian Huber @ 2014-06-06 12:49 UTC (permalink / raw)
  To: binutils

On 2014-06-06 14:15, Alan Modra wrote:
> On Fri, Jun 06, 2014 at 01:31:48PM +0200, Sebastian Huber wrote:
>> >On 2014-06-06 13:23, Sebastian Huber wrote:
>>> > >Ok, so this "cmplwi cr0, rX, ppc_exc_lock_std@sdarel" is illegal, since
>>> > >ppc_exc_lock_std@sdarel is signed and the immediate is unsigned 16-bit?  The
>>> > >assembler doesn't issue a warning about this.
>>> > >
>>> > >Exists there a way to rescue this cmplwi hack without relaxing the overflow
>>> > >checks?
>> >
>> >Hm, sorry, it was surprisingly simple.  This works:
>> >
>> >"cmplwi cr0, rX, ppc_exc_lock_std@sdarel@l"
>> >
>> >I was not aware that you can add several @ in a row.
> That is the wrong thing to use here.  sdarel@l translates to a VLE
> reloc which applies to a split 16-bit field in VLE insns.

Oh, yes.  This is not what I want.

>
> You want
>   cmpwi cr0, rX, ppc_exc_lock_std@sdarel
> to properly compare a 16-bit signed number from sym@sdarel.
>
> Note that the assembler does error if you write something like
>   cmplwi 3,-30000
> or
>   cmpwi 3,40000
> so what the linker is now doing is extending this behaviour to link
> time.

I actually need only the lower 16-bit of sym@sdarel here.  The code reads an 
opcode on a certain address and tests if the opcode is equal to "stw r1, 
ppc_exc_lock_std@sdarel(r13)".

/*
  *****************************************************************************
  * MACRO: TEST_1ST_OPCODE_crit
  *****************************************************************************
  *
  * USES:    REG, cr0
  * ON EXIT: REG available (contains *pc - STW_R1_R13(0)),
  *          return value in cr0.
  *
  * test opcode interrupted by critical (asynchronous) exception; set CR_LOCK if
  *
  *   *SRR0 == 'stw r1, ppc_exc_lock_std@sdarel(r13)'
  *
  */
	.macro	TEST_1ST_OPCODE_crit _REG

	lwz	\_REG, SRR0_FRAME_OFFSET(FRAME_REGISTER)
	lwz	\_REG, 0(\_REG)
	/*	opcode now in REG */

	/*	subtract upper 16bits of 'stw r1, 0(r13)' instruction */
	subis	\_REG, \_REG, STW_R1_R13(0)@h
	/*
	 * if what's left compares against the 'ppc_exc_lock_std@sdarel'
	 * address offset then we have a match...
	 */
	cmplwi	cr0, \_REG, ppc_exc_lock_std@sdarel

	.endm

I guess, I have to rewrite this a bit.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

* Re: 32-bit PowerPC sdata linker problem
       [not found]                 ` <5391CCFB.5060206@embedded-brains.de>
@ 2014-06-07 12:34                   ` Alan Modra
  2014-06-10  6:28                     ` Sebastian Huber
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2014-06-07 12:34 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: binutils

On Fri, Jun 06, 2014 at 04:15:23PM +0200, Sebastian Huber wrote:
> On 2014-06-06 15:49, Alan Modra wrote:
> >On Fri, Jun 06, 2014 at 03:23:52PM +0200, Sebastian Huber wrote:
> >>>On 2014-06-06 15:05, Alan Modra wrote:
> >>>> >On Fri, Jun 06, 2014 at 02:48:55PM +0200, Sebastian Huber wrote:
> >>>>> >>	cmplwi	cr0, \_REG, ppc_exc_lock_std@sdarel
> >>>>> >>
> >>>>> >>	.endm
> >>>>> >>
> >>>>> >>I guess, I have to rewrite this a bit.
> >>>> >
> >>>> >Doesn't using a cmpwi rather than cmplwi work?
> >>>> >
> >>>
> >>>No, the cmplwi uses 0x0000 || UIMM with the cmpwi we have
> >>>EXTS(SIMM), but the upper 16-bit must be zero so that the comparison
> >>>works in the macro.
> >Oh, of course.  Perhaps I should make cmpli accept both signed and
> >unsigned 16-bit fields.
> 
> I think our usage of this cmplwi with the implicit truncation from
> the linker is quite a hack.  On the other hand it worked for several
> years.

Applied.

bfd/
	* elf32-ppc.c (ppc_elf_relocate_section): Treat field of cmpli
	insn as a bitfield; Use complain_overflow_bitfield.
	* elf64-ppc.c (ppc64_elf_relocate_section): Likewise.
opcodes/
	* ppc-opc.c (UISIGNOPT): Define and use with cmpli.
gas/
	* config/tc-ppc.c (ppc_insert_operand): Handle PPC_OPERAND_SIGNOPT
	on unsigned fields.  Comment on PPC_OPERAND_SIGNOPT signed fields
	in 64-bit mode.
gold/
	* powerpc.cc (relocate): Treat field of cmpli insn as a bitfield.

-- 
Alan Modra
Australia Development Lab, IBM

diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 1bea6f8..344845d 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -9147,10 +9147,11 @@ ppc_elf_relocate_section (bfd *output_bfd,
 	      unsigned int insn;
 
 	      insn = bfd_get_32 (input_bfd, contents + (rel->r_offset & ~3));
-	      if ((insn & (0x3f << 26)) == 28u << 26 /* andi */
-		  || (insn & (0x3f << 26)) == 24u << 26 /* ori */
-		  || (insn & (0x3f << 26)) == 26u << 26 /* xori */
-		  || (insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
+	      if ((insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
+		complain = complain_overflow_bitfield;
+	      else if ((insn & (0x3f << 26)) == 28u << 26 /* andi */
+		       || (insn & (0x3f << 26)) == 24u << 26 /* ori */
+		       || (insn & (0x3f << 26)) == 26u << 26 /* xori */)
 		complain = complain_overflow_unsigned;
 	    }
 	  if (howto->complain_on_overflow != complain)
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index e7e2e39..b8d7465 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -14648,14 +14648,15 @@ ppc64_elf_relocate_section (bfd *output_bfd,
 	  enum complain_overflow complain = complain_overflow_signed;
 
 	  insn = bfd_get_32 (input_bfd, contents + (rel->r_offset & ~3));
-	  if (howto->rightshift == 0
-	      ? ((insn & (0x3f << 26)) == 28u << 26 /* andi */
-		 || (insn & (0x3f << 26)) == 24u << 26 /* ori */
-		 || (insn & (0x3f << 26)) == 26u << 26 /* xori */
-		 || (insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
-	      : ((insn & (0x3f << 26)) == 29u << 26 /* andis */
-		 || (insn & (0x3f << 26)) == 25u << 26 /* oris */
-		 || (insn & (0x3f << 26)) == 27u << 26 /* xoris */))
+	  if ((insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
+	    complain = complain_overflow_bitfield;
+	  else if (howto->rightshift == 0
+		   ? ((insn & (0x3f << 26)) == 28u << 26 /* andi */
+		      || (insn & (0x3f << 26)) == 24u << 26 /* ori */
+		      || (insn & (0x3f << 26)) == 26u << 26 /* xori */)
+		   : ((insn & (0x3f << 26)) == 29u << 26 /* andis */
+		      || (insn & (0x3f << 26)) == 25u << 26 /* oris */
+		      || (insn & (0x3f << 26)) == 27u << 26 /* xoris */))
 	    complain = complain_overflow_unsigned;
 	  if (howto->complain_on_overflow != complain)
 	    {
diff --git a/gas/config/tc-ppc.c b/gas/config/tc-ppc.c
index 2c8ce6a..ff4ea64 100644
--- a/gas/config/tc-ppc.c
+++ b/gas/config/tc-ppc.c
@@ -1781,10 +1781,23 @@ ppc_insert_operand (unsigned long insn,
   right = max & -max;
   min = 0;
 
-  if ((operand->flags & PPC_OPERAND_SIGNED) != 0)
+  if ((operand->flags & PPC_OPERAND_SIGNOPT) != 0)
+    {
+      /* Extend the allowed range for addis to [-65536, 65535].
+	 Similarly for some VLE high part insns.  For 64-bit it
+	 would be good to disable this for signed fields since the
+	 value is sign extended into the high 32 bits of the register.
+	 If the value is, say, an address, then we might care about
+	 the high bits.  However, gcc as of 2014-06 uses unsigned
+	 values when loading the high part of 64-bit constants using
+	 lis.
+	 Use the same extended range for cmpli, to allow at least
+	 [-32768, 65535].  */
+      min = ~max & -right;
+    }
+  else if ((operand->flags & PPC_OPERAND_SIGNED) != 0)
     {
-      if ((operand->flags & PPC_OPERAND_SIGNOPT) == 0)
-	max = (max >> 1) & -right;
+      max = (max >> 1) & -right;
       min = ~max & -right;
     }
 
diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index bd3994a..96432ed 100644
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -7409,14 +7409,15 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
       Insn insn = elfcpp::Swap<32, big_endian>::readval(iview);
 
       overflow = Reloc::CHECK_SIGNED;
-      if (overflow == Reloc::CHECK_LOW_INSN
-	  ? ((insn & (0x3f << 26)) == 28u << 26 /* andi */
-	     || (insn & (0x3f << 26)) == 24u << 26 /* ori */
-	     || (insn & (0x3f << 26)) == 26u << 26 /* xori */
-	     || (insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
-	  : ((insn & (0x3f << 26)) == 29u << 26 /* andis */
-	     || (insn & (0x3f << 26)) == 25u << 26 /* oris */
-	     || (insn & (0x3f << 26)) == 27u << 26 /* xoris */))
+      if ((insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
+	overflow = Reloc::CHECK_BITFIELD;
+      else if (overflow == Reloc::CHECK_LOW_INSN
+	       ? ((insn & (0x3f << 26)) == 28u << 26 /* andi */
+		  || (insn & (0x3f << 26)) == 24u << 26 /* ori */
+		  || (insn & (0x3f << 26)) == 26u << 26 /* xori */)
+	       : ((insn & (0x3f << 26)) == 29u << 26 /* andis */
+		  || (insn & (0x3f << 26)) == 25u << 26 /* oris */
+		  || (insn & (0x3f << 26)) == 27u << 26 /* xoris */))
 	overflow = Reloc::CHECK_UNSIGNED;
     }
 
diff --git a/opcodes/ppc-opc.c b/opcodes/ppc-opc.c
index 1d27961..a5cfe1a 100644
--- a/opcodes/ppc-opc.c
+++ b/opcodes/ppc-opc.c
@@ -654,8 +654,11 @@ const struct powerpc_operand powerpc_operands[] =
 #define UI TO + 1
   { 0xffff, 0, NULL, NULL, 0 },
 
+#define UISIGNOPT UI + 1
+  { 0xffff, 0, NULL, NULL, PPC_OPERAND_SIGNOPT },
+
   /* The IMM field in an SE_IM5 instruction.  */
-#define UI5 UI + 1
+#define UI5 UISIGNOPT + 1
   { 0x1f, 4, NULL, NULL, 0 },
 
   /* The OIMM field in an SE_OIM5 instruction.  */
@@ -3500,10 +3503,10 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 
 {"dozi",	OP(9),		OP_MASK,     M601,	PPCNONE,	{RT, RA, SI}},
 
-{"cmplwi",	OPL(10,0),	OPL_MASK,    PPCCOM,	PPCNONE,	{OBF, RA, UI}},
-{"cmpldi",	OPL(10,1),	OPL_MASK,    PPC64,	PPCNONE,	{OBF, RA, UI}},
-{"cmpli",	OP(10),		OP_MASK,     PPC,	PPCNONE,	{BF, L, RA, UI}},
-{"cmpli",	OP(10),		OP_MASK,     PWRCOM,	PPC,		{BF, RA, UI}},
+{"cmplwi",	OPL(10,0),	OPL_MASK,    PPCCOM,	PPCNONE,	{OBF, RA, UISIGNOPT}},
+{"cmpldi",	OPL(10,1),	OPL_MASK,    PPC64,	PPCNONE,	{OBF, RA, UISIGNOPT}},
+{"cmpli",	OP(10),		OP_MASK,     PPC,	PPCNONE,	{BF, L, RA, UISIGNOPT}},
+{"cmpli",	OP(10),		OP_MASK,     PWRCOM,	PPC,		{BF, RA, UISIGNOPT}},
 
 {"cmpwi",	OPL(11,0),	OPL_MASK,    PPCCOM,	PPCNONE,	{OBF, RA, SI}},
 {"cmpdi",	OPL(11,1),	OPL_MASK,    PPC64,	PPCNONE,	{OBF, RA, SI}},

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-06 12:17     ` Alan Modra
@ 2014-06-07 12:47       ` Alan Modra
  2014-06-16 11:21         ` Will Newton
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2014-06-07 12:47 UTC (permalink / raw)
  To: Sebastian Huber, binutils

On Fri, Jun 06, 2014 at 09:46:59PM +0930, Alan Modra wrote:
> Ah, light dawns.  I'm guessing you still have a definition for
> _SDA_BASE_ in your linker script, but using PROVIDE.  Due to the way I
> implemented the automatic define of _SDA_BASE_, PROVIDE in a linker
> script won't override the automatic define.  That's a bug.

Fixed like this.  Will fail to work if crt1.o happens to have a
.sdata section, due to the necessity of defining _SDA_BASE_ on the
first .sdata section.

	* ldexp.c (exp_fold_tree_1 <etree_provide>): Make PROVIDEd
	linker script symbol value override a built-in linker symbol.
diff --git a/ld/ldexp.c b/ld/ldexp.c
index d573fb7..5c4f8dd 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -1001,7 +1001,10 @@ exp_fold_tree_1 (etree_type *tree)
 	      if (h == NULL
 		  || (h->type != bfd_link_hash_new
 		      && h->type != bfd_link_hash_undefined
-		      && h->type != bfd_link_hash_common))
+		      && h->type != bfd_link_hash_common
+		      && !(h->type == bfd_link_hash_defined
+			   && (h->u.def.section->flags
+			       & SEC_LINKER_CREATED) != 0)))
 		{
 		  /* Do nothing.  The symbol was never referenced, or was
 		     defined by some object.  */

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-07 12:34                   ` Alan Modra
@ 2014-06-10  6:28                     ` Sebastian Huber
  0 siblings, 0 replies; 14+ messages in thread
From: Sebastian Huber @ 2014-06-10  6:28 UTC (permalink / raw)
  To: binutils

On 2014-06-07 14:34, Alan Modra wrote:
> On Fri, Jun 06, 2014 at 04:15:23PM +0200, Sebastian Huber wrote:
>> >On 2014-06-06 15:49, Alan Modra wrote:
>>> > >On Fri, Jun 06, 2014 at 03:23:52PM +0200, Sebastian Huber wrote:
>>>>> > >>>On 2014-06-06 15:05, Alan Modra wrote:
>>>>>>> > >>>> >On Fri, Jun 06, 2014 at 02:48:55PM +0200, Sebastian Huber wrote:
>>>>>>>>> > >>>>> >>	cmplwi	cr0, \_REG, ppc_exc_lock_std@sdarel
>>>>>>>>> > >>>>> >>
>>>>>>>>> > >>>>> >>	.endm
>>>>>>>>> > >>>>> >>
>>>>>>>>> > >>>>> >>I guess, I have to rewrite this a bit.
>>>>>>> > >>>> >
>>>>>>> > >>>> >Doesn't using a cmpwi rather than cmplwi work?
>>>>>>> > >>>> >
>>>>> > >>>
>>>>> > >>>No, the cmplwi uses 0x0000 || UIMM with the cmpwi we have
>>>>> > >>>EXTS(SIMM), but the upper 16-bit must be zero so that the comparison
>>>>> > >>>works in the macro.
>>> > >Oh, of course.  Perhaps I should make cmpli accept both signed and
>>> > >unsigned 16-bit fields.
>> >
>> >I think our usage of this cmplwi with the implicit truncation from
>> >the linker is quite a hack.  On the other hand it worked for several
>> >years.
> Applied.
>
> bfd/
> 	* elf32-ppc.c (ppc_elf_relocate_section): Treat field of cmpli
> 	insn as a bitfield; Use complain_overflow_bitfield.
> 	* elf64-ppc.c (ppc64_elf_relocate_section): Likewise.
> opcodes/
> 	* ppc-opc.c (UISIGNOPT): Define and use with cmpli.
> gas/
> 	* config/tc-ppc.c (ppc_insert_operand): Handle PPC_OPERAND_SIGNOPT
> 	on unsigned fields.  Comment on PPC_OPERAND_SIGNOPT signed fields
> 	in 64-bit mode.
> gold/
> 	* powerpc.cc (relocate): Treat field of cmpli insn as a bitfield.

Thanks, now it works again.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-07 12:47       ` Alan Modra
@ 2014-06-16 11:21         ` Will Newton
  2014-06-16 13:07           ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Will Newton @ 2014-06-16 11:21 UTC (permalink / raw)
  To: binutils, Alan Modra

On 7 June 2014 13:47, Alan Modra <amodra@gmail.com> wrote:

Hi Alan,

> On Fri, Jun 06, 2014 at 09:46:59PM +0930, Alan Modra wrote:
>> Ah, light dawns.  I'm guessing you still have a definition for
>> _SDA_BASE_ in your linker script, but using PROVIDE.  Due to the way I
>> implemented the automatic define of _SDA_BASE_, PROVIDE in a linker
>> script won't override the automatic define.  That's a bug.
>
> Fixed like this.  Will fail to work if crt1.o happens to have a
> .sdata section, due to the necessity of defining _SDA_BASE_ on the
> first .sdata section.
>
>         * ldexp.c (exp_fold_tree_1 <etree_provide>): Make PROVIDEd
>         linker script symbol value override a built-in linker symbol.
> diff --git a/ld/ldexp.c b/ld/ldexp.c
> index d573fb7..5c4f8dd 100644
> --- a/ld/ldexp.c
> +++ b/ld/ldexp.c
> @@ -1001,7 +1001,10 @@ exp_fold_tree_1 (etree_type *tree)
>               if (h == NULL
>                   || (h->type != bfd_link_hash_new
>                       && h->type != bfd_link_hash_undefined
> -                     && h->type != bfd_link_hash_common))
> +                     && h->type != bfd_link_hash_common
> +                     && !(h->type == bfd_link_hash_defined
> +                          && (h->u.def.section->flags
> +                              & SEC_LINKER_CREATED) != 0)))
>                 {
>                   /* Do nothing.  The symbol was never referenced, or was
>                      defined by some object.  */


This patch seems to break static ifunc links on ARM. I believe it is
caused by the default linker script providing symbols twice
(__rel_iplt_start, __rel_iplt_end) and the second definition now
overriding the first. At first glance that would appear to be sane
behaviour, but with the current linker script the result is a broken
binary. Any ideas on what the best fix would be?

Thanks,

  .rel.dyn        :
    {
      *(.rel.init)
      *(.rel.text .rel.text.* .rel.gnu.linkonce.t.*)
      *(.rel.fini)
      *(.rel.rodata .rel.rodata.* .rel.gnu.linkonce.r.*)
      *(.rel.data.rel.ro .rel.data.rel.ro.* .rel.gnu.linkonce.d.rel.ro.*)
      *(.rel.data .rel.data.* .rel.gnu.linkonce.d.*)
      *(.rel.tdata .rel.tdata.* .rel.gnu.linkonce.td.*)
      *(.rel.tbss .rel.tbss.* .rel.gnu.linkonce.tb.*)
      *(.rel.ctors)
      *(.rel.dtors)
      *(.rel.got)
      *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*)
      PROVIDE_HIDDEN (__rel_iplt_start = .);
      *(.rel.iplt)
      PROVIDE_HIDDEN (__rel_iplt_end = .);
      PROVIDE_HIDDEN (__rela_iplt_start = .);
      PROVIDE_HIDDEN (__rela_iplt_end = .);
    }
  .rela.dyn       :
    {
      *(.rela.init)
      *(.rela.text .rela.text.* .rela.gnu.linkonce.t.*)
      *(.rela.fini)
      *(.rela.rodata .rela.rodata.* .rela.gnu.linkonce.r.*)
      *(.rela.data .rela.data.* .rela.gnu.linkonce.d.*)
      *(.rela.tdata .rela.tdata.* .rela.gnu.linkonce.td.*)
      *(.rela.tbss .rela.tbss.* .rela.gnu.linkonce.tb.*)
      *(.rela.ctors)
      *(.rela.dtors)
      *(.rela.got)
      *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*)
      PROVIDE_HIDDEN (__rel_iplt_start = .);
      PROVIDE_HIDDEN (__rel_iplt_end = .);
      PROVIDE_HIDDEN (__rela_iplt_start = .);
      *(.rela.iplt)
      PROVIDE_HIDDEN (__rela_iplt_end = .);



-- 
Will Newton
Toolchain Working Group, Linaro

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-16 11:21         ` Will Newton
@ 2014-06-16 13:07           ` Alan Modra
  2014-06-16 14:10             ` Will Newton
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2014-06-16 13:07 UTC (permalink / raw)
  To: Will Newton; +Cc: binutils

On Mon, Jun 16, 2014 at 12:21:23PM +0100, Will Newton wrote:
>       PROVIDE_HIDDEN (__rel_iplt_start = .);
>       *(.rel.iplt)
>       PROVIDE_HIDDEN (__rel_iplt_end = .);
>       PROVIDE_HIDDEN (__rela_iplt_start = .);
>       PROVIDE_HIDDEN (__rela_iplt_end = .);

This should fix it.  Committed.

	* scripttempl/elf.sc: Edit out __rela_iplt symbol assignments from
	.rel sections, and __rel_iplt from .rela sections.
	* scripttempl/nds32elf.sc: Likewise.
	* Makefile.am (ends32*.c) Depend on nds32elf.sc.
	* Makefile.in: Regenerate.

diff --git a/ld/Makefile.am b/ld/Makefile.am
index 66795b3..a22006c 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -1490,27 +1490,27 @@ emsp430X.c: $(srcdir)/emulparams/msp430.sh $(srcdir)/emulparams/msp430X.sh \
 
 ends32elf.c: $(srcdir)/emulparams/nds32elf.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
-  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
 
 ends32elf16m.c: $(srcdir)/emulparams/nds32elf16m.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
-  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
 
 ends32belf.c: $(srcdir)/emulparams/nds32belf.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
-  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
 
 ends32belf16m.c: $(srcdir)/emulparams/nds32belf16m.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
-  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
 
 ends32elf_linux.c: $(srcdir)/emulparams/nds32elf_linux.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
-  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
 
 ends32belf_linux.c: $(srcdir)/emulparams/nds32belf_linux.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
-  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
 
 enews.c: $(srcdir)/emulparams/news.sh \
   $(srcdir)/emultempl/generic.em $(srcdir)/scripttempl/aout.sc ${GEN_DEPENDS}
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index e8126cb..6d0d13d 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -410,13 +410,13 @@ cat >> ldscripts/dyntmp.$$ <<EOF
   .rel.dyn      ${RELOCATING-0} :
     {
 EOF
-sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rela\./d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
+sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rela\./d;/__rela_iplt_/d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
 cat >> ldscripts/dyntmp.$$ <<EOF
     }
   .rela.dyn     ${RELOCATING-0} :
     {
 EOF
-sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rel\./d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
+sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rel\./d;/__rel_iplt_/d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
 cat >> ldscripts/dyntmp.$$ <<EOF
     }
 EOF
@@ -446,10 +446,10 @@ emit_dyn()
     cat ldscripts/dyntmp.$$
   else
     if test -z "${NO_REL_RELOCS}"; then
-      sed -e '/^[ 	]*\.rela\.[^}]*$/,/}/d' -e '/^[ 	]*\.rela\./d' ldscripts/dyntmp.$$
+      sed -e '/^[ 	]*\.rela\.[^}]*$/,/}/d;/^[ 	]*\.rela\./d;/__rela_iplt_/d' ldscripts/dyntmp.$$
     fi
     if test -z "${NO_RELA_RELOCS}"; then
-      sed -e '/^[ 	]*\.rel\.[^}]*$/,/}/d' -e '/^[ 	]*\.rel\./d' ldscripts/dyntmp.$$
+      sed -e '/^[ 	]*\.rel\.[^}]*$/,/}/d;/^[ 	]*\.rel\./d;/__rel_iplt_/d' ldscripts/dyntmp.$$
     fi
   fi
   rm -f ldscripts/dyntmp.$$
diff --git a/ld/scripttempl/nds32elf.sc b/ld/scripttempl/nds32elf.sc
index 80f60a1..076e120 100644
--- a/ld/scripttempl/nds32elf.sc
+++ b/ld/scripttempl/nds32elf.sc
@@ -361,13 +361,13 @@ cat >> ldscripts/dyntmp.$$ <<EOF
   .rel.dyn      ${RELOCATING-0} :
     {
 EOF
-sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rela\./d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
+sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rela\./d;/__rela_iplt_/d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
 cat >> ldscripts/dyntmp.$$ <<EOF
     }
   .rela.dyn     ${RELOCATING-0} :
     {
 EOF
-sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rel\./d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
+sed -e '/^[ 	]*[{}][ 	]*$/d;/:[ 	]*$/d;/\.rel\./d;/__rel_iplt_/d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
 cat >> ldscripts/dyntmp.$$ <<EOF
     }
 EOF
@@ -397,10 +397,10 @@ emit_dyn()
     cat ldscripts/dyntmp.$$
   else
     if test -z "${NO_REL_RELOCS}"; then
-      sed -e '/^[ 	]*\.rela\.[^}]*$/,/}/d' -e '/^[ 	]*\.rela\./d' ldscripts/dyntmp.$$
+      sed -e '/^[ 	]*\.rela\.[^}]*$/,/}/d;/^[ 	]*\.rela\./d;/__rela_iplt_/d' ldscripts/dyntmp.$$
     fi
     if test -z "${NO_RELA_RELOCS}"; then
-      sed -e '/^[ 	]*\.rel\.[^}]*$/,/}/d' -e '/^[ 	]*\.rel\./d' ldscripts/dyntmp.$$
+      sed -e '/^[ 	]*\.rel\.[^}]*$/,/}/d;/^[ 	]*\.rel\./d;/__rel_iplt_/d' ldscripts/dyntmp.$$
     fi
   fi
   rm -f ldscripts/dyntmp.$$

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-16 13:07           ` Alan Modra
@ 2014-06-16 14:10             ` Will Newton
  2014-06-16 14:11               ` Will Newton
  0 siblings, 1 reply; 14+ messages in thread
From: Will Newton @ 2014-06-16 14:10 UTC (permalink / raw)
  To: Will Newton, binutils

On 16 June 2014 14:07, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Jun 16, 2014 at 12:21:23PM +0100, Will Newton wrote:
>>       PROVIDE_HIDDEN (__rel_iplt_start = .);
>>       *(.rel.iplt)
>>       PROVIDE_HIDDEN (__rel_iplt_end = .);
>>       PROVIDE_HIDDEN (__rela_iplt_start = .);
>>       PROVIDE_HIDDEN (__rela_iplt_end = .);
>
> This should fix it.  Committed.
>
>         * scripttempl/elf.sc: Edit out __rela_iplt symbol assignments from
>         .rel sections, and __rel_iplt from .rela sections.
>         * scripttempl/nds32elf.sc: Likewise.
>         * Makefile.am (ends32*.c) Depend on nds32elf.sc.
>         * Makefile.in: Regenerate.

Yes, this fixes the problem for me. Thanks!

> diff --git a/ld/Makefile.am b/ld/Makefile.am
> index 66795b3..a22006c 100644
> --- a/ld/Makefile.am
> +++ b/ld/Makefile.am
> @@ -1490,27 +1490,27 @@ emsp430X.c: $(srcdir)/emulparams/msp430.sh $(srcdir)/emulparams/msp430X.sh \
>
>  ends32elf.c: $(srcdir)/emulparams/nds32elf.sh \
>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>
>  ends32elf16m.c: $(srcdir)/emulparams/nds32elf16m.sh \
>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>
>  ends32belf.c: $(srcdir)/emulparams/nds32belf.sh \
>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>
>  ends32belf16m.c: $(srcdir)/emulparams/nds32belf16m.sh \
>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>
>  ends32elf_linux.c: $(srcdir)/emulparams/nds32elf_linux.sh \
>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>
>  ends32belf_linux.c: $(srcdir)/emulparams/nds32belf_linux.sh \
>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>
>  enews.c: $(srcdir)/emulparams/news.sh \
>    $(srcdir)/emultempl/generic.em $(srcdir)/scripttempl/aout.sc ${GEN_DEPENDS}
> diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
> index e8126cb..6d0d13d 100644
> --- a/ld/scripttempl/elf.sc
> +++ b/ld/scripttempl/elf.sc
> @@ -410,13 +410,13 @@ cat >> ldscripts/dyntmp.$$ <<EOF
>    .rel.dyn      ${RELOCATING-0} :
>      {
>  EOF
> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;/__rela_iplt_/d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>  cat >> ldscripts/dyntmp.$$ <<EOF
>      }
>    .rela.dyn     ${RELOCATING-0} :
>      {
>  EOF
> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;/__rel_iplt_/d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>  cat >> ldscripts/dyntmp.$$ <<EOF
>      }
>  EOF
> @@ -446,10 +446,10 @@ emit_dyn()
>      cat ldscripts/dyntmp.$$
>    else
>      if test -z "${NO_REL_RELOCS}"; then
> -      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d' -e '/^[         ]*\.rela\./d' ldscripts/dyntmp.$$
> +      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d;/^[      ]*\.rela\./d;/__rela_iplt_/d' ldscripts/dyntmp.$$
>      fi
>      if test -z "${NO_RELA_RELOCS}"; then
> -      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d' -e '/^[  ]*\.rel\./d' ldscripts/dyntmp.$$
> +      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d;/^[       ]*\.rel\./d;/__rel_iplt_/d' ldscripts/dyntmp.$$
>      fi
>    fi
>    rm -f ldscripts/dyntmp.$$
> diff --git a/ld/scripttempl/nds32elf.sc b/ld/scripttempl/nds32elf.sc
> index 80f60a1..076e120 100644
> --- a/ld/scripttempl/nds32elf.sc
> +++ b/ld/scripttempl/nds32elf.sc
> @@ -361,13 +361,13 @@ cat >> ldscripts/dyntmp.$$ <<EOF
>    .rel.dyn      ${RELOCATING-0} :
>      {
>  EOF
> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;/__rela_iplt_/d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>  cat >> ldscripts/dyntmp.$$ <<EOF
>      }
>    .rela.dyn     ${RELOCATING-0} :
>      {
>  EOF
> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;/__rel_iplt_/d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>  cat >> ldscripts/dyntmp.$$ <<EOF
>      }
>  EOF
> @@ -397,10 +397,10 @@ emit_dyn()
>      cat ldscripts/dyntmp.$$
>    else
>      if test -z "${NO_REL_RELOCS}"; then
> -      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d' -e '/^[         ]*\.rela\./d' ldscripts/dyntmp.$$
> +      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d;/^[      ]*\.rela\./d;/__rela_iplt_/d' ldscripts/dyntmp.$$
>      fi
>      if test -z "${NO_RELA_RELOCS}"; then
> -      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d' -e '/^[  ]*\.rel\./d' ldscripts/dyntmp.$$
> +      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d;/^[       ]*\.rel\./d;/__rel_iplt_/d' ldscripts/dyntmp.$$
>      fi
>    fi
>    rm -f ldscripts/dyntmp.$$
>
> --
> Alan Modra
> Australia Development Lab, IBM



-- 
Will Newton
Toolchain Working Group, Linaro

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-16 14:10             ` Will Newton
@ 2014-06-16 14:11               ` Will Newton
  2014-06-18  0:29                 ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Will Newton @ 2014-06-16 14:11 UTC (permalink / raw)
  To: Will Newton, binutils

On 16 June 2014 15:09, Will Newton <will.newton@linaro.org> wrote:
> On 16 June 2014 14:07, Alan Modra <amodra@gmail.com> wrote:
>> On Mon, Jun 16, 2014 at 12:21:23PM +0100, Will Newton wrote:
>>>       PROVIDE_HIDDEN (__rel_iplt_start = .);
>>>       *(.rel.iplt)
>>>       PROVIDE_HIDDEN (__rel_iplt_end = .);
>>>       PROVIDE_HIDDEN (__rela_iplt_start = .);
>>>       PROVIDE_HIDDEN (__rela_iplt_end = .);
>>
>> This should fix it.  Committed.
>>
>>         * scripttempl/elf.sc: Edit out __rela_iplt symbol assignments from
>>         .rel sections, and __rel_iplt from .rela sections.
>>         * scripttempl/nds32elf.sc: Likewise.
>>         * Makefile.am (ends32*.c) Depend on nds32elf.sc.
>>         * Makefile.in: Regenerate.
>
> Yes, this fixes the problem for me. Thanks!

Would it be possible to apply the fix to the stable branch too?

>> diff --git a/ld/Makefile.am b/ld/Makefile.am
>> index 66795b3..a22006c 100644
>> --- a/ld/Makefile.am
>> +++ b/ld/Makefile.am
>> @@ -1490,27 +1490,27 @@ emsp430X.c: $(srcdir)/emulparams/msp430.sh $(srcdir)/emulparams/msp430X.sh \
>>
>>  ends32elf.c: $(srcdir)/emulparams/nds32elf.sh \
>>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
>> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
>> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>>
>>  ends32elf16m.c: $(srcdir)/emulparams/nds32elf16m.sh \
>>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
>> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
>> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>>
>>  ends32belf.c: $(srcdir)/emulparams/nds32belf.sh \
>>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
>> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
>> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>>
>>  ends32belf16m.c: $(srcdir)/emulparams/nds32belf16m.sh \
>>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
>> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
>> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>>
>>  ends32elf_linux.c: $(srcdir)/emulparams/nds32elf_linux.sh \
>>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
>> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
>> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>>
>>  ends32belf_linux.c: $(srcdir)/emulparams/nds32belf_linux.sh \
>>    $(ELF_DEPS) $(srcdir)/emultempl/nds32elf.em \
>> -  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
>> +  $(srcdir)/scripttempl/nds32elf.sc ${GEN_DEPENDS}
>>
>>  enews.c: $(srcdir)/emulparams/news.sh \
>>    $(srcdir)/emultempl/generic.em $(srcdir)/scripttempl/aout.sc ${GEN_DEPENDS}
>> diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
>> index e8126cb..6d0d13d 100644
>> --- a/ld/scripttempl/elf.sc
>> +++ b/ld/scripttempl/elf.sc
>> @@ -410,13 +410,13 @@ cat >> ldscripts/dyntmp.$$ <<EOF
>>    .rel.dyn      ${RELOCATING-0} :
>>      {
>>  EOF
>> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;/__rela_iplt_/d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>>  cat >> ldscripts/dyntmp.$$ <<EOF
>>      }
>>    .rela.dyn     ${RELOCATING-0} :
>>      {
>>  EOF
>> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;/__rel_iplt_/d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>>  cat >> ldscripts/dyntmp.$$ <<EOF
>>      }
>>  EOF
>> @@ -446,10 +446,10 @@ emit_dyn()
>>      cat ldscripts/dyntmp.$$
>>    else
>>      if test -z "${NO_REL_RELOCS}"; then
>> -      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d' -e '/^[         ]*\.rela\./d' ldscripts/dyntmp.$$
>> +      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d;/^[      ]*\.rela\./d;/__rela_iplt_/d' ldscripts/dyntmp.$$
>>      fi
>>      if test -z "${NO_RELA_RELOCS}"; then
>> -      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d' -e '/^[  ]*\.rel\./d' ldscripts/dyntmp.$$
>> +      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d;/^[       ]*\.rel\./d;/__rel_iplt_/d' ldscripts/dyntmp.$$
>>      fi
>>    fi
>>    rm -f ldscripts/dyntmp.$$
>> diff --git a/ld/scripttempl/nds32elf.sc b/ld/scripttempl/nds32elf.sc
>> index 80f60a1..076e120 100644
>> --- a/ld/scripttempl/nds32elf.sc
>> +++ b/ld/scripttempl/nds32elf.sc
>> @@ -361,13 +361,13 @@ cat >> ldscripts/dyntmp.$$ <<EOF
>>    .rel.dyn      ${RELOCATING-0} :
>>      {
>>  EOF
>> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rela\./d;/__rela_iplt_/d;s/^.*: { *\(.*\)}$/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>>  cat >> ldscripts/dyntmp.$$ <<EOF
>>      }
>>    .rela.dyn     ${RELOCATING-0} :
>>      {
>>  EOF
>> -sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>> +sed -e '/^[    ]*[{}][         ]*$/d;/:[       ]*$/d;/\.rel\./d;/__rel_iplt_/d;s/^.*: { *\(.*\)}/      \1/' $COMBRELOC >> ldscripts/dyntmp.$$
>>  cat >> ldscripts/dyntmp.$$ <<EOF
>>      }
>>  EOF
>> @@ -397,10 +397,10 @@ emit_dyn()
>>      cat ldscripts/dyntmp.$$
>>    else
>>      if test -z "${NO_REL_RELOCS}"; then
>> -      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d' -e '/^[         ]*\.rela\./d' ldscripts/dyntmp.$$
>> +      sed -e '/^[      ]*\.rela\.[^}]*$/,/}/d;/^[      ]*\.rela\./d;/__rela_iplt_/d' ldscripts/dyntmp.$$
>>      fi
>>      if test -z "${NO_RELA_RELOCS}"; then
>> -      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d' -e '/^[  ]*\.rel\./d' ldscripts/dyntmp.$$
>> +      sed -e '/^[      ]*\.rel\.[^}]*$/,/}/d;/^[       ]*\.rel\./d;/__rel_iplt_/d' ldscripts/dyntmp.$$
>>      fi
>>    fi
>>    rm -f ldscripts/dyntmp.$$
>>
>> --
>> Alan Modra
>> Australia Development Lab, IBM
>
>
>
> --
> Will Newton
> Toolchain Working Group, Linaro



-- 
Will Newton
Toolchain Working Group, Linaro

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

* Re: 32-bit PowerPC sdata linker problem
  2014-06-16 14:11               ` Will Newton
@ 2014-06-18  0:29                 ` Alan Modra
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Modra @ 2014-06-18  0:29 UTC (permalink / raw)
  To: Will Newton; +Cc: binutils

On Mon, Jun 16, 2014 at 03:11:12PM +0100, Will Newton wrote:
> On 16 June 2014 15:09, Will Newton <will.newton@linaro.org> wrote:
> > On 16 June 2014 14:07, Alan Modra <amodra@gmail.com> wrote:
> >> On Mon, Jun 16, 2014 at 12:21:23PM +0100, Will Newton wrote:
> >>>       PROVIDE_HIDDEN (__rel_iplt_start = .);
> >>>       *(.rel.iplt)
> >>>       PROVIDE_HIDDEN (__rel_iplt_end = .);
> >>>       PROVIDE_HIDDEN (__rela_iplt_start = .);
> >>>       PROVIDE_HIDDEN (__rela_iplt_end = .);
> >>
> >> This should fix it.  Committed.
> >>
> >>         * scripttempl/elf.sc: Edit out __rela_iplt symbol assignments from
> >>         .rel sections, and __rel_iplt from .rela sections.
> >>         * scripttempl/nds32elf.sc: Likewise.
> >>         * Makefile.am (ends32*.c) Depend on nds32elf.sc.
> >>         * Makefile.in: Regenerate.
> >
> > Yes, this fixes the problem for me. Thanks!
> 
> Would it be possible to apply the fix to the stable branch too?

Done.  I'm also going to commit the following to 2.24.

	PR 17047
	* ldlang.c (lang_finish): Don't free linker hash table.

diff --git a/ld/ldlang.c b/ld/ldlang.c
index ba7f493..9121aa2 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1238,7 +1238,14 @@ lang_init (void)
 void
 lang_finish (void)
 {
+  /* Some targets require access to the linker hash table during the
+     _bfd_write_contents call in bfd_close, so it can't be freed
+     before bfd_close.  It can't be freed after bfd_close either,
+     since bfd_alloc memory holding side data structures disappears
+     (PR17047).  So don't free it.
+
   bfd_link_hash_table_free (link_info.output_bfd, link_info.hash);
+  */
   bfd_hash_table_free (&lang_definedness_table);
   output_section_statement_table_free ();
 }

-- 
Alan Modra
Australia Development Lab, IBM

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

end of thread, other threads:[~2014-06-18  0:29 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <53918356.3040102@embedded-brains.de>
2014-06-06 10:54 ` 32-bit PowerPC sdata linker problem Alan Modra
2014-06-06 11:23   ` Sebastian Huber
2014-06-06 11:31     ` Sebastian Huber
2014-06-06 12:15       ` Alan Modra
2014-06-06 12:49         ` Sebastian Huber
     [not found]           ` <20140606130559.GK5592@bubble.grove.modra.org>
     [not found]             ` <5391C0E8.7010409@embedded-brains.de>
     [not found]               ` <20140606134915.GL5592@bubble.grove.modra.org>
     [not found]                 ` <5391CCFB.5060206@embedded-brains.de>
2014-06-07 12:34                   ` Alan Modra
2014-06-10  6:28                     ` Sebastian Huber
2014-06-06 12:17     ` Alan Modra
2014-06-07 12:47       ` Alan Modra
2014-06-16 11:21         ` Will Newton
2014-06-16 13:07           ` Alan Modra
2014-06-16 14:10             ` Will Newton
2014-06-16 14:11               ` Will Newton
2014-06-18  0:29                 ` Alan Modra

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