[PATCH] PR gold/17704

Sriraman Tallam tmsriram@google.com
Thu Jul 14 18:48:00 GMT 2016


Hi,

   This is a patch for PR 17704, which is identical to the patch
proposed by ngg@ here:
https://sourceware.org/bugzilla/attachment.cgi?id=8906&action=diff&collapsed=&headers=1&format=raw

Please take a look.

PR gold/17704
* icf.cc (match_sections): Add new parameter section_addraligns.
Check section alignment and keep the section with the strictest
alignment.
(find_identical_sections): New local variable section_addraligns.
Store each section's alignment.
* testsuite/pr17704a_test.S: New file.
* testsuite/Makefile.am (pr17704a_test): New test.
* testsuite/Makefile.in: Regenerate.

Thanks
Sri
-------------- next part --------------
	PR gold/17704
	* icf.cc (match_sections): Add new parameter section_addraligns.
	Check section alignment and keep the section with the strictest
	alignment.
	(find_identical_sections): New local variable section_addraligns.
	Store each section's alignment.
	* testsuite/pr17704a_test.S: New file.
	* testsuite/Makefile.am (pr17704a_test): New test.
	* testsuite/Makefile.in: Regenerate.
	

diff --git a/gold/icf.cc b/gold/icf.cc
--- a/gold/icf.cc
+++ b/gold/icf.cc
@@ -590,6 +590,7 @@ match_sections(unsigned int iteration_num,
                std::vector<unsigned int>* num_tracked_relocs,
                std::vector<unsigned int>* kept_section_id,
                const std::vector<Section_id>& id_section,
+	       const std::vector<uint64_t>& section_addraligns,
                std::vector<bool>* is_secn_or_group_unique,
                std::vector<std::string>* section_contents)
 {
@@ -598,6 +599,7 @@ match_sections(unsigned int iteration_num,
             Unordered_multimap<uint32_t, unsigned int>::iterator> key_range;
   bool converged = true;
 
+  (void)section_addraligns;
   if (iteration_num == 1)
     preprocess_for_unique_sections(id_section,
                                    is_secn_or_group_unique,
@@ -630,13 +632,7 @@ match_sections(unsigned int iteration_num,
         {
           if ((*kept_section_id)[i] != i)
             {
-              // This section is already folded into something.  See
-              // if it should point to a different kept section.
-              unsigned int kept_section = (*kept_section_id)[i];
-              if (kept_section != (*kept_section_id)[kept_section])
-                {
-                  (*kept_section_id)[i] = (*kept_section_id)[kept_section];
-                }
+              // This section is already folded into something.
               continue;
             }
           this_secn_contents = get_section_contents(false, secn, i, NULL,
@@ -671,7 +667,31 @@ match_sections(unsigned int iteration_num,
                          this_secn_contents.c_str(),
                          this_secn_contents.length()) != 0)
                   continue;
-              (*kept_section_id)[i] = kept_section;
+
+	      // Check section alignment here.
+              // The section with the larger alignment requirement
+              // should be kept. Do not fold if the larger is not
+              // divisible by the smaller, but that case will never
+              // happen if the alignments are powers of 2.
+              uint64_t align_i = section_addraligns[i];
+              uint64_t align_kept = section_addraligns[kept_section];
+              if (align_i <= align_kept)
+                {
+                  if (align_i != align_kept && align_i != 0
+		      && (align_kept % align_i) != 0)
+                      continue;
+                  (*kept_section_id)[i] = kept_section;
+                }
+              else
+                {
+                  if (align_kept != 0 && (align_i % align_kept) != 0)
+                      continue;
+                  (*kept_section_id)[kept_section] = i;
+                  it->second = i;
+                  full_section_contents[kept_section].swap(
+                      full_section_contents[i]);
+                }
+
               converged = false;
               break;
             }
@@ -688,6 +708,26 @@ match_sections(unsigned int iteration_num,
         (*is_secn_or_group_unique)[i] = true;
     }
 
+  // If a section was folded into another section that was later folded
+  // again then the former has to be updated.
+  for (unsigned int i = 0; i < id_section.size(); i++)
+    {
+      // Find the end of the folding chain
+      unsigned int kept = i;
+      while ((*kept_section_id)[kept] != kept)
+        {
+          kept = (*kept_section_id)[kept];
+        }
+      // Update every element of the chain
+      unsigned int current = i;
+      while ((*kept_section_id)[current] != kept)
+        {
+          unsigned int next = (*kept_section_id)[current];
+          (*kept_section_id)[current] = kept;
+          current = next;
+        }
+    }
+
   return converged;
 }
 
@@ -719,6 +759,7 @@ Icf::find_identical_sections(const Input_objects* input_objects,
 {
   unsigned int section_num = 0;
   std::vector<unsigned int> num_tracked_relocs;
+  std::vector<uint64_t> section_addraligns;
   std::vector<bool> is_secn_or_group_unique;
   std::vector<std::string> section_contents;
   const Target& target = parameters->target();
@@ -759,6 +800,7 @@ Icf::find_identical_sections(const Input_objects* input_objects,
           this->section_id_[Section_id(*p, i)] = section_num;
           this->kept_section_id_.push_back(section_num);
           num_tracked_relocs.push_back(0);
+	  section_addraligns.push_back((*p)->section_addralign(i));
           is_secn_or_group_unique.push_back(false);
           section_contents.push_back("");
           section_num++;
@@ -779,8 +821,8 @@ Icf::find_identical_sections(const Input_objects* input_objects,
       num_iterations++;
       converged = match_sections(num_iterations, symtab,
                                  &num_tracked_relocs, &this->kept_section_id_,
-                                 this->id_section_, &is_secn_or_group_unique,
-                                 &section_contents);
+                                 this->id_section_, section_addraligns,
+				 &is_secn_or_group_unique, &section_contents);
     }
 
   if (parameters->options().print_icf_sections())
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index c8d3093..7b9b267 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -355,6 +355,12 @@ icf_sht_rel_addend_test: icf_sht_rel_addend_test_1.o icf_sht_rel_addend_test_2.o
 icf_sht_rel_addend_test.stdout: icf_sht_rel_addend_test
 	$(TEST_NM) icf_sht_rel_addend_test > icf_sht_rel_addend_test.stdout
 
+check_PROGRAMS += pr17704a_test
+pr17704a_test.o: pr17704a_test.s
+	$(TEST_AS) --64  -o $@ $<
+pr17704a_test: pr17704a_test.o gcctestdir/ld
+	gcctestdir/ld --icf=all -o $@ $<
+
 check_PROGRAMS += large_symbol_alignment
 large_symbol_alignment_SOURCES = large_symbol_alignment.cc
 large_symbol_alignment_DEPENDENCIES = gcctestdir/ld
diff --git a/gold/testsuite/pr17704a_test.s b/gold/testsuite/pr17704a_test.s
index e69de29..2b39e64 100644
--- a/gold/testsuite/pr17704a_test.s
+++ b/gold/testsuite/pr17704a_test.s
@@ -0,0 +1,23 @@
+nop
+       .section        .text.foo,"axG",@progbits,foo,comdat
+foo:
+       ret
+
+       .section        .text.bar,"axG",@progbits,bar,comdat
+       .align 2
+bar:
+       ret
+
+       .section        .text._start,"ax",@progbits
+       .globl  _start
+_start:
+       leaq    bar(%rip), %rsi
+       testb   $1, %sil
+       je      .L9
+       mov $1, %eax
+       mov $1, %ebx
+       int $0x80
+.L9:
+       mov $1, %eax
+       mov $0, %ebx
+       int $0x80


More information about the Binutils mailing list