elf symbol size

This changes elf_obj_sy.size from being malloc'd to being on the notes
obstack.  That means no code needs to free these expressions, which in
turn means that the size expression can be shared when cloning
symbols.  Nothing modifies the size expressions except when resolving.
In all cases I could see, if the size changes the entire expression is
replaced.

The patch also extracts code from elf_copy_symbol_attributes into a
separate function for use by riscv and aarch64.

	* config/obj-elf.c (elf_obj_symbol_clone_hook): Delete.
	(elf_copy_symbol_size): New function, extracted and modified from..
	(elf_copy_symbol_attributes): ..here.
	(obj_elf_size): Don't free size and use notes_alloc.
	(elf_frob_symbol): Don't free size.
	(elf_format_ops): Zero symbol_clone_hook.
	* config/obj-elf.h (elf_obj_symbol_clone_hook): Delete.
	(obj_symbol_clone_hook): Don't define.
	(elf_copy_symbol_size): Declare.
	* config/tc-aarch64.c (aarch64_elf_copy_symbol_attributes): Delete.
	* config/tc-aarch64.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
	elf_copy_symbol_size.
	* config/tc-alpha.c (s_alpha_end): notes_alloc symbol size exp.
	* config/tc-ia64.c (dot_endp): Likewise.
	* config/tc-kvx.c (kvx_endp): Likewise.
	* config/tc-mips.c (s_mips_end): Likewise.
	* config/tc-riscv.c (riscv_elf_copy_symbol_attributes): Delete.
	* config/tc-riscv.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
	elf_copy_symbol_size.

Author: amodra

gas/config/obj-elf.c

@@ -2273,39 +2273,25 @@ elf_obj_symbol_new_hook (symbolS *symbolP)
 #endif
 }
 
-/* Deduplicate size expressions.  We might get into trouble with
-   multiple freeing or use after free if we leave them pointing to the
-   same expressionS.  */
-
+/* If size is unset, copy size from src.  Because we don't track whether
+   .size has been used, we can't differentiate .size dest, 0 from the case
+   where dest's size is unset.  */
 void
-elf_obj_symbol_clone_hook (symbolS *newsym, symbolS *orgsym ATTRIBUTE_UNUSED)
+elf_copy_symbol_size (symbolS *dest, symbolS *src)
 {
-  struct elf_obj_sy *newelf = symbol_get_obj (newsym);
-  if (newelf->size)
+  struct elf_obj_sy *srcelf = symbol_get_obj (src);
+  struct elf_obj_sy *destelf = symbol_get_obj (dest);
+  if (!destelf->size && S_GET_SIZE (dest) == 0)
     {
-      expressionS *exp = XNEW (expressionS);
-      *exp = *newelf->size;
-      newelf->size = exp;
+      destelf->size = srcelf->size;
+      S_SET_SIZE (dest, S_GET_SIZE (src));
     }
 }
 
 void
 elf_copy_symbol_attributes (symbolS *dest, symbolS *src)
 {
-  struct elf_obj_sy *srcelf = symbol_get_obj (src);
-  struct elf_obj_sy *destelf = symbol_get_obj (dest);
-  /* If size is unset, copy size from src.  Because we don't track whether
-     .size has been used, we can't differentiate .size dest, 0 from the case
-     where dest's size is unset.  */
-  if (!destelf->size && S_GET_SIZE (dest) == 0)
-    {
-      if (srcelf->size)
-	{
-	  destelf->size = XNEW (expressionS);
-	  *destelf->size = *srcelf->size;
-	}
-      S_SET_SIZE (dest, S_GET_SIZE (src));
-    }
+  elf_copy_symbol_size (dest, src);
   /* Don't copy visibility.  */
   S_SET_OTHER (dest, (ELF_ST_VISIBILITY (S_GET_OTHER (dest))
 		      | (S_GET_OTHER (src) & ~ELF_ST_VISIBILITY (-1))));
@@ -2404,12 +2390,11 @@ obj_elf_size (int ignore ATTRIBUTE_UNUSED)
   if (exp.X_op == O_constant)
     {
       S_SET_SIZE (sym, exp.X_add_number);
-      xfree (symbol_get_obj (sym)->size);
       symbol_get_obj (sym)->size = NULL;
     }
   else
     {
-      symbol_get_obj (sym)->size = XNEW (expressionS);
+      symbol_get_obj (sym)->size = notes_alloc (sizeof (exp));
       *symbol_get_obj (sym)->size = exp;
     }
 
@@ -2805,7 +2790,6 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 	    as_warn (_(".size expression for %s "
 		       "does not evaluate to a constant"), S_GET_NAME (symp));
 	}
-      free (sy_obj->size);
       sy_obj->size = NULL;
     }
 
