{GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

{GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Viktor Kutuzov]

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

Hi Ian,

I'm trying to cross build llvm and llvm-gcc on Linux for ARM by using
GOLD as the linker. This has exposed some problems we have in GOLD. 

One of them related to the assert in the relocate_for_relocatable()
method (target-reloc.h, line 557):

  new_symndx = object->symtab_index(r_sym);
  gold_assert(new_symndx != -1U);

This assert gets triggered when the build links glibc with the -r -X
flags (remove local symbols).

This happens because with the given -X option GOLD removes all local
symbols, including those which still needed to resolve static relocs
later.

LD keeps that kind of local symbols in the symbol table even if -X
requested. I guess GOLD should do the same.

Please find attached the patch that fixes this issue.

Thank you,
Viktor.


[-- Attachment #2: binutils-gold-prevent_discard_needed_local_symbols.patch --]
[-- Type: text/x-patch, Size: 881 bytes --]

Index: object.cc
===================================================================
RCS file: /cvs/src/src/gold/object.cc,v
retrieving revision 1.118
diff -u -p -r1.118 object.cc
--- object.cc	15 Jan 2010 01:44:22 -0000	1.118
+++ object.cc	10 Feb 2010 19:26:16 -0000
@@ -1609,9 +1609,13 @@ Sized_relobj<size, big_endian>::do_count
       //   - the symbol has a name.
       //
       // We do not discard a symbol if it needs a dynamic symbol entry.
+      // Also, if we are generating a relocatable output file, then 
+      // some of the local symbols may be required by relocs; we output them
+      // below if they are needed.
       if (discard_locals
 	  && sym.get_st_type() != elfcpp::STT_FILE
 	  && !lv.needs_output_dynsym_entry()
+	  && !lv.needs_output_symtab_entry()
 	  && parameters->target().is_local_label_name(name))
 	{
 	  lv.set_no_output_symtab_entry();

Re: {GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Ian Lance Taylor]

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> I'm trying to cross build llvm and llvm-gcc on Linux for ARM by using
> GOLD as the linker. This has exposed some problems we have in GOLD. 
>
> One of them related to the assert in the relocate_for_relocatable()
> method (target-reloc.h, line 557):
>
>   new_symndx = object->symtab_index(r_sym);
>   gold_assert(new_symndx != -1U);
>
> This assert gets triggered when the build links glibc with the -r -X
> flags (remove local symbols).
>
> This happens because with the given -X option GOLD removes all local
> symbols, including those which still needed to resolve static relocs
> later.
>
> LD keeps that kind of local symbols in the symbol table even if -X
> requested. I guess GOLD should do the same.
>
> Please find attached the patch that fixes this issue.

Thanks, but this patch doesn't work.  It fails the testsuite.  The
problem is that at the time your patch checks
needs_output_symtab_entry, it will always return true.
needs_output_symtab_entry will only return false if
set_no_output_symtab_entry has been called, and that only happens at
the end of the loop you patched.

I think what we need to do here is mark the local symbol as
appropriate in scan_relocatable_relocs.  The use of the
output_symtab_index_ field right now is a bit complicated.  It is
initialized to 0.  In Sized_relobj::do_count_local_symbols we set the
field to -1 if the symbol is not needed.  In
Sized_relobj::do_finalize_local_symbols, if the field is still 0, we
set it to the index in the output file.  Perhaps we should change that
0 == uninitialized, -1 == no index, -2 == needs index.  Then
do_finalize_local_symbols should never see 0.

Ian

Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Viktor Kutuzov]

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

Ian,
please find attached updated patch, which fix the local symbol
discarding behavior for the relocatable output objects (when used the -r
and -X/-x linker options together). I've updated this patch and the unit
tests accordingly. 

2010-02-24  Viktor Kutuzov (vkutuzov@accesssoftek.com)
    * target-reloc.h (scan_relocatable_relocs): Marking needed local
symbols for the relocatable objects.
    * object.h (Symbol_value<int>::set_output_symtab_index): Updated
assert to support a special index value (-2) for the needed local
symbols.
    (Symbol_value<size>::set_no_output_symtab_entry): Likewise.
    (Symbol_value<size>::set_needs_in_output_symtab): New method.
    (Symbol_value<size>::needs_in_output_symtab): New method.
    (Sized_relobj<size, big_endianl>::set_needs_in_output_symtab): New
method.
    * object.cc (Sized_relobj<size,
big_endian>::do_count_local_symbols): prevent discarding needed local
symbols for the relocatable output objects.
    (Sized_relobj<size, big_endian>::write_local_symbols): Likewise.
    * testsuite/discard_locals_relocatable_test.c: New file.
    * testsuite/discard_locals_test.sh: Updated with new test cases.
    * testsuite/Makefile.am: Likewise.
    * testsuite/Makefile.in: Likewise.

-Viktor.

On Wed, 2010-02-10 at 18:14 -0800, Ian Lance Taylor wrote:
> Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:
> 
> > I'm trying to cross build llvm and llvm-gcc on Linux for ARM by using
> > GOLD as the linker. This has exposed some problems we have in GOLD. 
> >
> > One of them related to the assert in the relocate_for_relocatable()
> > method (target-reloc.h, line 557):
> >
> >   new_symndx = object->symtab_index(r_sym);
> >   gold_assert(new_symndx != -1U);
> >
> > This assert gets triggered when the build links glibc with the -r -X
> > flags (remove local symbols).
> >
> > This happens because with the given -X option GOLD removes all local
> > symbols, including those which still needed to resolve static relocs
> > later.
> >
> > LD keeps that kind of local symbols in the symbol table even if -X
> > requested. I guess GOLD should do the same.
> >
> > Please find attached the patch that fixes this issue.
> 
> Thanks, but this patch doesn't work.  It fails the testsuite.  The
> problem is that at the time your patch checks
> needs_output_symtab_entry, it will always return true.
> needs_output_symtab_entry will only return false if
> set_no_output_symtab_entry has been called, and that only happens at
> the end of the loop you patched.
> 
> I think what we need to do here is mark the local symbol as
> appropriate in scan_relocatable_relocs.  The use of the
> output_symtab_index_ field right now is a bit complicated.  It is
> initialized to 0.  In Sized_relobj::do_count_local_symbols we set the
> field to -1 if the symbol is not needed.  In
> Sized_relobj::do_finalize_local_symbols, if the field is still 0, we
> set it to the index in the output file.  Perhaps we should change that
> 0 == uninitialized, -1 == no index, -2 == needs index.  Then
> do_finalize_local_symbols should never see 0.
> 
> Ian


-- 
-Viktor

[-- Attachment #2: binutils-gold-prevent_discard_needed_local_symbols-updated.patch --]
[-- Type: text/x-patch, Size: 11471 bytes --]

--- binutils.orig/src/gold/target-reloc.h	2010-01-14 10:23:06.000000000 -0800
+++ binutils.gold/src/gold/target-reloc.h	2010-02-24 12:19:57.000000000 -0800
@@ -491,6 +491,16 @@
 		  if (strategy != Relocatable_relocs::RELOC_DISCARD)
                     object->output_section(shndx)->set_needs_symtab_index();
 		}
+
+	      if ((parameters->options().discard_locals()
+		  || parameters->options().discard_all())
+		  && parameters->options().relocatable()
+		  && strategy != Relocatable_relocs::RELOC_DISCARD)
+		{
+		  // For the relocatable output objects, mark this local
+		  // symbol as needed in the output symbol table.
+		  object->set_needs_in_output_symtab(r_sym);
+		}
 	    }
 	}
 
