* [PATCH] PR gold/17704
@ 2016-07-14 18:48 Sriraman Tallam
2016-07-14 18:50 ` Sriraman Tallam
2016-07-18 5:51 ` Alan Modra
0 siblings, 2 replies; 12+ messages in thread
From: Sriraman Tallam @ 2016-07-14 18:48 UTC (permalink / raw)
To: binutils, ngg, Cary Coutant; +Cc: David Li, Roland McGrath
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
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
[-- Attachment #2: icf_section_alignment.txt --]
[-- Type: text/plain, Size: 6894 bytes --]
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,
- §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..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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-14 18:48 [PATCH] PR gold/17704 Sriraman Tallam
@ 2016-07-14 18:50 ` Sriraman Tallam
2016-07-15 20:54 ` Roland McGrath
2016-07-18 5:51 ` Alan Modra
1 sibling, 1 reply; 12+ messages in thread
From: Sriraman Tallam @ 2016-07-14 18:50 UTC (permalink / raw)
To: binutils, Cary Coutant, ngg; +Cc: David Li, Roland McGrath
+ngg@tresorit.com
On Thu, Jul 14, 2016 at 11:48 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-14 18:50 ` Sriraman Tallam
@ 2016-07-15 20:54 ` Roland McGrath
2016-07-15 23:35 ` Roland McGrath
0 siblings, 1 reply; 12+ messages in thread
From: Roland McGrath @ 2016-07-15 20:54 UTC (permalink / raw)
To: Sriraman Tallam; +Cc: binutils, Cary Coutant, ngg, David Li
I don't know the implementation well enough to review this patch. But
the logic of choosing among identical sections the one with the
largest sh_addralign value is clearly correct and fixes the problem.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-15 20:54 ` Roland McGrath
@ 2016-07-15 23:35 ` Roland McGrath
2016-07-18 8:20 ` Tristan Gingold
0 siblings, 1 reply; 12+ messages in thread
From: Roland McGrath @ 2016-07-15 23:35 UTC (permalink / raw)
To: Sriraman Tallam, Tristan Gingold; +Cc: binutils, Cary Coutant, ngg, David Li
Can we please make sure this fix gets into 2.27? It would be nice if
the initial 2.27 release were free of known gold --icf bugs. (This is
the second one I'm aware of that's caused problems in the Chromium
build. The other one, gold/19047, was already fixed on trunk and 2.26
branch.)
Thanks,
Roland
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-14 18:48 [PATCH] PR gold/17704 Sriraman Tallam
2016-07-14 18:50 ` Sriraman Tallam
@ 2016-07-18 5:51 ` Alan Modra
2016-07-21 18:11 ` Sriraman Tallam
1 sibling, 1 reply; 12+ messages in thread
From: Alan Modra @ 2016-07-18 5:51 UTC (permalink / raw)
To: Sriraman Tallam; +Cc: binutils, ngg, Cary Coutant, David Li, Roland McGrath
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.
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-15 23:35 ` Roland McGrath
@ 2016-07-18 8:20 ` Tristan Gingold
0 siblings, 0 replies; 12+ messages in thread
From: Tristan Gingold @ 2016-07-18 8:20 UTC (permalink / raw)
To: Roland McGrath; +Cc: Sriraman Tallam, binutils, Cary Coutant, ngg, David Li
> On 16 Jul 2016, at 01:35, Roland McGrath <mcgrathr@google.com> wrote:
>
> Can we please make sure this fix gets into 2.27? It would be nice if
> the initial 2.27 release were free of known gold --icf bugs. (This is
> the second one I'm aware of that's caused problems in the Chromium
> build. The other one, gold/19047, was already fixed on trunk and 2.26
> branch.)
That's fine with me, but I'd like to do the release before the end of month.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-18 5:51 ` Alan Modra
@ 2016-07-21 18:11 ` Sriraman Tallam
2016-08-01 16:56 ` Sriraman Tallam
0 siblings, 1 reply; 12+ messages in thread
From: Sriraman Tallam @ 2016-07-21 18:11 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils, ngg, Cary Coutant, David Li, Roland McGrath
[-- Attachment #1: Type: text/plain, Size: 3451 bytes --]
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.
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
[-- Attachment #2: icf_section_alignment.txt --]
[-- Type: text/plain, Size: 6204 bytes --]
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-07-21 18:11 ` Sriraman Tallam
@ 2016-08-01 16:56 ` Sriraman Tallam
2016-08-04 5:50 ` Cary Coutant
2016-08-12 15:33 ` Cary Coutant
0 siblings, 2 replies; 12+ messages in thread
From: Sriraman Tallam @ 2016-08-01 16:56 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils, Cary Coutant, David Li, Roland McGrath, ngg
[-- Attachment #1: Type: text/plain, Size: 3703 bytes --]
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
[-- Attachment #2: icf_section_alignment.txt --]
[-- Type: text/plain, Size: 6204 bytes --]
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-08-01 16:56 ` Sriraman Tallam
@ 2016-08-04 5:50 ` Cary Coutant
2016-08-12 15:33 ` Cary Coutant
1 sibling, 0 replies; 12+ messages in thread
From: Cary Coutant @ 2016-08-04 5:50 UTC (permalink / raw)
To: Sriraman Tallam; +Cc: Alan Modra, binutils, David Li, Roland McGrath, ngg
Hi,
I'm back from vacation, and am trying to catch up now.
-cary
On Mon, Aug 1, 2016 at 9:56 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-08-01 16:56 ` Sriraman Tallam
2016-08-04 5:50 ` Cary Coutant
@ 2016-08-12 15:33 ` Cary Coutant
2016-08-18 19:14 ` Sriraman Tallam
1 sibling, 1 reply; 12+ messages in thread
From: Cary Coutant @ 2016-08-12 15:33 UTC (permalink / raw)
To: Sriraman Tallam; +Cc: Alan Modra, binutils, David Li, Roland McGrath, ngg
>> 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.
+ (void)section_addraligns;
This shouldn't be necessary, but if it really is, I'd prefer to use
ATTRIBUTE_UNUSED on the parameter.
Also, I get this when trying to apply the patch:
patch: **** malformed patch at line 146: diff --git
a/gold/testsuite/pr17704a_test.s b/gold/testsuite/pr17704a_test.s
I think you may have deleted too much when deleting the hunks for
testsuite/Makefile.in.
-cary
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-08-12 15:33 ` Cary Coutant
@ 2016-08-18 19:14 ` Sriraman Tallam
2016-08-23 22:04 ` Cary Coutant
0 siblings, 1 reply; 12+ messages in thread
From: Sriraman Tallam @ 2016-08-18 19:14 UTC (permalink / raw)
To: Cary Coutant
Cc: Alan Modra, binutils, David Li, Roland McGrath, ngg, Luis Lozano
[-- Attachment #1: Type: text/plain, Size: 993 bytes --]
On Fri, Aug 12, 2016 at 8:33 AM, Cary Coutant <ccoutant@gmail.com> wrote:
>>> 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.
>
> + (void)section_addraligns;
>
> This shouldn't be necessary, but if it really is, I'd prefer to use
> ATTRIBUTE_UNUSED on the parameter.
>
> Also, I get this when trying to apply the patch:
>
> patch: **** malformed patch at line 146: diff --git
> a/gold/testsuite/pr17704a_test.s b/gold/testsuite/pr17704a_test.s
>
> I think you may have deleted too much when deleting the hunks for
> testsuite/Makefile.in.
Sorry for the late response, fixed both problems. Patch attached.
Thanks
Sri
>
> -cary
[-- Attachment #2: icf_section_alignment.txt --]
[-- Type: text/plain, Size: 5818 bytes --]
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..597e19a 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)
{
@@ -630,13 +631,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 +666,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 +701,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 +752,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 +793,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 +814,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..8b8fa81 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
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] PR gold/17704
2016-08-18 19:14 ` Sriraman Tallam
@ 2016-08-23 22:04 ` Cary Coutant
0 siblings, 0 replies; 12+ messages in thread
From: Cary Coutant @ 2016-08-23 22:04 UTC (permalink / raw)
To: Sriraman Tallam
Cc: Alan Modra, binutils, David Li, Roland McGrath, ngg, Luis Lozano
The only problem I have now is one of attribution and copyright
assignment. I think the patch is substantial enough to require
copyright assignment from the original author, but I don't know who
"ngg@tresorit.com" is, and I can't find a copyright assignment on file
for either that email or Tresorit as a company.
NGG, can you identify yourself and let me know if you have a copyright
assignment filed?
-cary
On Thu, Aug 18, 2016 at 12:13 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Fri, Aug 12, 2016 at 8:33 AM, Cary Coutant <ccoutant@gmail.com> wrote:
>>>> 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.
>>
>> + (void)section_addraligns;
>>
>> This shouldn't be necessary, but if it really is, I'd prefer to use
>> ATTRIBUTE_UNUSED on the parameter.
>>
>> Also, I get this when trying to apply the patch:
>>
>> patch: **** malformed patch at line 146: diff --git
>> a/gold/testsuite/pr17704a_test.s b/gold/testsuite/pr17704a_test.s
>>
>> I think you may have deleted too much when deleting the hunks for
>> testsuite/Makefile.in.
>
> Sorry for the late response, fixed both problems. Patch attached.
>
> Thanks
> Sri
>
>>
>> -cary
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-08-23 22:04 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-14 18:48 [PATCH] PR gold/17704 Sriraman Tallam
2016-07-14 18:50 ` Sriraman Tallam
2016-07-15 20:54 ` Roland McGrath
2016-07-15 23:35 ` Roland McGrath
2016-07-18 8:20 ` Tristan Gingold
2016-07-18 5:51 ` Alan Modra
2016-07-21 18:11 ` Sriraman Tallam
2016-08-01 16:56 ` Sriraman Tallam
2016-08-04 5:50 ` Cary Coutant
2016-08-12 15:33 ` Cary Coutant
2016-08-18 19:14 ` Sriraman Tallam
2016-08-23 22:04 ` 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).