Safe ICF for i386.
Sriraman Tallam
tmsriram@google.com
Thu Mar 4 01:38:00 GMT 2010
Hi Ian,
I checked this in with the changes you mentioned. I have attached the patch.
JFYI, Here is a simple example to show when R_386_GOTOFF is used as
reloc type for a function pointer. It is obtained when -fPIE is used.
foo.cc
------
#include <stdio.h>
int foo()
{
}
int main()
{
int (*p)() = foo;
}
$ g++ -m32 -O0 -ffunction-sections -fPIE -c foo.cc
$ readelf -r foo.o
Relocation section '.rel.text.main' at offset 0x694 contains 3 entries:
Offset Info Type Sym.Value Sym. Name
00000007 00001102 R_386_PC32 00000000 __x86.get_pc_thunk.cx
0000000d 0000120a R_386_GOTPC 00000000 _GLOBAL_OFFSET_TABLE_
00000013 00000e09 R_386_GOTOFF 00000000 _Z3foov
Thanks,
-Sriraman.
On Wed, Mar 3, 2010 at 5:15 PM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>> * i386.cc (Target_i386::can_check_for_function_pointers): New function.
>> (Scan::possible_function_pointer_reloc): New function.
>> (Scan::local_reloc_may_be_function_pointer): Change to call
>> possible_function_pointer_reloc.
>> (Scan::global_reloc_may_be_function_pointer): Ditto.
>> * icf.h (Icf::check_section_for_function_pointers): Change to reject
>> relocations in ".data.rel.ro._ZTV" section.
>> * testsuite/icf_safe_so_test.sh: Change to pass i386.
>> * testsuite/icf_safe_so_test.cc: Ditto.
>> * testsuite/icf_safe_test.cc: Ditto.
>> * testsuite/icf_safe_test.sh: Ditto.
>
>> +inline bool
>> +Target_i386::Scan::possible_function_pointer_reloc(unsigned int r_type)
>> +{
>> + switch (r_type)
>> + {
>> + case elfcpp::R_386_32:
>> + case elfcpp::R_386_16:
>> + case elfcpp::R_386_8:
>> + case elfcpp::R_386_GOTOFF:
>> + case elfcpp::R_386_GOT32:
>> + {
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>
> Write this using a default case rather than falling through the
> switch. Otherwise you may get a warning depending on the version of
> g++ you are using.
>
> I don't really see how R_386_GOTOFF could be used to get a function
> pointer. If you're sure that it can be, then it's OK.
>
>
>> +inline bool
>> +Target_i386::Scan::local_reloc_may_be_function_pointer(
>> + Symbol_table* ,
>> + Layout* ,
>> + Target_i386* ,
>> + Sized_relobj<32, false>* ,
>> + unsigned int ,
>> + Output_section* ,
>> + const elfcpp::Rel<32, false>& ,
>> + unsigned int r_type,
>> + const elfcpp::Sym<32, false>&)
>> +{
>> + return (possible_function_pointer_reloc(r_type));
>> +}
>
> Please remove unnecessary outer parentheses.
>
>> +inline bool
>> +Target_i386::Scan::global_reloc_may_be_function_pointer(
>> + Symbol_table* ,
>> + Layout* ,
>> + Target_i386* ,
>> + Sized_relobj<32, false>* ,
>> + unsigned int ,
>> + Output_section* ,
>> + const elfcpp::Rel<32, false>& ,
>> + unsigned int r_type,
>> + Symbol*)
>> +{
>> + return (possible_function_pointer_reloc(r_type));
>> +}
>
> Please remove unnecessary outer parentheses.
>
>
>> @@ -127,6 +127,7 @@ class Icf
>> return (parameters->options().icf_safe_folding()
>> && target->can_check_for_function_pointers()
>> && !is_prefix_of(".rodata._ZTV", section_name.c_str())
>> + && !is_prefix_of(".data.rel.ro._ZTV", section_name.c_str())
>> && !is_prefix_of(".eh_frame", section_name.c_str()));
>> }
>
> The indentation looks a little weird here but it may be a mail
> artifact, please check that it looks OK in the source.
>
>> +X86_32_specific_safe_fold()
>> +{
>> + grep_x86_32=`grep -q -e "Intel 80386" $1`
>> + arch_specific_safe_fold $? $2 $3 $4
>> +}
>>
>> +X86_64_specific_safe_fold()
>> +{
>> + grep_x86_64=`grep -q -e "Advanced Micro Devices X86-64" $1`
>> + arch_specific_safe_fold $? $2 $3 $4
>> +}
>
> I had to kind of puzzle over those grep statements. Please instead
> write
> grep -e ... $1 >/dev/null 2>&1
> rather than assigning to a variable.
>
>
> This is OK with those changes.
>
> Thanks.
>
> Ian
>
-------------- next part --------------
Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.114
diff -u -u -p -r1.114 i386.cc
--- i386.cc 13 Feb 2010 02:04:20 -0000 1.114
+++ i386.cc 4 Mar 2010 01:32:23 -0000
@@ -64,6 +64,10 @@ class Target_i386 : public Target_freebs
got_mod_index_offset_(-1U), tls_base_symbol_defined_(false)
{ }
+ inline bool
+ can_check_for_function_pointers() const
+ { return true; }
+
// Process the relocations to determine unreferenced sections for
// garbage collection.
void
@@ -203,24 +207,27 @@ class Target_i386 : public Target_freebs
Symbol* gsym);
inline bool
- local_reloc_may_be_function_pointer(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>&)
- { return false; }
+ local_reloc_may_be_function_pointer(Symbol_table* symtab, Layout* layout,
+ Target_i386* target,
+ Sized_relobj<32, false>* object,
+ unsigned int data_shndx,
+ Output_section* output_section,
+ const elfcpp::Rel<32, false>& reloc,
+ unsigned int r_type,
+ const elfcpp::Sym<32, false>& lsym);
+
+ inline bool
+ global_reloc_may_be_function_pointer(Symbol_table* symtab, Layout* layout,
+ Target_i386* target,
+ Sized_relobj<32, false>* object,
+ unsigned int data_shndx,
+ Output_section* output_section,
+ const elfcpp::Rel<32, false>& reloc,
+ unsigned int r_type,
+ Symbol* gsym);
inline bool
- global_reloc_may_be_function_pointer(Symbol_table* , Layout* ,
- Target_i386* ,
- Sized_relobj<32, false>* ,
- unsigned int ,
- Output_section* ,
- const elfcpp::Rel<32, false>& ,
- unsigned int , Symbol*)
- { return false; }
+ possible_function_pointer_reloc(unsigned int r_type);
static void
unsupported_reloc_local(Sized_relobj<32, false>*, unsigned int r_type);
@@ -1234,6 +1241,55 @@ Target_i386::Scan::unsupported_reloc_glo
object->name().c_str(), r_type, gsym->demangled_name().c_str());
}
+inline bool
+Target_i386::Scan::possible_function_pointer_reloc(unsigned int r_type)
+{
+ switch (r_type)
+ {
+ case elfcpp::R_386_32:
+ case elfcpp::R_386_16:
+ case elfcpp::R_386_8:
+ case elfcpp::R_386_GOTOFF:
+ case elfcpp::R_386_GOT32:
+ {
+ return true;
+ }
+ default:
+ return false;
+ }
+ return false;
+}
+
+inline bool
+Target_i386::Scan::local_reloc_may_be_function_pointer(
+ Symbol_table* ,
+ Layout* ,
+ Target_i386* ,
+ Sized_relobj<32, false>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rel<32, false>& ,
+ unsigned int r_type,
+ const elfcpp::Sym<32, false>&)
+{
+ return possible_function_pointer_reloc(r_type);
+}
+
+inline bool
+Target_i386::Scan::global_reloc_may_be_function_pointer(
+ Symbol_table* ,
+ Layout* ,
+ Target_i386* ,
+ Sized_relobj<32, false>* ,
+ unsigned int ,
+ Output_section* ,
+ const elfcpp::Rel<32, false>& ,
+ unsigned int r_type,
+ Symbol*)
+{
+ return possible_function_pointer_reloc(r_type);
+}
+
// Scan a relocation for a global symbol.
inline void
Index: icf.h
===================================================================
RCS file: /cvs/src/src/gold/icf.h,v
retrieving revision 1.6
diff -u -u -p -r1.6 icf.h
--- icf.h 21 Feb 2010 00:57:59 -0000 1.6
+++ icf.h 4 Mar 2010 01:32:23 -0000
@@ -127,6 +127,7 @@ class Icf
return (parameters->options().icf_safe_folding()
&& target->can_check_for_function_pointers()
&& !is_prefix_of(".rodata._ZTV", section_name.c_str())
+ && !is_prefix_of(".data.rel.ro._ZTV", section_name.c_str())
&& !is_prefix_of(".eh_frame", section_name.c_str()));
}
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/icf_safe_so_test.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_so_test.cc,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_so_test.cc
--- testsuite/icf_safe_so_test.cc 13 Feb 2010 02:04:21 -0000 1.1
+++ testsuite/icf_safe_so_test.cc 4 Mar 2010 01:32:23 -0000
@@ -24,7 +24,8 @@
// 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.
+// are addresses of PLT entries in shared objects. For 32-bit X86,
+// the hidden protected and internal symbols can be folded.
int __attribute__ ((visibility ("protected")))
foo_prot()
Index: testsuite/icf_safe_so_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_so_test.sh,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_so_test.sh
--- testsuite/icf_safe_so_test.sh 13 Feb 2010 02:04:21 -0000 1.1
+++ testsuite/icf_safe_so_test.sh 4 Mar 2010 01:32:23 -0000
@@ -25,8 +25,26 @@
# to verify if identical code folding in safe mode correctly folds
# functions in a shared object.
+error_if_symbol_absent()
+{
+ is_symbol_present $1 $2
+ if [ $? != 0 ];
+ then
+ echo "Symbol" $2 "not present, possibly folded."
+ exit 1
+ fi
+}
+
+is_symbol_present()
+{
+ result=`grep $2 $1`
+ return $?
+}
+
check_nofold()
{
+ error_if_symbol_absent $1 $2
+ error_if_symbol_absent $1 $3
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 ];
@@ -38,6 +56,18 @@ check_nofold()
check_fold()
{
+ is_symbol_present $1 $2
+ if [ $? != 0 ];
+ then
+ return 0
+ fi
+
+ is_symbol_present $1 $3
+ if [ $? != 0 ];
+ then
+ return 0
+ fi
+
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 ];
@@ -49,21 +79,31 @@ check_fold()
arch_specific_safe_fold()
{
- grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
- if [ $? == 0 ];
+ if [ $1 == 0 ];
then
- check_fold $1 $3 $4
+ check_fold $2 $3 $4
else
- check_nofold $1 $3 $4
+ check_nofold $2 $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"
+X86_32_specific_safe_fold()
+{
+ grep -q -e "Intel 80386" $1 >& /dev/null
+ arch_specific_safe_fold $? $2 $3 $4
+}
+X86_64_specific_safe_fold()
+{
+ grep -q -e "Advanced Micro Devices X86-64" $1 >& /dev/null
+ arch_specific_safe_fold $? $2 $3 $4
+}
+
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_hidden"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_internal"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_static"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_hidden" "foo_internal"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_hidden" "foo_static"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_internal" "foo_static"
+X86_64_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.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.2
diff -u -u -p -r1.2 icf_safe_test.cc
--- testsuite/icf_safe_test.cc 13 Feb 2010 02:04:21 -0000 1.2
+++ testsuite/icf_safe_test.cc 4 Mar 2010 01:32:23 -0000
@@ -22,10 +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 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.
+// not be folded into kept_func_2 other than for X86 (32 and 64 bit)
+// 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
{
Index: testsuite/icf_safe_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.sh,v
retrieving revision 1.2
diff -u -u -p -r1.2 icf_safe_test.sh
--- testsuite/icf_safe_test.sh 13 Feb 2010 02:04:21 -0000 1.2
+++ testsuite/icf_safe_test.sh 4 Mar 2010 01:32:23 -0000
@@ -22,8 +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, except for x86-64, which uses relocation
-# types to detect if function pointers are taken.
+# ctors and dtors are folded, except for x86 (32 and 64 bit), which
+# uses relocation types to detect if function pointers are taken.
check_nofold()
{
@@ -49,7 +49,7 @@ check_fold()
arch_specific_safe_fold()
{
- grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
+ grep_x86=`grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" $2`
if [ $? == 0 ];
then
check_fold $1 $3 $4
More information about the Binutils
mailing list