--- binutils.orig/src/gold/object.h	2010-02-24 12:01:23.000000000 -0800
+++ binutils.gold/src/gold/object.h	2010-02-24 12:25:59.000000000 -0800
@@ -1105,7 +1105,8 @@
   void
   set_output_symtab_index(unsigned int i)
   {
-    gold_assert(this->output_symtab_index_ == 0);
+    gold_assert(this->output_symtab_index_ == 0
+	        || this->output_symtab_index_ == -2U);
     this->output_symtab_index_ = i;
   }
 
@@ -1114,10 +1115,27 @@
   void
   set_no_output_symtab_entry()
   {
-    gold_assert(this->output_symtab_index_ == 0);
+    gold_assert(this->output_symtab_index_ == 0
+	        || this->output_symtab_index_ == -2U);
     this->output_symtab_index_ = -1U;
   }
 
+  // Record that this symbol should go into the output symbol table.
+  void
+  set_needs_in_output_symtab()
+  {
+    // Keep a valid symtab index
+    if (this->output_symtab_index_ == 0
+	|| this->output_symtab_index_ == -1U)
+      this->output_symtab_index_ = -2U;
+  }
+
+  // For the relocatable ouput object, return whether this symbol
+  // should go into the output symbol table. This method always returns
+  // false for any other type of the output objects.
+  bool needs_in_output_symtab()
+  { return (this->output_symtab_index_ == -2U); }
+
   // Set the index in the output dynamic symbol table.
   void
   set_needs_output_dynsym_entry()
@@ -1429,6 +1447,14 @@
     this->local_values_[sym].set_needs_output_dynsym_entry();
   }
 
+  // Record that local symbol SYM should an output symbol entry.
+  void
+  set_needs_in_output_symtab(unsigned int sym)
+  {
+    gold_assert(sym < this->local_values_.size());
+    this->local_values_[sym].set_needs_in_output_symtab();
+  }
+
   // Return whether the local symbol SYMNDX has a GOT offset.
   // For TLS symbols, the GOT entry will hold its tp-relative offset.
   bool
--- binutils.orig/src/gold/object.cc	2010-02-24 12:01:23.000000000 -0800
+++ binutils.gold/src/gold/object.cc	2010-02-24 12:18:17.000000000 -0800
@@ -1591,7 +1590,7 @@
           ++dyncount;
         }
 
