public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [Patch] Fix xcoff relocation adjustment in gas
@ 2011-06-10  9:13 Tristan Gingold
  2011-06-11  4:14 ` Alan Modra
  0 siblings, 1 reply; 3+ messages in thread
From: Tristan Gingold @ 2011-06-10  9:13 UTC (permalink / raw)
  To: binutils Development

Hi,

currently in order to adjust relocation on xcoff, gas searches the csect using the symbol address.
But this is flawed because the address of the end of a csect is the same as the address of the
next csect.  To illustrate this issue, see the following reproducer:

	.file	"repro.s"
	.csect .text[PR]
	.toc
	.csect .text[PR]
Ltext..0:
  	nop
        nop
        nop
        nop
	.csect _alignmenttest.ro_[RO],4
	.align 2
e1:
	.byte	1
	.csect .text[PR]
Letext..0:
	.csect _alignmenttest.rw_[RW],4
rw1:	.vbyte 4, Letext..0

Letext..0 (which is a common pattern from gcc) is at the border of two csects.
And unfortunately gas currently selects the wrong csect in this case:

$ objdump -r repro-old.o 

repro-old.o:     file format aixcoff-rs6000

RELOCATION RECORDS FOR [.data]:
OFFSET   TYPE              VALUE 
00000000 R_POS             _alignmenttest.ro_+0xfffffff0


While the native AIX assembler (and gas when this patch is applied) generates the right reloc:

$ objdump -r repro-gas.o 

repro-gas.o:     file format aixcoff-rs6000

RELOCATION RECORDS FOR [.data]:
OFFSET   TYPE              VALUE 
00000000 R_POS             .text

To fix this issue, we reuse the tc field 'within' to attach each labels to its csect.  As a consequence,
the reloc adjustment algorithm is now O(1) instead of O(n), which is a nice result too.

There were two adjustments needed in the testsuite for test1xcoff32.d:
* a symbol (.text) has been added in the symbol table as a consequence of the patch (and because it was
  never needed before)

* more interesting, the code generated for:

	.csect	.crazy_table[RO]
xdsym0:	.long	0xbeefed
xdsym1:
	.csect	[PR]
	.lglobl	reference_csect_relative_symbols
reference_csect_relative_symbols:
	lwz	3,xdsym0(3)
	lwz	3,xdsym1(3)
	lwz	3,xusym0(3)
	lwz	3,xusym1(3)
[...]
	.csect	.crazy_table[RO]
xusym0:	.long	0xbeefed
xusym1:


is now:
    8:  80 63 00 00     l       r3,0\(r3\)
    c:  80 63 00 04     l       r3,4\(r3\)
   10:  80 63 00 04     l       r3,4\(r3\)
+  14:  80 63 00 08     l       r3,8\(r3\)

instead of:
    8:  80 63 00 00     l       r3,0\(r3\)
    c:  80 63 00 04     l       r3,4\(r3\)
   10:  80 63 00 04     l       r3,4\(r3\)
-  14:  80 63 00 00     l       r3,0\(r3\)

The previous code was clearly wrong as the offset of xusym1 is not the same as xusym1.

I haven't added a testcase as test1xcoff also covers this bug.

Ok for trunk ?

Tristan.

gas/
2011-06-10  Tristan Gingold  <gingold@adacore.com>

        * config/tc-ppc.h (struct ppc_tc_sy): Complete comment on within.
        (tc_new_dot_label): Define.
        (ppc_new_dot_label): Declare.
        * config/tc-ppc.c (ppc_frob_label): Set within target field.
        (ppc_fix_adjustable): Use this field to adjust the reloc.
        (ppc_new_dot_label): New function.

gas/testsuite/
2011-06-10  Tristan Gingold  <gingold@adacore.com>

        * gas/ppc/test1xcoff32.d: Adjust for csect anchor.

diff --git a/gas/config/tc-ppc.c b/gas/config/tc-ppc.c
index fe2c4ff..f5a3748 100644
--- a/gas/config/tc-ppc.c
+++ b/gas/config/tc-ppc.c
@@ -5375,6 +5375,7 @@ ppc_frob_label (symbolS *sym)
       symbol_append (sym, symbol_get_tc (ppc_current_csect)->within,
 		     &symbol_rootP, &symbol_lastP);
       symbol_get_tc (ppc_current_csect)->within = sym;
+      symbol_get_tc (sym)->within = ppc_current_csect;
     }
 
 #ifdef OBJ_ELF
@@ -5864,55 +5865,17 @@ ppc_fix_adjustable (fixS *fix)
 	  || (ppc_after_toc_frag != NULL
 	      && val >= ppc_after_toc_frag->fr_address)))
     {
-      symbolS *csect;
-      symbolS *next_csect;
-
-      if (symseg == text_section)
-	csect = ppc_text_csects;
-      else if (symseg == data_section)
-	csect = ppc_data_csects;
-      else
-	abort ();
+      symbolS *csect = tc->within;
 
-      /* Skip the initial dummy symbol.  */
-      csect = symbol_get_tc (csect)->next;
-
-      if (csect != (symbolS *) NULL)
-	{
-	  while ((next_csect = symbol_get_tc (csect)->next) != (symbolS *) NULL
-		 && (symbol_get_frag (next_csect)->fr_address <= val))
-	    {
-	      /* If the csect address equals the symbol value, then we
-		 have to look through the full symbol table to see
-		 whether this is the csect we want.  Note that we will
-		 only get here if the csect has zero length.  */
-	      if (symbol_get_frag (csect)->fr_address == val
-		  && S_GET_VALUE (csect) == val)
-		{
-		  symbolS *scan;
+      /* If the symbol was not declared by a label (eg: a section symbol),
+         use the section instead of the csect.  This doesn't happen in
+         normal AIX assembly code.  */
+      if (csect == NULL)
+        csect = seg_info (symseg)->sym;
 
-		  for (scan = symbol_next (csect);
-		       scan != NULL;
-		       scan = symbol_next (scan))
-		    {
-		      if (symbol_get_tc (scan)->subseg != 0)
-			break;
-		      if (scan == fix->fx_addsy)
-			break;
-		    }
+      fix->fx_offset += val - symbol_get_frag (csect)->fr_address;
+      fix->fx_addsy = csect;
 
-		  /* If we found the symbol before the next csect
-		     symbol, then this is the csect we want.  */
-		  if (scan == fix->fx_addsy)
-		    break;
-		}
-
-	      csect = next_csect;
-	    }
-
-	  fix->fx_offset += val - symbol_get_frag (csect)->fr_address;
-	  fix->fx_addsy = csect;
-	}
       return 0;
     }
 
@@ -5954,6 +5917,13 @@ ppc_force_relocation (fixS *fix)
   return generic_force_reloc (fix);
 }
 
+void
+ppc_new_dot_label (symbolS *sym)
+{
+  /* Anchor this label to the current csect for relocations.  */
+  symbol_get_tc (sym)->within = ppc_current_csect;
+}
+
 #endif /* OBJ_XCOFF */
 
 #ifdef OBJ_ELF
diff --git a/gas/config/tc-ppc.h b/gas/config/tc-ppc.h
index 9706f6f..a11d396 100644
--- a/gas/config/tc-ppc.h
+++ b/gas/config/tc-ppc.h
@@ -143,8 +143,9 @@ struct ppc_tc_sy
      for symbols that are not csects.  */
   subsegT subseg;
   /* For a csect symbol, the last symbol which has been defined in
-     this csect, or NULL if none have been defined so far.  For a .bs
-     symbol, the referenced csect symbol.  */
+     this csect, or NULL if none have been defined so far.
+     For a .bs symbol, the referenced csect symbol.
+     For a label, the enclosing csect.  */
   symbolS *within;
   union
   {
@@ -207,6 +208,9 @@ do {							\
 extern void ppc_xcoff_end (void);
 #define md_end ppc_xcoff_end
 
+#define tc_new_dot_label(sym) ppc_new_dot_label (sym)
+extern void ppc_new_dot_label (symbolS *);
+
 #endif /* OBJ_XCOFF */
 
 extern const char       ppc_symbol_chars[];
diff --git a/gas/testsuite/gas/ppc/test1xcoff32.d b/gas/testsuite/gas/ppc/test1x
index 56de4d4..75e93a8 100644
--- a/gas/testsuite/gas/ppc/test1xcoff32.d
+++ b/gas/testsuite/gas/ppc/test1xcoff32.d
@@ -55,6 +55,8 @@ AUX val     4 prmhsh 0 snhsh 0 typ 1 algn 2 clss 3 stb 0 snstb
 AUX val     0 prmhsh 0 snhsh 0 typ 0 algn 0 clss 0 stb 0 snstb 0
 \[ 36\]\(sec  0\)\(fl 0x00\)\(ty   0\)\(scl   2\) \(nx 1\) 0x00000000 esym1
 AUX val     0 prmhsh 0 snhsh 0 typ 0 algn 0 clss 0 stb 0 snstb 0
+\[ 38\]\(sec  1\)\(fl 0x00\)\(ty   0\)\(scl   3\) \(nx 1\) 0x00000000 \.text
+AUX scnlen 0x68 nreloc 7 nlnno 0
 
 
 Disassembly of section \.text:
@@ -67,7 +69,7 @@ Disassembly of section \.text:
    8:	80 63 00 00 	l       r3,0\(r3\)
    c:	80 63 00 04 	l       r3,4\(r3\)
   10:	80 63 00 04 	l       r3,4\(r3\)
-  14:	80 63 00 00 	l       r3,0\(r3\)
+  14:	80 63 00 08 	l       r3,8\(r3\)
 
 0+0018 <dubious_references_to_default_RW_csect>:
   18:	80 63 00 00 	l       r3,0\(r3\)
@@ -136,4 +138,4 @@ Disassembly of section \.data:
 
 0+008c <ignored6>:
   8c:	00 00 00 00 	\.long 0x0
-			8c: R_POS	\.crazy_table
+			8c: R_POS	\.text


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

* Re: [Patch] Fix xcoff relocation adjustment in gas
  2011-06-10  9:13 [Patch] Fix xcoff relocation adjustment in gas Tristan Gingold
@ 2011-06-11  4:14 ` Alan Modra
  2011-06-14  9:04   ` Tristan Gingold
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2011-06-11  4:14 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: binutils Development

On Fri, Jun 10, 2011 at 11:13:32AM +0200, Tristan Gingold wrote:
> gas/
> 2011-06-10  Tristan Gingold  <gingold@adacore.com>
> 
>         * config/tc-ppc.h (struct ppc_tc_sy): Complete comment on within.
>         (tc_new_dot_label): Define.
>         (ppc_new_dot_label): Declare.
>         * config/tc-ppc.c (ppc_frob_label): Set within target field.
>         (ppc_fix_adjustable): Use this field to adjust the reloc.
>         (ppc_new_dot_label): New function.
> 
> gas/testsuite/
> 2011-06-10  Tristan Gingold  <gingold@adacore.com>
> 
>         * gas/ppc/test1xcoff32.d: Adjust for csect anchor.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [Patch] Fix xcoff relocation adjustment in gas
  2011-06-11  4:14 ` Alan Modra
