Re: PATCH: Don't allow ia64 unwind section to point to section in different files

["H. J. Lu" <hjl@lucon.org>]

On Tue, May 17, 2005 at 01:38:57PM -0700, James E Wilson wrote:
> On Fri, 2005-05-13 at 14:46, H. J. Lu wrote:
> > When weak functions are used on ia64, part of the unwind section may
> > point to the strong definition in a different file. This will lead to
> > wrong unwind info. Basically, on ia64, we have to use comdat to get the
> > right unwind info. This patch will check it.
> 
> This doesn't look like the right fix to me.
> 
> I looked at the unwind info with old and new binutils, and noticed that
> the relocs are different.  In old binutils, we have
> RELOCATION RECORDS FOR [.IA_64.unwind]:
> OFFSET           TYPE              VALUE
> 0000000000000000 SEGREL64LSB       .text
> in new binutils we have
> RELOCATION RECORDS FOR [.IA_64.unwind]:
> OFFSET           TYPE              VALUE
> 0000000000000000 SEGREL64LSB       _start
> 
> Since _start is at offset 0 in .text, these are the same value normally,
> but they aren't if we have duplicate weak/strong functions.  Since the
> unwind info should always bind locally, I think it is wrong to have the
> unresolved _start symbol here.
> 
> This change comes from one of Jan Beulich's patches.  It used to be that
> unwind.proc_start was expr_build_dot(), now it is set to "sym", which is
> the symbol of the current function.  When that change was made, we
> didn't know why expr_build_dot was being used here.  Now it appears it
> was necessary to fix a problem with bad unwind info in the kernel.
> 
> Unfortunately, Jan's change was added to improve unwind info checking,
> so we can't easily change back without losing some new features.  But
> maybe there is a way to create the relocation differently, so that the
> symbol name gets resolved into a section offset in the assembler?  Or
> maybe we can create a fake local symbol with the same address to put in
> the relocation, to ensure it is resolved locally?  I am not sure of the
> best approach here.

How about this patch?


H.J.
----
2005-05-17  H.J. Lu  <hongjiu.lu@intel.com>

	* config/tc-ia64.c (unwind): Add proc_start_addr.
	(dot_proc): Set unwind.proc_start_addr to expr_build_dot ().
	(dot_endp): Use unwind.proc_start_addr instead of
	unwind.proc_start for unwind info.

--- gas/config/tc-ia64.c.unwind	2005-05-08 12:02:03.000000000 -0700
+++ gas/config/tc-ia64.c	2005-05-17 15:04:03.000000000 -0700
@@ -723,6 +723,9 @@ static struct
 
   /* These are used to create the unwind table entry for this function.  */
   symbolS *proc_start;
+  /* The unwind info has to be resolved locally even if the function is
+     global.  */
+  symbolS *proc_start_addr;
   symbolS *info;		/* pointer to unwind info */
   symbolS *personality_routine;
   segT saved_text_seg;
@@ -4298,8 +4301,9 @@ dot_proc (dummy)
 	break;
       ++input_line_pointer;
     }
+  unwind.proc_start_addr = expr_build_dot ();
   if (unwind.proc_start == 0)
-    unwind.proc_start = expr_build_dot ();
+    unwind.proc_start = unwind.proc_start_addr;
   demand_empty_rest_of_line ();
   ia64_do_align (16);
 
@@ -4442,7 +4446,7 @@ dot_endp (dummy)
       e.X_op = O_pseudo_fixup;
       e.X_op_symbol = pseudo_func[FUNC_SEG_RELATIVE].u.sym;
       e.X_add_number = 0;
-      e.X_add_symbol = unwind.proc_start;
+      e.X_add_symbol = unwind.proc_start_addr;
       ia64_cons_fix_new (frag_now, where, bytes_per_address, &e);
 
       e.X_op = O_pseudo_fixup;
@@ -4548,7 +4552,7 @@ dot_endp (dummy)
       ++input_line_pointer;
     }
   demand_empty_rest_of_line ();
-  unwind.proc_start = unwind.info = 0;
+  unwind.proc_start = unwind.proc_start_addr = unwind.info = 0;
 }
 
 static void