-      if (discard_all)
+      if (discard_all && !lv.needs_in_output_symtab())
 	{
 	  lv.set_no_output_symtab_entry();
 	  continue;
@@ -1612,6 +1611,7 @@
       if (discard_locals
 	  && sym.get_st_type() != elfcpp::STT_FILE
 	  && !lv.needs_output_dynsym_entry()
+	  && !lv.needs_in_output_symtab()
 	  && parameters->target().is_local_label_name(name))
 	{
 	  lv.set_no_output_symtab_entry();
@@ -1937,7 +1937,8 @@
 	  st_shndx = out_sections[st_shndx]->out_shndx();
 	  if (st_shndx >= elfcpp::SHN_LORESERVE)
 	    {
-	      if (lv.needs_output_symtab_entry() && !strip_all)
+	      if (lv.needs_output_symtab_entry()
+		  && !(strip_all && !lv.needs_in_output_symtab()))
 		symtab_xindex->add(lv.output_symtab_index(), st_shndx);
 	      if (lv.needs_output_dynsym_entry())
 		dynsym_xindex->add(lv.output_dynsym_index(), st_shndx);
@@ -1946,7 +1947,8 @@
 	}
 
       // Write the symbol to the output symbol table.
-      if (!strip_all && lv.needs_output_symtab_entry())
+      if (!(strip_all && !lv.needs_in_output_symtab())
+	  && lv.needs_output_symtab_entry())
         {
           elfcpp::Sym_write<size, big_endian> osym(ov);
 
--- binutils.orig/src/gold/testsuite/discard_locals_relocatable_test.c	1969-12-31 16:00:00.000000000 -0800
+++ binutils.gold/src/gold/testsuite/discard_locals_relocatable_test.c	2010-02-24 11:38:15.000000000 -0800
@@ -0,0 +1,48 @@
+/* discard_locals_relocatable_test.c -- test --discard-locals/--discard-all 
+   options for the relocatable output objects 
+   (linked with -relocatable option).
+
+   Note: use GCC -fPIC option to compile this test.
+
+   Copyright 2010 Free Software Foundation, Inc.
+   Viktor Kutuzov <vkutuzov@accesssoftek.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.
+
+   This is a test of a common symbol in the main program and a
+   versioned symbol in a shared library.  The common symbol in the
+   main program should override the shared library symbol.  */
+
+/* Local symbol format for generic ELF target. 
+   Use GCC -Wa,-L option to preserve this local symbol
+   in the output object file. */
+asm (".Lshould_be_discarded:");
+
+int func(void);
+
+extern void print_func(const char* s);
+
+int func (void)
+{
+  print_func( 
+    "This string is splitted to force generation of"
+    "the temporary local symbols like .LXx and the related relocations."
+    "We expect the .LC0 symbol in the source object file.");
+  return 0;
+}
+
--- binutils.orig/src/gold/testsuite/discard_locals_test.sh	2009-06-05 14:32:57.000000000 -0700
+++ binutils.gold/src/gold/testsuite/discard_locals_test.sh	2010-02-24 10:51:35.000000000 -0800
@@ -27,11 +27,12 @@
 # the resulting executable and check that symbols from two test library
 # archives are correctly hidden or left unmodified.
 
-check()
+check_discarded()
 {
     file=$1
+    sym=$2
 
-    found=`egrep "should_be_discarded" $file`
+    found=`egrep $sym $file`
     if test -n "$found"; then
 	echo "These local symbols are not discarded in $file:"
 	echo "$found"
@@ -39,6 +40,24 @@
     fi
 }
 
-check "discard_locals_test.syms"
+check_non_discarded()
+{
+    file=$1
+    sym=$2
+
+    found=`egrep $sym $file`
+    if test -z "$found"; then
+	echo "This local symbol is discarded in $file:"
+	echo "$2"
+	exit 1
+    fi
+}
+
+check_discarded     "discard_locals_test.syms" "should_be_discarded"
+
+check_non_discarded "discard_locals_relocatable_test1.syms" ".LC0"
+check_discarded     "discard_locals_relocatable_test1.syms" "should_be_discarded"
+check_non_discarded "discard_locals_relocatable_test2.syms" ".LC0"
+check_discarded     "discard_locals_relocatable_test2.syms" "should_be_discarded"
 
 exit 0
--- binutils.orig/src/gold/testsuite/Makefile.am	2010-02-24 12:01:23.000000000 -0800
+++ binutils.gold/src/gold/testsuite/Makefile.am	2010-02-24 10:32:03.000000000 -0800
@@ -1285,8 +1285,11 @@
 
 check_PROGRAMS += discard_locals_test
 check_SCRIPTS += discard_locals_test.sh
-check_DATA += discard_locals_test.syms
-MOSTLYCLEANFILES += discard_locals_test.syms
+check_DATA += discard_locals_test.syms \
+    discard_locals_relocatable_test1.syms discard_locals_relocatable_test2.syms
+MOSTLYCLEANFILES += discard_locals_test.syms \
+    discard_locals_relocatable_test1.syms discard_locals_relocatable_test2.syms \
+    discard_locals_relocatable_test1.out discard_locals_relocatable_test2.out
 discard_locals_test_SOURCES = discard_locals_test.c
 discard_locals_test_LDFLAGS = -Bgcctestdir/ -Wl,--discard-locals
 discard_locals_test.syms: discard_locals_test
@@ -1295,6 +1298,18 @@
 discard_locals_test.o: discard_locals_test.c
 	$(COMPILE) -c -Wa,-L -o $@ $<
 
+discard_locals_relocatable_test1.syms: discard_locals_relocatable_test1.out
+	$(TEST_READELF) -sW $< >$@ 2>/dev/null
+discard_locals_relocatable_test.o: discard_locals_relocatable_test.c
+	$(COMPILE) -c -Wa,-L -fPIC -o $@ $<
+discard_locals_relocatable_test1.out: discard_locals_relocatable_test.o
+	../ld-new --discard-locals -relocatable -o $@ $<
+
+discard_locals_relocatable_test2.syms: discard_locals_relocatable_test2.out
+	$(TEST_READELF) -sW $< >$@ 2>/dev/null
+discard_locals_relocatable_test2.out: discard_locals_relocatable_test.o
+	../ld-new --discard-all -relocatable -o $@ $<
+
 if MCMODEL_MEDIUM
 check_PROGRAMS += large
 large_SOURCES = large.c
--- binutils.orig/src/gold/testsuite/Makefile.in	2010-02-24 12:01:23.000000000 -0800
+++ binutils.gold/src/gold/testsuite/Makefile.in	2010-02-24 10:41:20.000000000 -0800
@@ -276,6 +276,8 @@
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	no_version_test.sh
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_28 = exclude_libs_test.syms \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_test.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_relocatable_test1.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_relocatable_test2.syms \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	hidden_test.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	retain_symbols_file_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	no_version_test.stdout
@@ -284,6 +286,10 @@
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	libexclude_libs_test_2.a \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	alt/libexclude_libs_test_3.a \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_test.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_relocatable_test1.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_relocatable_test2.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_relocatable_test1.out \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	discard_locals_relocatable_test2.out \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	hidden_test hidden_test.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	retain_symbols_file_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	retain_symbols_file_test.in \
@@ -3136,6 +3142,18 @@
 # '-Wa,-L' is required to preserve the local label used for testing.
 @GCC_TRUE@@NATIVE_LINKER_TRUE@discard_locals_test.o: discard_locals_test.c
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(COMPILE) -c -Wa,-L -o $@ $<
+
+@GCC_TRUE@@NATIVE_LINKER_TRUE@discard_locals_relocatable_test1.syms: discard_locals_relocatable_test1.out
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -sW $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@discard_locals_relocatable_test.o: discard_locals_relocatable_test.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(COMPILE) -c -Wa,-L -fPIC -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@discard_locals_relocatable_test1.out: discard_locals_relocatable_test.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	../ld-new --discard-locals -relocatable -o $@ $<
+
+@GCC_TRUE@@NATIVE_LINKER_TRUE@discard_locals_relocatable_test2.syms: discard_locals_relocatable_test2.out
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -sW $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@discard_locals_relocatable_test2.out: discard_locals_relocatable_test.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	../ld-new --discard-all -relocatable -o $@ $<
 @GCC_TRUE@@NATIVE_LINKER_TRUE@libhidden.so: hidden_test_1.c gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(COMPILE) -Bgcctestdir/ -g -shared -fPIC -w -o $@ $(srcdir)/hidden_test_1.c
 @GCC_TRUE@@NATIVE_LINKER_TRUE@hidden_test: hidden_test_main.o libhidden.so gcctestdir/ld

Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Ian Lance Taylor]

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

Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:

> please find attached updated patch, which fix the local symbol
> discarding behavior for the relocatable output objects (when used the -r
> and -X/-x linker options together). I've updated this patch and the unit
> tests accordingly. 
>
> 2010-02-24  Viktor Kutuzov (vkutuzov@accesssoftek.com)
>     * target-reloc.h (scan_relocatable_relocs): Marking needed local
> symbols for the relocatable objects.
>     * object.h (Symbol_value<int>::set_output_symtab_index): Updated
> assert to support a special index value (-2) for the needed local
> symbols.
>     (Symbol_value<size>::set_no_output_symtab_entry): Likewise.
>     (Symbol_value<size>::set_needs_in_output_symtab): New method.
>     (Symbol_value<size>::needs_in_output_symtab): New method.
>     (Sized_relobj<size, big_endianl>::set_needs_in_output_symtab): New
> method.
>     * object.cc (Sized_relobj<size,
> big_endian>::do_count_local_symbols): prevent discarding needed local
> symbols for the relocatable output objects.
>     (Sized_relobj<size, big_endian>::write_local_symbols): Likewise.
>     * testsuite/discard_locals_relocatable_test.c: New file.
>     * testsuite/discard_locals_test.sh: Updated with new test cases.
>     * testsuite/Makefile.am: Likewise.
>     * testsuite/Makefile.in: Likewise.

Thanks.  I decided to do this somewhat differently, to try to make the
use of the output_symtab functions and values slightly more coherent.

Sorry for the delay.

Ian


2010-03-03  Viktor Kutuzov  <vkutuzov@accesssoftek.com>
	    Ian Lance Taylor  <iant@google.com>

	* target-reloc.h (relocate_section): Check the symbol table index
	for -1U before setting the local symbol index.
	(scan_relocatable_relocs): If copying the relocation, record that
	the local symbol is required.
	* object.h (Symbol_value::is_output_symtab_index_set): New
	function.
	(Symbol_value::may_be_discarded_from_output_symtab): New
	function.
	(Symbol_value::has_output_symtab_entry): New function.
	(Symbol_value::needs_output_symtab_entry): Remove.
	(Symbol_value::output_symtab_index): Make sure the symbol index is
	set.
	(Symbol_value::set_output_symtab_index): Make sure the symbol
	index is not set.  Make sure the new index is valid.
	(Symbol_value::set_must_have_output_symtab_entry): New function.
	(Symbol_value::has_output_dynsym_entry): New function.
	(Symbol_value::set_output_dynsym_index): Make sure the new index
	is valid.
	(Sized_relobj::set_must_have_output_symtab_entry): New function.
	* object.cc (Sized_relobj::do_count_local_symbols): Only discard a
	local symbol if permitted.
	(Sized_relobj::do_finalize_local_symbols): Call
	is_output_symtab_index_set rather than needs_output_symtab_entry.
	(Sized_relobj::write_local_symbols): Call has_output_symtab_entry
	rather than needs_output_symtab_entry.  Call
	has_output_dynsym_entry rather than needs_output_dynsym_entry.
	* arm.cc (Arm_relobj::update_output_local_symbol_count): Call
	is_output_symtab_index_set rather than needs_output_symtab_entry.
	* testsuite/discard_locals_relocatable_test.c: New file.
	* testsuite/discard_locals_test.sh: Test -r.
	* testsuite/Makefile.am (check_DATA): Add
	discard_locals_relocatable_test1.syms,
	discard_local_relocatable_test2.syms.
	(MOSTLYCLEANFILES): Likewise.  Also add
	discard_locals_relocatable_test1.lout and
	discard_locals_relocatable_test2.out.
	(discard_locals_relocatable_test1.syms): New target.
	(discard_locals_relocatable_test.o): New target.
	(discard_locals_relocatable_test1.out): New target.
	(discard_locals_relocatable_test2.syms): New target.
	(discard_locals_relocatable_test2.out): New target.
	(various): Add missing ../ld-new dependencies.
	* testsuite/Makefile.in: Rebuild.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: discard --]
[-- Type: text/x-diff, Size: 16897 bytes --]

Index: arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.90
retrieving revision 1.91
diff -p -u -r1.90 -r1.91
--- arm.cc	27 Feb 2010 00:46:00 -0000	1.90
+++ arm.cc	3 Mar 2010 19:31:54 -0000	1.91
@@ -6506,7 +6506,7 @@ Arm_relobj<big_endian>::update_output_lo
       Symbol_value<32>& lv((*this->local_values())[i]);
 
       // This local symbol was already discarded by do_count_local_symbols.
-      if (!lv.needs_output_symtab_entry())
+      if (!lv.is_output_symtab_index_set())
 	continue;
 
       bool is_ordinary;
Index: object.cc
===================================================================
RCS file: /cvs/src/src/gold/object.cc,v
retrieving revision 1.119
retrieving revision 1.120
diff -p -u -r1.119 -r1.120
--- object.cc	11 Feb 2010 07:40:11 -0000	1.119
+++ object.cc	3 Mar 2010 19:31:54 -0000	1.120
@@ -1591,7 +1591,7 @@ Sized_relobj<size, big_endian>::do_count
           ++dyncount;
         }
 