@ 2011-06-14  9:04   ` Tristan Gingold
  0 siblings, 0 replies; 3+ messages in thread
From: Tristan Gingold @ 2011-06-14  9:04 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils Development


On Jun 11, 2011, at 6:14 AM, Alan Modra wrote:

> On Fri, Jun 10, 2011 at 11:13:32AM +0200, Tristan Gingold wrote:
>> gas/
>> 2011-06-10  Tristan Gingold  <gingold@adacore.com>
>> 
>>        * config/tc-ppc.h (struct ppc_tc_sy): Complete comment on within.
>>        (tc_new_dot_label): Define.
>>        (ppc_new_dot_label): Declare.
>>        * config/tc-ppc.c (ppc_frob_label): Set within target field.
>>        (ppc_fix_adjustable): Use this field to adjust the reloc.
>>        (ppc_new_dot_label): New function.
>> 
>> gas/testsuite/
>> 2011-06-10  Tristan Gingold  <gingold@adacore.com>
>> 
>>        * gas/ppc/test1xcoff32.d: Adjust for csect anchor.
> 
> OK.

Thanks, committed.

Tristan.

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

end of thread, other threads:[~2011-06-14  9:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-10  9:13 [Patch] Fix xcoff relocation adjustment in gas Tristan Gingold
2011-06-11  4:14 ` Alan Modra
2011-06-14  9:04   ` Tristan Gingold

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