Fix treatment of symbol versions with unused as-needed libraries.

When we have a weak reference to a symbol defined in an
as-needed library, and that library ends up not-needed, gold
simply clears the version information in the symbol table, even
if the symbol could have been resolved by a needed library later
in the link order. This results in a loss of version information,
which can cause the program to bind to the wrong version at run
time.

This patch lets a dynamic definition override an earlier one if
the earlier one is from a not-needed library, so that we can
retain the version information from the binding to the needed
library. In order to do that, the tracking of needed/not-needed
had to be moved up to symbol resolution time, instead of during
Symbol_table::set_dynsym_indexes().

In cases where we still end up discarding version information,
I've added a warning.

For the original problem report and discussion, see:

https://stackoverflow.com/questions/50751421/undefined-behavior-in-shared-lib-using-libpthread-but-not-having-it-in-elf-as-d

gold/
	* resolve.cc (Symbol_table::resolve): Rename tobinding to
	orig_tobinding.  Call set_is_needed() for objects that resolve
	non-weak references.
	(Symbol_table::should_override): Allow a dynamic definition to
	override an earlier one in a not-needed library.
	* symtab.cc (Symbol_table::set_dynsym_indexes): Remove separate
	processing for as-needed symbols.  Add warning when discarding
	version informatin.
	* testsuite/Makefile.am (weak_as_needed): New test case.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/weak_as_needed.sh: New test script.
	* testsuite/weak_as_needed_a.c: New source file.
	* testsuite/weak_as_needed_b.c: New source file.
	* testsuite/weak_as_needed_b.script: New version script.
	* testsuite/weak_as_needed_c.c: New source file.
	* testsuite/weak_as_needed_c.script: New version script.
This commit is contained in:
Cary Coutant 2018-06-21 13:51:16 -07:00
parent 1ced1a5f10
commit cea6ffbd06
11 changed files with 246 additions and 35 deletions

View File

@ -1,3 +1,22 @@
2018-06-21 Cary Coutant <ccoutant@gmail.com>
* resolve.cc (Symbol_table::resolve): Rename tobinding to
orig_tobinding. Call set_is_needed() for objects that resolve
non-weak references.
(Symbol_table::should_override): Allow a dynamic definition to
override an earlier one in a not-needed library.
* symtab.cc (Symbol_table::set_dynsym_indexes): Remove separate
processing for as-needed symbols. Add warning when discarding
version informatin.
* testsuite/Makefile.am (weak_as_needed): New test case.
* testsuite/Makefile.in: Regenerate.
* testsuite/weak_as_needed.sh: New test script.
* testsuite/weak_as_needed_a.c: New source file.
* testsuite/weak_as_needed_b.c: New source file.
* testsuite/weak_as_needed_b.script: New version script.
* testsuite/weak_as_needed_c.c: New source file.
* testsuite/weak_as_needed_c.script: New version script.
2018-06-20 Cary Coutant <ccoutant@gmail.com>
PR gold/23268

View File

