Re: [PATCH][RFC] Explicitly set ELF symbol size to zero for undefined symbols -- libbfd, gold

[Simon Baldwin <simonb at google dot com>]

Alan Modra wrote:
> ...
> Oh, of course.  Silly me.  Ok, I'm happy if the size is zeroed in
> elf_link_output_extsym, but I think it should happen
>
>     case bfd_link_hash_defined:
>     case bfd_link_hash_defweak:
>       {
> 	input_sec = h->root.u.def.section;
> 	if (input_sec->output_section != NULL)
> 	  {
> ...
> 	  }
> 	else
> 	  {
> 	    BFD_ASSERT (input_sec->owner == NULL
> 			|| (input_sec->owner->flags & DYNAMIC) != 0);
> 	    sym.st_shndx = SHN_UNDEF;
> here.
> 	    input_sec = bfd_und_section_ptr;
> 	  }
>
> That way we only zap st_size for the particular case of symbols
> defined in dynamic libs.
>   

Thanks for reviews and suggestion.  Attached is a revised version of the 
patch, with complementary change to gold.

As before, test parity with pre-patch binutils-2.18.50 snapshot testsuite.

Okay for submit, or not yet?  Thanks.  --S


This patch explicitly sets the size of undefined ELF symbols acquired from
dynamic libraries to zero for linker output.

Currently, the size of an undefined ELF symbol is copied out of the object
file or DSO that supplies the symbol, on linking.  This size is unreliable,
for example in the case of two DSOs, one linking to the other.  The lower-
level DSO could make an ABI-preserving change that alters the symbol size,
with no hard requirement to rebuild the higher-level DSO.  And if the higher-
level DSO is rebuilt, tools that monitor file checksums will register a
change due to the altered size of the undefined symbol, even though nothing
else about the higher-level DSO has altered.  This can lead to unnecessary
and undesirable rebuild and change cascades in checksum-based systems.

Confirmed test parity between pre- and post-patch source trees.


bfd/ChangeLog:
2008-07-21  Simon Baldwin  <simonb@google.com>

	* elflink.c (elf_link_output_extsym): Set st_size to zero for
	undefined symbols from dynamic libraries.

gold/ChangeLog:
2008-07-21  Simon Baldwin  <simonb@google.com>

	* symtab.cc (Symbol_table::sized_write_symbol): Set st_size to
	zero for undefined symbols from dynamic libraries.


*** ./bfd/elflink.c.orig	Thu Jul 10 15:39:15 2008
--- ./bfd/elflink.c	Mon Jul 21 12:07:33 2008
*************** elf_link_output_extsym (struct elf_link_
*** 8550,8569 ****
--- 8550,8574 ----
  				    && !input_sec->gc_mark);
  		      }
  		  }
  	      }
  	  }
  	else
  	  {
  	    BFD_ASSERT (input_sec->owner == NULL
  			|| (input_sec->owner->flags & DYNAMIC) != 0);
  	    sym.st_shndx = SHN_UNDEF;
+             /* Reset size to zero for undefined symbols.  Undefined symbols
+                may be picked up from shared libraries, and a shared library
+                can change between here and use with the dynamic loader,
+                making sizes other than zero invalid.  */
+             sym.st_size = 0;
  	    input_sec = bfd_und_section_ptr;
  	  }
        }
        break;
  
      case bfd_link_hash_common:
        input_sec = h->root.u.c.p->section;
        sym.st_shndx = bed->common_section_index (input_sec);
        sym.st_value = 1 << h->root.u.c.p->alignment_power;
        break;
*** ./gold/symtab.cc.orig	Fri Jul 11 11:17:22 2008
--- ./gold/symtab.cc	Mon Jul 21 12:57:53 2008
*************** void
*** 2292,2312 ****
  Symbol_table::sized_write_symbol(
      Sized_symbol<size>* sym,
      typename elfcpp::Elf_types<size>::Elf_Addr value,
      unsigned int shndx,
      const Stringpool* pool,
      unsigned char* p) const
  {
    elfcpp::Sym_write<size, big_endian> osym(p);
    osym.put_st_name(pool->get_offset(sym->name()));
    osym.put_st_value(value);
!   osym.put_st_size(sym->symsize());
    // A version script may have overridden the default binding.
    if (sym->is_forced_local())
      osym.put_st_info(elfcpp::elf_st_info(elfcpp::STB_LOCAL, sym->type()));
    else
      osym.put_st_info(elfcpp::elf_st_info(sym->binding(), sym->type()));
    osym.put_st_other(elfcpp::elf_st_other(sym->visibility(), sym->nonvis()));
    osym.put_st_shndx(shndx);
  }
  
  // Check for unresolved symbols in shared libraries.  This is
--- 2292,2316 ----
  Symbol_table::sized_write_symbol(
      Sized_symbol<size>* sym,
      typename elfcpp::Elf_types<size>::Elf_Addr value,
      unsigned int shndx,
      const Stringpool* pool,
      unsigned char* p) const
  {
    elfcpp::Sym_write<size, big_endian> osym(p);
    osym.put_st_name(pool->get_offset(sym->name()));
    osym.put_st_value(value);
!   // Use a symbol size of zero for undefined symbols from shared libraries.
!   if (shndx == elfcpp::SHN_UNDEF && sym->in_dyn())
!     osym.put_st_size(0);
!   else
!     osym.put_st_size(sym->symsize());
    // A version script may have overridden the default binding.
    if (sym->is_forced_local())
      osym.put_st_info(elfcpp::elf_st_info(elfcpp::STB_LOCAL, sym->type()));
    else
      osym.put_st_info(elfcpp::elf_st_info(sym->binding(), sym->type()));
    osym.put_st_other(elfcpp::elf_st_other(sym->visibility(), sym->nonvis()));
    osym.put_st_shndx(shndx);
  }
  
  // Check for unresolved symbols in shared libraries.  This is