-      if (discard_all)
+      if (discard_all && lv.may_be_discarded_from_output_symtab())
 	{
 	  lv.set_no_output_symtab_entry();
 	  continue;
@@ -1612,6 +1612,7 @@ Sized_relobj<size, big_endian>::do_count
       if (discard_locals
 	  && sym.get_st_type() != elfcpp::STT_FILE
 	  && !lv.needs_output_dynsym_entry()
+	  && lv.may_be_discarded_from_output_symtab()
 	  && parameters->target().is_local_label_name(name))
 	{
 	  lv.set_no_output_symtab_entry();
@@ -1774,7 +1775,7 @@ Sized_relobj<size, big_endian>::do_final
 				+ lv.input_value());
 	}
 
-      if (lv.needs_output_symtab_entry())
+      if (!lv.is_output_symtab_index_set())
         {
           lv.set_output_symtab_index(index);
           ++index;
@@ -1937,16 +1938,16 @@ Sized_relobj<size, big_endian>::write_lo
 	  st_shndx = out_sections[st_shndx]->out_shndx();
 	  if (st_shndx >= elfcpp::SHN_LORESERVE)
 	    {
-	      if (lv.needs_output_symtab_entry() && !strip_all)
+	      if (lv.has_output_symtab_entry())
 		symtab_xindex->add(lv.output_symtab_index(), st_shndx);
-	      if (lv.needs_output_dynsym_entry())
+	      if (lv.has_output_dynsym_entry())
 		dynsym_xindex->add(lv.output_dynsym_index(), st_shndx);
 	      st_shndx = elfcpp::SHN_XINDEX;
 	    }
 	}
 
       // Write the symbol to the output symbol table.
