IA64 linker relaxation bug

IA64 linker relaxation bug

[Andreas Schwab]

The testcase at <ftp://ftp.suse.com/pub/people/schwab/ia64-relax.tar.bz2>
demonstrates a bug in the linker relaxation on ia64:

$ make
ld -o temacs temacs.o libncurses.so libm.so libc.so elf-init.oS crtn.o
elf-init.oS(.text+0x101): In function `__libc_csu_init':
: relocation truncated to fit: GPREL22 __init_array_start
make: *** [temacs] Error 1

The problem is that __init_array_start is changing its value multiple
times during linking, always alternating between 0x6000000000000430 and
0x6000000000004fa0.  There is an LTOFF22X relocation against this symbol,
and by the time the relaxing pass looks at this relocation the value of
__init_array_start is 0x6000000000004fa0 which is close enough to the gp
value.  But the final value happens to be 0x6000000000000430, outside of
the range of a GPREL22 relocation.

This bug is reproducable both with mainline and 2.14 branch.  I think it
is a side effect of
<http://sources.redhat.com/ml/binutils/2001-02/msg00304.html>.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

PATCH: IA64 linker relaxation bug

[H. J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]

On Sat, Nov 01, 2003 at 02:37:29AM +0100, Andreas Schwab wrote:
> The testcase at <ftp://ftp.suse.com/pub/people/schwab/ia64-relax.tar.bz2>
> demonstrates a bug in the linker relaxation on ia64:
> 
> $ make
> ld -o temacs temacs.o libncurses.so libm.so libc.so elf-init.oS crtn.o
> elf-init.oS(.text+0x101): In function `__libc_csu_init':
> : relocation truncated to fit: GPREL22 __init_array_start
> make: *** [temacs] Error 1
> 
> The problem is that __init_array_start is changing its value multiple
> times during linking, always alternating between 0x6000000000000430 and
> 0x6000000000004fa0.  There is an LTOFF22X relocation against this symbol,
> and by the time the relaxing pass looks at this relocation the value of
> __init_array_start is 0x6000000000004fa0 which is close enough to the gp
> value.  But the final value happens to be 0x6000000000000430, outside of
> the range of a GPREL22 relocation.
> 

The problem is lang_do_assignments assigns 0x6000000000004fa0 and
lang_size_sections assigns 0x6000000000000430. The final one is
0x6000000000000430.

> This bug is reproducable both with mainline and 2.14 branch.  I think it
> is a side effect of
> <http://sources.redhat.com/ml/binutils/2001-02/msg00304.html>.
> 

I am testing this patch. Does this make any senses?


H.J.

[-- Attachment #2: ld-final-relax-2.patch --]
[-- Type: text/plain, Size: 1428 bytes --]

2003-11-04  H.J. Lu  <hongjiu.lu@intel.com>

	* ldlang.c (lang_process): Move the relax finalize pass after
	all sizes are finalized.

--- ld/ldlang.c.relax	2003-11-04 14:25:03.000000000 -0800
+++ ld/ldlang.c	2003-11-04 15:48:24.000000000 -0800
@@ -4257,14 +4257,6 @@ lang_process (void)
 	     globals are, so can make a better guess.  */
 	  lang_size_sections (statement_list.head, abs_output_section,
 			      &statement_list.head, 0, 0, &relax_again, FALSE);
-
-	  /* If the normal relax is done and the relax finalize pass
-	     is not performed yet, we perform another relax pass.  */
-	  if (!relax_again && !link_info.relax_finalizing)
-	    {
-	      link_info.relax_finalizing = TRUE;
-	      relax_again = TRUE;
-	    }
 	}
       while (relax_again);
 
@@ -4273,6 +4265,15 @@ lang_process (void)
       lang_do_assignments (statement_list.head, abs_output_section, NULL, 0);
       lang_size_sections (statement_list.head, abs_output_section,
 			  &statement_list.head, 0, 0, NULL, TRUE);
+
+      /* The relax finalize pass should be done after all sizes are
+	 finalized and cause no size change. */
+      link_info.relax_finalizing = TRUE;
+      relax_again = TRUE;
+      lang_reset_memory_regions ();
+      lang_size_sections (statement_list.head, abs_output_section,
+			  &statement_list.head, 0, 0, &relax_again,
+			  FALSE);
     }
 
   /* See if anything special should be done now we know how big

Re: PATCH: IA64 linker relaxation bug

[Jim Wilson]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

On Tue, 2003-11-04 at 16:10, H. J. Lu wrote:
> The problem is lang_do_assignments assigns 0x6000000000004fa0 and
> lang_size_sections assigns 0x6000000000000430. The final one is
> 0x6000000000000430.

> I am testing this patch. Does this make any senses?

It would help if you gave some reason why you think it makes sense.  I
couldn't make any sense of it, as it seemed like you were just adding
another arbitrary relaxation pass at the end.

The real problem seems to me to be the fact that lang_do_assignments is
computing a different value than lang_size_sections.  I tracked this
down to a problem with _raw_size.  The first thing we do is call
lang_reset_memory_regions which clears _raw_size.  Then we call
lang_do_assignments.  After each section, it sets dot to
       dot = os->bfd_section->vma + os->bfd_section->_raw_size / opb;
but since we already cleared _raw_size, this means dot is set to the
address of the start of the section rather than the address of the end
of the section.  The result is that any symbol which is between
sections, and whose value depends on dot, gets the wrong address during
lang_do_assignment.

As far as I can tell, the values cleared by lang_reset_memory_regions
are not used in lang_do_assignments, other than _raw_size, so the right
solution is to move the lang_reset_memory_regions call after the
lang_do_assignments call.  This does solve the problem.

While looking at this, I noticed that we are accidentally running two
relax_finalize passes instead of the expected one.  This is because
elfNN_ia64_relax_section sets relax_again to true, and the relation loop
blindly gives us another iteration.  It seems reasonable to fix the ia64
port to not ask for another relaxation round in this case.  This part I
am not as sure about, but it does seem to make sense.

I also noticed that we always run the relax_finalize pass, even though
the ia64 port is the only one that uses it.  Perhaps we need some target
flag or hook to indicate which targets need this extra relaxation pass. 
I did not try to solve this problem.

The patch I came up with follows in an attachment.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

[-- Attachment #2: tmp.relax.patch --]
[-- Type: text/plain, Size: 2813 bytes --]

ld/ChangeLog
2003-11-14  James E Wilson  <wilson@specifixinc.com>

	* ldlang.c (lang_process): Move lang_reset_memory_regions call after
	lang_do_assignments call.

Index: ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.127
diff -p -r1.127 ldlang.c
*** ldlang.c	31 Oct 2003 10:27:34 -0000	1.127
--- ldlang.c	15 Nov 2003 07:55:07 -0000
*************** lang_process (void)
*** 4240,4247 ****
  
        do
  	{
- 	  lang_reset_memory_regions ();
- 
  	  relax_again = FALSE;
  
  	  /* Note: pe-dll.c does something like this also.  If you find
--- 4240,4245 ----
*************** lang_process (void)
*** 4253,4258 ****
--- 4251,4260 ----
  	  lang_do_assignments (statement_list.head, abs_output_section,
  			       NULL, 0);
  
+ 	  /* We must do this after lang_do_assignments, because it uses
+ 	     _raw_size.  */
+ 	  lang_reset_memory_regions ();
+ 
  	  /* Perform another relax pass - this time we know where the
  	     globals are, so can make a better guess.  */
  	  lang_size_sections (statement_list.head, abs_output_section,
*************** lang_process (void)
*** 4269,4276 ****
        while (relax_again);
  
        /* Final extra sizing to report errors.  */
-       lang_reset_memory_regions ();
        lang_do_assignments (statement_list.head, abs_output_section, NULL, 0);
        lang_size_sections (statement_list.head, abs_output_section,
  			  &statement_list.head, 0, 0, NULL, TRUE);
      }
--- 4271,4278 ----
        while (relax_again);
  
        /* Final extra sizing to report errors.  */
        lang_do_assignments (statement_list.head, abs_output_section, NULL, 0);
+       lang_reset_memory_regions ();
        lang_size_sections (statement_list.head, abs_output_section,
  			  &statement_list.head, 0, 0, NULL, TRUE);
      }

bfd/ChangeLog
2003-11-14  James E Wilson  <wilson@specifixinc.com>

	* elfxx-ia64.c (elfNN_ia64_relax_section): Don't set *again if
	relax_finalizing.

Index: elfxx-ia64.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-ia64.c,v
retrieving revision 1.108
diff -p -r1.108 elfxx-ia64.c
*** elfxx-ia64.c	5 Nov 2003 13:17:09 -0000	1.108
--- elfxx-ia64.c	15 Nov 2003 07:55:50 -0000
*************** elfNN_ia64_relax_section (abfd, sec, lin
*** 1075,1082 ****
  
    if (link_info->relax_finalizing)
      sec->need_finalize_relax = 0;
  
-   *again = changed_contents || changed_relocs;
    return TRUE;
  
   error_return:
--- 1075,1083 ----
  
    if (link_info->relax_finalizing)
      sec->need_finalize_relax = 0;
+   else
+     *again = changed_contents || changed_relocs;
  
    return TRUE;
  
   error_return:

Re: PATCH: IA64 linker relaxation bug

[Hans-Peter Nilsson]

On 15 Nov 2003, Jim Wilson wrote:
> The patch I came up with follows in an attachment.

Did any of you possibly extract a test-case suitable for the
linker test-suite?  Changes in this area are likely to break,
you know...

brgds, H-P

Re: PATCH: IA64 linker relaxation bug

[Alan Modra]

On Sat, Nov 15, 2003 at 12:13:18AM -0800, Jim Wilson wrote:
> 	* ldlang.c (lang_process): Move lang_reset_memory_regions call after
> 	lang_do_assignments call.

Makes sense to me.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: IA64 linker relaxation bug

[Jim Wilson]

On Sun, 2003-11-16 at 14:54, Alan Modra wrote:
> On Sat, Nov 15, 2003 at 12:13:18AM -0800, Jim Wilson wrote:
> > 	* ldlang.c (lang_process): Move lang_reset_memory_regions call after
> > 	lang_do_assignments call.
> 
> Makes sense to me.

Thanks for the review.  I have checked in this patch.  I tested it on a
debian testing (sarge) IA-64 system by running make check.

I am dropping the elfxx-ia64.c change I suggested, because I don't know
if it is right, and it isn't necessary to fix this problem.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: PATCH: IA64 linker relaxation bug

[Jim Wilson]

On Sat, 2003-11-15 at 04:22, Hans-Peter Nilsson wrote:
> Did any of you possibly extract a test-case suitable for the
> linker test-suite?  Changes in this area are likely to break,
> you know...

I spent a couple of hours trying to create a small testcase, but I
wasn't able to exactly reproduce the problem that created the linker
failure with a small synthetic testcase.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com