public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [gold commit] PR 21090: Disallow --incremental with -pie and force -no-pie for incremental tests
@ 2017-12-02  7:41 Cary Coutant
  0 siblings, 0 replies; only message in thread
From: Cary Coutant @ 2017-12-02  7:41 UTC (permalink / raw)
  To: Binutils

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

This is a partial fix for the gold testsuite failures documented in
PR 21090. The use of -fpie triggers some mov-to-lea optimizations that
are not compatible with incremental linking, so those optimizations need
to be disabled. We also diagnose the attempt to use -pie with incremental
linking, and force -no-pie for the incremental tests in case the build has
been configured to have GCC pass -pie all the time.

We still have a problem where compiling with -fpie results in some GOT
entries even when linking with -no-pie. This combination still causes test
failures because we are not updating the GOT entries in an incremental update
link.

-cary


2017-12-01  Cary Coutant  <ccoutant@gmail.com>

gold/
        PR gold/21090
        * incremental.cc (Sized_relobj_incr::do_relocate): Fix comment.
        * options.cc (General_options::finalize): Disallow -pie with
        incremental linking.
        * x86_64.cc (Target_x86_64::Scan::local): Don't do mov-to-lea
        or callq-to-direct optimizations for incremental links.
        (Target_x86_64::Scan::global): Likewise.
        (Target_x86_64::Relocate::relocate): Likewise.
        * testsuite/Makefile.am (incremental_test): Force -no-pie.
        (incremental_test_2): Likewise.
        (incremental_test_3): Likewise.
        (incremental_test_4): Likewise.
        (incremental_test_5): Likewise.
        (incremental_test_6): Likewise.
        (incremental_copy_test): Likewise.
        (incremental_common_test_1): Likewise.
        (incremental_comdat_test_1):  Likewise.
        * testsuite/Makefile.in: Regenerate.

