public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [gold patch] Fix problems with mixed PIC/non-PIC objects in PIE output
@ 2010-09-11  7:16 Cary Coutant
  2010-09-11  7:18 ` Cary Coutant
  0 siblings, 1 reply; 3+ messages in thread
From: Cary Coutant @ 2010-09-11  7:16 UTC (permalink / raw)
  To: Ian Lance Taylor, Binutils

When building a PIE executable with mixed PIC and non-PIC input files,
a non-PIC call to a shared library function will result in a dynamic
relocation on the call. If there is also a PIC call to the same
symbol, a PLT entry will have been created, and the logic for
determining if a dynamic relocation is necessary will erroneously
return false. If the non-PIC call precedes the PIC call,
Symbol::needs_dynamic_reloc() will return true during scanning (before
the PIC call is seen), and false when applying relocations (after the
PIC call was scanned). For targets that use REL relocations (namely
i386), this causes gold to apply the static relocation instead of
leaving the original addend intact. This patch fixes the test in
needs_dynamic_reloc() to check for output_is_position_independent
instead of shared.

In addition, a non-PIC reference to data in a shared library will
result in the creation of a COPY relocation, but no dynamic relocation
for the reference to the new home. I checked how Gnu ld behaves, and
it does not appear to make COPY relocs for PIE executables. This patch
also changes Symbol::may_need_copy_reloc() to test for
output_is_position_independent.

The new test case covers both of these problems.

Tested on i386 and x86_64 (although x86_64 doesn't run the non-PIC-in-PIE test).

OK?

-cary


        * symtab.h (Symbol::needs_dynamic_reloc): Non-PIC calls from
        position-independent executables to shared libraries need dynamic
        relocations.
        (Symbol::may_need_copy_reloc): Do not generate COPY relocs in
        position-independent executables.
        * testsuite/Makefile.am (two_file_mixed_pie_test): New test.
        * testsuite/Makefile.in: Regenerate.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [gold patch] Fix problems with mixed PIC/non-PIC objects in PIE output
  2010-09-11  7:16 [gold patch] Fix problems with mixed PIC/non-PIC objects in PIE output Cary Coutant
@ 2010-09-11  7:18 ` Cary Coutant
  2010-09-12  6:04   ` Ian Lance Taylor
  0 siblings, 1 reply; 3+ messages in thread
From: Cary Coutant @ 2010-09-11  7:18 UTC (permalink / raw)
  To: Ian Lance Taylor, Binutils

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

[This time with the patch...]

On Sat, Sep 11, 2010 at 12:16 AM, Cary Coutant <ccoutant@google.com> wrote:
> When building a PIE executable with mixed PIC and non-PIC input files,
> a non-PIC call to a shared library function will result in a dynamic
> relocation on the call. If there is also a PIC call to the same
> symbol, a PLT entry will have been created, and the logic for
> determining if a dynamic relocation is necessary will erroneously
> return false. If the non-PIC call precedes the PIC call,
> Symbol::needs_dynamic_reloc() will return true during scanning (before
> the PIC call is seen), and false when applying relocations (after the
> PIC call was scanned). For targets that use REL relocations (namely
> i386), this causes gold to apply the static relocation instead of
> leaving the original addend intact. This patch fixes the test in
> needs_dynamic_reloc() to check for output_is_position_independent
> instead of shared.
>
> In addition, a non-PIC reference to data in a shared library will
> result in the creation of a COPY relocation, but no dynamic relocation
> for the reference to the new home. I checked how Gnu ld behaves, and
> it does not appear to make COPY relocs for PIE executables. This patch
> also changes Symbol::may_need_copy_reloc() to test for
> output_is_position_independent.
>
> The new test case covers both of these problems.
>
> Tested on i386 and x86_64 (although x86_64 doesn't run the non-PIC-in-PIE test).
>
> OK?
>
> -cary
>
>
>        * symtab.h (Symbol::needs_dynamic_reloc): Non-PIC calls from
>        position-independent executables to shared libraries need dynamic
>        relocations.
>        (Symbol::may_need_copy_reloc): Do not generate COPY relocs in
>        position-independent executables.
>        * testsuite/Makefile.am (two_file_mixed_pie_test): New test.
>        * testsuite/Makefile.in: Regenerate.
>

[-- Attachment #2: gold-pie-patch.txt --]
[-- Type: text/plain, Size: 9546 bytes --]

	* symtab.h (Symbol::needs_dynamic_reloc): Non-PIC calls from
	position-independent executables to shared libraries need dynamic
	relocations.
	(Symbol::may_need_copy_reloc): Do not generate COPY relocs in
	position-independent executables.
	* testsuite/Makefile.am (two_file_mixed_pie_test): New test.
	* testsuite/Makefile.in: Regenerate.


Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.113
diff -u -p -r1.113 symtab.h
--- symtab.h	25 Aug 2010 08:36:54 -0000	1.113
+++ symtab.h	11 Sep 2010 06:54:56 -0000
@@ -657,7 +657,8 @@ class Symbol
     // shared library cannot use a PLT entry.
     if ((flags & FUNCTION_CALL)
         && this->has_plt_offset()
-        && !((flags & NON_PIC_REF) && parameters->options().shared()))
+        && !((flags & NON_PIC_REF)
+             && parameters->options().output_is_position_independent()))
       return false;
 
     // A reference to any PLT entry in a non-position-independent executable
@@ -798,7 +799,7 @@ class Symbol
   bool
   may_need_copy_reloc() const
   {
-    return (!parameters->options().shared()
+    return (!parameters->options().output_is_position_independent()
 	    && parameters->options().copyreloc()
 	    && this->is_from_dynobj()
 	    && !this->is_func());
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.146
diff -u -p -r1.146 Makefile.am
--- testsuite/Makefile.am	8 Sep 2010 23:54:51 -0000	1.146
+++ testsuite/Makefile.am	11 Sep 2010 06:54:56 -0000
@@ -461,6 +461,11 @@ two_file_mixed_2_shared_test_DEPENDENCIE
 two_file_mixed_2_shared_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
 two_file_mixed_2_shared_test_LDADD = two_file_shared_mixed_1.so two_file_shared_2.so
 
+check_PROGRAMS += two_file_mixed_pie_test
+two_file_mixed_pie_test: two_file_test_1.o two_file_test_1b_pie.o \
+		two_file_test_main_pie.o two_file_shared_2.so gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,-R,. -pie two_file_test_1.o two_file_test_1b_pie.o two_file_test_main_pie.o two_file_shared_2.so
+
 endif FN_PTRS_IN_SO_WITHOUT_PIC
 
 check_PROGRAMS += two_file_strip_test
Index: testsuite/Makefile.in
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.in,v
retrieving revision 1.155
diff -u -p -r1.155 Makefile.in
--- testsuite/Makefile.in	8 Sep 2010 23:54:51 -0000	1.155
+++ testsuite/Makefile.in	11 Sep 2010 06:54:57 -0000
@@ -160,7 +160,8 @@ check_PROGRAMS = object_unittest$(EXEEXT
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_12_nonpic_test \
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_21_nonpic_test \
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_shared_test \
-@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_2_shared_test
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_2_shared_test \
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_pie_test
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_6 = two_file_strip_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_same_shared_strip_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	common_test_1 common_test_2 \
@@ -592,7 +593,8 @@ libgoldtest_a_OBJECTS = $(am_libgoldtest
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_12_nonpic_test$(EXEEXT) \
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_21_nonpic_test$(EXEEXT) \
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_shared_test$(EXEEXT) \
-@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_2_shared_test$(EXEEXT)
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_2_shared_test$(EXEEXT) \
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_pie_test$(EXEEXT)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__EXEEXT_3 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_strip_test$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_same_shared_strip_test$(EXEEXT) \
@@ -1269,6 +1271,12 @@ two_file_mixed_2_shared_test_OBJECTS =  
 two_file_mixed_2_shared_test_LINK = $(CXXLD) $(AM_CXXFLAGS) \
 	$(CXXFLAGS) $(two_file_mixed_2_shared_test_LDFLAGS) $(LDFLAGS) \
 	-o $@
+two_file_mixed_pie_test_SOURCES = two_file_mixed_pie_test.c
+two_file_mixed_pie_test_OBJECTS = two_file_mixed_pie_test.$(OBJEXT)
+two_file_mixed_pie_test_LDADD = $(LDADD)
+two_file_mixed_pie_test_DEPENDENCIES = libgoldtest.a ../libgold.a \
+	../../libiberty/libiberty.a $(am__DEPENDENCIES_1) \
+	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am_two_file_mixed_shared_test_OBJECTS = two_file_test_main.$(OBJEXT)
 two_file_mixed_shared_test_OBJECTS =  \
 	$(am_two_file_mixed_shared_test_OBJECTS)
@@ -1518,6 +1526,7 @@ SOURCES = $(libgoldtest_a_SOURCES) basic
 	$(tls_shared_nonpic_test_SOURCES) $(tls_shared_test_SOURCES) \
 	$(tls_static_pic_test_SOURCES) $(tls_static_test_SOURCES) \
 	$(tls_test_SOURCES) $(two_file_mixed_2_shared_test_SOURCES) \
+	two_file_mixed_pie_test.c \
 	$(two_file_mixed_shared_test_SOURCES) \
 	$(two_file_pic_test_SOURCES) two_file_pie_test.c \
 	$(two_file_relocatable_test_SOURCES) \
@@ -2765,6 +2774,15 @@ tls_test$(EXEEXT): $(tls_test_OBJECTS) $
 two_file_mixed_2_shared_test$(EXEEXT): $(two_file_mixed_2_shared_test_OBJECTS) $(two_file_mixed_2_shared_test_DEPENDENCIES) 
 	@rm -f two_file_mixed_2_shared_test$(EXEEXT)
 	$(two_file_mixed_2_shared_test_LINK) $(two_file_mixed_2_shared_test_OBJECTS) $(two_file_mixed_2_shared_test_LDADD) $(LIBS)
+@FN_PTRS_IN_SO_WITHOUT_PIC_FALSE@two_file_mixed_pie_test$(EXEEXT): $(two_file_mixed_pie_test_OBJECTS) $(two_file_mixed_pie_test_DEPENDENCIES) 
+@FN_PTRS_IN_SO_WITHOUT_PIC_FALSE@	@rm -f two_file_mixed_pie_test$(EXEEXT)
+@FN_PTRS_IN_SO_WITHOUT_PIC_FALSE@	$(LINK) $(two_file_mixed_pie_test_OBJECTS) $(two_file_mixed_pie_test_LDADD) $(LIBS)
+@GCC_FALSE@two_file_mixed_pie_test$(EXEEXT): $(two_file_mixed_pie_test_OBJECTS) $(two_file_mixed_pie_test_DEPENDENCIES) 
+@GCC_FALSE@	@rm -f two_file_mixed_pie_test$(EXEEXT)
+@GCC_FALSE@	$(LINK) $(two_file_mixed_pie_test_OBJECTS) $(two_file_mixed_pie_test_LDADD) $(LIBS)
+@NATIVE_LINKER_FALSE@two_file_mixed_pie_test$(EXEEXT): $(two_file_mixed_pie_test_OBJECTS) $(two_file_mixed_pie_test_DEPENDENCIES) 
+@NATIVE_LINKER_FALSE@	@rm -f two_file_mixed_pie_test$(EXEEXT)
+@NATIVE_LINKER_FALSE@	$(LINK) $(two_file_mixed_pie_test_OBJECTS) $(two_file_mixed_pie_test_LDADD) $(LIBS)
 two_file_mixed_shared_test$(EXEEXT): $(two_file_mixed_shared_test_OBJECTS) $(two_file_mixed_shared_test_DEPENDENCIES) 
 	@rm -f two_file_mixed_shared_test$(EXEEXT)
 	$(two_file_mixed_shared_test_LINK) $(two_file_mixed_shared_test_OBJECTS) $(two_file_mixed_shared_test_LDADD) $(LIBS)
@@ -2956,6 +2974,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tls_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tls_test_file2.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tls_test_main.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/two_file_mixed_pie_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/two_file_pie_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/two_file_strip_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/two_file_test_1.Po@am__quote@
@@ -3380,6 +3399,8 @@ two_file_mixed_shared_test.log: two_file
 	@p='two_file_mixed_shared_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 two_file_mixed_2_shared_test.log: two_file_mixed_2_shared_test$(EXEEXT)
 	@p='two_file_mixed_2_shared_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+two_file_mixed_pie_test.log: two_file_mixed_pie_test$(EXEEXT)
+	@p='two_file_mixed_pie_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 two_file_strip_test.log: two_file_strip_test$(EXEEXT)
 	@p='two_file_strip_test$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 two_file_same_shared_strip_test.log: two_file_same_shared_strip_test$(EXEEXT)
@@ -3877,6 +3898,9 @@ uninstall-am:
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_1b_pic.o two_file_test_2.o
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared_mixed_1.so: two_file_test_1.o two_file_test_1b_pic.o two_file_shared_2.so gcctestdir/ld
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1.o two_file_test_1b_pic.o two_file_shared_2.so
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_mixed_pie_test: two_file_test_1.o two_file_test_1b_pie.o \
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@		two_file_test_main_pie.o two_file_shared_2.so gcctestdir/ld
+@FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,-R,. -pie two_file_test_1.o two_file_test_1b_pie.o two_file_test_main_pie.o two_file_shared_2.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_strip_test: two_file_test
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_STRIP) -o two_file_strip_test two_file_test
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared_strip.so: two_file_shared.so

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [gold patch] Fix problems with mixed PIC/non-PIC objects in PIE output
  2010-09-11  7:18 ` Cary Coutant
@ 2010-09-12  6:04   ` Ian Lance Taylor
  0 siblings, 0 replies; 3+ messages in thread
From: Ian Lance Taylor @ 2010-09-12  6:04 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

Cary Coutant <ccoutant@google.com> writes:

> 	* symtab.h (Symbol::needs_dynamic_reloc): Non-PIC calls from
> 	position-independent executables to shared libraries need dynamic
> 	relocations.
> 	(Symbol::may_need_copy_reloc): Do not generate COPY relocs in
> 	position-independent executables.
> 	* testsuite/Makefile.am (two_file_mixed_pie_test): New test.
> 	* testsuite/Makefile.in: Regenerate.

This is OK.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-09-12  6:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-11  7:16 [gold patch] Fix problems with mixed PIC/non-PIC objects in PIE output Cary Coutant
2010-09-11  7:18 ` Cary Coutant
2010-09-12  6:04   ` Ian Lance Taylor

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).