-      if (!strip_all && lv.needs_output_symtab_entry())
+      if (lv.has_output_symtab_entry())
         {
           elfcpp::Sym_write<size, big_endian> osym(ov);
 
@@ -1963,7 +1964,7 @@ Sized_relobj<size, big_endian>::write_lo
         }
 
       // Write the symbol to the output dynamic symbol table.
-      if (lv.needs_output_dynsym_entry())
+      if (lv.has_output_dynsym_entry())
         {
           gold_assert(dyn_ov < dyn_oview + dyn_output_size);
           elfcpp::Sym_write<size, big_endian> osym(dyn_ov);
Index: object.h
===================================================================
RCS file: /cvs/src/src/gold/object.h,v
retrieving revision 1.92
retrieving revision 1.93
diff -p -u -r1.92 -r1.93
--- object.h	11 Feb 2010 07:42:17 -0000	1.92
+++ object.h	3 Mar 2010 19:31:54 -0000	1.93
@@ -1087,17 +1087,39 @@ class Symbol_value
   input_value() const
   { return this->u_.value; }
 
-  // Return whether this symbol should go into the output symbol
+  // Return whether we have set the index in the output symbol table
+  // yet.
+  bool
+  is_output_symtab_index_set() const
+  {
+    return (this->output_symtab_index_ != 0
+	    && this->output_symtab_index_ != -2U);
+  }
+
+  // Return whether this symbol may be discarded from the normal
+  // symbol table.
+  bool
+  may_be_discarded_from_output_symtab() const
+  {
+    gold_assert(!this->is_output_symtab_index_set());
+    return this->output_symtab_index_ != -2U;
+  }
+
+  // Return whether this symbol has an entry in the output symbol
   // table.
   bool
-  needs_output_symtab_entry() const
-  { return this->output_symtab_index_ != -1U; }
+  has_output_symtab_entry() const
+  {
+    gold_assert(this->is_output_symtab_index_set());
+    return this->output_symtab_index_ != -1U;
+  }
 
   // Return the index in the output symbol table.
   unsigned int
   output_symtab_index() const
   {
-    gold_assert(this->output_symtab_index_ != 0);
+    gold_assert(this->is_output_symtab_index_set()
+		&& this->output_symtab_index_ != -1U);
     return this->output_symtab_index_;
   }
 
@@ -1105,7 +1127,8 @@ class Symbol_value
   void
   set_output_symtab_index(unsigned int i)
   {
-    gold_assert(this->output_symtab_index_ == 0);
+    gold_assert(!this->is_output_symtab_index_set());
+    gold_assert(i != 0 && i != -1U && i != -2U);
     this->output_symtab_index_ = i;
   }
 
@@ -1118,6 +1141,15 @@ class Symbol_value
     this->output_symtab_index_ = -1U;
   }
 
+  // Record that this symbol must go into the output symbol table,
+  // because it there is a relocation that uses it.
+  void
+  set_must_have_output_symtab_entry()
+  {
+    gold_assert(!this->is_output_symtab_index_set());
+    this->output_symtab_index_ = -2U;
+  }
+
   // Set the index in the output dynamic symbol table.
   void
   set_needs_output_dynsym_entry()
@@ -1126,7 +1158,7 @@ class Symbol_value
     this->output_dynsym_index_ = 0;
   }
 
-  // Return whether this symbol should go into the output symbol
+  // Return whether this symbol should go into the dynamic symbol
   // table.
   bool
   needs_output_dynsym_entry() const
@@ -1134,11 +1166,21 @@ class Symbol_value
     return this->output_dynsym_index_ != -1U;
   }
 
+  // Return whether this symbol has an entry in the dynamic symbol
+  // table.
+  bool
+  has_output_dynsym_entry() const
+  {
+    gold_assert(this->output_dynsym_index_ != 0);
+    return this->output_dynsym_index_ != -1U;
+  }
+
   // Record that this symbol should go into the dynamic symbol table.
   void
   set_output_dynsym_index(unsigned int i)
   {
     gold_assert(this->output_dynsym_index_ == 0);
+    gold_assert(i != 0 && i != -1U);
     this->output_dynsym_index_ = i;
   }
 
@@ -1195,10 +1237,13 @@ class Symbol_value
 
  private:
   // The index of this local symbol in the output symbol table.  This
-  // will be -1 if the symbol should not go into the symbol table.
+  // will be 0 if no value has been assigned yet, and the symbol may
+  // be omitted.  This will be -1U if the symbol should not go into
+  // the symbol table.  This will be -2U if the symbol must go into
+  // the symbol table, but no index has been assigned yet.
   unsigned int output_symtab_index_;
   // The index of this local symbol in the dynamic symbol table.  This
