Safe Identical Code Folding for X86-64.
Sriraman Tallam
tmsriram@google.com
Fri Feb 12 23:46:00 GMT 2010
Hi Ian,
Thanks for the review. I have made all the changes. Please take
another look.
2010-02-12 Sriraman Tallam <tmsriram@google.com>
* arm.cc (Scan::local_for_icf): New function.
(Scan::global_for_icf): New function.
* sparc.cc (Scan::local_for_icf): New function.
(Scan::global_for_icf): New function.
* powerpc.cc (Scan::local_for_icf): New function.
(Scan::global_for_icf): New function.
* i386.cc (Scan::local_for_icf): New function.
(Scan::global_for_icf): New function.
* x86_64.cc (Scan::local_for_icf): New function.
(Scan::global_for_icf): New function.
(Scan::possible_function_pointer_reloc): New function.
(Target_x86_64::can_check_for_function_pointers): New function.
* gc.h (gc_process_relocs): Scan relocation types to determine if
function pointers were taken for targets that support it.
* icf.cc (Icf::find_identical_sections): Include functions for
folding in safe ICF whose pointer is not taken.
* icf.h (Secn_fptr_taken_set): New typedef.
(fptr_section_id_): New member.
(section_has_function_pointers): New function.
(set_section_has_function_pointers): New function.
(check_section_for_function_pointers): New function.
* options.h: Fix comment for safe ICF option.
* target.h (can_check_for_function_pointers): New function.
* testsuite/Makefile.am: Add icf_safe_so_test test case.
Modify icf_safe_test for X86-64.
* testsuite/Makefile.in: Regenerate.
* testsuite/icf_safe_so_test.cc: New file.
* testsuite/icf_safe_so_test.sh: New file.
* testsuite/icf_safe_test.cc (kept_func_3): New function.
(main): Change to take pointer to function kept_func_3.
* testsuite/icf_safe_test.sh (arch_specific_safe_fold): Check if safe
folding is done correctly for X86-64.
Thanks,
-Sriraman.
On Thu, Feb 11, 2010 at 9:08 PM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>> 2010-01-21 Sriraman Tallam <tmsriram@google.com>
>>
>> * arm.cc (Scan::local_for_icf): New function.
>> (Scan::global_for_icf): New function.
>> * sparc.cc (Scan::local_for_icf): New function.
>> (Scan::global_for_icf): New function.
>> * powerpc.cc (Scan::local_for_icf): New function.
>> (Scan::global_for_icf): New function.
>> * i386.cc (Scan::local_for_icf): New function.
>> (Scan::global_for_icf): New function.
>> * x86_64.cc (Scan::local_for_icf): New function.
>> (Scan::global_for_icf): New function.
>> (Scan::is_non_pic_reloc_type_func_ptr): New function.
>> * gc.h (gc_process_relocs): Scan relocation types to determine if
>> function pointers were taken for targets that support it.
>
> No extra indentation here.
>
>> * icf.cc (Icf::find_identical_sections): Include functions for
>> folding in safe ICF whose pointer is not taken.
>> * icf.h (Secn_fptr_taken_set): New typedef.
>> (fptr_section_id_): New member.
>> (is_section_fptr_taken): New function.
>> (set_section_fptr_taken): New function.
>> (check_section_for_fptr_taken): New function.
>> * options.h: Fix comment for safe ICF option.
>> * target.h (is_fptr_taken_checked): New function.
>> * testsuite/Makefile.am: Add icf_safe_so_test test case.
>> Modify icf_safe_test for X86-64.
>> * testsuite/Makefile.in: Regenerate.
>> * testsuite/icf_safe_so_test.cc: New file.
>> * testsuite/icf_safe_so_test.sh: New file.
>> * testsuite/icf_safe_test.cc (kept_func_3): New function.
>> (main): Change to take pointer to function kept_func_3.
>> * testsuite/icf_safe_test.sh (arch_specific_safe_fold): Check if safe
>> folding is done correctly for X86-64.
>
>> Index: arm.cc
>> ===================================================================
>> RCS file: /cvs/src/src/gold/arm.cc,v
>> retrieving revision 1.59
>> diff -u -u -p -r1.59 arm.cc
>> --- arm.cc 20 Jan 2010 17:29:52 -0000 1.59
>> +++ arm.cc 22 Jan 2010 00:18:11 -0000
>> @@ -1873,6 +1873,24 @@ class Target_arm : public Sized_target<3
>> const elfcpp::Rel<32, big_endian>& reloc, unsigned int r_type,
>> Symbol* gsym);
>>
>> + inline void
>> + local_for_icf(Symbol_table* , Layout* , Target_arm* ,
>> + Sized_relobj<32, big_endian>* ,
>> + unsigned int ,
>> + Output_section* ,
>> + const elfcpp::Rel<32, big_endian>& , unsigned int ,
>> + const elfcpp::Sym<32, big_endian>& )
>> + { }
>
> Please remove the extra spaces before commas and parenthesis.
>
>> +
>> + inline void
>> + global_for_icf(Symbol_table* , Layout* , Target_arm* ,
>> + Sized_relobj<32, big_endian>* ,
>> + unsigned int ,
>> + Output_section* ,
>> + const elfcpp::Rel<32, big_endian>& , unsigned int ,
>> + Symbol* )
>> + { }
>
> Same here.
>
>
>> Index: gc.h
>> ===================================================================
>> RCS file: /cvs/src/src/gold/gc.h,v
>> retrieving revision 1.9
>> diff -u -u -p -r1.9 gc.h
>> --- gc.h 20 Jan 2010 17:29:52 -0000 1.9
>> +++ gc.h 22 Jan 2010 00:18:11 -0000
>> @@ -162,19 +162,20 @@ template<int size, bool big_endian, type
>> inline void
>> gc_process_relocs(
>> Symbol_table* symtab,
>> - Layout*,
>> - Target_type* ,
>> + Layout* ,
>> + Target_type* target,
>
> No extra space before comma.
>
>> Sized_relobj<size, big_endian>* src_obj,
>> unsigned int src_indx,
>> const unsigned char* prelocs,
>> size_t reloc_count,
>> - Output_section*,
>> + Output_section* ,
>
> Same here.
>
>
>> + if (parameters->options().icf_enabled()
>> + && parameters->options().icf_safe_folding()
>> + && target->is_fptr_taken_checked())
>> + {
>> + src_section_name = src_obj->section_name(src_indx);
>> + }
>
> Getting the name of a section is slow, and you just did it a few lines
> above. Don't do it twice.
>
>
>> + // When doing safe folding, check to see if this relocation is that
>> + // of a function pointer being taken.
>> + if (parameters->options().icf_enabled()
>> + && symtab->icf()->check_section_for_fptr_taken(src_section_name,
>> + target))
>> + {
>> + scan.local_for_icf(symtab, NULL, NULL, src_obj, src_indx,
>> + NULL, reloc, r_type, lsym);
>> + }
>
> How about renaming check_section_for_fptr_taken to
> check_section_for_function_pointers, here and elsewhere.
>
>
>> +// Safe Folding :
>> +// ------------
>> +//
>> +// ICF in safe mode, folds only ctors and dtors as their function pointers can
>> +// never be taken. Also, for AMD X86-64, safe folding uses the relocation
>> +// type to determine if a function's pointer was taken or not and only folds
>> +// functions whose pointers are definitely not taken.
>> +//
>> +// For X86-64, it is not correct to use safe folding to build non-pie
>> +// executables using PIC/PIE objects. This can cause the resulting binary
>> +// to have unexpected run-time behaviour in the presence of pointer
>> +// comparisons.
>
> s/mode,/mode/
> s/AMD //
>
>
>> -static bool
>> +bool
>> is_function_ctor_or_dtor(const char* mangled_func_name)
>
> Keep this static, no reason to change it.
>
>> @@ -577,14 +591,18 @@ Icf::find_identical_sections(const Input
>> if (parameters->options().gc_sections()
>> && symtab->gc()->is_section_garbage(*p, i))
>> continue;
>> + const char* mangled_func_name = strrchr(section_name, '.');
>> + gold_assert(mangled_func_name != NULL);
>> // With --icf=safe, check if the mangled function name is a ctor
>> // or a dtor. The mangled function name can be obtained from the
>> // section name by stripping the section prefix.
>> - const char* mangled_func_name = strrchr(section_name, '.');
>> - gold_assert(mangled_func_name != NULL);
>> if (parameters->options().icf_safe_folding()
>> - && !is_function_ctor_or_dtor(mangled_func_name + 1))
>> - continue;
>> + && !is_function_ctor_or_dtor(mangled_func_name + 1)
>> + && (target.is_fptr_taken_checked() == false
>> + || is_section_fptr_taken(*p, i)))
>> + {
>> + continue;
>> + }
>
> Don't write == false, write !target.is_fptr_taken_checked().
>
> Actually I don't like that name either. How about
> can_check_for_function_pointers. And how about changing
> is_section_fptr_taken to section_has_function_pointers.
>
>
>> DEFINE_enum(icf, options::TWO_DASHES, '\0', "none",
>> N_("Identical Code Folding. "
>> - "\'--icf=safe\' folds only ctors and dtors."),
>> + "\'--icf=safe\' folds only ctors and dtors. "
>> + "For X86-64, also folds functions whose pointers are "
>> + "definitely not taken. For X86-64, do not use safe "
>> + "ICF to build non-pie executables with pic objects."),
>> ("[none,all,safe]"),
>> {"none", "all", "safe"});
>
> Doesn't that make the --help output look kind of ridiculous? Not sure
> what to do here, but I don't think we should do that.
>
>> @@ -1364,6 +1393,94 @@ Target_x86_64::Scan::unsupported_reloc_g
>> object->name().c_str(), r_type, gsym->demangled_name().c_str());
>> }
>>
>> +// Returns true if this relocation type could be that of a function pointer
>> +// only if the target is not position-independent code.
>> +inline bool
>> +Target_x86_64::Scan::is_non_pic_reloc_type_func_ptr(unsigned int r_type)
>
> Let's call this possible_function_pointer_reloc. The non_pic part
> doesn't need to be in the name, I think.
>
>> + // 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())
>> + {
>> + symtab->icf()->set_section_fptr_taken(object,
>> + lsym.get_st_shndx());
>> + return;
>> + }
>> +
>> + if (is_non_pic_reloc_type_func_ptr(r_type))
>> + symtab->icf()->set_section_fptr_taken(object, lsym.get_st_shndx());
>> +}
>
> The then-block is the same here; combine the conditions with ||.
>
> Why don't you check the symbol type? If the type is STT_OBJECT, you
> know it is not a function.
>
>
>> +inline void
>> +Target_x86_64::Scan::global_for_icf(Symbol_table* symtab,
>> + Layout* ,
>> + Target_x86_64* ,
>> + Sized_relobj<64, false>* ,
>> + unsigned int ,
>> + Output_section* ,
>> + const elfcpp::Rela<64, false>& ,
>> + unsigned int r_type,
>> + Symbol* gsym)
>> +{
>> + if (parameters->options().shared())
>> + {
>> + // When building a shared library, do not fold symbols whose visibility
>> + // is hidden, internal or protected.
>> + if (gsym->visibility() == elfcpp::STV_INTERNAL
>> + || gsym->visibility() == elfcpp::STV_PROTECTED
>> + || gsym->visibility() == elfcpp::STV_HIDDEN)
>> + {
>> + bool is_ordinary;
>> + symtab->icf()->set_section_fptr_taken(gsym->object(),
>> + gsym->shndx(&is_ordinary));
>> + }
>> + return;
>> + }
>> +
>> + if (is_non_pic_reloc_type_func_ptr(r_type))
>> + {
>> + bool is_ordinary;
>> + symtab->icf()->set_section_fptr_taken(gsym->object(),
>> + gsym->shndx(&is_ordinary));
>> + }
>> +}
>
> These conditions can also be combined.
>
> Here too it seems like you should check the symbol type.
>
> You can't ignore is_ordinary here. If is_ordinary is false, you must
> not treat the the symbol index as usual. I believe that could happen
> here for a common symbol. It's probably simplest to punt if
> is_ordinary is false--to treat it as a taking a function pointer.
>
>> +// not be folded into kept_func_2 other than for AMD X86-64 which can
>> +// use relocation types to determine if function pointers are taken.
>> +// kept_func_3 should never be folded as its pointer is taken. The ctor
>> +// and dtor of class A must be folded.
>
> s/AMD // Intel makes them too, after all.
>
>
> Please fix and resend.
>
> Thanks.
>
> Ian
>
-------------- next part --------------
Index: arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.80
diff -u -u -p -r1.80 arm.cc
--- arm.cc 12 Feb 2010 05:51:31 -0000 1.80
+++ arm.cc 12 Feb 2010 23:32:02 -0000
@@ -2171,6 +2171,24 @@ class Target_arm : public Sized_target<3
const elfcpp::Rel<32, big_endian>& reloc, unsigned int r_type,
Symbol* gsym);
+ inline void
+ local_for_icf(Symbol_table* , Layout* , Target_arm* ,
+ Sized_relobj<32, big_endian>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rel<32, big_endian>& , unsigned int ,
+ const elfcpp::Sym<32, big_endian>&)
+ { }
+
+ inline void
+ global_for_icf(Symbol_table* , Layout* , Target_arm* ,
+ Sized_relobj<32, big_endian>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rel<32, big_endian>& , unsigned int ,
+ Symbol*)
+ { }
+
private:
static void
unsupported_reloc_local(Sized_relobj<32, big_endian>*,
Index: gc.h
===================================================================
RCS file: /cvs/src/src/gold/gc.h,v
retrieving revision 1.9
diff -u -u -p -r1.9 gc.h
--- gc.h 20 Jan 2010 17:29:52 -0000 1.9
+++ gc.h 12 Feb 2010 23:32:02 -0000
@@ -163,18 +163,19 @@ inline void
gc_process_relocs(
Symbol_table* symtab,
Layout*,
- Target_type* ,
+ Target_type* target,
Sized_relobj<size, big_endian>* src_obj,
unsigned int src_indx,
const unsigned char* prelocs,
size_t reloc_count,
Output_section*,
- bool ,
+ bool,
size_t local_count,
const unsigned char* plocal_syms)
{
Object *dst_obj;
unsigned int dst_indx;
+ Scan scan;
typedef typename Reloc_types<sh_type, size, big_endian>::Reloc Reltype;
const int reloc_size = Reloc_types<sh_type, size, big_endian>::reloc_size;
@@ -186,8 +187,14 @@ gc_process_relocs(
bool is_icf_tracked = false;
const char* cident_section_name = NULL;
+ std::string src_section_name = (parameters->options().icf_enabled()
+ ? src_obj->section_name(src_indx)
+ : "");
+
+ bool check_section_for_function_pointers = false;
+
if (parameters->options().icf_enabled()
- && is_section_foldable_candidate(src_obj->section_name(src_indx).c_str()))
+ && is_section_foldable_candidate(src_section_name.c_str()))
{
is_icf_tracked = true;
Section_id src_id(src_obj, src_indx);
@@ -196,11 +203,16 @@ gc_process_relocs(
addendvec = &symtab->icf()->addend_reloc_list()[src_id];
}
+ check_section_for_function_pointers =
+ symtab->icf()->check_section_for_function_pointers(src_section_name,
+ target);
+
for (size_t i = 0; i < reloc_count; ++i, prelocs += reloc_size)
{
Reltype reloc(prelocs);
typename elfcpp::Elf_types<size>::Elf_WXword r_info = reloc.get_r_info();
unsigned int r_sym = elfcpp::elf_r_sym<size>(r_info);
+ unsigned int r_type = elfcpp::elf_r_type<size>(r_info);
typename elfcpp::Elf_types<size>::Elf_Swxword addend =
Reloc_types<sh_type, size, big_endian>::get_reloc_addend_noerror(&reloc);
@@ -225,6 +237,12 @@ gc_process_relocs(
(*addendvec).push_back(std::make_pair(symvalue,
static_cast<long long>(addend)));
}
+ // When doing safe folding, check to see if this relocation is that
+ // of a function pointer being taken.
+ if (check_section_for_function_pointers
+ && lsym.get_st_type() != elfcpp::STT_OBJECT)
+ scan.local_for_icf(symtab, NULL, NULL, src_obj, src_indx,
+ NULL, reloc, r_type, lsym);
if (shndx == src_indx)
continue;
}
@@ -265,7 +283,13 @@ gc_process_relocs(
static_cast<long long>(sized_gsym->value());
(*addendvec).push_back(std::make_pair(symvalue,
static_cast<long long>(addend)));
- }
+ }
+ // When doing safe folding, check to see if this relocation is that
+ // of a function pointer being taken.
+ if (check_section_for_function_pointers
+ && gsym->type() != elfcpp::STT_OBJECT)
+ scan.global_for_icf(symtab, NULL, NULL, src_obj, src_indx,
+ NULL, reloc, r_type, gsym);
}
if (parameters->options().gc_sections())
{
Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.113
diff -u -u -p -r1.113 i386.cc
--- i386.cc 10 Feb 2010 23:00:29 -0000 1.113
+++ i386.cc 12 Feb 2010 23:32:02 -0000
@@ -202,6 +202,24 @@ class Target_i386 : public Target_freebs
const elfcpp::Rel<32, false>& reloc, unsigned int r_type,
Symbol* gsym);
+ inline void
+ local_for_icf(Symbol_table* , Layout* , Target_i386* ,
+ Sized_relobj<32, false>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rel<32, false>& , unsigned int ,
+ const elfcpp::Sym<32, false>&)
+ { }
+
+ inline void
+ global_for_icf(Symbol_table* , Layout* , Target_i386* ,
+ Sized_relobj<32, false>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rel<32, false>& , unsigned int ,
+ Symbol*)
+ { }
+
static void
unsupported_reloc_local(Sized_relobj<32, false>*, unsigned int r_type);
Index: icf.cc
===================================================================
RCS file: /cvs/src/src/gold/icf.cc,v
retrieving revision 1.7
diff -u -u -p -r1.7 icf.cc
--- icf.cc 4 Jan 2010 19:08:39 -0000 1.7
+++ icf.cc 12 Feb 2010 23:32:02 -0000
@@ -101,6 +101,19 @@
// when folding takes place. This could lead to unexpected run-time
// behaviour.
//
+// Safe Folding :
+// ------------
+//
+// ICF in safe mode folds only ctors and dtors as their function pointers can
+// never be taken. Also, for X86-64, safe folding uses the relocation
+// type to determine if a function's pointer is taken or not and only folds
+// functions whose pointers are definitely not taken.
+//
+// For X86-64, it is not correct to use safe folding to build non-pie
+// executables using PIC/PIE objects. This can cause the resulting binary
+// to have unexpected run-time behaviour in the presence of pointer
+// comparisons.
+//
//
// How to run : --icf=[safe|all|none]
// Optional parameters : --icf-iterations <num> --print-icf-sections
@@ -560,6 +573,7 @@ Icf::find_identical_sections(const Input
std::vector<unsigned int> num_tracked_relocs;
std::vector<bool> is_secn_or_group_unique;
std::vector<std::string> section_contents;
+ const Target& target = parameters->target();
// Decide which sections are possible candidates first.
@@ -577,14 +591,18 @@ Icf::find_identical_sections(const Input
if (parameters->options().gc_sections()
&& symtab->gc()->is_section_garbage(*p, i))
continue;
+ const char* mangled_func_name = strrchr(section_name, '.');
+ gold_assert(mangled_func_name != NULL);
// With --icf=safe, check if the mangled function name is a ctor
// or a dtor. The mangled function name can be obtained from the
// section name by stripping the section prefix.
- const char* mangled_func_name = strrchr(section_name, '.');
- gold_assert(mangled_func_name != NULL);
if (parameters->options().icf_safe_folding()
- && !is_function_ctor_or_dtor(mangled_func_name + 1))
- continue;
+ && !is_function_ctor_or_dtor(mangled_func_name + 1)
+ && (!target.can_check_for_function_pointers()
+ || section_has_function_pointers(*p, i)))
+ {
+ continue;
+ }
this->id_section_.push_back(Section_id(*p, i));
this->section_id_[Section_id(*p, i)] = section_num;
this->kept_section_id_.push_back(section_num);
Index: icf.h
===================================================================
RCS file: /cvs/src/src/gold/icf.h,v
retrieving revision 1.4
diff -u -u -p -r1.4 icf.h
--- icf.h 20 Jan 2010 17:29:52 -0000 1.4
+++ icf.h 12 Feb 2010 23:32:02 -0000
@@ -50,9 +50,11 @@ class Icf
typedef Unordered_map<Section_id,
unsigned int,
Section_id_hash> Uniq_secn_id_map;
+ typedef Unordered_set<Section_id, Section_id_hash> Secn_fptr_taken_set;
Icf()
: id_section_(), section_id_(), kept_section_id_(),
+ fptr_section_id_(),
num_tracked_relocs(NULL), icf_ready_(false),
section_reloc_list_(), symbol_reloc_list_(),
addend_reloc_list_()
@@ -88,7 +90,37 @@ class Icf
// given section.
bool
is_section_folded(Object* obj, unsigned int shndx);
-
+
+ // Given an object and a section index, this returns true if the
+ // pointer of the function defined in this section is taken.
+ bool
+ section_has_function_pointers(Object *obj, unsigned int shndx)
+ {
+ return (this->fptr_section_id_.find(Section_id(obj, shndx))
+ != this->fptr_section_id_.end());
+ }
+
+ // Records that a pointer of the function defined in this section
+ // is taken.
+ void
+ set_section_has_function_pointers(Object *obj, unsigned int shndx)
+ {
+ this->fptr_section_id_.insert(Section_id(obj, shndx));
+ }
+
+ // Checks if the section_name should be searched for relocs
+ // corresponding to taken function pointers. Ignores eh_frame
+ // and vtable sections.
+ inline bool
+ check_section_for_function_pointers(std::string section_name,
+ Target* target)
+ {
+ return (parameters->options().icf_safe_folding()
+ && target->can_check_for_function_pointers()
+ && !is_prefix_of(".rodata._ZTV", section_name.c_str())
+ && !is_prefix_of(".eh_frame", section_name.c_str()));
+ }
+
// Returns a map of a section to a list of all sections referenced
// by its relocations.
Section_list&
@@ -122,6 +154,10 @@ class Icf
// section. If the id's are the same then this section is
// not folded.
std::vector<unsigned int> kept_section_id_;
+ // Given a section id, this says if the pointer to this
+ // function is taken in which case it is dangerous to fold
+ // this function.
+ Secn_fptr_taken_set fptr_section_id_;
unsigned int* num_tracked_relocs;
// Flag to indicate if ICF has been run.
bool icf_ready_;
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.139
diff -u -u -p -r1.139 options.h
--- options.h 22 Jan 2010 19:43:00 -0000 1.139
+++ options.h 12 Feb 2010 23:32:03 -0000
@@ -914,7 +914,8 @@ class General_options
DEFINE_enum(icf, options::TWO_DASHES, '\0', "none",
N_("Identical Code Folding. "
- "\'--icf=safe\' folds only ctors and dtors."),
+ "\'--icf=safe\' Folds ctors, dtors and functions whose"
+ " pointers are definitely not taken."),
("[none,all,safe]"),
{"none", "all", "safe"});
Index: powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.28
diff -u -u -p -r1.28 powerpc.cc
--- powerpc.cc 9 Feb 2010 20:29:44 -0000 1.28
+++ powerpc.cc 12 Feb 2010 23:32:03 -0000
@@ -183,6 +183,24 @@ class Target_powerpc : public Sized_targ
const elfcpp::Rela<size, big_endian>& reloc, unsigned int r_type,
Symbol* gsym);
+ inline void
+ local_for_icf(Symbol_table* , Layout* , Target_powerpc* ,
+ Sized_relobj<size, big_endian>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rela<size, big_endian>& , unsigned int ,
+ const elfcpp::Sym<size, big_endian>&)
+ { }
+
+ inline void
+ global_for_icf(Symbol_table* , Layout* , Target_powerpc* ,
+ Sized_relobj<size, big_endian>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rela<size, big_endian>& , unsigned int ,
+ Symbol*)
+ { }
+
private:
static void
unsupported_reloc_local(Sized_relobj<size, big_endian>*,
Index: sparc.cc
===================================================================
RCS file: /cvs/src/src/gold/sparc.cc,v
retrieving revision 1.35
diff -u -u -p -r1.35 sparc.cc
--- sparc.cc 9 Feb 2010 20:29:44 -0000 1.35
+++ sparc.cc 12 Feb 2010 23:32:03 -0000
@@ -193,6 +193,25 @@ class Target_sparc : public Sized_target
const elfcpp::Rela<size, big_endian>& reloc, unsigned int r_type,
Symbol* gsym);
+ inline void
+ local_for_icf(Symbol_table* , Layout* , Target_sparc* ,
+ Sized_relobj<size, big_endian>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rela<size, big_endian>& , unsigned int ,
+ const elfcpp::Sym<size, big_endian>&)
+ { }
+
+ inline void
+ global_for_icf(Symbol_table* , Layout* , Target_sparc* ,
+ Sized_relobj<size, big_endian>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rela<size, big_endian>& , unsigned int ,
+ Symbol*)
+ { }
+
+
private:
static void
unsupported_reloc_local(Sized_relobj<size, big_endian>*,
Index: target.h
===================================================================
RCS file: /cvs/src/src/gold/target.h,v
retrieving revision 1.44
diff -u -u -p -r1.44 target.h
--- target.h 3 Feb 2010 05:36:55 -0000 1.44
+++ target.h 12 Feb 2010 23:32:03 -0000
@@ -64,6 +64,13 @@ class Target
virtual ~Target()
{ }
+ // Virtual function which is set to return true by a target if
+ // it can use relocation types to determine if a function's
+ // pointer is taken.
+ virtual bool
+ can_check_for_function_pointers() const
+ { return false; }
+
// Return the bit size that this target implements. This should
// return 32 or 64.
int
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.106
diff -u -u -p -r1.106 x86_64.cc
--- x86_64.cc 10 Feb 2010 23:00:29 -0000 1.106
+++ x86_64.cc 12 Feb 2010 23:32:03 -0000
@@ -39,6 +39,7 @@
#include "tls.h"
#include "freebsd.h"
#include "gc.h"
+#include "icf.h"
namespace
{
@@ -69,6 +70,15 @@ class Target_x86_64 : public Target_free
tls_base_symbol_defined_(false)
{ }
+ // This function should be defined in targets that can use relocation
+ // types to determine (implemented in local_for_icf and global_for_icf)
+ // if a function's pointer is taken. ICF uses this in safe mode to only
+ // fold those functions whose pointer is defintely not taken. For x86_64
+ // pie binaries, safe ICF cannot be done by looking at relocation types.
+ inline bool
+ can_check_for_function_pointers() const
+ { return !parameters->options().pie(); }
+
// Hook for a new output section.
void
do_new_output_section(Output_section*) const;
@@ -224,6 +234,22 @@ class Target_x86_64 : public Target_free
const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
Symbol* gsym);
+ inline void
+ local_for_icf(Symbol_table* symtab, Layout* layout, Target_x86_64* target,
+ Sized_relobj<64, false>* object,
+ unsigned int data_shndx,
+ Output_section* output_section,
+ const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
+ const elfcpp::Sym<64, false>& lsym);
+
+ inline void
+ global_for_icf(Symbol_table* symtab, Layout* layout, Target_x86_64* target,
+ Sized_relobj<64, false>* object,
+ unsigned int data_shndx,
+ Output_section* output_section,
+ const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
+ Symbol* gsym);
+
private:
static void
unsupported_reloc_local(Sized_relobj<64, false>*, unsigned int r_type);
@@ -235,6 +261,9 @@ class Target_x86_64 : public Target_free
void
check_non_pic(Relobj*, unsigned int r_type);
+ inline bool
+ possible_function_pointer_reloc(unsigned int r_type);
+
// Whether we have issued an error about a non-PIC compilation.
bool issued_non_pic_error_;
};
@@ -1364,6 +1393,81 @@ Target_x86_64::Scan::unsupported_reloc_g
object->name().c_str(), r_type, gsym->demangled_name().c_str());
}
+// Returns true if this relocation type could be that of a function pointer
+// only if the target is not position-independent code.
+inline bool
+Target_x86_64::Scan::possible_function_pointer_reloc(unsigned int r_type)
+{
+ if (parameters->options().shared())
+ return false;
+ switch (r_type)
+ {
+ case elfcpp::R_X86_64_64:
+ case elfcpp::R_X86_64_32:
+ case elfcpp::R_X86_64_32S:
+ case elfcpp::R_X86_64_16:
+ case elfcpp::R_X86_64_8:
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
+// For safe ICF, scan a relocation for a local symbol to check if it
+// corresponds to a function pointer being taken. In that case mark
+// the function whose pointer was taken as not foldable.
+
+inline void
+Target_x86_64::Scan::local_for_icf(Symbol_table* symtab,
+ Layout* ,
+ Target_x86_64* ,
+ Sized_relobj<64, false>* object,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rela<64, false>& ,
+ unsigned int r_type,
+ const elfcpp::Sym<64, false>& lsym)
+{
+ // 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()
+ || possible_function_pointer_reloc(r_type))
+ symtab->icf()->set_section_has_function_pointers(object,
+ lsym.get_st_shndx());
+
+}
+
+// For safe ICF, scan a relocation for a global symbol to check if it
+// corresponds to a function pointer being taken. In that case mark
+// the function whose pointer was taken as not foldable.
+
+inline void
+Target_x86_64::Scan::global_for_icf(Symbol_table* symtab,
+ Layout* ,
+ Target_x86_64* ,
+ Sized_relobj<64, false>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rela<64, false>& ,
+ unsigned int r_type,
+ Symbol* gsym)
+{
+ bool is_ordinary;
+ unsigned int shndx = gsym->shndx(&is_ordinary);
+
+ // 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))
+ || !is_ordinary
+ || possible_function_pointer_reloc(r_type))
+ symtab->icf()->set_section_has_function_pointers(gsym->object(), shndx);
+}
+
// Scan a relocation for a global symbol.
inline void
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.121
diff -u -u -p -r1.121 Makefile.am
--- testsuite/Makefile.am 12 Feb 2010 05:51:32 -0000 1.121
+++ testsuite/Makefile.am 12 Feb 2010 23:32:03 -0000
@@ -170,14 +170,28 @@ icf_keep_unique_test.stdout: icf_keep_un
$(TEST_NM) -C icf_keep_unique_test > icf_keep_unique_test.stdout
check_SCRIPTS += icf_safe_test.sh
-check_DATA += icf_safe_test.stdout
+check_DATA += icf_safe_test_1.stdout icf_safe_test_2.stdout
MOSTLYCLEANFILES += icf_safe_test
-icf_safe_test.o: icf_safe_test.cc
+icf_safe_test.o: icf_safe_test.cc
$(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
icf_safe_test: icf_safe_test.o gcctestdir/ld
$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_test.o
-icf_safe_test.stdout: icf_safe_test
- $(TEST_NM) icf_safe_test > icf_safe_test.stdout
+icf_safe_test_1.stdout: icf_safe_test
+ $(TEST_NM) icf_safe_test > icf_safe_test_1.stdout
+icf_safe_test_2.stdout: icf_safe_test
+ $(TEST_READELF) -h icf_safe_test > icf_safe_test_2.stdout
+
+check_SCRIPTS += icf_safe_so_test.sh
+check_DATA += icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout
+MOSTLYCLEANFILES += icf_safe_so_test
+icf_safe_so_test.o: icf_safe_so_test.cc
+ $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $<
+icf_safe_so_test: icf_safe_so_test.o gcctestdir/ld
+ $(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_so_test.o -fPIC -shared
+icf_safe_so_test_1.stdout: icf_safe_so_test
+ $(TEST_NM) icf_safe_so_test > icf_safe_so_test_1.stdout
+icf_safe_so_test_2.stdout: icf_safe_so_test
+ $(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
check_PROGRAMS += basic_test
check_PROGRAMS += basic_static_test
Index: testsuite/icf_safe_so_test.cc
===================================================================
RCS file: testsuite/icf_safe_so_test.cc
diff -N testsuite/icf_safe_so_test.cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_safe_so_test.cc 12 Feb 2010 23:32:03 -0000
@@ -0,0 +1,73 @@
+// icf_safe_so_test.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify if identical code folding
+// in safe mode correctly folds functions in a shared object. The
+// foo_* functions below should not be folded. For x86-64,
+// foo_glob and bar_glob should be folded as their function pointers
+// are addresses of PLT entries in shared objects.
+
+int __attribute__ ((visibility ("protected")))
+foo_prot()
+{
+ return 1;
+}
+
+int __attribute__ ((visibility ("hidden")))
+foo_hidden()
+{
+ return 1;
+}
+
+int __attribute__ ((visibility ("internal")))
+foo_internal()
+{
+ return 1;
+}
+
+static int
+foo_static()
+{
+ return 1;
+}
+
+int foo_glob()
+{
+ return 2;
+}
+
+int bar_glob()
+{
+ return 2;
+}
+
+int main()
+{
+ int (*p)() = foo_glob;
+ (void)p;
+ foo_static();
+ foo_prot();
+ foo_hidden();
+ foo_internal();
+ return 0;
+}
+
Index: testsuite/icf_safe_so_test.sh
===================================================================
RCS file: testsuite/icf_safe_so_test.sh
diff -N testsuite/icf_safe_so_test.sh
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_safe_so_test.sh 12 Feb 2010 23:32:03 -0000
@@ -0,0 +1,69 @@
+# icf_safe_so_test.sh -- test --icf=safe
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if --icf=safe works as expected.
+# File icf_safe_so_test.cc is in this test. The goal of this script is
+# to verify if identical code folding in safe mode correctly folds
+# functions in a shared object.
+
+check_nofold()
+{
+ func_addr_1=`grep $2 $1 | awk '{print $1}'`
+ func_addr_2=`grep $3 $1 | awk '{print $1}'`
+ if [ $func_addr_1 = $func_addr_2 ];
+ then
+ echo "Safe Identical Code Folding folded" $2 "and" $3
+ exit 1
+ fi
+}
+
+check_fold()
+{
+ func_addr_1=`grep $2 $1 | awk '{print $1}'`
+ func_addr_2=`grep $3 $1 | awk '{print $1}'`
+ if [ $func_addr_1 != $func_addr_2 ];
+ then
+ echo "Safe Identical Code Folding did not fold " $2 "and" $3
+ exit 1
+ fi
+}
+
+arch_specific_safe_fold()
+{
+ grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
+ if [ $? == 0 ];
+ then
+ check_fold $1 $3 $4
+ else
+ check_nofold $1 $3 $4
+ fi
+}
+
+check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_hidden"
+check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_internal"
+check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_static"
+check_nofold icf_safe_so_test_1.stdout "foo_hidden" "foo_internal"
+check_nofold icf_safe_so_test_1.stdout "foo_hidden" "foo_static"
+check_nofold icf_safe_so_test_1.stdout "foo_internal" "foo_static"
+arch_specific_safe_fold icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout \
+ "foo_glob" "bar_glob"
+
Index: testsuite/icf_safe_test.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.cc,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_test.cc
--- testsuite/icf_safe_test.cc 13 Oct 2009 21:17:43 -0000 1.1
+++ testsuite/icf_safe_test.cc 12 Feb 2010 23:32:03 -0000
@@ -22,8 +22,10 @@
// The goal of this program is to verify if identical code folding
// in safe mode correctly folds only ctors and dtors. kept_func_1 must
-// not be folded into kept_func_2. The ctor and dtor of class A must
-// be folded.
+// not be folded into kept_func_2 other than for X86-64 which can
+// use relocation types to determine if function pointers are taken.
+// kept_func_3 should never be folded as its pointer is taken. The ctor
+// and dtor of class A must be folded.
class A
{
@@ -48,7 +50,14 @@ int kept_func_2()
return 1;
}
+int kept_func_3()
+{
+ return 1;
+}
+
int main()
{
+ int (*p)() = kept_func_3;
+ p();
return 0;
}
Index: testsuite/icf_safe_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.sh,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_test.sh
--- testsuite/icf_safe_test.sh 13 Oct 2009 21:17:43 -0000 1.1
+++ testsuite/icf_safe_test.sh 12 Feb 2010 23:32:03 -0000
@@ -22,7 +22,8 @@
# The goal of this program is to verify if --icf=safe works as expected.
# File icf_safe_test.cc is in this test. This program checks if only
-# ctors and dtors are folded.
+# ctors and dtors are folded, except for x86-64, which uses relocation
+# types to detect if function pointers are taken.
check_nofold()
{
@@ -46,5 +47,19 @@ check_fold()
fi
}
-check_nofold icf_safe_test.stdout "kept_func_1" "kept_func_2"
-check_fold icf_safe_test.stdout "_ZN1AD1Ev" "_ZN1AC1Ev"
+arch_specific_safe_fold()
+{
+ grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
+ if [ $? == 0 ];
+ then
+ check_fold $1 $3 $4
+ else
+ check_nofold $1 $3 $4
+ fi
+}
+
+arch_specific_safe_fold icf_safe_test_1.stdout icf_safe_test_2.stdout \
+ "kept_func_1" "kept_func_2"
+check_fold icf_safe_test_1.stdout "_ZN1AD1Ev" "_ZN1AC1Ev"
+check_nofold icf_safe_test_1.stdout "kept_func_3" "kept_func_1"
+check_nofold icf_safe_test_1.stdout "kept_func_3" "kept_func_2"
More information about the Binutils
mailing list