This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi, I made the final changes and submitted this patch. 2010-02-12 Sriraman Tallam <tmsriram@google.com> * arm.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * sparc.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * powerpc.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * i386.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * x86_64.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): 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 Fri, Feb 12, 2010 at 4:18 PM, Ian Lance Taylor <iant@google.com> wrote: > Sriraman Tallam <tmsriram@google.com> writes: > >> 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. > > >> +// 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. > > s/as their/if their/ > > Can you add another paragraph here or somewhere describing in detail > the problem with PIC/PIE objects? > > >> +// 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); >> +} > > Feel free to disagree, but I think this code would be easier to write > if these functions returned a boolean. ?Then you could call the > functions something like global_reloc_may_be_function_pointer. ?And > you can move the set_section_has_function_pointers into > gc_process_relocs, rather than having to duplicate it. > > This is OK with those changes. > > Thanks. > > Ian >
Attachment:
gold_safe_icf_2010_02_12_patch.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |