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]

Re: Plugin interfaces to do Pettis Hansen style code layout in the gold linker.


On Tue, Jul 5, 2011 at 2:29 PM, Ian Lance Taylor <iant@google.com> wrote:
>
> Sriraman Tallam <tmsriram@google.com> writes:
>
> > * layout.cc (Layout::Layout): Initialize section_ordering_specified_,
> > input_section_position_, and input_section_glob_.
> > (read_layout_from_file): Call function section_ordering_specified.
> > * layout.h (is_section_ordering_specified): New function.
> > (section_ordering_specified): New function.
> > (section_ordering_specified_): New boolean member.
> > * main.cc(main): Call load_plugins after layout object is defined.
> > * output.cc (Output_section::add_input_section): Use
> > function section_ordering_specified to check if section ordering is
> > needed.
> > * output.cc (Output_section::add_relaxed_input_section): Use
> > function section_ordering_specified to check if section ordering is
> > needed.
> > (Output_section::update_section_layout): New function.
> > (Output_section::sort_attached_input_sections): Check if input section
> > must be reordered.
> > * output.h (Output_section::update_section_layout): New function.
> > * plugin.cc (register_inspect_unclaimed_object): New function.
> > (get_section_count): New function.
> > (get_section_type): New function.
> > (get_section_name): New function.
> > (get_section_contents): New function.
> > (update_section_order): New function.
> > (allow_section_ordering): New function.
> > (Plugin::inspect_unclaimed_object): New function.
> > (Plugin_manager::inspect_unclaimed_object): New function.
> > (Plugin_manager::get_unclaimed_object): New function.
> > (Plugin::load): Add the new interfaces to the transfer vector.
> > (Plugin_manager::load_plugins): New parameter.
> > (Plugin_manager::all_symbols_read): New parameter.
> > * plugin.h (input_objects): New function
> > (Plugin::inspect_unclaimed_object): New function.
> > (Plugin::set_inspect_unclaimed_object_handler): New function.
> > (Plugin__manager::load_plugins): New parameter.
> > (Plugin_manager::inspect_unclaimed_object): New function.
> > (Plugin_manager::get_unclaimed_object): New function.
> > (Plugin_manager::in_inspect_unclaimed_object_handler): New function.
> > (Plugin_manager::set_inspect_unclaimed_object_handler): New function.
> > (Plugin_manager::unclaimed_object): New function.
> > (Plugin_manager::Unclaimed_object_list): New typedef.
> > (Plugin_manager::unclaimed_objects_): New member.
> > (Plugin_manager::in_inspect_unclaimed_object_handler_): New member.
> > (layout): New function.
> > * readsyms.cc (Read_symbols::do_read_symbols): Call the plugin handler.
> > * testsuite/plugin_test.c (register_inspect_unclaimed_object_hook):
> > New function pointer.
> > (get_section_count): New function pointer.
> > (get_section_type): New function pointer.
> > (get_section_name): New function pointer.
> > (get_section_contents): New function pointer.
> > (update_section_order): New function pointer.
> > (allow_section_ordering): New function pointer.
> > (inspect_unclaimed_object_hook): New function.
> > (onload): Check if the new interfaces exist.
> > * plugin-api.h
> > (unclaimed_obkect_handle): New typedef.
> > (ld_plugin_section): New struct.
> > (ld_plugin_inspect_unclaimed_object_handler): New typedef.
> > (ld_plugin_register_inspect_unclaimed_object): New typedef.
> > (ld_plugin_get_section_count): New typedef.
> > (ld_plugin_get_section_type): New typedef.
> > (ld_plugin_get_section_name): New typedef.
> > (ld_plugin_get_section_contents): New typedef.
> > (ld_plugin_update_section_order): New typedef.
> > (ld_plugin_allow_section_ordering): New typedef.
> > (LDPT_REGISTER_INSPECT_UNCLAIMED_OBJECT_HOOK): New enum value.
> > (LDPT_GET_SECTION_COUNT): New enum value.
> > (LDPT_GET_SECTION_TYPE): New enum value.
> > (LDPT_GET_SECTION_NAME): New enum value.
> > (LDPT_GET_SECTION_CONTENTS): New enum value.
> > (LDPT_UPDATE_SECTION_ORDER): New enum value.
> > (LDPT_ALLOW_SECTION_ORDERING): New enum value.
> > (tv_get_section_count): New struct members.
> > (tv_get_section_type): New struct members.
> > (tv_get_section_name): New struct members.
> > (tv_get_section_contents): New struct members.
> > (tv_update_section_order): New struct members.
> > (tv_allow_section_ordering): New struct members.
>
>
> > +static enum ld_plugin_status
> > +get_input_section_contents(const struct ld_plugin_section section,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? const unsigned char** section_contents,
> > + ? ? ? ? ? ? ? ? ? ? ? ?unsigned int* len);
>
> Let's change the len parameter from "unsigned int*" to "size_t*".
>
>
> > +// This updates the section order index of input sections according to the
> > +// the order specified in the mapping from Section id to order index.
> > +
> > +void
> > +Output_section::update_section_layout(
> > + ?const Section_layout_order& order_map)
> > +{
> > + ?for (Input_section_list::iterator p = this->input_sections_.begin();
> > + ? ? ? p != this->input_sections_.end();
> > + ? ? ? ++p)
> > + ? ?{
> > + ? ? ?if ((*p).is_input_section()
> > + ? ? ? || (*p).is_relaxed_input_section())
> > + ? ? ? ?{
> > + ? ? ? Object* obj = ((*p).is_input_section()
> > + ? ? ? ? ? ? ? ? ? ? ?? (*p).relobj()
> > + ? ? ? ? ? ? ? ? ? ? ?: (*p).relaxed_input_section()->relobj());
> > + ? ? ? unsigned int shndx = (*p).shndx();
> > + ? ? ? Section_layout_order::const_iterator it
> > + ? ? ? ? = order_map.find(Section_id(obj, shndx));
> > + ? ? ? if (it == order_map.end())
> > + ? ? ? ? continue;
> > + ? ? ? unsigned int section_order_index = it->second;
> > + ? ? ? if (section_order_index != 0)
> > + ? ? ? ? ? ?{
> > + ? ? ? ? ? ? ?(*p).set_section_order_index(section_order_index);
> > + ? ? ? ? ? ? ?this->set_input_section_order_specified();
> > + ? ? ? ? }
> > + ? ? ? ?}
> > + ? ?}
> > +}
>
> Please write "p->" instead of "(*p).".
>
>
> > +// Specify the ordering of sections in the final layout. The sections are
> > +// specified as (handle,shndx) pairs in the two arrays in the order in
> > +// which they should appear in the final layout.
> > +
> > +static enum ld_plugin_status
> > +update_section_order(const struct ld_plugin_section *section_list,
> > + ? ? ? ? ? ? ? ? ?unsigned int num_sections)
> > +{
> > + ?gold_assert(parameters->options().has_plugins());
> > +
> > + ?if (num_sections == 0)
> > + ? ?return LDPS_OK;
> > +
> > + ?if (section_list == NULL)
> > + ? ?return LDPS_ERR;
> > +
> > + ?std::map<Section_id, unsigned int> order_map;
> > +
> > + ?for (unsigned int i = 0; i < num_sections; ++i)
> > + ? ?{
> > + ? ? ?Object* obj = parameters->options().plugins()->get_unclaimed_object(
> > + ? ? ? ?section_list[i].handle);
> > + ? ? ?unsigned int shndx = section_list[i].shndx;
> > + ? ? ?Section_id secn_id(obj, shndx);
> > + ? ? ?order_map[secn_id] = i + 1;
> > + ? ?}
> > +
> > + ?Layout* layout = parameters->options().plugins()->layout();
> > + ?gold_assert (layout != NULL);
> > +
> > + ?for (Layout::Section_list::const_iterator p = layout->section_list().begin();
> > + ? ? ? p != layout->section_list().end();
> > + ? ? ? ++p)
> > + ? ?(*p)->update_section_layout(order_map);
> > +
> > + ?return LDPS_OK;
> > +}
>
> You need to check the return value of get_unclaimed_object, and return
> LDPS_ERR if it returns NULL.
>
>
> > + ? ? ?// Allow the plugins to inspect the unclaimed object now.
> > + ? ? ?if (parameters->options().has_plugins())
> > + ? ? ? ?parameters->options().plugins()->inspect_unclaimed_object(obj);
>
> Can you remind me of why we need both inspect_unclaimed_object and
> claim_file? ?To me it seems that it might be logical to let claim_file
> call get_section_count and so forth, and even update_section_order, and
> then simply set *claimed to 0 if it doesn't want to claim the file.
>
>
> > +/* The linker's interface for retrieving the name of specific section in
> > + ? an object. This interface should only be invoked in the
> > + ? inspect_unclaimed_object handler. ?This function sets *SECTION_NAME_PTR
> > + ? to a null-terminated buffer allocated by malloc. ?The plugin must free
> > + ? *SECTION_NAME_PTR. ?*/
>
> s/of specific section/of a specific section/
>
>
> > +/* The linker's interface for retrieving the contents of a specific section
> > + ? in an object. ?This interface should only be invoked in the
> > + ? inspect_unclaimed_object handler. ?This function sets *SECTION_CONTENTS to
> > + ? point to a buffer that is valid ?until inspect_unclaimed_object handler
> > + ? returns. ?It sets *LEN to the size of the buffer. ?*/
>
> There seems to be an extra space after "valid".
>
>
> The only important point in the above is whether we can eliminate
> inspect_unclaimed_object.

Originally, I added inspect_unclaimed_object because it needs an elf
object to be created before hand where claim_file does not create any
object before hand. However,  it looks like I can create an elf object
for claim_file which means I can inspect all objects in the claim_file
hook of the plugin. If the plugin claims the object then the elf
object will be deleted and replaced with the new Pluginobj that is
created. I will get back with the modified patch doing this.

Thanks,
-Sri.

>
> Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]