@ -394,7 +394,7 @@ Symbol_table::resolve(Sized_symbol<size>* to,
object, &adjust_common_sizes,
&adjust_dyndef, is_default_version))
{
elfcpp::STB tobinding = to->binding();
elfcpp::STB orig_tobinding = to->binding();
typename Sized_symbol<size>::Value_type tovalue = to->value();
this->override(to, sym, st_shndx, is_ordinary, object, version);
if (adjust_common_sizes)
@ -408,7 +408,7 @@ Symbol_table::resolve(Sized_symbol<size>* to,
{
// We are overriding an UNDEF or WEAK UNDEF with a DYN DEF.
// Remember which kind of UNDEF it was for future reference.
to->set_undef_binding(tobinding);
to->set_undef_binding(orig_tobinding);
}
}
else
@ -431,6 +431,11 @@ Symbol_table::resolve(Sized_symbol<size>* to,
to->override_visibility(sym.get_st_visibility());
}
// If we have a non-WEAK reference from a regular object to a
// dynamic object, mark the dynamic object as needed.
if (to->is_from_dynobj() && to->in_reg() && !to->is_undef_binding_weak())
to->object()->set_is_needed();
if (adjust_common_sizes && parameters->options().warn_common())
{
if (tosize > sym.get_st_size())
@ -621,6 +626,13 @@ Symbol_table::should_override(const Symbol* to, unsigned int frombits,
&& to->version() == NULL
&& is_default_version)
return true;
// Or, if the existing definition is in an unused --as-needed library,
// and the reference is weak, let the new definition override.
if (to->in_reg()
&& to->is_undef_binding_weak()
&& to->object()->as_needed()
&& !to->object()->is_needed())
return true;
return false;
case UNDEF * 16 + DYN_DEF:
@ -637,16 +649,12 @@ Symbol_table::should_override(const Symbol* to, unsigned int frombits,
case COMMON * 16 + DYN_DEF:
case WEAK_COMMON * 16 + DYN_DEF:
case DYN_COMMON * 16 + DYN_DEF:
case DYN_WEAK_COMMON * 16 + DYN_DEF:
// Ignore a dynamic definition if we already have a common
// definition.
return false;
case DEF * 16 + DYN_WEAK_DEF:
case WEAK_DEF * 16 + DYN_WEAK_DEF:
case DYN_DEF * 16 + DYN_WEAK_DEF:
case DYN_WEAK_DEF * 16 + DYN_WEAK_DEF:
// Ignore a weak dynamic definition if we already have a
// definition.
return false;
@ -670,12 +678,25 @@ Symbol_table::should_override(const Symbol* to, unsigned int frombits,
case COMMON * 16 + DYN_WEAK_DEF:
case WEAK_COMMON * 16 + DYN_WEAK_DEF:
case DYN_COMMON * 16 + DYN_WEAK_DEF:
case DYN_WEAK_COMMON * 16 + DYN_WEAK_DEF:
// Ignore a weak dynamic definition if we already have a common
// definition.
return false;
case DYN_COMMON * 16 + DYN_DEF:
case DYN_WEAK_COMMON * 16 + DYN_DEF:
case DYN_DEF * 16 + DYN_WEAK_DEF:
case DYN_WEAK_DEF * 16 + DYN_WEAK_DEF:
case DYN_COMMON * 16 + DYN_WEAK_DEF:
case DYN_WEAK_COMMON * 16 + DYN_WEAK_DEF:
// If the existing definition is in an unused --as-needed library,
// and the reference is weak, let a new dynamic definition override.
if (to->in_reg()
&& to->is_undef_binding_weak()
&& to->object()->as_needed()
&& !to->object()->is_needed())
return true;
return false;
case DEF * 16 + UNDEF:
case WEAK_DEF * 16 + UNDEF:
case UNDEF * 16 + UNDEF:

View File

@ -2543,8 +2543,6 @@ Symbol_table::set_dynsym_indexes(unsigned int index,
Stringpool* dynpool,
Versions* versions)
{
std::vector<Symbol*> as_needed_sym;
// First process all the symbols which have been forced to be local,
// as they must appear before all global symbols.
unsigned int forced_local_count = 0;
@ -2611,15 +2609,6 @@ Symbol_table::set_dynsym_indexes(unsigned int index,
syms->push_back(sym);
dynpool->add(sym->name(), false, NULL);
// If the symbol is defined in a dynamic object and is
// referenced strongly in a regular object, then mark the
// dynamic object as needed. This is used to implement
// --as-needed.
if (sym->is_from_dynobj()
&& sym->in_reg()
&& !sym->is_undef_binding_weak())
sym->object()->set_is_needed();
// Record any version information, except those from
// as-needed libraries not seen to be needed. Note that the
// is_needed state for such libraries can change in this loop.
@ -2630,24 +2619,18 @@ Symbol_table::set_dynsym_indexes(unsigned int index,
|| sym->object()->is_needed())
versions->record_version(this, dynpool, sym);
else
as_needed_sym.push_back(sym);
{
gold_warning(_("discarding version information for "
"%s@%s, defined in unused shared library %s "
"(linked with --as-needed)"),
sym->name(), sym->version(),
sym->object()->name().c_str());
sym->clear_version();
}
}
}
}
// Process version information for symbols from as-needed libraries.
for (std::vector<Symbol*>::iterator p = as_needed_sym.begin();
p != as_needed_sym.end();
++p)
{
Symbol* sym = *p;
if (sym->object()->is_needed())
versions->record_version(this, dynpool, sym);
else
sym->clear_version();
}
// Finish up the versions. In some cases this may add new dynamic
// symbols.
index = versions->finalize(this, index, syms);

View File

@ -1880,6 +1880,23 @@ ver_test_14.syms: ver_test_14
ver_test_14: gcctestdir/ld ver_test_main.o ver_test_1.so ver_test_2.so ver_test_4.so ver_test_14.script
$(CXXLINK) -Bgcctestdir/ -Wl,--version-script,$(srcdir)/ver_test_14.script -Wl,-E -Wl,-R,. ver_test_main.o ver_test_1.so ver_test_2.so ver_test_4.so
check_SCRIPTS += weak_as_needed.sh
check_DATA += weak_as_needed.stdout
weak_as_needed.stdout: weak_as_needed_a.so
$(TEST_READELF) -dW --dyn-syms $< >$@
weak_as_needed_a.so: gcctestdir/ld weak_as_needed_a.o weak_as_needed_b.so weak_as_needed_c.so
gcctestdir/ld -shared -rpath . -o $@ weak_as_needed_a.o --as-needed weak_as_needed_b.so weak_as_needed_c.so
weak_as_needed_b.so: gcctestdir/ld weak_as_needed_b.o weak_as_needed_b.script
gcctestdir/ld -shared -rpath . -o $@ --version-script $(srcdir)/weak_as_needed_b.script weak_as_needed_b.o
weak_as_needed_c.so: gcctestdir/ld weak_as_needed_c.o weak_as_needed_c.script
gcctestdir/ld -shared -rpath . -o $@ --version-script $(srcdir)/weak_as_needed_c.script weak_as_needed_c.o
weak_as_needed_a.o: weak_as_needed_a.c
$(COMPILE) -c -fpic -o $@ $<
weak_as_needed_b.o: weak_as_needed_b.c
$(COMPILE) -c -fpic -o $@ $<
weak_as_needed_c.o: weak_as_needed_c.c
$(COMPILE) -c -fpic -o $@ $<
check_PROGRAMS += protected_1
protected_1_SOURCES = \
protected_main_1.cc protected_main_2.cc protected_main_3.cc

View File

@ -479,7 +479,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_4.sh ver_test_5.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_7.sh ver_test_8.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_10.sh ver_test_13.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_14.sh relro_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_14.sh weak_as_needed.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ relro_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_matching_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_3.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_4.sh \
@ -534,7 +535,9 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_8_2.so.syms \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_10.syms \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_13.syms \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_14.syms protected_3.err \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_14.syms \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ weak_as_needed.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ protected_3.err \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ relro_test.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_matching_test.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_3.stdout \
@ -5802,6 +5805,13 @@ ver_test_14.sh.log: ver_test_14.sh
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
"$$tst" $(AM_TESTS_FD_REDIRECT)
weak_as_needed.sh.log: weak_as_needed.sh
@p='weak_as_needed.sh'; \
b='weak_as_needed.sh'; \
$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
"$$tst" $(AM_TESTS_FD_REDIRECT)
relro_test.sh.log: relro_test.sh
@p='relro_test.sh'; \
b='relro_test.sh'; \
@ -8712,6 +8722,20 @@ uninstall-am:
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_OBJDUMP) -T $< | $(TEST_CXXFILT) >$@
@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_14: gcctestdir/ld ver_test_main.o ver_test_1.so ver_test_2.so ver_test_4.so ver_test_14.script
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Bgcctestdir/ -Wl,--version-script,$(srcdir)/ver_test_14.script -Wl,-E -Wl,-R,. ver_test_main.o ver_test_1.so ver_test_2.so ver_test_4.so
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed.stdout: weak_as_needed_a.so
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -dW --dyn-syms $< >$@
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_a.so: gcctestdir/ld weak_as_needed_a.o weak_as_needed_b.so weak_as_needed_c.so
@GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -rpath . -o $@ weak_as_needed_a.o --as-needed weak_as_needed_b.so weak_as_needed_c.so
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_b.so: gcctestdir/ld weak_as_needed_b.o weak_as_needed_b.script
@GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -rpath . -o $@ --version-script $(srcdir)/weak_as_needed_b.script weak_as_needed_b.o
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_c.so: gcctestdir/ld weak_as_needed_c.o weak_as_needed_c.script
@GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -rpath . -o $@ --version-script $(srcdir)/weak_as_needed_c.script weak_as_needed_c.o
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_a.o: weak_as_needed_a.c
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(COMPILE) -c -fpic -o $@ $<
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_b.o: weak_as_needed_b.c
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(COMPILE) -c -fpic -o $@ $<
@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_c.o: weak_as_needed_c.c
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(COMPILE) -c -fpic -o $@ $<
@GCC_TRUE@@NATIVE_LINKER_TRUE@protected_1.so: gcctestdir/ld protected_1_pic.o protected_2_pic.o protected_3_pic.o
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Bgcctestdir/ -shared protected_1_pic.o protected_2_pic.o protected_3_pic.o

View File

@ -0,0 +1,62 @@
#!/bin/sh
# weak_as_needed.sh -- a test case for version handling with weak symbols
# and --as-needed libraries.
# Copyright (C) 2018 Free Software Foundation, Inc.
# Written by Cary Coutant <ccoutant@gmail.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 test verifies that a weak reference is properly bound to
# an as-needed library, when it is first resolved to a symbol in
# a library that ends up being not needed.
# Ref: https://stackoverflow.com/questions/50751421/undefined-behavior-in-shared-lib-using-libpthread-but-not-having-it-in-elf-as-d
check()
{
if ! grep -q "$2" "$1"
then
echo "Did not find expected output in $1:"
echo " $2"
echo ""
echo "Actual output below:"
cat "$1"
exit 1
fi
}
check_missing()
{
if grep -q "$2" "$1"
then
echo "Found unexpected output in $1:"
echo " $2"
echo ""
echo "Actual output below:"
cat "$1"
exit 1
fi
}
check weak_as_needed.stdout "WEAK .* UND *bar@v2"
check weak_as_needed.stdout "NEEDED.*weak_as_needed_c\.so"
check_missing weak_as_needed.stdout "NEEDED.*weak_as_needed_b\.so"
exit 0

View File

@ -0,0 +1,10 @@
extern void bar(void) __attribute__ (( weak ));
extern void t4(void);
void foo(void);
void foo(void)
{
bar();
t4();
}

View File

@ -0,0 +1,23 @@
#include <stdio.h>
__asm__ (".symver bar_v1, bar@v1");
__asm__ (".symver bar_v2, bar@@v2");
void bar_v1(void);
void bar_v2(void);
void baz(void);
void bar_v1(void)
{
printf("weak_as_needed_b: bar_v1\n");
}
void bar_v2(void)
{
printf("weak_as_needed_b: bar_v2\n");
}
void baz(void)
{
printf("weak_as_needed_b: baz\n");
}

View File

@ -0,0 +1,11 @@
v1 {
global:
bar;
};
v2 {
global:
bar;
baz;
local:
*;
} v1;

View File

@ -0,0 +1,29 @@
#include <stdio.h>
__asm__ (".symver bar_v1, bar@v1");
__asm__ (".symver bar_v2, bar@@v2");
void bar_v1(void);
void bar_v2(void);
void baz(void);
void t4(void);
void bar_v1(void)
{
printf("weak_as_needed_c: bar_v1\n");
}
void bar_v2(void)
{
printf("weak_as_needed_c: bar_v2\n");
}
void baz(void)
{
printf("weak_as_needed_c: baz\n");
}
void t4(void)
{
printf("weak_as_needed_c: t4\n");
}

View File

@ -0,0 +1,12 @@
v1 {
global:
bar;
};
v2 {
global:
bar;
baz;
t4;
local:
*;
} v1;