* [PATCH 0/2] gold: Update ICF
@ 2020-10-11 15:06 H.J. Lu
2020-10-11 15:06 ` [PATCH 1/2] Gold: Skip zero-sized sections for ICF H.J. Lu
2020-10-11 15:06 ` [PATCH 2/2] Gold: Enable safe ICF for shared object on x86-64 H.J. Lu
0 siblings, 2 replies; 7+ messages in thread
From: H.J. Lu @ 2020-10-11 15:06 UTC (permalink / raw)
To: binutils
There is no need to do ICF on zero-sized sections and we can do ICF for
shared object on x86-64. I will check in them next Tuesday if there are
no objections.
H.J. Lu (2):
Gold: Skip zero-sized sections for ICF
Gold: Enable safe ICF for shared object on x86-64
gold/icf.cc | 2 ++
gold/testsuite/icf_safe_so_test.cc | 8 ++++++++
gold/testsuite/icf_safe_so_test.sh | 2 +-
gold/x86_64.cc | 18 ++----------------
4 files changed, 13 insertions(+), 17 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] Gold: Skip zero-sized sections for ICF
2020-10-11 15:06 [PATCH 0/2] gold: Update ICF H.J. Lu
@ 2020-10-11 15:06 ` H.J. Lu
2020-10-12 0:02 ` Fangrui Song
2020-10-11 15:06 ` [PATCH 2/2] Gold: Enable safe ICF for shared object on x86-64 H.J. Lu
1 sibling, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2020-10-11 15:06 UTC (permalink / raw)
To: binutils
Skip zero-sized sections since there is no need to do ICF on them.
* icf.cc (Icf::find_identical_sections): Skip zero-sized sections.
---
gold/icf.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gold/icf.cc b/gold/icf.cc
index a60db7abc8d..54af4126696 100644
--- a/gold/icf.cc
+++ b/gold/icf.cc
@@ -973,6 +973,8 @@ Icf::find_identical_sections(const Input_objects* input_objects,
for (unsigned int i = 0; i < (*p)->shnum(); ++i)
{
+ if ((*p)->section_size(i) == 0)
+ continue;
const std::string section_name = (*p)->section_name(i);
if (!is_section_foldable_candidate(section_name))
{
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] Gold: Enable safe ICF for shared object on x86-64
2020-10-11 15:06 [PATCH 0/2] gold: Update ICF H.J. Lu
2020-10-11 15:06 ` [PATCH 1/2] Gold: Skip zero-sized sections for ICF H.J. Lu
@ 2020-10-11 15:06 ` H.J. Lu
2020-10-12 0:06 ` Fangrui Song
1 sibling, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2020-10-11 15:06 UTC (permalink / raw)
To: binutils
With
commit 4aebb6312eb5dcd12f2f8420028547584b708907
Author: Rahul Chaudhry <rahulchaudhry@google.com>
Date: Wed Feb 15 00:37:10 2017 -0800
Improved support for --icf=safe when used with -pie.
we now check opcode with R_X86_64_PC32 relocation, which tell branches
from other instructions. We can enable safe ICF for shared object on
x86-64. Also, global symbols with non-default visibility should be
folded like local symbols.
PR gold/21452
* x86_64.cc (Scan::local_reloc_may_be_function_pointer): Remove
check for shared library.
(Scan::global_reloc_may_be_function_pointer): Remove check for
shared library and symbol visibility.
* testsuite/icf_safe_so_test.cc (bar_static): New function.
(main): Take function address of bar_static and use it.
* testsuite/icf_safe_so_test.sh (arch_specific_safe_fold): Also
check fold on x86-64. Check bar_static isn't folded.
---
gold/testsuite/icf_safe_so_test.cc | 8 ++++++++
gold/testsuite/icf_safe_so_test.sh | 2 +-
gold/x86_64.cc | 18 ++----------------
3 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/gold/testsuite/icf_safe_so_test.cc b/gold/testsuite/icf_safe_so_test.cc
index 1c593031d05..32566553276 100644
--- a/gold/testsuite/icf_safe_so_test.cc
+++ b/gold/testsuite/icf_safe_so_test.cc
@@ -61,10 +61,18 @@ int bar_glob()
return 2;
}
+static int
+bar_static()
+{
+ return 2;
+}
+
int main()
{
int (*p)() = foo_glob;
(void)p;
+ p = bar_static;
+ (void)p;
foo_static();
foo_prot();
foo_hidden();
diff --git a/gold/testsuite/icf_safe_so_test.sh b/gold/testsuite/icf_safe_so_test.sh
index 10f8782d1f5..4c253c55d20 100755
--- a/gold/testsuite/icf_safe_so_test.sh
+++ b/gold/testsuite/icf_safe_so_test.sh
@@ -83,7 +83,7 @@ END {
arch_specific_safe_fold()
{
- if grep -q -e "Intel 80386" -e "ARM" -e "PowerPC" $1;
+ if grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" -e "ARM" -e "PowerPC" $1;
then
shift
shift
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index bacd89f2eff..378bac16f78 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -4022,12 +4022,6 @@ Target_x86_64<size>::Scan::local_reloc_may_be_function_pointer(
unsigned int r_type,
const elfcpp::Sym<size, false>&)
{
- // When building a shared library, do not fold any local symbols as it is
- // not possible to distinguish pointer taken versus a call by looking at
- // the relocation types.
- if (parameters->options().shared())
- return true;
-
return possible_function_pointer_reloc(src_obj, src_indx,
reloc.get_r_offset(), r_type);
}
@@ -4047,16 +4041,8 @@ Target_x86_64<size>::Scan::global_reloc_may_be_function_pointer(
Output_section* ,
const elfcpp::Rela<size, false>& reloc,
unsigned int r_type,
- Symbol* gsym)
-{
- // When building a shared library, do not fold symbols whose visibility
- // is hidden, internal or protected.
- if (parameters->options().shared()
- && (gsym->visibility() == elfcpp::STV_INTERNAL
- || gsym->visibility() == elfcpp::STV_PROTECTED
- || gsym->visibility() == elfcpp::STV_HIDDEN))
- return true;
-
+ Symbol*)
+{
return possible_function_pointer_reloc(src_obj, src_indx,
reloc.get_r_offset(), r_type);
}
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Gold: Skip zero-sized sections for ICF
2020-10-11 15:06 ` [PATCH 1/2] Gold: Skip zero-sized sections for ICF H.J. Lu
@ 2020-10-12 0:02 ` Fangrui Song
2020-10-12 1:56 ` H.J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Fangrui Song @ 2020-10-12 0:02 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On 2020-10-11, H.J. Lu via Binutils wrote:
>Skip zero-sized sections since there is no need to do ICF on them.
>
> * icf.cc (Icf::find_identical_sections): Skip zero-sized sections.
>---
> gold/icf.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/gold/icf.cc b/gold/icf.cc
>index a60db7abc8d..54af4126696 100644
>--- a/gold/icf.cc
>+++ b/gold/icf.cc
>@@ -973,6 +973,8 @@ Icf::find_identical_sections(const Input_objects* input_objects,
>
> for (unsigned int i = 0; i < (*p)->shnum(); ++i)
> {
>+ if ((*p)->section_size(i) == 0)
>+ continue;
> const std::string section_name = (*p)->section_name(i);
> if (!is_section_foldable_candidate(section_name))
> {
>--
>2.26.2
>
Does anything break without the special case? ld.lld --icf={all,safe} does not
special case st_size=0
.section .text.1,"ax"; .globl foo; foo:
.section .text.2,"ax"; .globl bar; bar:
% ld.lld a.o --icf=all --print-icf-sections
selected section a.o:(.text)
removing identical section a.o:(.text.1)
removing identical section a.o:(.text.2)
ld.lld: warning: cannot find entry symbol _start; defaulting to 0x201158
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Gold: Enable safe ICF for shared object on x86-64
2020-10-11 15:06 ` [PATCH 2/2] Gold: Enable safe ICF for shared object on x86-64 H.J. Lu
@ 2020-10-12 0:06 ` Fangrui Song
0 siblings, 0 replies; 7+ messages in thread
From: Fangrui Song @ 2020-10-12 0:06 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On 2020-10-11, H.J. Lu via Binutils wrote:
>With
>
>commit 4aebb6312eb5dcd12f2f8420028547584b708907
>Author: Rahul Chaudhry <rahulchaudhry@google.com>
>Date: Wed Feb 15 00:37:10 2017 -0800
>
> Improved support for --icf=safe when used with -pie.
>
>we now check opcode with R_X86_64_PC32 relocation, which tell branches
>from other instructions. We can enable safe ICF for shared object on
>x86-64. Also, global symbols with non-default visibility should be
>folded like local symbols.
>
> PR gold/21452
> * x86_64.cc (Scan::local_reloc_may_be_function_pointer): Remove
> check for shared library.
> (Scan::global_reloc_may_be_function_pointer): Remove check for
> shared library and symbol visibility.
> * testsuite/icf_safe_so_test.cc (bar_static): New function.
> (main): Take function address of bar_static and use it.
> * testsuite/icf_safe_so_test.sh (arch_specific_safe_fold): Also
> check fold on x86-64. Check bar_static isn't folded.
>---
> gold/testsuite/icf_safe_so_test.cc | 8 ++++++++
> gold/testsuite/icf_safe_so_test.sh | 2 +-
> gold/x86_64.cc | 18 ++----------------
> 3 files changed, 11 insertions(+), 17 deletions(-)
>
>diff --git a/gold/testsuite/icf_safe_so_test.cc b/gold/testsuite/icf_safe_so_test.cc
>index 1c593031d05..32566553276 100644
>--- a/gold/testsuite/icf_safe_so_test.cc
>+++ b/gold/testsuite/icf_safe_so_test.cc
>@@ -61,10 +61,18 @@ int bar_glob()
> return 2;
> }
>
>+static int
>+bar_static()
>+{
>+ return 2;
>+}
>+
> int main()
> {
> int (*p)() = foo_glob;
> (void)p;
>+ p = bar_static;
>+ (void)p;
> foo_static();
> foo_prot();
> foo_hidden();
>diff --git a/gold/testsuite/icf_safe_so_test.sh b/gold/testsuite/icf_safe_so_test.sh
>index 10f8782d1f5..4c253c55d20 100755
>--- a/gold/testsuite/icf_safe_so_test.sh
>+++ b/gold/testsuite/icf_safe_so_test.sh
>@@ -83,7 +83,7 @@ END {
>
> arch_specific_safe_fold()
> {
>- if grep -q -e "Intel 80386" -e "ARM" -e "PowerPC" $1;
>+ if grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" -e "ARM" -e "PowerPC" $1;
> then
> shift
> shift
>diff --git a/gold/x86_64.cc b/gold/x86_64.cc
>index bacd89f2eff..378bac16f78 100644
>--- a/gold/x86_64.cc
>+++ b/gold/x86_64.cc
>@@ -4022,12 +4022,6 @@ Target_x86_64<size>::Scan::local_reloc_may_be_function_pointer(
> unsigned int r_type,
> const elfcpp::Sym<size, false>&)
> {
>- // When building a shared library, do not fold any local symbols as it is
>- // not possible to distinguish pointer taken versus a call by looking at
>- // the relocation types.
>- if (parameters->options().shared())
>- return true;
>-
> return possible_function_pointer_reloc(src_obj, src_indx,
> reloc.get_r_offset(), r_type);
> }
>@@ -4047,16 +4041,8 @@ Target_x86_64<size>::Scan::global_reloc_may_be_function_pointer(
> Output_section* ,
> const elfcpp::Rela<size, false>& reloc,
> unsigned int r_type,
>- Symbol* gsym)
>-{
>- // When building a shared library, do not fold symbols whose visibility
>- // is hidden, internal or protected.
>- if (parameters->options().shared()
>- && (gsym->visibility() == elfcpp::STV_INTERNAL
>- || gsym->visibility() == elfcpp::STV_PROTECTED
>- || gsym->visibility() == elfcpp::STV_HIDDEN))
>- return true;
>-
>+ Symbol*)
>+{
> return possible_function_pointer_reloc(src_obj, src_indx,
> reloc.get_r_offset(), r_type);
> }
>--
>2.26.2
>
Thanks for dropping special cases! The -shared special cases are
questionable.
I think "Improved support for --icf=safe when used with -pie." is not
needed nowadays since function calls use R_X86_64_PLT32.
(Solaris does not like R_X86_64_PLT32 and as a result they will not get
ICF benefits.)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Gold: Skip zero-sized sections for ICF
2020-10-12 0:02 ` Fangrui Song
@ 2020-10-12 1:56 ` H.J. Lu
2020-10-13 16:49 ` Fangrui Song
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2020-10-12 1:56 UTC (permalink / raw)
To: Fangrui Song; +Cc: Binutils
On Sun, Oct 11, 2020 at 5:02 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2020-10-11, H.J. Lu via Binutils wrote:
> >Skip zero-sized sections since there is no need to do ICF on them.
> >
> > * icf.cc (Icf::find_identical_sections): Skip zero-sized sections.
> >---
> > gold/icf.cc | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/gold/icf.cc b/gold/icf.cc
> >index a60db7abc8d..54af4126696 100644
> >--- a/gold/icf.cc
> >+++ b/gold/icf.cc
> >@@ -973,6 +973,8 @@ Icf::find_identical_sections(const Input_objects* input_objects,
> >
> > for (unsigned int i = 0; i < (*p)->shnum(); ++i)
> > {
> >+ if ((*p)->section_size(i) == 0)
> >+ continue;
> > const std::string section_name = (*p)->section_name(i);
> > if (!is_section_foldable_candidate(section_name))
> > {
> >--
> >2.26.2
> >
>
> Does anything break without the special case? ld.lld --icf={all,safe} does not
> special case st_size=0
>
>
> .section .text.1,"ax"; .globl foo; foo:
> .section .text.2,"ax"; .globl bar; bar:
>
> % ld.lld a.o --icf=all --print-icf-sections
> selected section a.o:(.text)
> removing identical section a.o:(.text.1)
> removing identical section a.o:(.text.2)
> ld.lld: warning: cannot find entry symbol _start; defaulting to 0x201158
The zero-sized input functions should have no impact on output,
ICF or not.
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Gold: Skip zero-sized sections for ICF
2020-10-12 1:56 ` H.J. Lu
@ 2020-10-13 16:49 ` Fangrui Song
0 siblings, 0 replies; 7+ messages in thread
From: Fangrui Song @ 2020-10-13 16:49 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On 2020-10-11, H.J. Lu wrote:
>On Sun, Oct 11, 2020 at 5:02 PM Fangrui Song <i@maskray.me> wrote:
>>
>> On 2020-10-11, H.J. Lu via Binutils wrote:
>> >Skip zero-sized sections since there is no need to do ICF on them.
>> >
>> > * icf.cc (Icf::find_identical_sections): Skip zero-sized sections.
>> >---
>> > gold/icf.cc | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> >diff --git a/gold/icf.cc b/gold/icf.cc
>> >index a60db7abc8d..54af4126696 100644
>> >--- a/gold/icf.cc
>> >+++ b/gold/icf.cc
>> >@@ -973,6 +973,8 @@ Icf::find_identical_sections(const Input_objects* input_objects,
>> >
>> > for (unsigned int i = 0; i < (*p)->shnum(); ++i)
>> > {
>> >+ if ((*p)->section_size(i) == 0)
>> >+ continue;
>> > const std::string section_name = (*p)->section_name(i);
>> > if (!is_section_foldable_candidate(section_name))
>> > {
>> >--
>> >2.26.2
>> >
>>
>> Does anything break without the special case? ld.lld --icf={all,safe} does not
>> special case st_size=0
>>
>>
>> .section .text.1,"ax"; .globl foo; foo:
>> .section .text.2,"ax"; .globl bar; bar:
>>
>> % ld.lld a.o --icf=all --print-icf-sections
>> selected section a.o:(.text)
>> removing identical section a.o:(.text.1)
>> removing identical section a.o:(.text.2)
>> ld.lld: warning: cannot find entry symbol _start; defaulting to 0x201158
>
>The zero-sized input functions should have no impact on output,
>ICF or not.
Folding a zero-sized section has no benefit, so we can either do or not
do it. Since not doing it adds 2 lines of code, I'd rather we do it.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-10-13 16:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-11 15:06 [PATCH 0/2] gold: Update ICF H.J. Lu
2020-10-11 15:06 ` [PATCH 1/2] Gold: Skip zero-sized sections for ICF H.J. Lu
2020-10-12 0:02 ` Fangrui Song
2020-10-12 1:56 ` H.J. Lu
2020-10-13 16:49 ` Fangrui Song
2020-10-11 15:06 ` [PATCH 2/2] Gold: Enable safe ICF for shared object on x86-64 H.J. Lu
2020-10-12 0:06 ` Fangrui Song
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).