-  // will be -1 if the symbol should not go into the symbol table.
+  // will be -1U if the symbol should not go into the symbol table.
   unsigned int output_dynsym_index_;
   // The section index in the input file in which this symbol is
   // defined.
@@ -1421,6 +1466,14 @@ class Sized_relobj : public Relobj
     return this->local_values_[sym].input_shndx(is_ordinary);
   }
 
+  // Record that local symbol SYM must be in the output symbol table.
+  void
+  set_must_have_output_symtab_entry(unsigned int sym)
+  {
+    gold_assert(sym < this->local_values_.size());
+    this->local_values_[sym].set_must_have_output_symtab_entry();
+  }
+
   // Record that local symbol SYM needs a dynamic symbol entry.
   void
   set_needs_output_dynsym_entry(unsigned int sym)
Index: target-reloc.h
===================================================================
RCS file: /cvs/src/src/gold/target-reloc.h,v
retrieving revision 1.39
retrieving revision 1.40
diff -p -u -r1.39 -r1.40
--- target-reloc.h	12 Jan 2010 19:12:40 -0000	1.39
+++ target-reloc.h	3 Mar 2010 19:31:54 -0000	1.40
@@ -279,7 +279,7 @@ relocate_section(
 	    }
 
 	  sym = static_cast<const Sized_symbol<size>*>(gsym);
-	  if (sym->has_symtab_index())
+	  if (sym->has_symtab_index() && sym->symtab_index() != -1U)
 	    symval.set_output_symtab_index(sym->symtab_index());
 	  else
 	    symval.set_no_output_symtab_entry();
@@ -491,6 +491,9 @@ scan_relocatable_relocs(
 		  if (strategy != Relocatable_relocs::RELOC_DISCARD)
                     object->output_section(shndx)->set_needs_symtab_index();
 		}
+
+	      if (strategy == Relocatable_relocs::RELOC_COPY)
+		object->set_must_have_output_symtab_entry(r_sym);
 	    }
 	}
 
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.124
retrieving revision 1.125
diff -p -u -r1.124 -r1.125
--- testsuite/Makefile.am	27 Feb 2010 00:36:49 -0000	1.124
+++ testsuite/Makefile.am	3 Mar 2010 19:31:54 -0000	1.125
@@ -1285,8 +1285,14 @@ local_labels_test: local_labels_test.o
 
 check_PROGRAMS += discard_locals_test
 check_SCRIPTS += discard_locals_test.sh
-check_DATA += discard_locals_test.syms
-MOSTLYCLEANFILES += discard_locals_test.syms
+check_DATA += discard_locals_test.syms \
+	discard_locals_relocatable_test1.syms \
+	discard_locals_relocatable_test2.syms
+MOSTLYCLEANFILES += discard_locals_test.syms \
+	discard_locals_relocatable_test1.syms \
+	discard_locals_relocatable_test2.syms \
+	discard_locals_relocatable_test1.out \
+	discard_locals_relocatable_test2.out
 discard_locals_test_SOURCES = discard_locals_test.c
 discard_locals_test_LDFLAGS = -Bgcctestdir/ -Wl,--discard-locals
 discard_locals_test.syms: discard_locals_test
@@ -1295,6 +1301,18 @@ discard_locals_test.syms: discard_locals
 discard_locals_test.o: discard_locals_test.c
 	$(COMPILE) -c -Wa,-L -o $@ $<
 
+discard_locals_relocatable_test1.syms: discard_locals_relocatable_test1.out
+	$(TEST_READELF) -sW $< >$@ 2>/dev/null
+discard_locals_relocatable_test.o: discard_locals_relocatable_test.c
+	$(COMPILE) -c -Wa,-L -fPIC -o $@ $<
+discard_locals_relocatable_test1.out: discard_locals_relocatable_test.o ../ld-new
+	../ld-new --discard-locals -relocatable -o $@ $<
+
+discard_locals_relocatable_test2.syms: discard_locals_relocatable_test2.out
+	$(TEST_READELF) -sW $< >$@ 2>/dev/null
+discard_locals_relocatable_test2.out: discard_locals_relocatable_test.o ../ld-new
+	../ld-new --discard-all -relocatable -o $@ $<
+
 if MCMODEL_MEDIUM
 check_PROGRAMS += large
 large_SOURCES = large.c
@@ -1456,11 +1474,11 @@ check_SCRIPTS += arm_abs_global.sh
 check_DATA += arm_abs_global.stdout
 arm_abs_lib.o: arm_abs_lib.s
 	$(TEST_AS) -march=armv7-a -o $@ $<
-libarm_abs.so: arm_abs_lib.o
+libarm_abs.so: arm_abs_lib.o ../ld-new
 	../ld-new -shared -o $@ arm_abs_lib.o
 arm_abs_global.o: arm_abs_global.s
 	$(TEST_AS) -march=armv7-a -o $@ $<
-arm_abs_global: arm_abs_global.o libarm_abs.so
+arm_abs_global: arm_abs_global.o libarm_abs.so ../ld-new
 	../ld-new -o $@ arm_abs_global.o -L. -larm_abs
 arm_abs_global.stdout: arm_abs_global
 	$(TEST_READELF) -r $< > $@
@@ -1475,7 +1493,7 @@ check_DATA += arm_bl_in_range.stdout arm
 arm_bl_in_range.stdout: arm_bl_in_range
 	$(TEST_OBJDUMP) -D $< > $@
 
-arm_bl_in_range: arm_bl_in_range.o
+arm_bl_in_range: arm_bl_in_range.o ../ld-new
 	../ld-new -T $(srcdir)/arm_branch_range.t -o $@ $<
 
 arm_bl_in_range.o: arm_bl_in_range.s
@@ -1484,7 +1502,7 @@ arm_bl_in_range.o: arm_bl_in_range.s
 arm_bl_out_of_range.stdout: arm_bl_out_of_range
 	$(TEST_OBJDUMP) -S $< > $@
 
