* Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.
@ 2015-04-11 0:16 Sriraman Tallam
2015-04-11 0:22 ` Sriraman Tallam
2015-04-11 2:18 ` Cary Coutant
0 siblings, 2 replies; 9+ messages in thread
From: Sriraman Tallam @ 2015-04-11 0:16 UTC (permalink / raw)
To: binutils, Cary Coutant, Paul Pluzhnikov, Rong Xu, Brooks Moses,
Ollie Wild, David Li, Teresa Johnson, H.J. Lu, Ian Lance Taylor
[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]
Hi,
We have a problem where we use --warn-unresolved-symbols to tide
over an issue where we know the symbol is undefined but we also know
that the it is never accessed.
Example:
extern int foo();
int (*p)() = NULL;
int main() {
if (p == &foo)
foo();
return 0;
}
Now,
$ g++ -O2 foo.cc
foo.cc:function main: warning: undefined reference to 'foo()'
gives the right warning but builds a.out just fine and runs fine as
long as p is not equal to foo which is our case.
However, with -fPIE
$ g++ -O2 -fPIE foo.cc -pie
foo.cc:function main: warning: undefined reference to 'foo()'
but
$./a.out
./a.out: symbol lookup error: ./a.out: undefined symbol: _Z3foov
because with fPIE, a function pointer access is using a GOTPCREL
relocation which creates a GOT entry and a dynamic relocation for it.
The dynamic linker does not like the unresolved symbol any more.
I have attached a patch to prevent creation of GOT and dynamic
relocation entries with --warn-unresolved-symbols in general. Is this
reasonable?
Thanks
Sri
[-- Attachment #2: warn_unresolved.txt --]
[-- Type: text/plain, Size: 1450 bytes --]
* x86_64.cc (Scan::global): Do not create GOT entry for
unresolved symbol with --warn-unresolved-symbols.
(Relocate::relocate): Check for GOT entry only when the
above condition is not true.
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index 007af1d..bccb0ab 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -2931,6 +2931,11 @@ Target_x86_64<size>::Scan::global(Symbol_table* symtab,
}
else
{
+ // If the symbol is unresolved and we use --warn-unresolved-symbols
+ // do not create GOT entry and dynamic relocation for it.
+ if (parameters->options().warn_unresolved_symbols()
+ && issue_undefined_symbol_error(gsym))
+ break;
// If this symbol is not fully resolved, we need to add a
// dynamic relocation for it.
Reloc_section* rela_dyn = target->rela_dyn_section(layout);
@@ -3557,8 +3562,14 @@ Target_x86_64<size>::Relocate::relocate(
{
if (gsym != NULL)
{
- gold_assert(gsym->has_got_offset(GOT_TYPE_STANDARD));
- got_offset = gsym->got_offset(GOT_TYPE_STANDARD) - target->got_size();
+ // We dont create GOT entries for unresolved symbols
+ // with --warn-unresolved-symbols.
+ if (!parameters->options().warn_unresolved_symbols()
+ || !issue_undefined_symbol_error(gsym))
+ {
+ gold_assert(gsym->has_got_offset(GOT_TYPE_STANDARD));
+ got_offset = gsym->got_offset(GOT_TYPE_STANDARD) - target->got_size();
+ }
}
else
{
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-11 0:16 Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols Sriraman Tallam @ 2015-04-11 0:22 ` Sriraman Tallam 2015-04-11 2:18 ` Cary Coutant 1 sibling, 0 replies; 9+ messages in thread From: Sriraman Tallam @ 2015-04-11 0:22 UTC (permalink / raw) To: binutils, Cary Coutant, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, H.J. Lu, Ian Lance Taylor, ccoutant +ccoutant@gmail.com On Fri, Apr 10, 2015 at 5:15 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi, > > We have a problem where we use --warn-unresolved-symbols to tide > over an issue where we know the symbol is undefined but we also know > that the it is never accessed. > > Example: > > extern int foo(); > > int (*p)() = NULL; > int main() { > if (p == &foo) > foo(); > return 0; > } > > Now, > > $ g++ -O2 foo.cc > foo.cc:function main: warning: undefined reference to 'foo()' > > gives the right warning but builds a.out just fine and runs fine as > long as p is not equal to foo which is our case. > > However, with -fPIE > > $ g++ -O2 -fPIE foo.cc -pie > foo.cc:function main: warning: undefined reference to 'foo()' > > but > $./a.out > ./a.out: symbol lookup error: ./a.out: undefined symbol: _Z3foov > > because with fPIE, a function pointer access is using a GOTPCREL > relocation which creates a GOT entry and a dynamic relocation for it. > The dynamic linker does not like the unresolved symbol any more. > > I have attached a patch to prevent creation of GOT and dynamic > relocation entries with --warn-unresolved-symbols in general. Is this > reasonable? > > > Thanks > Sri ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-11 0:16 Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols Sriraman Tallam 2015-04-11 0:22 ` Sriraman Tallam @ 2015-04-11 2:18 ` Cary Coutant 2015-04-11 13:28 ` H.J. Lu 1 sibling, 1 reply; 9+ messages in thread From: Cary Coutant @ 2015-04-11 2:18 UTC (permalink / raw) To: Sriraman Tallam Cc: binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, H.J. Lu, Ian Lance Taylor > However, with -fPIE > > $ g++ -O2 -fPIE foo.cc -pie > foo.cc:function main: warning: undefined reference to 'foo()' > > but > $./a.out > ./a.out: symbol lookup error: ./a.out: undefined symbol: _Z3foov > > because with fPIE, a function pointer access is using a GOTPCREL > relocation which creates a GOT entry and a dynamic relocation for it. > The dynamic linker does not like the unresolved symbol any more. > > I have attached a patch to prevent creation of GOT and dynamic > relocation entries with --warn-unresolved-symbols in general. Is this > reasonable? No, I don't think so. For static links, we statically resolve undefined symbols to zero, but for non-static links, we leave them for dynamic resolution, presuming that they might be resolvable at runtime. This patch would change linker behavior that is fairly well established. (This patch also changes only x86_64 behavior; you'd need a similar patch for all targets. For a problem that isn't really target-specific, I'd prefer a target-independent solution.) The difference between non-PIE and PIE here is not due to the linker's -pie option, but because the compilers -fPIE option caused the compiler to use the GOTPCREL relocation, which requires the linker to create a GOT entry. If the reference to foo is weak, the dynamic loader won't complain -- I think that's really what you want. I'm not sure whether it's reasonable for you to have the compiler make these symbols weak unsats or not, but if you can, you wouldn't need --warn-unresolved-symbols any longer. If not, I wouldn't object to a new --weak-unresolved-symbols option that would have the linker convert any unresolved symbols (that it warned about) into weak references. I don't think we want to do that by default, since it changes existing expectations, but I think it's fine as an option. HJ, does that sound like a reasonable option for Gnu ld as well? -cary ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-11 2:18 ` Cary Coutant @ 2015-04-11 13:28 ` H.J. Lu 2015-04-20 23:06 ` Sriraman Tallam 0 siblings, 1 reply; 9+ messages in thread From: H.J. Lu @ 2015-04-11 13:28 UTC (permalink / raw) To: Cary Coutant Cc: Sriraman Tallam, binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, Ian Lance Taylor On Fri, Apr 10, 2015 at 7:18 PM, Cary Coutant <ccoutant@gmail.com> wrote: >> However, with -fPIE >> >> $ g++ -O2 -fPIE foo.cc -pie >> foo.cc:function main: warning: undefined reference to 'foo()' >> >> but >> $./a.out >> ./a.out: symbol lookup error: ./a.out: undefined symbol: _Z3foov >> >> because with fPIE, a function pointer access is using a GOTPCREL >> relocation which creates a GOT entry and a dynamic relocation for it. >> The dynamic linker does not like the unresolved symbol any more. >> >> I have attached a patch to prevent creation of GOT and dynamic >> relocation entries with --warn-unresolved-symbols in general. Is this >> reasonable? > > No, I don't think so. For static links, we statically resolve > undefined symbols to zero, but for non-static links, we leave them for > dynamic resolution, presuming that they might be resolvable at > runtime. This patch would change linker behavior that is fairly well > established. (This patch also changes only x86_64 behavior; you'd need > a similar patch for all targets. For a problem that isn't really > target-specific, I'd prefer a target-independent solution.) > > The difference between non-PIE and PIE here is not due to the linker's > -pie option, but because the compilers -fPIE option caused the > compiler to use the GOTPCREL relocation, which requires the linker to > create a GOT entry. > > If the reference to foo is weak, the dynamic loader won't complain -- > I think that's really what you want. I'm not sure whether it's > reasonable for you to have the compiler make these symbols weak unsats > or not, but if you can, you wouldn't need --warn-unresolved-symbols > any longer. If not, I wouldn't object to a new > --weak-unresolved-symbols option that would have the linker convert > any unresolved symbols (that it warned about) into weak references. I > don't think we want to do that by default, since it changes existing > expectations, but I think it's fine as an option. > > HJ, does that sound like a reasonable option for Gnu ld as well? > This sounds reasonable to me. -- H.J. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-11 13:28 ` H.J. Lu @ 2015-04-20 23:06 ` Sriraman Tallam 2015-04-22 19:33 ` Cary Coutant 0 siblings, 1 reply; 9+ messages in thread From: Sriraman Tallam @ 2015-04-20 23:06 UTC (permalink / raw) To: H.J. Lu Cc: Cary Coutant, binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, Ian Lance Taylor [-- Attachment #1: Type: text/plain, Size: 2313 bytes --] I have attached a patch to add --weak-unresolved-symbols to gold. Please let me know what you think. Thanks Sri On Sat, Apr 11, 2015 at 6:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, Apr 10, 2015 at 7:18 PM, Cary Coutant <ccoutant@gmail.com> wrote: >>> However, with -fPIE >>> >>> $ g++ -O2 -fPIE foo.cc -pie >>> foo.cc:function main: warning: undefined reference to 'foo()' >>> >>> but >>> $./a.out >>> ./a.out: symbol lookup error: ./a.out: undefined symbol: _Z3foov >>> >>> because with fPIE, a function pointer access is using a GOTPCREL >>> relocation which creates a GOT entry and a dynamic relocation for it. >>> The dynamic linker does not like the unresolved symbol any more. >>> >>> I have attached a patch to prevent creation of GOT and dynamic >>> relocation entries with --warn-unresolved-symbols in general. Is this >>> reasonable? >> >> No, I don't think so. For static links, we statically resolve >> undefined symbols to zero, but for non-static links, we leave them for >> dynamic resolution, presuming that they might be resolvable at >> runtime. This patch would change linker behavior that is fairly well >> established. (This patch also changes only x86_64 behavior; you'd need >> a similar patch for all targets. For a problem that isn't really >> target-specific, I'd prefer a target-independent solution.) >> >> The difference between non-PIE and PIE here is not due to the linker's >> -pie option, but because the compilers -fPIE option caused the >> compiler to use the GOTPCREL relocation, which requires the linker to >> create a GOT entry. >> >> If the reference to foo is weak, the dynamic loader won't complain -- >> I think that's really what you want. I'm not sure whether it's >> reasonable for you to have the compiler make these symbols weak unsats >> or not, but if you can, you wouldn't need --warn-unresolved-symbols >> any longer. If not, I wouldn't object to a new >> --weak-unresolved-symbols option that would have the linker convert >> any unresolved symbols (that it warned about) into weak references. I >> don't think we want to do that by default, since it changes existing >> expectations, but I think it's fine as an option. >> >> HJ, does that sound like a reasonable option for Gnu ld as well? >> > > This sounds reasonable to me. > > -- > H.J. [-- Attachment #2: weak_unresolved_symbols_patch.txt --] [-- Type: text/plain, Size: 13704 bytes --] This patch adds option --weak-unresolved-symbols to treat unresolved symbols as weak references. This is helpful when we want the link to succeed with unresolved symbols and the dynamic loader to not complain at run-time. Option --warn-unresolved-symbols lets the link succeed but could fail at run-time with unresolved symbol warnings especially when the unresolved symbols have GOT entries and dynamic relocations against them, like when -fPIE is used. * options.h (--warn-unresolved-symbols): New option. * symtab.cc (Symbol_table::sized_write_globals): Change symbol binding to weak with new option. * symtab.h (is_weak_undefined): Check for new option. (is_strong_undefined): Check for new option. * testsuite/Makefile.am(weak_unresolved_symbols_test): New test. * testsuite/Makefile.in: Regenerate. * testsuite/weak_unresolved_symbols_test.cc: New file. * Makefile.in: Side-effect from running automake. diff --git a/gold/options.h b/gold/options.h index c1a5743..bb3bee6 100644 --- a/gold/options.h +++ b/gold/options.h @@ -1219,6 +1219,9 @@ class General_options options::TWO_DASHES, '\0', N_("Report unresolved symbols as errors"), NULL, true); + DEFINE_bool(weak_unresolved_symbols, options::TWO_DASHES, '\0', false, + N_("Convert unresolved symbols to weak references"), + NULL); DEFINE_bool(wchar_size_warning, options::TWO_DASHES, '\0', true, NULL, N_("(ARM only) Do not warn about objects with incompatible " diff --git a/gold/symtab.cc b/gold/symtab.cc index d4f40c8..29deebf 100644 --- a/gold/symtab.cc +++ b/gold/symtab.cc @@ -2916,6 +2916,14 @@ Symbol_table::sized_write_globals(const Stringpool* sympool, typename elfcpp::Elf_types<size>::Elf_Addr dynsym_value = sym_value; elfcpp::STB binding = sym->binding(); + // If --weak-unresolved-symbols is set, change binding of unresolved + // global symbols to STB_WEAK. + if (parameters->options().weak_unresolved_symbols() + && (binding == elfcpp::STB_GLOBAL + || binding == elfcpp::STB_LOCAL) + && sym->is_undefined()) + binding = elfcpp::STB_WEAK; + // If --no-gnu-unique is set, change STB_GNU_UNIQUE to STB_GLOBAL. if (binding == elfcpp::STB_GNU_UNIQUE && !parameters->options().gnu_unique()) diff --git a/gold/symtab.h b/gold/symtab.h index 9413360..6f47c5e 100644 --- a/gold/symtab.h +++ b/gold/symtab.h @@ -526,7 +526,8 @@ class Symbol { return (this->is_undefined() && (this->binding() == elfcpp::STB_WEAK - || this->is_undef_binding_weak())); + || this->is_undef_binding_weak() + || parameters->options().weak_unresolved_symbols())); } // Return whether this is a strong undefined symbol. @@ -535,7 +536,8 @@ class Symbol { return (this->is_undefined() && this->binding() != elfcpp::STB_WEAK - && !this->is_undef_binding_weak()); + && !this->is_undef_binding_weak() + && !parameters->options().weak_unresolved_symbols()); } // Return whether this is an absolute symbol. diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index c57b2b3..e47174d 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -533,6 +533,11 @@ pie_copyrelocs_shared_test.o: pie_copyrelocs_shared_test.cc pie_copyrelocs_shared_test.so: pie_copyrelocs_shared_test.o gcctestdir/ld $(CXXLINK) -Bgcctestdir/ -shared pie_copyrelocs_shared_test.o +check_PROGRAMS += weak_unresolved_symbols_test +weak_unresolved_symbols_test_SOURCES = weak_unresolved_symbols_test.cc +weak_unresolved_symbols_test_CXXFLAGS = -fPIE +weak_unresolved_symbols_test_LDFLAGS = -Bgcctestdir/ -pie -Wl,--weak-unresolved-symbols + check_SCRIPTS += two_file_shared.sh check_DATA += two_file_shared.dbg MOSTLYCLEANFILES += two_file_shared.dbg diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in index 90c3054..4d7a54f 100644 --- a/gold/testsuite/Makefile.in +++ b/gold/testsuite/Makefile.in @@ -147,7 +147,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_separate_shared_21_test \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_relocatable_test \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_pie_test \ -@GCC_TRUE@@NATIVE_LINKER_TRUE@ pie_copyrelocs_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ pie_copyrelocs_test \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ weak_unresolved_symbols_test # The nonpic tests will fail on platforms which can not put non-PIC # code into shared libraries, so we just don't run them in that case. @@ -842,7 +843,8 @@ libgoldtest_a_OBJECTS = $(am_libgoldtest_a_OBJECTS) @GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_separate_shared_21_test$(EXEEXT) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_relocatable_test$(EXEEXT) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_pie_test$(EXEEXT) \ -@GCC_TRUE@@NATIVE_LINKER_TRUE@ pie_copyrelocs_test$(EXEEXT) +@GCC_TRUE@@NATIVE_LINKER_TRUE@ pie_copyrelocs_test$(EXEEXT) \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ weak_unresolved_symbols_test$(EXEEXT) @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am__EXEEXT_9 = two_file_shared_1_nonpic_test$(EXEEXT) \ @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_shared_2_nonpic_test$(EXEEXT) \ @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_same_shared_nonpic_test$(EXEEXT) \ @@ -1966,6 +1968,17 @@ weak_undef_test_LINK = $(CXXLD) $(AM_CXXFLAGS) $(CXXFLAGS) \ weak_undef_test_2_OBJECTS = $(am_weak_undef_test_2_OBJECTS) weak_undef_test_2_LINK = $(CXXLD) $(AM_CXXFLAGS) $(CXXFLAGS) \ $(weak_undef_test_2_LDFLAGS) $(LDFLAGS) -o $@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@am_weak_unresolved_symbols_test_OBJECTS = weak_unresolved_symbols_test-weak_unresolved_symbols_test.$(OBJEXT) +weak_unresolved_symbols_test_OBJECTS = \ + $(am_weak_unresolved_symbols_test_OBJECTS) +weak_unresolved_symbols_test_LDADD = $(LDADD) +weak_unresolved_symbols_test_DEPENDENCIES = libgoldtest.a ../libgold.a \ + ../../libiberty/libiberty.a $(am__DEPENDENCIES_1) \ + $(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \ + $(am__DEPENDENCIES_1) +weak_unresolved_symbols_test_LINK = $(CXXLD) \ + $(weak_unresolved_symbols_test_CXXFLAGS) $(CXXFLAGS) \ + $(weak_unresolved_symbols_test_LDFLAGS) $(LDFLAGS) -o $@ DEFAULT_INCLUDES = -I.@am__isrc@ -I$(top_builddir) depcomp = $(SHELL) $(top_srcdir)/../depcomp am__depfiles_maybe = depfiles @@ -2057,7 +2070,8 @@ SOURCES = $(libgoldtest_a_SOURCES) basic_pic_test.c basic_pie_test.c \ $(ver_test_8_SOURCES) $(ver_test_9_SOURCES) \ $(weak_alias_test_SOURCES) weak_plt.c $(weak_test_SOURCES) \ $(weak_undef_nonpic_test_SOURCES) $(weak_undef_test_SOURCES) \ - $(weak_undef_test_2_SOURCES) + $(weak_undef_test_2_SOURCES) \ + $(weak_unresolved_symbols_test_SOURCES) ETAGS = etags CTAGS = ctags am__tty_colors = \ @@ -2465,6 +2479,9 @@ LDADD = libgoldtest.a ../libgold.a ../../libiberty/libiberty.a $(LIBINTL) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@pie_copyrelocs_test_CXXFLAGS = -fno-exceptions -fno-asynchronous-unwind-tables @GCC_TRUE@@NATIVE_LINKER_TRUE@pie_copyrelocs_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,. -pie @GCC_TRUE@@NATIVE_LINKER_TRUE@pie_copyrelocs_test_LDADD = pie_copyrelocs_shared_test.so +@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_unresolved_symbols_test_SOURCES = weak_unresolved_symbols_test.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_unresolved_symbols_test_CXXFLAGS = -fPIE +@GCC_TRUE@@NATIVE_LINKER_TRUE@weak_unresolved_symbols_test_LDFLAGS = -Bgcctestdir/ -pie -Wl,--weak-unresolved-symbols @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared_1_nonpic_test_SOURCES = \ @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_test_2.cc two_file_test_main.cc @@ -3722,6 +3739,9 @@ weak_undef_test$(EXEEXT): $(weak_undef_test_OBJECTS) $(weak_undef_test_DEPENDENC weak_undef_test_2$(EXEEXT): $(weak_undef_test_2_OBJECTS) $(weak_undef_test_2_DEPENDENCIES) @rm -f weak_undef_test_2$(EXEEXT) $(weak_undef_test_2_LINK) $(weak_undef_test_2_OBJECTS) $(weak_undef_test_2_LDADD) $(LIBS) +weak_unresolved_symbols_test$(EXEEXT): $(weak_unresolved_symbols_test_OBJECTS) $(weak_unresolved_symbols_test_DEPENDENCIES) + @rm -f weak_unresolved_symbols_test$(EXEEXT) + $(weak_unresolved_symbols_test_LINK) $(weak_unresolved_symbols_test_OBJECTS) $(weak_unresolved_symbols_test_LDADD) $(LIBS) mostlyclean-compile: -rm -f *.$(OBJEXT) @@ -3852,6 +3872,7 @@ distclean-compile: @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/weak_test.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/weak_undef_test.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/weak_undef_test_2.Po@am__quote@ +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Po@am__quote@ .c.o: @am__fastdepCC_TRUE@ $(COMPILE) -MT $@ -MD -MP -MF $(DEPDIR)/$*.Tpo -c -o $@ $< @@ -3979,6 +4000,20 @@ pie_copyrelocs_test-pie_copyrelocs_test.obj: pie_copyrelocs_test.cc @AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ @am__fastdepCXX_FALSE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(pie_copyrelocs_test_CXXFLAGS) $(CXXFLAGS) -c -o pie_copyrelocs_test-pie_copyrelocs_test.obj `if test -f 'pie_copyrelocs_test.cc'; then $(CYGPATH_W) 'pie_copyrelocs_test.cc'; else $(CYGPATH_W) '$(srcdir)/pie_copyrelocs_test.cc'; fi` +weak_unresolved_symbols_test-weak_unresolved_symbols_test.o: weak_unresolved_symbols_test.cc +@am__fastdepCXX_TRUE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(weak_unresolved_symbols_test_CXXFLAGS) $(CXXFLAGS) -MT weak_unresolved_symbols_test-weak_unresolved_symbols_test.o -MD -MP -MF $(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Tpo -c -o weak_unresolved_symbols_test-weak_unresolved_symbols_test.o `test -f 'weak_unresolved_symbols_test.cc' || echo '$(srcdir)/'`weak_unresolved_symbols_test.cc +@am__fastdepCXX_TRUE@ $(am__mv) $(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Tpo $(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Po +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ source='weak_unresolved_symbols_test.cc' object='weak_unresolved_symbols_test-weak_unresolved_symbols_test.o' libtool=no @AMDEPBACKSLASH@ +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ +@am__fastdepCXX_FALSE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(weak_unresolved_symbols_test_CXXFLAGS) $(CXXFLAGS) -c -o weak_unresolved_symbols_test-weak_unresolved_symbols_test.o `test -f 'weak_unresolved_symbols_test.cc' || echo '$(srcdir)/'`weak_unresolved_symbols_test.cc + +weak_unresolved_symbols_test-weak_unresolved_symbols_test.obj: weak_unresolved_symbols_test.cc +@am__fastdepCXX_TRUE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(weak_unresolved_symbols_test_CXXFLAGS) $(CXXFLAGS) -MT weak_unresolved_symbols_test-weak_unresolved_symbols_test.obj -MD -MP -MF $(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Tpo -c -o weak_unresolved_symbols_test-weak_unresolved_symbols_test.obj `if test -f 'weak_unresolved_symbols_test.cc'; then $(CYGPATH_W) 'weak_unresolved_symbols_test.cc'; else $(CYGPATH_W) '$(srcdir)/weak_unresolved_symbols_test.cc'; fi` +@am__fastdepCXX_TRUE@ $(am__mv) $(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Tpo $(DEPDIR)/weak_unresolved_symbols_test-weak_unresolved_symbols_test.Po +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ source='weak_unresolved_symbols_test.cc' object='weak_unresolved_symbols_test-weak_unresolved_symbols_test.obj' libtool=no @AMDEPBACKSLASH@ +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ +@am__fastdepCXX_FALSE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(weak_unresolved_symbols_test_CXXFLAGS) $(CXXFLAGS) -c -o weak_unresolved_symbols_test-weak_unresolved_symbols_test.obj `if test -f 'weak_unresolved_symbols_test.cc'; then $(CYGPATH_W) 'weak_unresolved_symbols_test.cc'; else $(CYGPATH_W) '$(srcdir)/weak_unresolved_symbols_test.cc'; fi` + ID: $(HEADERS) $(SOURCES) $(LISP) $(TAGS_FILES) list='$(SOURCES) $(HEADERS) $(LISP) $(TAGS_FILES)'; \ unique=`for i in $$list; do \ @@ -4403,6 +4438,8 @@ two_file_pie_test.log: two_file_pie_test$(EXEEXT) @p='two_file_pie_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) pie_copyrelocs_test.log: pie_copyrelocs_test$(EXEEXT) @p='pie_copyrelocs_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) +weak_unresolved_symbols_test.log: weak_unresolved_symbols_test$(EXEEXT) + @p='weak_unresolved_symbols_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) two_file_shared_1_nonpic_test.log: two_file_shared_1_nonpic_test$(EXEEXT) @p='two_file_shared_1_nonpic_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) two_file_shared_2_nonpic_test.log: two_file_shared_2_nonpic_test$(EXEEXT) diff --git a/gold/Makefile.in b/gold/Makefile.in index 8e0ff0e..28d777f 100644 --- a/gold/Makefile.in +++ b/gold/Makefile.in @@ -70,8 +70,8 @@ subdir = . DIST_COMMON = NEWS README ChangeLog $(srcdir)/Makefile.in \ $(srcdir)/Makefile.am $(top_srcdir)/configure \ $(am__configure_deps) $(srcdir)/config.in \ - $(srcdir)/../mkinstalldirs $(top_srcdir)/po/Make-in ffsll.c \ - ftruncate.c pread.c mremap.c yyscript.h yyscript.c \ + $(srcdir)/../mkinstalldirs $(top_srcdir)/po/Make-in pread.c \ + mremap.c ftruncate.c ffsll.c yyscript.h yyscript.c \ $(srcdir)/../depcomp $(srcdir)/../ylwrap ACLOCAL_M4 = $(top_srcdir)/aclocal.m4 am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-20 23:06 ` Sriraman Tallam @ 2015-04-22 19:33 ` Cary Coutant 2015-04-23 15:09 ` Cary Coutant 0 siblings, 1 reply; 9+ messages in thread From: Cary Coutant @ 2015-04-22 19:33 UTC (permalink / raw) To: Sriraman Tallam Cc: H.J. Lu, binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, Ian Lance Taylor * options.h (--warn-unresolved-symbols): New option. s/warn/weak/ * symtab.cc (Symbol_table::sized_write_globals): Change symbol binding to weak with new option. * symtab.h (is_weak_undefined): Check for new option. (is_strong_undefined): Check for new option. * testsuite/Makefile.am(weak_unresolved_symbols_test): New test. * testsuite/Makefile.in: Regenerate. * testsuite/weak_unresolved_symbols_test.cc: New file. This file is missing from the patch. (Don't forget to "git add".) * Makefile.in: Side-effect from running automake. Since you didn't change Makefile, just remove this from the patch ("git checkout Makefile" after running automake). Otherwise, we'll keep ping-ponging uselessly between the two (or more) variations that automake generates. + // If --weak-unresolved-symbols is set, change binding of unresolved + // global symbols to STB_WEAK. + if (parameters->options().weak_unresolved_symbols() + && (binding == elfcpp::STB_GLOBAL + || binding == elfcpp::STB_LOCAL) Why check for STB_LOCAL? Local symbols can't be unresolved. What about STB_GNU_UNIQUE symbols? (Perhaps that's what you meant.) -cary ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-22 19:33 ` Cary Coutant @ 2015-04-23 15:09 ` Cary Coutant 2015-04-23 17:27 ` Sriraman Tallam 0 siblings, 1 reply; 9+ messages in thread From: Cary Coutant @ 2015-04-23 15:09 UTC (permalink / raw) To: Sriraman Tallam Cc: H.J. Lu, binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, Ian Lance Taylor > + // If --weak-unresolved-symbols is set, change binding of unresolved > + // global symbols to STB_WEAK. > + if (parameters->options().weak_unresolved_symbols() > + && (binding == elfcpp::STB_GLOBAL > + || binding == elfcpp::STB_LOCAL) > > Why check for STB_LOCAL? Local symbols can't be unresolved. What about > STB_GNU_UNIQUE symbols? (Perhaps that's what you meant.) On further thought, I doubt we will see an undefined GNU_UNIQUE symbol, and if we do, I'm not sure we should be changing it to WEAK (unless the very next if statement converts it to GLOBAL). -cary ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-23 15:09 ` Cary Coutant @ 2015-04-23 17:27 ` Sriraman Tallam 2015-04-23 18:01 ` Cary Coutant 0 siblings, 1 reply; 9+ messages in thread From: Sriraman Tallam @ 2015-04-23 17:27 UTC (permalink / raw) To: Cary Coutant Cc: H.J. Lu, binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, Ian Lance Taylor [-- Attachment #1: Type: text/plain, Size: 740 bytes --] On Thu, Apr 23, 2015 at 8:09 AM, Cary Coutant <ccoutant@gmail.com> wrote: >> + // If --weak-unresolved-symbols is set, change binding of unresolved >> + // global symbols to STB_WEAK. >> + if (parameters->options().weak_unresolved_symbols() >> + && (binding == elfcpp::STB_GLOBAL >> + || binding == elfcpp::STB_LOCAL) >> >> Why check for STB_LOCAL? Local symbols can't be unresolved. What about >> STB_GNU_UNIQUE symbols? (Perhaps that's what you meant.) > > On further thought, I doubt we will see an undefined GNU_UNIQUE > symbol, and if we do, I'm not sure we should be changing it to WEAK > (unless the very next if statement converts it to GLOBAL). Patch redone and attached. Thanks Sri > > -cary [-- Attachment #2: weak_unresolved_symbols_patch.txt --] [-- Type: text/plain, Size: 5260 bytes --] This patch adds option --weak-unresolved-symbols to treat unresolved symbols as weak references. This is helpful when we want the link to succeed with unresolved symbols and the dynamic loader to not complain at run-time. Option --warn-unresolved-symbols lets the link succeed but could fail at run-time with unresolved symbol warnings especially when the unresolved symbols have GOT entries and dynamic relocations against them, like when -fPIE is used. * options.h (--weak-unresolved-symbols): New option. * symtab.cc (Symbol_table::sized_write_globals): Change symbol binding to weak with new option. * symtab.h (is_weak_undefined): Check for new option. (is_strong_undefined): Check for new option. * testsuite/Makefile.am(weak_unresolved_symbols_test): New test. * testsuite/Makefile.in: Regenerate. * testsuite/weak_unresolved_symbols_test.cc: New file. diff --git a/gold/options.h b/gold/options.h index c1a5743..bb3bee6 100644 --- a/gold/options.h +++ b/gold/options.h @@ -1219,6 +1219,9 @@ class General_options options::TWO_DASHES, '\0', N_("Report unresolved symbols as errors"), NULL, true); + DEFINE_bool(weak_unresolved_symbols, options::TWO_DASHES, '\0', false, + N_("Convert unresolved symbols to weak references"), + NULL); DEFINE_bool(wchar_size_warning, options::TWO_DASHES, '\0', true, NULL, N_("(ARM only) Do not warn about objects with incompatible " diff --git a/gold/symtab.cc b/gold/symtab.cc index d4f40c8..19f79367a 100644 --- a/gold/symtab.cc +++ b/gold/symtab.cc @@ -2916,6 +2916,13 @@ Symbol_table::sized_write_globals(const Stringpool* sympool, typename elfcpp::Elf_types<size>::Elf_Addr dynsym_value = sym_value; elfcpp::STB binding = sym->binding(); + // If --weak-unresolved-symbols is set, change binding of unresolved + // global symbols to STB_WEAK. + if (parameters->options().weak_unresolved_symbols() + && binding == elfcpp::STB_GLOBAL + && sym->is_undefined()) + binding = elfcpp::STB_WEAK; + // If --no-gnu-unique is set, change STB_GNU_UNIQUE to STB_GLOBAL. if (binding == elfcpp::STB_GNU_UNIQUE && !parameters->options().gnu_unique()) diff --git a/gold/symtab.h b/gold/symtab.h index 9413360..6f47c5e 100644 --- a/gold/symtab.h +++ b/gold/symtab.h @@ -526,7 +526,8 @@ class Symbol { return (this->is_undefined() && (this->binding() == elfcpp::STB_WEAK - || this->is_undef_binding_weak())); + || this->is_undef_binding_weak() + || parameters->options().weak_unresolved_symbols())); } // Return whether this is a strong undefined symbol. @@ -535,7 +536,8 @@ class Symbol { return (this->is_undefined() && this->binding() != elfcpp::STB_WEAK - && !this->is_undef_binding_weak()); + && !this->is_undef_binding_weak() + && !parameters->options().weak_unresolved_symbols()); } // Return whether this is an absolute symbol. diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index c57b2b3..e47174d 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -533,6 +533,11 @@ pie_copyrelocs_shared_test.o: pie_copyrelocs_shared_test.cc pie_copyrelocs_shared_test.so: pie_copyrelocs_shared_test.o gcctestdir/ld $(CXXLINK) -Bgcctestdir/ -shared pie_copyrelocs_shared_test.o +check_PROGRAMS += weak_unresolved_symbols_test +weak_unresolved_symbols_test_SOURCES = weak_unresolved_symbols_test.cc +weak_unresolved_symbols_test_CXXFLAGS = -fPIE +weak_unresolved_symbols_test_LDFLAGS = -Bgcctestdir/ -pie -Wl,--weak-unresolved-symbols + check_SCRIPTS += two_file_shared.sh check_DATA += two_file_shared.dbg MOSTLYCLEANFILES += two_file_shared.dbg --- /dev/null +++ b/gold/testsuite/weak_unresolved_symbols_test.cc @@ -0,0 +1,45 @@ +// weak_unresolved_symbols_test.cc -- a test case for gold + +// Copyright (C) 2014-2015 Free Software Foundation, Inc. +// Written by Sriraman Tallam <tmsriram@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. + +// Test --weak-unresolved-symbols. Symbol foo remains unresolved but +// with -fPIE, needs a GOT entry and has a dynsym entry and a dynamic +// relocation against it created. This will fail to link and run without +// --weak-unresolved-symbols. With --warn-unresolved-symbols, it will link +// but the dynamic linker will complain that foo(_Z3foov) is unresolved. + +extern int foo(); + +int bar() { + return 0; +} + +int (*p)() = &bar; + +int main() { + if (p == &foo) + { + foo(); + } + else + (*p)(); + return 0; +} ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols. 2015-04-23 17:27 ` Sriraman Tallam @ 2015-04-23 18:01 ` Cary Coutant 0 siblings, 0 replies; 9+ messages in thread From: Cary Coutant @ 2015-04-23 18:01 UTC (permalink / raw) To: Sriraman Tallam Cc: H.J. Lu, binutils, Paul Pluzhnikov, Rong Xu, Brooks Moses, Ollie Wild, David Li, Teresa Johnson, Ian Lance Taylor * options.h (--weak-unresolved-symbols): New option. * symtab.cc (Symbol_table::sized_write_globals): Change symbol binding to weak with new option. * symtab.h (is_weak_undefined): Check for new option. (is_strong_undefined): Check for new option. * testsuite/Makefile.am(weak_unresolved_symbols_test): New test. * testsuite/Makefile.in: Regenerate. * testsuite/weak_unresolved_symbols_test.cc: New file. Space before '('. +// Copyright (C) 2014-2015 Free Software Foundation, Inc. This should just say '2015'. OK with those fixes. Thanks! -cary ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-23 18:01 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-11 0:16 Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols Sriraman Tallam 2015-04-11 0:22 ` Sriraman Tallam 2015-04-11 2:18 ` Cary Coutant 2015-04-11 13:28 ` H.J. Lu 2015-04-20 23:06 ` Sriraman Tallam 2015-04-22 19:33 ` Cary Coutant 2015-04-23 15:09 ` Cary Coutant 2015-04-23 17:27 ` Sriraman Tallam 2015-04-23 18:01 ` Cary Coutant
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).