This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects
- From: Ian Lance Taylor <iant at google dot com>
- To: vkutuzov at accesssoftek dot com
- Cc: binutils <binutils at sourceware dot org>
- Date: Wed, 03 Mar 2010 11:33:53 -0800
- Subject: Re: [GOLD][PATCH PROPOSAL] prevent discarding of needed local symbols for the relocatable objects
- References: <1265843004.2150.342.camel@dp690-dev5v4> <mcraavgcydc.fsf@dhcp-172-17-9-151.mtv.corp.google.com> <1267053412.6817.190.camel@dp690-dev5v4>
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.
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