-arm_bl_out_of_range: arm_bl_out_of_range.o
+arm_bl_out_of_range: arm_bl_out_of_range.o ../ld-new
 	../ld-new -T $(srcdir)/arm_branch_range.t -o $@ $<
 
 arm_bl_out_of_range.o: arm_bl_out_of_range.s
@@ -1493,7 +1511,7 @@ arm_bl_out_of_range.o: arm_bl_out_of_ran
 thumb_bl_in_range.stdout: thumb_bl_in_range
 	$(TEST_OBJDUMP) -D $< > $@
 
-thumb_bl_in_range: thumb_bl_in_range.o
+thumb_bl_in_range: thumb_bl_in_range.o ../ld-new
 	../ld-new -T $(srcdir)/thumb_branch_range.t -o $@ $<
 
 thumb_bl_in_range.o: thumb_bl_in_range.s
@@ -1502,7 +1520,7 @@ thumb_bl_in_range.o: thumb_bl_in_range.s
 thumb_bl_out_of_range.stdout: thumb_bl_out_of_range
 	$(TEST_OBJDUMP) -D $< > $@
 
-thumb_bl_out_of_range: thumb_bl_in_range.o
+thumb_bl_out_of_range: thumb_bl_in_range.o ../ld-new
 	../ld-new -T $(srcdir)/thumb_branch_range.t -o $@ $<
 
 thumb_bl_out_of_range.o: thumb_bl_in_range.s
@@ -1511,7 +1529,7 @@ thumb_bl_out_of_range.o: thumb_bl_in_ran
 thumb2_bl_in_range.stdout: thumb2_bl_in_range
 	$(TEST_OBJDUMP) -D $< > $@
 
-thumb2_bl_in_range: thumb2_bl_in_range.o
+thumb2_bl_in_range: thumb2_bl_in_range.o ../ld-new
 	../ld-new -T $(srcdir)/thumb2_branch_range.t -o $@ $<
 
 thumb2_bl_in_range.o: thumb_bl_in_range.s
@@ -1520,7 +1538,7 @@ thumb2_bl_in_range.o: thumb_bl_in_range.
 thumb2_bl_out_of_range.stdout: thumb2_bl_out_of_range
 	$(TEST_OBJDUMP) -D $< > $@
 
-thumb2_bl_out_of_range: thumb2_bl_in_range.o
+thumb2_bl_out_of_range: thumb2_bl_in_range.o ../ld-new
 	../ld-new -T $(srcdir)/thumb2_branch_range.t -o $@ $<
 
 thumb2_bl_out_of_range.o: thumb_bl_in_range.s
@@ -1536,7 +1554,7 @@ check_DATA += arm_fix_v4bx.stdout arm_fi
 arm_fix_v4bx.stdout: arm_fix_v4bx
 	$(TEST_OBJDUMP) -D -j.text $< > $@
 
-arm_fix_v4bx: arm_fix_v4bx.o
+arm_fix_v4bx: arm_fix_v4bx.o ../ld-new
 	../ld-new --fix-v4bx -o $@ $<
 
 arm_fix_v4bx.o: arm_fix_v4bx.s
@@ -1545,13 +1563,13 @@ arm_fix_v4bx.o: arm_fix_v4bx.s
 arm_fix_v4bx_interworking.stdout: arm_fix_v4bx_interworking
 	$(TEST_OBJDUMP) -D -j.text $< > $@
 
-arm_fix_v4bx_interworking: arm_fix_v4bx.o
+arm_fix_v4bx_interworking: arm_fix_v4bx.o ../ld-new
 	../ld-new --fix-v4bx-interworking -o $@ $<
 
 arm_no_fix_v4bx.stdout: arm_no_fix_v4bx
 	$(TEST_OBJDUMP) -D -j.text $< > $@
 
-arm_no_fix_v4bx: arm_fix_v4bx.o
+arm_no_fix_v4bx: arm_fix_v4bx.o ../ld-new
 	../ld-new -o $@ $<
 
 MOSTLYCLEANFILES += arm_fix_v4bx arm_fix_v4bx_interworking arm_no_fix_v4bx
Index: testsuite/discard_locals_relocatable_test.c
===================================================================
RCS file: testsuite/discard_locals_relocatable_test.c
diff -N testsuite/discard_locals_relocatable_test.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/discard_locals_relocatable_test.c	3 Mar 2010 19:31:54 -0000	1.1
@@ -0,0 +1,43 @@
+/* discard_locals_relocatable_test.c -- test --discard-locals/--discard-all -r
+
+   Copyright 2010 Free Software Foundation, Inc.
+   Viktor Kutuzov <vkutuzov@accesssoftek.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.
+
+   This is a test of a common symbol in the main program and a
+   versioned symbol in a shared library.  The common symbol in the
+   main program should override the shared library symbol.  */
+
+/* Note: use GCC -fPIC option to compile this test.  */
+
+/* Local symbol format for generic ELF target. 
+   Use GCC -Wa,-L option to preserve this local symbol
+   in the output object file. */
+asm (".Lshould_be_discarded:");
+
+extern void print_func (const char* s);
+
+extern int func (void);
+
+int
+func (void)
+{
+  print_func ("local string");
+  return 0;
+}
Index: testsuite/discard_locals_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/discard_locals_test.sh,v
retrieving revision 1.1
retrieving revision 1.2
diff -p -u -r1.1 -r1.2
--- testsuite/discard_locals_test.sh	5 Jun 2009 21:32:57 -0000	1.1
+++ testsuite/discard_locals_test.sh	3 Mar 2010 19:31:54 -0000	1.2
@@ -27,11 +27,12 @@
 # the resulting executable and check that symbols from two test library
 # archives are correctly hidden or left unmodified.
 
-check()
+check_discarded()
 {
     file=$1
+    sym=$2
 
-    found=`egrep "should_be_discarded" $file`
+    found=`egrep $sym $file`
     if test -n "$found"; then
 	echo "These local symbols are not discarded in $file:"
 	echo "$found"
@@ -39,6 +40,24 @@ check()
     fi
 }
 
