Re: [GOLD] PATCH: PR gold/10893: gold doesn't support IFUNC

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Dec 4, 2009 at 9:46 PM, Ian Lance Taylor <iant@google.com> wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>
>> elfcpp/
>>
>> 2009-12-03 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>
>> ? ? ? PR gold/10893
>> ? ? ? * elfcpp.h (Sym): Add is_func,
>>
>> gold/
>>
>> 2009-12-03 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>
>> ? ? ? PR gold/10893
>> ? ? ? * i386.cc (Target_i386::Scan::globa): Use is_func instead
>> ? ? ? of checking elfcpp::STT_FUNC.
>> ? ? ? (Target_i386::Relocate::relocate): Likewise.
>> ? ? ? * reloc.cc (Sized_relobj<size, big_endian>::split_stack_adjust_reltype): Likewise.
>> ? ? ? (Sized_relobj<size, big_endian>::find_functions): Likewise.
>> ? ? ? * x86_64.cc (Target_x86_64::Scan::global): Likewise.
>>
>> ? ? ? * symtab.cc (Symbol_table::sized_write_symbol): Turn IFUNC
>> ? ? ? symbols from shared libraries into normal FUNC symbols.
>>
>> ? ? ? * symtab.h (Symbol): Add is_func an use it.
>
>
>> --- a/elfcpp/elfcpp.h
>> +++ b/elfcpp/elfcpp.h
>> @@ -1292,6 +1292,11 @@ class Sym
>> ? ?get_st_shndx() const
>> ? ?{ return Convert<16, big_endian>::convert_host(this->p_->st_shndx); }
>>
>> + ?bool
>> + ?is_func() const
>> + ?{ return (this->get_st_type() == STT_FUNC
>> + ? ? ? ? || this->get_st_type() == STT_GNU_IFUNC); }
>> +
>> ? private:
>> ? ?const internal::Sym_data<size>* p_;
>> ?};
>
> I don't think you need this one. ?In any case, it shouldn't be there,
> and please don't add it. ?If you explain why you need it, we can
> figure out where to handle it.
>
>
>> --- a/gold/i386.cc
>> +++ b/gold/i386.cc
>
> You didn't fix anything in arm.cc, powerpc.cc, or sparc.cc. ?Why not?
> Is it because STT_GNU_IFUNC does not work for those targets?

STT_GNU_IFUNC works on i386 and x86-64 in Fedora 12. It may
work on PPC. But I can't test it.

My patch only allows gold to work with IFUNC symbols defined
in shared libraries.

>
>> @@ -1233,7 +1233,7 @@ Sized_relobj<size, big_endian>::find_functions(
>>
>> ? ? ? ?// FIXME: Some targets can have functions which do not have type
>> ? ? ? ?// STT_FUNC, e.g., STT_ARM_TFUNC.
>> - ? ? ?if (isym.get_st_type() != elfcpp::STT_FUNC
>> + ? ? ?if (!isym.is_func()
>> ? ? ? ? || isym.get_st_size() == 0)
>> ? ? ? continue;
>
> Ah, here is where you are using the elfcpp method. ?You should write
> this as
> ? ?if ((isym.get_st_type() != elfcpp::STT_FUNC
> ? ? ? ? && isym.get_st_type() != elfcpp::STT_GNU_IFUNC)
> ? ? ? ?|| isym.get_st_size() == 0)
>
>> --- a/gold/symtab.h
>> +++ b/gold/symtab.h
>> @@ -205,6 +205,12 @@ class Symbol
>> ? ?type() const
>> ? ?{ return this->type_; }
>>
>> + ?// Return true for function symbol.
>> + ?bool
>> + ?is_func() const
>> + ?{ return (this->type_ == elfcpp::STT_FUNC
>> + ? ? ? ? || this->type_ == elfcpp::STT_GNU_IFUNC); }
>> +
>
> Format this as:
>
> ?bool
> ?is_func() const
> ?{
> ? ?return (this->type_ == elfcpp::STT_FUNC
> ? ? ? ? ? ?|| this->type_ == elfcpp::STT_GNU_IFUNC);
> ?}
>
>
> This patch is OK with those change if it still works.
>

This is the patch I checked in.

Thanks.

-- 
H.J.
---
2009-12-07  H.J. Lu  <hongjiu.lu@intel.com>

	PR gold/10893
	* i386.cc (Target_i386::Scan::globa): Use is_func instead of
	checking elfcpp::STT_FUNC.
	(Target_i386::Relocate::relocate): Likewise.
	* x86_64.cc (Target_x86_64::Scan::global): Likewise.

	* symtab.cc (Symbol_table::sized_write_symbol): Turn IFUNC
	symbols from shared libraries into normal FUNC symbols.

	* symtab.h (Symbol): Add is_func and use it.

Attachment: gold-ifunc-2.patch
Description: Text document