@@ -3336,7 +3320,7 @@ const struct format_ops elf_format_ops =
 #endif
   elf_obj_read_begin_hook,
   elf_obj_symbol_new_hook,
-  elf_obj_symbol_clone_hook,
+  0,
   elf_adjust_symtab
 };
 

gas/config/obj-elf.h

@@ -224,11 +224,7 @@ void elf_obj_symbol_new_hook (symbolS *);
 #define obj_symbol_new_hook	elf_obj_symbol_new_hook
 #endif
 
-void elf_obj_symbol_clone_hook (symbolS *, symbolS *);
-#ifndef obj_symbol_clone_hook
-#define obj_symbol_clone_hook	elf_obj_symbol_clone_hook
-#endif
-
+void elf_copy_symbol_size (symbolS *, symbolS *);
 void elf_copy_symbol_attributes (symbolS *, symbolS *);
 #ifndef OBJ_COPY_SYMBOL_ATTRIBUTES
 #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC) \

gas/config/tc-aarch64.c

@@ -11277,34 +11277,3 @@ aarch64_copy_symbol_attributes (symbolS * dest, symbolS * src)
 {
   AARCH64_GET_FLAG (dest) = AARCH64_GET_FLAG (src);
 }
-
-#ifdef OBJ_ELF
-/* Same as elf_copy_symbol_attributes, but without copying st_other.
-   This is needed so AArch64 specific st_other values can be independently
-   specified for an IFUNC resolver (that is called by the dynamic linker)
-   and the symbol it resolves (aliased to the resolver).  In particular,
-   if a function symbol has special st_other value set via directives,
-   then attaching an IFUNC resolver to that symbol should not override
-   the st_other setting.  Requiring the directive on the IFUNC resolver
-   symbol would be unexpected and problematic in C code, where the two
-   symbols appear as two independent function declarations.  */
-
-void
-aarch64_elf_copy_symbol_attributes (symbolS *dest, symbolS *src)
-{
-  struct elf_obj_sy *srcelf = symbol_get_obj (src);
-  struct elf_obj_sy *destelf = symbol_get_obj (dest);
-  /* If size is unset, copy size from src.  Because we don't track whether
-     .size has been used, we can't differentiate .size dest, 0 from the case
-     where dest's size is unset.  */
-  if (!destelf->size && S_GET_SIZE (dest) == 0)
-    {
-      if (srcelf->size)
-	{
-	  destelf->size = XNEW (expressionS);
-	  *destelf->size = *srcelf->size;
-	}
-      S_SET_SIZE (dest, S_GET_SIZE (src));
-    }
-}
-#endif

gas/config/tc-aarch64.h

@@ -133,9 +133,17 @@ void aarch64_copy_symbol_attributes (symbolS *, symbolS *);
 #endif
 
 #ifdef OBJ_ELF
-void aarch64_elf_copy_symbol_attributes (symbolS *, symbolS *);
+/* Don't copy st_other.
+   This is needed so AArch64 specific st_other values can be independently
+   specified for an IFUNC resolver (that is called by the dynamic linker)
+   and the symbol it resolves (aliased to the resolver).  In particular,
+   if a function symbol has special st_other value set via directives,
+   then attaching an IFUNC resolver to that symbol should not override
+   the st_other setting.  Requiring the directive on the IFUNC resolver
+   symbol would be unexpected and problematic in C code, where the two
+   symbols appear as two independent function declarations.  */
 #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC) \
-  aarch64_elf_copy_symbol_attributes (DEST, SRC)
+  elf_copy_symbol_size (DEST, SRC)
 #endif
 
 #define TC_START_LABEL(STR, NUL_CHAR, NEXT_CHAR)			\

gas/config/tc-alpha.c

@@ -3789,7 +3789,7 @@ s_alpha_end (int dummy ATTRIBUTE_UNUSED)
 	  if (sym && cur_frame_data)
 	    {
 	      OBJ_SYMFIELD_TYPE *obj = symbol_get_obj (sym);
-	      expressionS *exp = XNEW (expressionS);
+	      expressionS *exp = notes_alloc (sizeof (*exp));
 
 	      obj->size = exp;
 	      exp->X_op = O_subtract;

gas/config/tc-ia64.c

@@ -4476,13 +4476,14 @@ dot_endp (int dummy ATTRIBUTE_UNUSED)
 		    S_SET_SIZE (sym, frag_now_fix () - S_GET_VALUE (sym));
 		  else
 		    {
-		      symbol_get_obj (sym)->size = XNEW (expressionS);
-		      symbol_get_obj (sym)->size->X_op = O_subtract;
-		      symbol_get_obj (sym)->size->X_add_symbol
+		      OBJ_SYMFIELD_TYPE *obj = symbol_get_obj (sym);
+		      obj->size = notes_alloc (sizeof (*obj->size));
+		      obj->size->X_op = O_subtract;
+		      obj->size->X_add_symbol
 			= symbol_new (FAKE_LABEL_NAME, now_seg,
 				      frag_now, frag_now_fix ());
-		      symbol_get_obj (sym)->size->X_op_symbol = sym;
-		      symbol_get_obj (sym)->size->X_add_number = 0;
+		      obj->size->X_op_symbol = sym;
+		      obj->size->X_add_number = 0;
 		    }
 		}
 	    }

