[PATCH] PR gold/17704
Sriraman Tallam
tmsriram@google.com
Mon Aug 1 16:56:00 GMT 2016
On Thu, Jul 21, 2016 at 11:11 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Sun, Jul 17, 2016 at 10:50 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Thu, Jul 14, 2016 at 11:48:39AM -0700, Sriraman Tallam wrote:
>>> @@ -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;
>>> }
>>
>> The gabi says of sh_addralign "Currently, only 0 and positive integral
>> powers of two are allowed", so I think you can discount the
>> possibility of alignment not a power of two. It's not worth
>> supporting here when other parts of gold do not, eg. see layout.cc
>> calls to align_address.
>>
>> Please also fix indentation, only some of the above lines use tabs.
>
> New patch attached with the changes.
>
> 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.
Hi Cary,
Ping, in case you didnt take a look at this.
Thanks
Sri
>
> Thanks
> Sri
>
>>
>>> 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
>>
>> The new test contains x86 assembly, but is enabled for all native
>> targets using gcc. Please move to a more appropriate part of the
>> makefile.
>>
>> --
>> Alan Modra
>> Australia Development Lab, IBM
-------------- 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
index dce0b8b..01958e5 100644
--- 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,25 @@ 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. We assume alignment can only be
+ // zero or postive integral powers of two.
+ uint64_t align_i = section_addraligns[i];
+ uint64_t align_kept = section_addraligns[kept_section];
+ if (align_i <= align_kept)
+ {
+ (*kept_section_id)[i] = kept_section;
+ }
+ else
+ {
+ (*kept_section_id)[kept_section] = i;
+ it->second = i;
+ full_section_contents[kept_section].swap(
+ full_section_contents[i]);
+ }
+
converged = false;
break;
}
@@ -688,6 +702,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 +753,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 +794,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 +815,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,
- §ion_contents);
+ this->id_section_, section_addraligns,
+ &is_secn_or_group_unique, §ion_contents);
}
if (parameters->options().print_icf_sections())
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index c8d3093..ecb7e2c 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1131,6 +1131,12 @@ x86_64_overflow_pc32.err: x86_64_overflow_pc32.o gcctestdir/ld
exit 1; \
fi
+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_SCRIPTS += x32_overflow_pc32.sh
check_DATA += x32_overflow_pc32.err
MOSTLYCLEANFILES += x32_overflow_pc32.err
@@ -1149,40 +1155,40 @@ endif DEFAULT_TARGET_X86_64
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