This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
gold patch committed: Fix symbol visibility (PR 9836)
- From: Ian Lance Taylor <iant at google dot com>
- To: binutils at sourceware dot org
- Date: Fri, 27 Feb 2009 11:59:16 -0800
- Subject: gold patch committed: Fix symbol visibility (PR 9836)
PR 9836 is about a bug in gold's symbol visibility handling: it was not
forcing symbols with hidden visibility to be local. I committed this
patch to fix the bug, along with a test.
Ian
2009-02-27 Ian Lance Taylor <iant@google.com>
PR 9836
* symtab.cc (Symbol_table::add_from_object): If the visibility is
hidden or internal, force the symbol to be local.
* resolve.cc (Symbol::override_visibility): Define.
(Symbol::override_base): Use override_visibility.
(Symbol_table::resolve): Likewise.
(Symbol::override_base_with_special): Likewise.
(Symbol_table::override_with_special): If the visibility is hidden
or internal, force the symbol to be local.
* symtab.h (class Symbol): Add set_visibility and
override_visibility.
* testsuite/ver_test_1.sh: New file.
* testsuite/Makefile.am (check_SCRIPTS): Add ver_test_1.sh.
(check_DATA): Add ver_test_1.syms.
(ver_test_1.syms): New target.
* testsuite/Makefile.in: Rebuild.
Index: resolve.cc
===================================================================
RCS file: /cvs/src/src/gold/resolve.cc,v
retrieving revision 1.38
diff -p -u -r1.38 resolve.cc
--- resolve.cc 19 Sep 2008 22:54:57 -0000 1.38
+++ resolve.cc 27 Feb 2009 19:50:48 -0000
@@ -1,6 +1,6 @@
// resolve.cc -- symbol resolution for gold
-// Copyright 2006, 2007, 2008 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
// Written by Ian Lance Taylor <iant@google.com>.
// This file is part of gold.
@@ -63,6 +63,26 @@ Symbol::override_version(const char* ver
}
}
+// This symbol is being overidden by another symbol whose visibility
+// is VISIBILITY. Updated the VISIBILITY_ field accordingly.
+
+inline void
+Symbol::override_visibility(elfcpp::STV visibility)
+{
+ // The rule for combining visibility is that we always choose the
+ // most constrained visibility. In order of increasing constraint,
+ // visibility goes PROTECTED, HIDDEN, INTERNAL. This is the reverse
+ // of the numeric values, so the effect is that we always want the
+ // smallest non-zero value.
+ if (visibility != elfcpp::STV_DEFAULT)
+ {
+ if (this->visibility_ == elfcpp::STV_DEFAULT)
+ this->visibility_ = visibility;
+ else if (this->visibility_ > visibility)
+ this->visibility_ = visibility;
+ }
+}
+
// Override the fields in Symbol.
template<int size, bool big_endian>
@@ -78,7 +98,7 @@ Symbol::override_base(const elfcpp::Sym<
this->is_ordinary_shndx_ = is_ordinary;
this->type_ = sym.get_st_type();
this->binding_ = sym.get_st_bind();
- this->visibility_ = sym.get_st_visibility();
+ this->override_visibility(sym.get_st_visibility());
this->nonvis_ = sym.get_st_nonvis();
if (object->is_dynamic())
this->in_dyn_ = true;
@@ -279,6 +299,9 @@ Symbol_table::resolve(Sized_symbol<size>
{
if (adjust_common_sizes && sym.get_st_size() > to->symsize())
to->set_symsize(sym.get_st_size());
+ // The ELF ABI says that even for a reference to a symbol we
+ // merge the visibility.
+ to->override_visibility(sym.get_st_visibility());
}
// A new weak undefined reference, merging with an old weak
@@ -721,7 +744,7 @@ Symbol::override_base_with_special(const
this->override_version(from->version_);
this->type_ = from->type_;
this->binding_ = from->binding_;
- this->visibility_ = from->visibility_;
+ this->override_visibility(from->visibility_);
this->nonvis_ = from->nonvis_;
// Special symbols are always considered to be regular symbols.
@@ -776,7 +799,12 @@ Symbol_table::override_with_special(Size
}
while (ssym != tosym);
}
- if (tosym->binding() == elfcpp::STB_LOCAL)
+ if (tosym->binding() == elfcpp::STB_LOCAL
+ || ((tosym->visibility() == elfcpp::STV_HIDDEN
+ || tosym->visibility() == elfcpp::STV_INTERNAL)
+ && (tosym->binding() == elfcpp::STB_GLOBAL
+ || tosym->binding() == elfcpp::STB_WEAK)
+ && !parameters->options().relocatable()))
this->force_local(tosym);
}
Index: symtab.cc
===================================================================
RCS file: /cvs/src/src/gold/symtab.cc,v
retrieving revision 1.114
diff -p -u -r1.114 symtab.cc
--- symtab.cc 6 Feb 2009 19:20:09 -0000 1.114
+++ symtab.cc 27 Feb 2009 19:50:48 -0000
@@ -648,7 +648,8 @@ Symbol_table::resolve(Sized_symbol<size>
this->gc_mark_dyn_syms(to);
}
-// Record that a symbol is forced to be local by a version script.
+// Record that a symbol is forced to be local by a version script or
+// by visibility.
void
Symbol_table::force_local(Symbol* sym)
@@ -962,6 +963,15 @@ Symbol_table::add_from_object(Object* ob
this->tls_commons_.push_back(ret);
}
+ // If we're not doing a relocatable link, then any symbol with
+ // hidden or internal visibility is local.
+ if ((ret->visibility() == elfcpp::STV_HIDDEN
+ || ret->visibility() == elfcpp::STV_INTERNAL)
+ && (ret->binding() == elfcpp::STB_GLOBAL
+ || ret->binding() == elfcpp::STB_WEAK)
+ && !parameters->options().relocatable())
+ this->force_local(ret);
+
if (def)
ret->set_is_default();
return ret;
@@ -1179,7 +1189,7 @@ Symbol_table::add_from_pluginobj(
def, *sym, st_shndx, true, st_shndx);
if (local)
- this->force_local(res);
+ this->force_local(res);
return res;
}
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.84
diff -p -u -r1.84 symtab.h
--- symtab.h 28 Jan 2009 02:25:33 -0000 1.84
+++ symtab.h 27 Feb 2009 19:50:49 -0000
@@ -1,6 +1,6 @@
// symtab.h -- the gold symbol table -*- C++ -*-
-// Copyright 2006, 2007, 2008 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
// Written by Ian Lance Taylor <iant@google.com>.
// This file is part of gold.
@@ -210,6 +210,15 @@ class Symbol
visibility() const
{ return this->visibility_; }
+ // Set the visibility.
+ void
+ set_visibility(elfcpp::STV visibility)
+ { this->visibility_ = visibility; }
+
+ // Override symbol visibility.
+ void
+ override_visibility(elfcpp::STV);
+
// Return the non-visibility part of the st_other field.
unsigned char
nonvis() const
@@ -1384,7 +1393,8 @@ class Symbol_table
void
resolve(Sized_symbol<size>* to, const Sized_symbol<size>* from);
- // Record that a symbol is forced to be local by a version script.
+ // Record that a symbol is forced to be local by a version script or
+ // by visibility.
void
force_local(Symbol*);
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.84
diff -p -u -r1.84 Makefile.am
--- testsuite/Makefile.am 25 Feb 2009 19:05:21 -0000 1.84
+++ testsuite/Makefile.am 27 Feb 2009 19:50:49 -0000
@@ -742,6 +742,11 @@ ver_test_3.o: ver_test_3.cc
ver_test_4.o: ver_test_4.cc
$(CXXCOMPILE) -c -fpic -o $@ $<
+check_SCRIPTS += ver_test_1.sh
+check_DATA += ver_test_1.syms
+ver_test_1.syms: ver_test_1.so
+ $(TEST_READELF) -s $< >$@ 2>/dev/null
+
check_PROGRAMS += ver_test_2
ver_test_2_SOURCES = ver_test_main_2.cc
ver_test_2_DEPENDENCIES = gcctestdir/ld ver_test_4.so ver_test_2.so
Index: testsuite/ver_test_1.sh
===================================================================
RCS file: testsuite/ver_test_1.sh
diff -N testsuite/ver_test_1.sh
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/ver_test_1.sh 27 Feb 2009 19:50:49 -0000
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+# ver_test_1.sh -- check that protected symbols are local
+
+# Copyright 2009 Free Software Foundation, Inc.
+# Written by Ian Lance Taylor <iant@google.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.
+
+syms=`grep ' HIDDEN ' ver_test_1.syms | grep ' GLOBAL '`
+if test -n "$syms"; then
+ echo "Found GLOBAL HIDDEN symbols"
+ echo $syms
+ exit 1
+fi