gas/config/tc-kvx.c

@@ -2355,16 +2355,11 @@ kvx_endp (int start ATTRIBUTE_UNUSED)
 	if (exp.X_op == O_constant)
 	  {
 	    S_SET_SIZE (last_proc_sym, exp.X_add_number);
-	    if (symbol_get_obj (last_proc_sym)->size)
-	      {
-		xfree (symbol_get_obj (last_proc_sym)->size);
-		symbol_get_obj (last_proc_sym)->size = NULL;
-	      }
+	    symbol_get_obj (last_proc_sym)->size = NULL;
 	  }
 	else
 	  {
-	    symbol_get_obj (last_proc_sym)->size =
-	      (expressionS *) xmalloc (sizeof (expressionS));
+	    symbol_get_obj (last_proc_sym)->size = notes_alloc (sizeof (exp));
 	    *symbol_get_obj (last_proc_sym)->size = exp;
 	  }
 

gas/config/tc-mips.c

@@ -19809,7 +19809,7 @@ s_mips_end (int x ATTRIBUTE_UNUSED)
   if (p && cur_proc_ptr)
     {
       OBJ_SYMFIELD_TYPE *obj = symbol_get_obj (p);
-      expressionS *exp = XNEW (expressionS);
+      expressionS *exp = notes_alloc (sizeof (*exp));
 
       obj->size = exp;
       exp->X_op = O_subtract;

gas/config/tc-riscv.c

@@ -5889,35 +5889,6 @@ s_variant_cc (int ignored ATTRIBUTE_UNUSED)
   elfsym->internal_elf_sym.st_other |= STO_RISCV_VARIANT_CC;
 }
 
-/* Same as elf_copy_symbol_attributes, but without copying st_other.
-   This is needed so RISC-V specific st_other values can be independently
-   specified for an IFUNC resolver (that is called by the dynamic linker)
-   and the symbol it resolves (aliased to the resolver).  In particular,
-   if a function symbol has special st_other value set via directives,
-   then attaching an IFUNC resolver to that symbol should not override
-   the st_other setting.  Requiring the directive on the IFUNC resolver
-   symbol would be unexpected and problematic in C code, where the two
-   symbols appear as two independent function declarations.  */
-
-void
-riscv_elf_copy_symbol_attributes (symbolS *dest, symbolS *src)
-{
-  struct elf_obj_sy *srcelf = symbol_get_obj (src);
-  struct elf_obj_sy *destelf = symbol_get_obj (dest);
-  /* If size is unset, copy size from src.  Because we don't track whether
-     .size has been used, we can't differentiate .size dest, 0 from the case
-     where dest's size is unset.  */
-  if (!destelf->size && S_GET_SIZE (dest) == 0)
-    {
-      if (srcelf->size)
-	{
-	  destelf->size = XNEW (expressionS);
-	  *destelf->size = *srcelf->size;
-	}
-      S_SET_SIZE (dest, S_GET_SIZE (src));
-    }
-}
-
 /* RISC-V pseudo-ops table.  */
 static const pseudo_typeS riscv_pseudo_table[] =
 {

gas/config/tc-riscv.h

@@ -175,8 +175,16 @@ extern void riscv_init_frag (struct frag *, int);
 #define obj_adjust_symtab() riscv_adjust_symtab ()
 extern void riscv_adjust_symtab (void);
 
-void riscv_elf_copy_symbol_attributes (symbolS *, symbolS *);
+/* Don't copy st_other.
+   This is needed so RISC-V specific st_other values can be independently
+   specified for an IFUNC resolver (that is called by the dynamic linker)
+   and the symbol it resolves (aliased to the resolver).  In particular,
+   if a function symbol has special st_other value set via directives,
+   then attaching an IFUNC resolver to that symbol should not override
+   the st_other setting.  Requiring the directive on the IFUNC resolver
+   symbol would be unexpected and problematic in C code, where the two
+   symbols appear as two independent function declarations.  */
 #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC)  \
-  riscv_elf_copy_symbol_attributes (DEST, SRC)
+  elf_copy_symbol_size (DEST, SRC)
 
 #endif /* TC_RISCV */