Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Sriraman Tallam]

[-- 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
 	    {

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Sriraman Tallam]

+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

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Cary Coutant]

> 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

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[H.J. Lu]

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.

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Sriraman Tallam]

[-- 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 \

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Cary Coutant]

        * 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

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Cary Coutant]

> +      // 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

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Sriraman Tallam]

[-- 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;
+}

Re: Patch to not create GOT and dynamic relocation entries for unresolved symbols with --warn-unresolved-symbols.

[Cary Coutant]

        * 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