[-- Attachment #2: pr21090-a.patch --]
[-- Type: application/octet-stream, Size: 14793 bytes --]

Disallow --incremental with -pie and force -no-pie for incremental tests.

This is a partial fix for the gold testsuite failures documented in
PR 21090. The use of -fpie triggers some mov-to-lea optimizations that
are not compatible with incremental linking, so those optimizations need
to be disabled. We also diagnose the attempt to use -pie with incremental
linking, and force -no-pie for the incremental tests in case the build has
been configured to have GCC pass -pie all the time.

We still have a problem where compiling with -fpie results in some GOT
entries even when linking with -no-pie. This combination still causes test
failures because we are not updating the GOT entries in an incremental update
link.

2017-12-01  Cary Coutant  <ccoutant@gmail.com>

gold/
	PR gold/21090
	* incremental.cc (Sized_relobj_incr::do_relocate): Fix comment.
	* options.cc (General_options::finalize): Disallow -pie with
	incremental linking.
	* x86_64.cc (Target_x86_64::Scan::local): Don't do mov-to-lea
	or callq-to-direct optimizations for incremental links.
	(Target_x86_64::Scan::global): Likewise.
	(Target_x86_64::Relocate::relocate): Likewise.
	* testsuite/Makefile.am (incremental_test): Force -no-pie.
	(incremental_test_2): Likewise.
	(incremental_test_3): Likewise.
	(incremental_test_4): Likewise.
	(incremental_test_5): Likewise.
	(incremental_test_6): Likewise.
	(incremental_copy_test): Likewise.
	(incremental_common_test_1): Likewise.
	(incremental_comdat_test_1):  Likewise.
	* testsuite/Makefile.in: Regenerate.


diff --git a/gold/incremental.cc b/gold/incremental.cc
index 6d6b34ae16..8db7bc6cf7 100644
--- a/gold/incremental.cc
+++ b/gold/incremental.cc
@@ -2536,8 +2536,8 @@ Sized_relobj_incr<size, big_endian>::do_set_local_dynsym_offset(off_t)
 // Relocate the input sections and write out the local symbols.
 // We don't actually do any relocation here.  For unchanged input files,
 // we reapply relocations only for symbols that have changed; that happens
-// in queue_final_tasks.  We do need to rewrite the incremental relocations
-// for this object.
+// in Layout_task_runner::run().  We do need to rewrite the incremental
+// relocations for this object.
 
 template<int size, bool big_endian>
 void
diff --git a/gold/options.cc b/gold/options.cc
index f54fb9b1cb..21b95e38d8 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -1343,6 +1343,8 @@ General_options::finalize()
 	gold_fatal(_("incremental linking is not compatible with --plugin"));
       if (this->relro())
 	gold_fatal(_("incremental linking is not compatible with -z relro"));
+      if (this->pie())
+	gold_fatal(_("incremental linking is not compatible with -pie"));
       if (this->gc_sections())
 	{
 	  gold_warning(_("ignoring --gc-sections for an incremental link"));
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index f490453f1f..aad244c595 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -178,7 +178,7 @@ incremental_test_1.o: incremental_test_1.c
 incremental_test_2.o: incremental_test_2.c
 	$(COMPILE) -O0 -c -ffunction-sections -g -o $@ $<
 incremental_test: incremental_test_1.o incremental_test_2.o gcctestdir/ld
-	$(LINK) -Bgcctestdir/ -Wl,--incremental-full -Wl,-z,norelro incremental_test_1.o incremental_test_2.o -Wl,-debug 2> incremental_test.cmdline
+	$(LINK) -Bgcctestdir/ -Wl,--incremental-full -Wl,-z,norelro,-no-pie incremental_test_1.o incremental_test_2.o -Wl,-debug 2> incremental_test.cmdline
 incremental_test.stdout: incremental_test ../incremental-dump
 	../incremental-dump incremental_test > $@
 
@@ -3011,31 +3011,31 @@ MOSTLYCLEANFILES += two_file_test_tmp_2.o
 incremental_test_2: two_file_test_1_v1_ndebug.o two_file_test_1_ndebug.o two_file_test_1b_ndebug.o \
 		    two_file_test_2_ndebug.o two_file_test_main_ndebug.o gcctestdir/ld
 	cp -f two_file_test_1_v1_ndebug.o two_file_test_tmp_2.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b_ndebug.o two_file_test_2_ndebug.o two_file_test_main_ndebug.o
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b_ndebug.o two_file_test_2_ndebug.o two_file_test_main_ndebug.o
 	@sleep 1
 	cp -f two_file_test_1_ndebug.o two_file_test_tmp_2.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b_ndebug.o two_file_test_2_ndebug.o two_file_test_main_ndebug.o
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b_ndebug.o two_file_test_2_ndebug.o two_file_test_main_ndebug.o
 
 check_PROGRAMS += incremental_test_3
 MOSTLYCLEANFILES += two_file_test_tmp_3.o
 incremental_test_3: two_file_test_1.o two_file_test_1b_v1.o two_file_test_1b.o \
 		    two_file_test_2.o two_file_test_main.o gcctestdir/ld
 	cp -f two_file_test_1b_v1.o two_file_test_tmp_3.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ two_file_test_1.o two_file_test_tmp_3.o two_file_test_2.o two_file_test_main.o
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_1.o two_file_test_tmp_3.o two_file_test_2.o two_file_test_main.o
 	@sleep 1
 	cp -f two_file_test_1b.o two_file_test_tmp_3.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ two_file_test_1.o two_file_test_tmp_3.o two_file_test_2.o two_file_test_main.o
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_1.o two_file_test_tmp_3.o two_file_test_2.o two_file_test_main.o
 
 check_PROGRAMS += incremental_test_4
 MOSTLYCLEANFILES += incremental_test_4.base two_file_test_tmp_4.o
 incremental_test_4: two_file_test_1.o two_file_test_1b.o two_file_test_2_v1.o \
 		    two_file_test_2.o two_file_test_main.o gcctestdir/ld
 	cp -f two_file_test_2_v1.o two_file_test_tmp_4.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ two_file_test_1.o two_file_test_1b.o two_file_test_tmp_4.o two_file_test_main.o
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_1.o two_file_test_1b.o two_file_test_tmp_4.o two_file_test_main.o
 	mv -f incremental_test_4 incremental_test_4.base
 	@sleep 1
 	cp -f two_file_test_2.o two_file_test_tmp_4.o
-	$(CXXLINK) -Wl,--incremental-update,--incremental-base=incremental_test_4.base -Wl,-z,norelro -Bgcctestdir/ two_file_test_1.o two_file_test_1b.o two_file_test_tmp_4.o two_file_test_main.o
+	$(CXXLINK) -Wl,--incremental-update,--incremental-base=incremental_test_4.base -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_1.o two_file_test_1b.o two_file_test_tmp_4.o two_file_test_main.o
 
 check_PROGRAMS += incremental_test_5
 MOSTLYCLEANFILES += two_file_test_5.a
@@ -3043,11 +3043,11 @@ incremental_test_5: two_file_test_1.o two_file_test_1b_v1.o two_file_test_1b.o \
 		    two_file_test_2.o two_file_test_main.o gcctestdir/ld
 	cp -f two_file_test_1b_v1.o two_file_test_tmp_5.o
 	$(TEST_AR) rc two_file_test_5.a two_file_test_1.o two_file_test_tmp_5.o two_file_test_2.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ two_file_test_main.o two_file_test_5.a
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_main.o two_file_test_5.a
 	@sleep 1
 	cp -f two_file_test_1b.o two_file_test_tmp_5.o
 	$(TEST_AR) rc two_file_test_5.a two_file_test_1.o two_file_test_tmp_5.o two_file_test_2.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ two_file_test_main.o two_file_test_5.a
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_main.o two_file_test_5.a
 
 # Test the --incremental-unchanged flag with an archive library.
 # The second link should not update the library.
@@ -3057,38 +3057,38 @@ incremental_test_6: two_file_test_1.o two_file_test_1b_v1.o two_file_test_1b.o \
 		    two_file_test_2.o two_file_test_main.o gcctestdir/ld
 	cp -f two_file_test_1b.o two_file_test_tmp_6.o
 	$(TEST_AR) rc two_file_test_6.a two_file_test_1.o two_file_test_tmp_6.o two_file_test_2.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ two_file_test_main.o two_file_test_6.a
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_main.o two_file_test_6.a
 	@sleep 1
 	cp -f two_file_test_1b_v1.o two_file_test_tmp_6.o
 	$(TEST_AR) rc two_file_test_6.a two_file_test_1.o two_file_test_tmp_6.o two_file_test_2.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ two_file_test_main.o -Wl,--incremental-unchanged two_file_test_6.a -Wl,--incremental-unknown
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ two_file_test_main.o -Wl,--incremental-unchanged two_file_test_6.a -Wl,--incremental-unknown
 
 check_PROGRAMS += incremental_copy_test
 incremental_copy_test: copy_test_v1.o copy_test.o copy_test_1.so copy_test_2.so
 	cp -f copy_test_v1.o copy_test_tmp.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ -Wl,-R,. -Wl,--no-as-needed copy_test_tmp.o copy_test_1.so copy_test_2.so
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ -Wl,-R,. -Wl,--no-as-needed copy_test_tmp.o copy_test_1.so copy_test_2.so
 	@sleep 1
 	cp -f copy_test.o copy_test_tmp.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ -Wl,-R,. -Wl,--no-as-needed copy_test_tmp.o copy_test_1.so copy_test_2.so
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ -Wl,-R,. -Wl,--no-as-needed copy_test_tmp.o copy_test_1.so copy_test_2.so
 
 check_PROGRAMS += incremental_common_test_1
 incremental_common_test_1: common_test_1_v1.o common_test_1_v2.o gcctestdir/ld
 	cp -f common_test_1_v1.o common_test_1_tmp.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ common_test_1_tmp.o
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ common_test_1_tmp.o
 	@sleep 1
 	cp -f common_test_1_v2.o common_test_1_tmp.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ common_test_1_tmp.o
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ common_test_1_tmp.o
 
 check_PROGRAMS += incremental_comdat_test_1
 incremental_comdat_test_1: incr_comdat_test_1.o incr_comdat_test_2_v1.o incr_comdat_test_2_v2.o incr_comdat_test_2_v3.o gcctestdir/ld
 	cp -f incr_comdat_test_2_v1.o incr_comdat_test_1_tmp.o
-	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro -Bgcctestdir/ incr_comdat_test_1.o incr_comdat_test_1_tmp.o
+	$(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Wl,-z,norelro,-no-pie -Bgcctestdir/ incr_comdat_test_1.o incr_comdat_test_1_tmp.o
 	@sleep 1
 	cp -f incr_comdat_test_2_v2.o incr_comdat_test_1_tmp.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ incr_comdat_test_1.o incr_comdat_test_1_tmp.o
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ incr_comdat_test_1.o incr_comdat_test_1_tmp.o
 	@sleep 1
 	cp -f incr_comdat_test_2_v3.o incr_comdat_test_1_tmp.o
-	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro -Bgcctestdir/ incr_comdat_test_1.o incr_comdat_test_1_tmp.o
+	$(CXXLINK) -Wl,--incremental-update -Wl,-z,norelro,-no-pie -Bgcctestdir/ incr_comdat_test_1.o incr_comdat_test_1_tmp.o
 
 endif DEFAULT_TARGET_X86_64
 
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index da5087faad..c83759561f 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -3066,9 +3066,10 @@ Target_x86_64<size>::Scan::local(Symbol_table* symtab,
 	// mov foo@GOTPCREL(%rip), %reg
 	// to lea foo(%rip), %reg.
 	// in Relocate::relocate.
-	if ((r_type == elfcpp::R_X86_64_GOTPCREL
-	     || r_type == elfcpp::R_X86_64_GOTPCRELX
-	     || r_type == elfcpp::R_X86_64_REX_GOTPCRELX)
+	if (!parameters->incremental()
+	    && (r_type == elfcpp::R_X86_64_GOTPCREL
+		|| r_type == elfcpp::R_X86_64_GOTPCRELX
+		|| r_type == elfcpp::R_X86_64_REX_GOTPCRELX)
 	    && reloc.get_r_offset() >= 2
 	    && !is_ifunc)
 	  {
@@ -3079,7 +3080,6 @@ Target_x86_64<size>::Scan::local(Symbol_table* symtab,
 	      break;
 	  }
 
-
 	// The symbol requires a GOT entry.
 	unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
 
@@ -3546,15 +3546,21 @@ Target_x86_64<size>::Scan::global(Symbol_table* symtab,
 	// (callq|jmpq) *foo@GOTPCRELX(%rip) to
 	// (callq|jmpq) foo
 	// in Relocate::relocate, then there is nothing to do here.
+	// We cannot make these optimizations in incremental linking mode,
+	// because we look at the opcode to decide whether or not to make
+	// change, and during an incremental update, the change may have
+	// already been applied.
 
         Lazy_view<size> view(object, data_shndx);
         size_t r_offset = reloc.get_r_offset();
-        if (r_offset >= 2
+        if (!parameters->incremental()
+	    && r_offset >= 2
             && Target_x86_64<size>::can_convert_mov_to_lea(gsym, r_type,
                                                            r_offset, &view))
           break;
 
-	if (r_offset >= 2
+	if (!parameters->incremental()
+	    && r_offset >= 2
 	    && Target_x86_64<size>::can_convert_callq_to_direct(gsym, r_type,
 								r_offset,
 								&view))
@@ -4243,14 +4249,15 @@ Target_x86_64<size>::Relocate::relocate(
       // mov foo@GOTPCREL(%rip), %reg
       // to lea foo(%rip), %reg.
       // if possible.
-       if ((gsym == NULL
-             && rela.get_r_offset() >= 2
-             && view[-2] == 0x8b
-             && !psymval->is_ifunc_symbol())
-            || (gsym != NULL
-                && rela.get_r_offset() >= 2
-                && Target_x86_64<size>::can_convert_mov_to_lea(gsym, r_type,
-                                                               0, &view)))
+      if (!parameters->incremental()
+	  && ((gsym == NULL
+	       && rela.get_r_offset() >= 2
+	       && view[-2] == 0x8b
+	       && !psymval->is_ifunc_symbol())
+	      || (gsym != NULL
+		  && rela.get_r_offset() >= 2
+		  && Target_x86_64<size>::can_convert_mov_to_lea(gsym, r_type,
+								 0, &view))))
 	{
 	  view[-2] = 0x8d;
 	  Reloc_funcs::pcrela32(view, object, psymval, addend, address);
@@ -4261,7 +4268,8 @@ Target_x86_64<size>::Relocate::relocate(
       // and jmpq *foo@GOTPCRELX(%rip) to
       // jmpq foo
       // nop
-      else if (gsym != NULL
+      else if (!parameters->incremental()
+	       && gsym != NULL
 	       && rela.get_r_offset() >= 2
 	       && Target_x86_64<size>::can_convert_callq_to_direct(gsym,
 								   r_type,

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-12-02  7:41 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-02  7:41 [gold commit] PR 21090: Disallow --incremental with -pie and force -no-pie for incremental tests 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).