RE: [PATCH v2,GOLD] direct bindings support

["Liang, Zhenyu" <zhenyu dot liang at intel dot com>]

>On 3/11/13, Liang, Zhenyu <zhenyu.liang@intel.com> wrote:
>> I'm trying to add direct bindings support to GNU binutils. This 
>> feature was introduced by Solaris linker. See 
>> http://docs.oracle.com/cd/E19963-01/html/819-0690/gejgf.html for more 
>> details.
>> Here is my first patch for gold linker. Currently, support -Bdirect, 
>> -Bnodirect and -z [direct|nodirect] options.
>> Request to review.
>
>This looks pretty good.  Do you have a copyright assignment with the FSF?  As far as I know Intel prefers to make copyright assignments on an individual rather than a company basis.

No. Could you please send me the form? And I'll confirm with my manager.

>
>In general comments should be complete sentences with proper capitalization and punctuation.  A number of new comments needs to be fixed.
>
>> +  // Return the index of DT_NEEDED should be in the target shared object.
>
>That comment doesn't quite make sense as written.  Should "of" be "that"?  Same with the immediately following comment on set_dtndx.

Change to "Return the index of DT_NEEDED that this object should appear in the target shared object". Is this OK?

>
>> +  bool direct = false;
>
>This variable should be named is_direct.

I add a data member named "is_direct_enabled_" as a flag of whether direct bindings is enabled.
The DF_1_DIRECT should be set in dynamic section when the flag is true. I forgot to do this in my previous patch.

>
>> +      Output_section_data* sidata = new Output_data_fixed_space(index * 4, 4,
> >+								"** syminfo");
>
>Rather than a hard-coded 4 in "index * 4" I'd prefer to add the struct to Elf_sizes in elfcpp.h and use that.
>
>> +  const section_size_type syminfo_size = dynamic_count * 4;
>
>Use Elf_sizes here too.
>
>> +	      unsigned char* pi = syminfo_view + (dynsym_index * 4);
>
>Here too.

Agree.

>
>Thanks.
>
>Ian


ChangeLog of elfcpp/

2013-03-13  Zhenyu Liang  <zhenyu.liang@intel.com>

	* elfcpp.h (SHT_SUNW_syminfo, SYMINFO_FLG_DIRECT, SYMINFO_FLG_FILTER,
	SYMINFO_FLG_PASSTHRU, SYMINFO_FLG_COPY, SYMINFO_FLG_LAZYLOAD,
	SYMINFO_FLG_DIRECTBIND, SYMINFO_FLG_NOEXTDIRECT, SYMINFO_FLG_AUXILIARY,
	SYMINFO_FLG_INTERPOSE, SYMINFO_FLG_CAP, SYMINFO_FLG_DEFERRED,
	SYMINFO_BT_SELF, SYMINFO_BT_PARENT, SYMINFO_BT_NONE, SYMINFO_BT_EXTERN,
	SYMINFO_BT_LOWRESERVE, SYMINFO_NONE, SYMINFO_CURRENT, SYMINFO_NUM):
	New enum constants.
	(Elf_sizes::syminfo_size): New size constant.
	(class Syminfo, class Syminfo_write): New accessor classes.
	* elfcpp_internal.h (struct Syminfo_data): New struct.

ChangeLog of gold/

2013-03-13  Zhenyu Liang  <zhenyu.liang@intel.com>

	* dynobj.cc (Dynobj::dtndx_, Dynobj::dtndx, Dynobj::set_dtndx):
	New data member and accessors.
	(Dynobj::Dynobj): Initialize new data member.
	* layout.h (Layout::is_direct_enabled_, Layout::syminfo_section_):
	New data members.
	* layout.cc (Layout::Layout): Initialize new data members.
	(Layout::default_section_order): Add SHT_SUNW_syminfo section support.
	(Layout::create_symtab_sections): Call Symbol_table::finalize with the
	file offset of syminfo.
	(Layout::create_dynamic_symtab): Create syminfo section, if needed.
	(Layout::finish_dynamic_section): Add DF_1_DIRECT flag, if needed.
	* options.h (class General_options): Add -Bdirect and -Bnodirect
	options.
	(class Position_dependent_options): Add -z [direct|nodirect] options.
	* output.h (Output_data_dynamic::dtnum): New member function.
	* symtab.h (Symbol_table::syminfo_offset_): Add file offset of syminfo
	as parameter.
	* symtab.cc (Symbol_table::finalize): Generate and write syminfo data.

Attachment: binutils-trunk-gold-direct-v2.patch
Description: binutils-trunk-gold-direct-v2.patch