-check "discard_locals_test.syms"
+check_non_discarded()
+{
+    file=$1
+    sym=$2
+
+    found=`egrep $sym $file`
+    if test -z "$found"; then
+	echo "This local symbol is discarded in $file:"
+	echo "$2"
+	exit 1
+    fi
+}
+
+check_discarded     "discard_locals_test.syms" "should_be_discarded"
+
+check_non_discarded "discard_locals_relocatable_test1.syms" ".LC0"
+check_discarded     "discard_locals_relocatable_test1.syms" "should_be_discarded"
+check_non_discarded "discard_locals_relocatable_test2.syms" ".LC0"
+check_discarded     "discard_locals_relocatable_test2.syms" "should_be_discarded"
 
 exit 0

Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Cary Coutant]

I think this part of the patch may have broken -s:

@@ -1937,16 +1938,16 @@ Sized_relobj<size, big_endian>::write_lo
 	  st_shndx = out_sections[st_shndx]->out_shndx();
 	  if (st_shndx >= elfcpp::SHN_LORESERVE)
 	    {
-	      if (lv.needs_output_symtab_entry() && !strip_all)
+	      if (lv.has_output_symtab_entry())
 		symtab_xindex->add(lv.output_symtab_index(), st_shndx);
-	      if (lv.needs_output_dynsym_entry())
+	      if (lv.has_output_dynsym_entry())
 		dynsym_xindex->add(lv.output_dynsym_index(), st_shndx);
 	      st_shndx = elfcpp::SHN_XINDEX;
 	    }
 	}

       // Write the symbol to the output symbol table.
-      if (!strip_all && lv.needs_output_symtab_entry())
+      if (lv.has_output_symtab_entry())
         {
           elfcpp::Sym_write<size, big_endian> osym(ov);

Even with -s, do_finalize_local_symbols() is going to count local
symbols and assign them indexes in the symbol table, and
lv.has_output_symtab_entry() will return true. write_local_symbols
will then try to write out a local symbol table entry, and will fail
an assertion here:

          osym.put_st_name(sympool->get_offset(name));

Interestingly, the assertion at the bottom of the loop will not trigger:

  if (output_size > 0)
    {
      gold_assert(ov - oview == output_size);
      of->write_output_view(symtab_off + this->local_symbol_offset_,
			    output_size, oview);
    }

because output_local_symbol_count and output_size were set to 0 by the
test for the -s option.

This patch fixes the problem. Is this OK, or were you thinking that
do_finalize_local_symbols() shouldn't even assign the symbol index
when -s is set?

-cary

	* object.cc (write_local_symbols): Check for strip_all when writing symbols.

Index: object.cc
===================================================================
RCS file: /cvs/src/src/gold/object.cc,v
retrieving revision 1.138
diff -u -p -r1.138 object.cc
--- object.cc	18 Apr 2011 05:39:43 -0000	1.138
+++ object.cc	10 May 2011 22:26:23 -0000
@@ -2302,7 +2302,7 @@ Sized_relobj<size, big_endian>::write_lo
 	  st_shndx = out_sections[st_shndx]->out_shndx();
 	  if (st_shndx >= elfcpp::SHN_LORESERVE)
 	    {
-	      if (lv.has_output_symtab_entry())
+	      if (lv.has_output_symtab_entry() && !strip_all)
 		symtab_xindex->add(lv.output_symtab_index(), st_shndx);
 	      if (lv.has_output_dynsym_entry())
 		dynsym_xindex->add(lv.output_dynsym_index(), st_shndx);
@@ -2311,7 +2311,7 @@ Sized_relobj<size, big_endian>::write_lo
 	}

       // Write the symbol to the output symbol table.
-      if (lv.has_output_symtab_entry())
+      if (!strip_all && lv.has_output_symtab_entry())
         {
           elfcpp::Sym_write<size, big_endian> osym(ov);

Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Ian Lance Taylor]

Cary Coutant <ccoutant@google.com> writes:

> This patch fixes the problem. Is this OK, or were you thinking that
> do_finalize_local_symbols() shouldn't even assign the symbol index
> when -s is set?

I was thinking the latter.  I see that do_count_local_symbols checks
discard_locals.  It sounds like it should also test strip_all.  A patch
for that is preapproved if it works.

Thanks.

Ian

Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects

[Cary Coutant]

>> This patch fixes the problem. Is this OK, or were you thinking that
>> do_finalize_local_symbols() shouldn't even assign the symbol index
>> when -s is set?
>
> I was thinking the latter.  I see that do_count_local_symbols checks
> discard_locals.  It sounds like it should also test strip_all.  A patch
> for that is preapproved if it works.

That works. I've committed the following patch.

-cary


	* object.cc (Sized_relobj::do_count_local_symbols): Check for
	strip_all (-s).


Index: object.cc
===================================================================
RCS file: /cvs/src/src/gold/object.cc,v
retrieving revision 1.138
diff -u -p -r1.138 object.cc
--- object.cc	18 Apr 2011 05:39:43 -0000	1.138
+++ object.cc	11 May 2011 00:25:55 -0000
@@ -1824,6 +1824,7 @@ Sized_relobj<size, big_endian>::do_count
   unsigned int dyncount = 0;
   // Skip the first, dummy, symbol.
   psyms += sym_size;
+  bool strip_all = parameters->options().strip_all();
   bool discard_all = parameters->options().discard_all();
   bool discard_locals = parameters->options().discard_locals();
   for (unsigned int i = 1; i < loccount; ++i, psyms += sym_size)
@@ -1882,7 +1883,8 @@ Sized_relobj<size, big_endian>::do_count
           ++dyncount;
         }

-      if (discard_all && lv.may_be_discarded_from_output_symtab())
+      if (strip_all
+	  || (discard_all && lv.may_be_discarded_from_output_symtab()))
 	{
 	  lv.set_no_output_symtab_entry();
 	  continue;