This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: Plugin interfaces to do Pettis Hansen style code layout in the gold linker.
- From: Cary Coutant <ccoutant at google dot com>
- To: Sriraman Tallam <tmsriram at google dot com>
- Cc: Ian Lance Taylor <iant at google dot com>, binutils <binutils at sourceware dot org>
- Date: Fri, 24 Jun 2011 15:24:21 -0700
- Subject: Re: Plugin interfaces to do Pettis Hansen style code layout in the gold linker.
- References: <AANLkTint2GrjcsfS0cjs1Hgkn4XvoCEQanBv_afztcrX@mail.gmail.com> <AANLkTikX7Z0q27LxNP_nHQnjZHcy0yEavkOo2C4MUcGK@mail.gmail.com> <AANLkTikWfZ_ru7Ltv=t8EOdL+b1xuwj_+wGnH4SaWxde@mail.gmail.com> <AANLkTimwq3L+TkKT3DFtgohfbi+1i-G+kCA0bZTXHkK7@mail.gmail.com> <AANLkTikWow8A06VhQTuhyFAOWupCwnZo8=qxZiq-XxEC@mail.gmail.com> <mcrwrk0xx31.fsf@google.com> <BANLkTikCPFWecCPaz-ZT3VBMJ9Acdx_TeqX=oCfXXWZENK73fQ@mail.gmail.com>
> ?Finally, I managed to make all these changes and I have attached the
> new patch. Thanks.
Here are my comments. This looks good to me (with changes as noted),
but you still need Ian's approval.
Thanks, and sorry for the delay!
BTW, it looks like you did the diff for plugin-api.h separately and
concatenated it to the rest of the patch. It would probably have been
better to do the diff from one level up, so that someone reading your
email can apply the patch more easily.
-cary
> Index: layout.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/layout.cc,v
> retrieving revision 1.197
> diff -u -u -p -r1.197 layout.cc
> --- layout.cc 25 May 2011 00:17:46 -0000 1.197
> +++ layout.cc 4 Jun 2011 00:26:16 -0000
> @@ -386,11 +386,14 @@ Layout::Layout(int number_of_input_files
> any_postprocessing_sections_(false),
> resized_signatures_(false),
> have_stabstr_section_(false),
> + section_ordering_specified_(false),
> incremental_inputs_(NULL),
> record_output_section_data_from_script_(false),
> script_output_section_data_list_(),
> segment_states_(NULL),
> relaxation_debug_check_(NULL),
> + input_section_position_(),
> + input_section_glob_(),
> incremental_base_(NULL),
> free_list_()
> {
> @@ -2063,7 +2066,7 @@ Layout::find_section_order_index(const s
> }
>
> // Read the sequence of input sections from the file specified with
> -// --section-ordering-file.
> +// option --section-ordering-file.
>
> void
> Layout::read_layout_from_file()
> @@ -2079,6 +2082,7 @@ Layout::read_layout_from_file()
>
> std::getline(in, line); // this chops off the trailing \n, if any
> unsigned int position = 1;
> + this->set_section_ordering_specified();
>
> while (in)
> {
> Index: layout.h
> ===================================================================
> RCS file: /cvs/src/src/gold/layout.h,v
> retrieving revision 1.91
> diff -u -u -p -r1.91 layout.h
> --- layout.h 24 May 2011 21:41:10 -0000 1.91
> +++ layout.h 4 Jun 2011 00:26:16 -0000
> @@ -492,6 +492,14 @@ class Layout
> const char* name, const elfcpp::Shdr<size, big_endian>& shdr,
> unsigned int reloc_shndx, unsigned int reloc_type, off_t* offset);
>
> + bool
> + is_section_ordering_specified()
> + { return section_ordering_specified_; }
Should be "this->section_ordering_specified_".
> +
> + void
> + set_section_ordering_specified()
> + { section_ordering_specified_ = true; }
Likewise.
> +
> // For incremental updates, allocate a block of memory from the
> // free list. Find a block starting at or after MINOFF.
> off_t
> @@ -501,6 +509,8 @@ class Layout
> unsigned int
> find_section_order_index(const std::string&);
>
> + // Read the sequence of input sections from the file specified with
> + // linker option --section-ordering-file.
> void
> read_layout_from_file();
>
> @@ -1224,6 +1234,9 @@ class Layout
> bool resized_signatures_;
> // Whether we have created a .stab*str output section.
> bool have_stabstr_section_;
> + // True if the input sections in the output sections should be sorted
> + // as specified in a section ordering file.
> + bool section_ordering_specified_;
> // In incremental build, holds information check the inputs and build the
> // .gnu_incremental_inputs section.
> Incremental_inputs* incremental_inputs_;
> Index: main.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/main.cc,v
> retrieving revision 1.43
> diff -u -u -p -r1.43 main.cc
> --- main.cc 12 Apr 2011 00:44:48 -0000 1.43
> +++ main.cc 4 Jun 2011 00:26:16 -0000
> @@ -195,10 +195,6 @@ main(int argc, char** argv)
> if (parameters->options().relocatable())
> command_line.script_options().version_script_info()->clear();
>
> - // Load plugin libraries.
> - if (command_line.options().has_plugins())
> - command_line.options().plugins()->load_plugins();
> -
> // The work queue.
> Workqueue workqueue(command_line.options());
>
> @@ -234,6 +230,10 @@ main(int argc, char** argv)
> if (parameters->options().section_ordering_file())
> layout.read_layout_from_file();
>
> + // Load plugin libraries.
> + if (command_line.options().has_plugins())
> + command_line.options().plugins()->load_plugins(&layout);
> +
> // Get the search path from the -L options.
> Dirsearch search_path;
> search_path.initialize(&workqueue, &command_line.options().library_path());
> Index: output.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/output.cc,v
> retrieving revision 1.148
> diff -u -u -p -r1.148 output.cc
> --- output.cc 2 Jun 2011 13:43:21 -0000 1.148
> +++ output.cc 4 Jun 2011 00:26:16 -0000
> @@ -2291,7 +2291,7 @@ Output_section::add_input_section(Layout
> && (sh_flags & elfcpp::SHF_EXECINSTR) != 0
> && parameters->target().has_code_fill()
> && (parameters->target().may_relax()
> - || parameters->options().section_ordering_file()))
> + || layout->is_section_ordering_specified()))
> {
> gold_assert(this->fills_.empty());
> this->generate_code_fills_at_write_ = true;
> @@ -2330,10 +2330,10 @@ Output_section::add_input_section(Layout
> || this->must_sort_attached_input_sections()
> || parameters->options().user_set_Map()
> || parameters->target().may_relax()
> - || parameters->options().section_ordering_file())
> + || layout->is_section_ordering_specified())
> {
> Input_section isecn(object, shndx, input_section_size, addralign);
> - if (parameters->options().section_ordering_file())
> + if (layout->is_section_ordering_specified())
> {
> unsigned int section_order_index =
> layout->find_section_order_index(std::string(secname));
> @@ -2414,7 +2414,7 @@ Output_section::add_relaxed_input_sectio
>
> // If the --section-ordering-file option is used to specify the order of
> // sections, we need to keep track of sections.
> - if (parameters->options().section_ordering_file())
> + if (layout->is_section_ordering_specified())
> {
> unsigned int section_order_index =
> layout->find_section_order_index(name);
> @@ -3266,6 +3266,39 @@ Output_section::Input_section_sort_secti
> return s1_secn_index < s2_secn_index;
> }
>
> +// 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(
> + std::map<Section_id, unsigned int>& order_map)
Make the parameter const. By convention, we don't use non-const references
as parameters.
A typedef for std::map<Section_id, unsigned int> might be nice.
> +{
> + 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());
> +
Trailing white space.
> + unsigned int shndx = (*p).shndx();
> + std::map<Section_id, unsigned int>::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();
> + }
> + }
> + }
> +}
> +
> // Sort the input sections attached to an output section.
>
> void
> @@ -3308,7 +3341,7 @@ Output_section::sort_attached_input_sect
> }
> else
> {
> - gold_assert(parameters->options().section_ordering_file());
> + gold_assert(this->input_section_order_specified());
> std::sort(sort_list.begin(), sort_list.end(),
> Input_section_sort_section_order_index_compare());
> }
> Index: output.h
> ===================================================================
> RCS file: /cvs/src/src/gold/output.h,v
> retrieving revision 1.124
> diff -u -u -p -r1.124 output.h
> --- output.h 25 May 2011 00:17:46 -0000 1.124
> +++ output.h 4 Jun 2011 00:26:16 -0000
> @@ -2665,6 +2665,9 @@ class Output_section : public Output_dat
> flags() const
> { return this->flags_; }
>
> + void
> + update_section_layout(std::map<Section_id, unsigned int>& order_map);
> +
> // Update the output section flags based on input section flags.
> void
> update_flags_for_input_section(elfcpp::Elf_Xword flags);
> Index: plugin.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/plugin.cc,v
> retrieving revision 1.47
> diff -u -u -p -r1.47 plugin.cc
> --- plugin.cc 25 May 2011 00:17:47 -0000 1.47
> +++ plugin.cc 4 Jun 2011 00:26:16 -0000
> @@ -57,6 +57,10 @@ static enum ld_plugin_status
> register_claim_file(ld_plugin_claim_file_handler handler);
>
> static enum ld_plugin_status
> +register_inspect_unclaimed_object(
> + ld_plugin_inspect_unclaimed_object_handler handler);
> +
> +static enum ld_plugin_status
> register_all_symbols_read(ld_plugin_all_symbols_read_handler handler);
>
> static enum ld_plugin_status
> @@ -89,6 +93,30 @@ set_extra_library_path(const char *path)
> static enum ld_plugin_status
> message(int level, const char *format, ...);
>
> +static enum ld_plugin_status
> +get_input_section_count(const unclaimed_object_handle handle,
> + unsigned int* count);
For extern "C" interfaces, use the C style: "unsigned int *count" (here and
the next 4 declarations).
> +
> +static enum ld_plugin_status
> +get_input_section_type(unclaimed_object_handle handle, unsigned int shndx,
> + unsigned int* type);
> +
> +static enum ld_plugin_status
> +get_input_section_name(unclaimed_object_handle handle, unsigned int shndx,
> + char** section_name_ptr);
> +
> +static enum ld_plugin_status
> +get_input_section_contents(unclaimed_object_handle handle, unsigned int shndx,
> + const unsigned char** section_contents,
> + unsigned int* len);
> +
> +static enum ld_plugin_status
> +update_section_order(void **handles, unsigned int *shndx,
> + unsigned int num_sections);
> +
> +static enum ld_plugin_status
> +allow_section_ordering();
> +
> };
>
> #endif // ENABLE_PLUGINS
> @@ -133,7 +161,8 @@ Plugin::load()
> sscanf(ver, "%d.%d", &major, &minor);
>
> // Allocate and populate a transfer vector.
> - const int tv_fixed_size = 17;
> + const int tv_fixed_size = 24;
> +
> int tv_size = this->args_.size() + tv_fixed_size;
> ld_plugin_tv* tv = new ld_plugin_tv[tv_size];
>
> @@ -176,6 +205,11 @@ Plugin::load()
> tv[i].tv_u.tv_register_claim_file = register_claim_file;
>
> ++i;
> + tv[i].tv_tag = LDPT_REGISTER_INSPECT_UNCLAIMED_OBJECT_HOOK;
> + tv[i].tv_u.tv_register_inspect_unclaimed_object
> + = register_inspect_unclaimed_object;
> +
> + ++i;
> tv[i].tv_tag = LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK;
> tv[i].tv_u.tv_register_all_symbols_read = register_all_symbols_read;
>
> @@ -216,6 +250,30 @@ Plugin::load()
> tv[i].tv_u.tv_set_extra_library_path = set_extra_library_path;
>
> ++i;
> + tv[i].tv_tag = LDPT_GET_INPUT_SECTION_COUNT;
> + tv[i].tv_u.tv_get_input_section_count = get_input_section_count;
> +
> + ++i;
> + tv[i].tv_tag = LDPT_GET_INPUT_SECTION_TYPE;
> + tv[i].tv_u.tv_get_input_section_type = get_input_section_type;
> +
> + ++i;
> + tv[i].tv_tag = LDPT_GET_INPUT_SECTION_NAME;
> + tv[i].tv_u.tv_get_input_section_name = get_input_section_name;
> +
> + ++i;
> + tv[i].tv_tag = LDPT_GET_INPUT_SECTION_CONTENTS;
> + tv[i].tv_u.tv_get_input_section_contents = get_input_section_contents;
> +
> + ++i;
> + tv[i].tv_tag = LDPT_UPDATE_SECTION_ORDER;
> + tv[i].tv_u.tv_update_section_order = update_section_order;
> +
> + ++i;
> + tv[i].tv_tag = LDPT_ALLOW_SECTION_ORDERING;
> + tv[i].tv_u.tv_allow_section_ordering = allow_section_ordering;
> +
> + ++i;
> tv[i].tv_tag = LDPT_NULL;
> tv[i].tv_u.tv_val = 0;
>
> @@ -244,6 +302,17 @@ Plugin::claim_file(struct ld_plugin_inpu
> return false;
> }
>
> +// Call the plugin inspect_unclaimed_object handler.
> +
> +inline void
> +Plugin::inspect_unclaimed_object(unsigned int index)
> +{
> + unclaimed_object_handle handle
> + = reinterpret_cast<unclaimed_object_handle>(index);
> + if (this->inspect_unclaimed_object_handler_ != NULL)
> + (*this->inspect_unclaimed_object_handler_)(handle);
> +}
> +
> // Call the all-symbols-read handler.
>
> inline void
> @@ -326,8 +395,9 @@ Plugin_manager::~Plugin_manager()
> // Load all plugin libraries.
>
> void
> -Plugin_manager::load_plugins()
> +Plugin_manager::load_plugins(Layout* layout)
> {
> + this->layout_ = layout;
> for (this->current_ = this->plugins_.begin();
> this->current_ != this->plugins_.end();
> ++this->current_)
> @@ -372,6 +442,27 @@ Plugin_manager::claim_file(Input_file* i
> return NULL;
> }
>
> +// Call the plugin inspect_unclaimed_object handlers to allow them to inspect
> +// the unclaimed object. Pass the handle of the object which is the index
> +// into the vector containing the unclaimed objects.
> +
> +void
> +Plugin_manager::inspect_unclaimed_object(Object* obj)
> +{
> + unsigned int index;
> + index = this->unclaimed_objects_.size();
How about converting index to an unclaimed_object_handle here, and
passing that to Plugin::inspect_unclaimed_object()?
> + this->unclaimed_objects_.push_back(obj);
> +
> + this->in_inspect_unclaimed_object_handler_ = true;
> +
> + for (this->current_ = this->plugins_.begin();
> + this->current_ != this->plugins_.end();
> + ++this->current_)
> + (*this->current_)->inspect_unclaimed_object(index);
> +
> + this->in_inspect_unclaimed_object_handler_ = false;
> +}
> +
> // Save an archive. This is used so that a plugin can add a file
> // which refers to a symbol which was not previously referenced. In
> // that case we want to pretend that the symbol was referenced before,
> @@ -402,7 +493,7 @@ Plugin_manager::save_input_group(Input_g
> void
> Plugin_manager::all_symbols_read(Workqueue* workqueue, Task* task,
> Input_objects* input_objects,
> - Symbol_table* symtab, Layout* layout,
> + Symbol_table* symtab,
> Dirsearch* dirpath, Mapfile* mapfile,
> Task_token** last_blocker)
> {
> @@ -411,7 +502,6 @@ Plugin_manager::all_symbols_read(Workque
> this->task_ = task;
> this->input_objects_ = input_objects;
> this->symtab_ = symtab;
> - this->layout_ = layout;
> this->dirpath_ = dirpath;
> this->mapfile_ = mapfile;
> this->this_blocker_ = NULL;
> @@ -642,6 +732,21 @@ Plugin_manager::release_input_file(unsig
> return LDPS_OK;
> }
>
> +// Get the object from the list of unclaimed objects with
> +// index specified in handle.
> +
> +Object*
> +Plugin_manager::get_unclaimed_object(unclaimed_object_handle handle)
> +{
> + unsigned int index
> + = static_cast<unsigned int>(reinterpret_cast<intptr_t>(handle));
> + Object* obj = NULL;
> +
> + obj = parameters->options().plugins()->unclaimed_object(index);
> +
> + return obj;
> +}
This can be simplified to:
Object*
Plugin_manager::get_unclaimed_object(unclaimed_object_handle handle)
{
unsigned int index =
static_cast<unsigned int>(reinterpret_cast<intptr_t>(handle));
return this->unclaimed_object(index);
}
> +
> ld_plugin_status
> Plugin_manager::get_view(unsigned int handle, const void **viewp)
> {
> @@ -1226,7 +1331,6 @@ Plugin_hook::run(Workqueue* workqueue)
> this,
> this->input_objects_,
> this->symtab_,
> - this->layout_,
> this->dirpath_,
> this->mapfile_,
> &this->this_blocker_);
> @@ -1248,6 +1352,18 @@ register_claim_file(ld_plugin_claim_file
> return LDPS_OK;
> }
>
> +//Register a inspect_unclaimed_object handler.
> +
> +static enum ld_plugin_status
> +register_inspect_unclaimed_object
> + (ld_plugin_inspect_unclaimed_object_handler handler)
> +{
> + gold_assert(parameters->options().has_plugins());
> + parameters->options().plugins()->set_inspect_unclaimed_object_handler(
> + handler);
> + return LDPS_OK;
> +}
> +
> // Register an all-symbols-read handler.
>
> static enum ld_plugin_status
> @@ -1384,6 +1500,145 @@ message(int level, const char* format, .
> return LDPS_OK;
> }
>
> +// Get the section count of the object corresponding to the handle. This
> +// plugin interface can only be called in the inspect_unclaimed_object
> +// handler of the plugin.
> +
> +static enum ld_plugin_status
> +get_input_section_count(const unclaimed_object_handle handle,
> + unsigned int* count)
> +{
> + gold_assert(parameters->options().has_plugins());
> +
> + if (!parameters->options().plugins()->in_inspect_unclaimed_object_handler())
> + return LDPS_ERR;
> +
> + Object* obj = parameters->options().plugins()->get_unclaimed_object(handle);
> +
> + if (obj == NULL)
> + return LDPS_BAD_HANDLE;
> +
> + *count = obj->shnum();
> + return LDPS_OK;
> +}
> +
> +// Get the type of the specified section in the object corresponding
> +// to the handle. This plugin interface can only be called in the
> +// inspect_unclaimed_object handler of the plugin.
> +
> +static enum ld_plugin_status
> +get_input_section_type(unclaimed_object_handle handle,
> + unsigned int shndx,
> + unsigned int* type)
> +{
> + gold_assert(parameters->options().has_plugins());
> +
> + if (!parameters->options().plugins()->in_inspect_unclaimed_object_handler())
> + return LDPS_ERR;
> +
> + Object* obj = parameters->options().plugins()->get_unclaimed_object(handle);
> +
> + if (obj == NULL)
> + return LDPS_BAD_HANDLE;
> +
> + *type = obj->section_type(shndx);
> + return LDPS_OK;
> +}
> +
> +// Get the name of the specified section in the object corresponding
> +// to the handle. This plugin interface can only be called in the
> +// inspect_unclaimed_object handler of the plugin.
> +
> +static enum ld_plugin_status
> +get_input_section_name(unclaimed_object_handle handle,
> + unsigned int shndx,
> + char** section_name_ptr)
> +{
> + gold_assert(parameters->options().has_plugins());
> +
> + if (!parameters->options().plugins()->in_inspect_unclaimed_object_handler())
> + return LDPS_ERR;
> +
> + Object* obj = parameters->options().plugins()->get_unclaimed_object(handle);
> +
> + if (obj == NULL)
> + return LDPS_BAD_HANDLE;
I'd suggest asserting "obj->is_locked()" here.
> +
> + const char* section_name = obj->section_name(shndx).c_str();
> + *section_name_ptr = static_cast<char*>(malloc(strlen(section_name) + 1));
> + strcpy(*section_name_ptr, section_name);
Will the plugin free this memory?
> + return LDPS_OK;
> +}
> +
> +// Get the contents of the specified section in the object corresponding
> +// to the handle. This plugin interface can only be called in the
> +// inspect_unclaimed_object handler of the plugin.
> +
> +static enum ld_plugin_status
> +get_input_section_contents(unclaimed_object_handle handle, unsigned int shndx,
> + const unsigned char** section_contents_ptr,
Spaces before tab.
> + unsigned int* len)
> +{
> + gold_assert(parameters->options().has_plugins());
> +
> + if (!parameters->options().plugins()->in_inspect_unclaimed_object_handler())
> + return LDPS_ERR;
> +
> + Object* obj = parameters->options().plugins()->get_unclaimed_object(handle);
> +
> + if (obj == NULL)
> + return LDPS_BAD_HANDLE;
Assert "obj->is_locked()".
> +
> + section_size_type plen;
> + *section_contents_ptr
> + = obj->section_contents(shndx, &plen, false);
> + *len = plen;
> + return LDPS_OK;
> +}
> +
> +// 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(unclaimed_object_handle* handles, unsigned int* shndx,
> + unsigned int num_sections)
> +{
> + gold_assert(parameters->options().has_plugins());
> + gold_assert(handles && shndx && num_sections);
Use this style for bools only. This should be:
gold_assert(handles != NULL && shndx != NULL && num_sections > 0);
On further consideration, none of these conditions should trigger an
internal error in gold -- these are API usage errors, and you should
return LDPS_ERR. If num_sections == 0, I'd be tempted to do nothing
and return LDPS_OK.
I'd suggest changing the name of the parameter "shndx" to "pshndx" or
"shndx_list". (Or, use an array of structs instead of parallel arrays.)
> +
> + 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(handles[i]);
> + unsigned int indx = shndx[i];
> + Section_id secn_id(obj, indx);
If you rename "shndx" as suggested above, you can use that name instead
of "indx" here. We use "shndx" throughout gold in this context.
> + order_map[secn_id] = i + 1;
> + }
> +
> + Layout* layout = parameters->options().plugins()->layout();
> +
Trailing white space.
I'd suggest asserting that "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;
> +}
> +
> +// Let the linker know that the sections could be reordered.
> +
> +static enum ld_plugin_status
> +allow_section_ordering()
> +{
> + gold_assert(parameters->options().has_plugins());
> + Layout* layout = parameters->options().plugins()->layout();
I'd suggest asserting that "layout != NULL".
> + layout->set_section_ordering_specified();
> + return LDPS_OK;
> +}
> +
> #endif // ENABLE_PLUGINS
>
> // Allocate a Pluginobj object of the appropriate size and endianness.
> Index: plugin.h
> ===================================================================
> RCS file: /cvs/src/src/gold/plugin.h,v
> retrieving revision 1.19
> diff -u -u -p -r1.19 plugin.h
> --- plugin.h 12 Apr 2011 00:44:48 -0000 1.19
> +++ plugin.h 4 Jun 2011 00:26:16 -0000
> @@ -58,6 +58,7 @@ class Plugin
> filename_(filename),
> args_(),
> claim_file_handler_(NULL),
> + inspect_unclaimed_object_handler_(NULL),
> all_symbols_read_handler_(NULL),
> cleanup_handler_(NULL),
> cleanup_done_(false)
> @@ -74,6 +75,10 @@ class Plugin
> bool
> claim_file(struct ld_plugin_input_file* plugin_input_file);
>
> + // Call the inspect_unclaimed_object handler.
> + void
> + inspect_unclaimed_object(unsigned int index);
> +
> // Call the all-symbols-read handler.
> void
> all_symbols_read();
> @@ -87,6 +92,12 @@ class Plugin
> set_claim_file_handler(ld_plugin_claim_file_handler handler)
> { this->claim_file_handler_ = handler; }
>
> + // Register a inspect_unclaimed_object handler.
> + void
> + set_inspect_unclaimed_object_handler(
> + ld_plugin_inspect_unclaimed_object_handler handler)
> + { this->inspect_unclaimed_object_handler_ = handler; }
> +
> // Register an all-symbols-read handler.
> void
> set_all_symbols_read_handler(ld_plugin_all_symbols_read_handler handler)
> @@ -116,6 +127,7 @@ class Plugin
> std::vector<std::string> args_;
> // The plugin's event handlers.
> ld_plugin_claim_file_handler claim_file_handler_;
> + ld_plugin_inspect_unclaimed_object_handler inspect_unclaimed_object_handler_;
> ld_plugin_all_symbols_read_handler all_symbols_read_handler_;
> ld_plugin_cleanup_handler cleanup_handler_;
> // TRUE if the cleanup handlers have been called.
> @@ -128,9 +140,11 @@ class Plugin_manager
> {
> public:
> Plugin_manager(const General_options& options)
> - : plugins_(), objects_(), deferred_layout_objects_(), input_file_(NULL),
> + : plugins_(), objects_(), unclaimed_objects_(),
> + deferred_layout_objects_(), input_file_(NULL),
> plugin_input_file_(), rescannable_(), undefined_symbols_(),
> any_claimed_(false), in_replacement_phase_(false), any_added_(false),
> + in_inspect_unclaimed_object_handler_(false),
> options_(options), workqueue_(NULL), task_(NULL), input_objects_(NULL),
> symtab_(NULL), layout_(NULL), dirpath_(NULL), mapfile_(NULL),
> this_blocker_(NULL), extra_search_path_()
> @@ -153,12 +167,26 @@ class Plugin_manager
>
> // Load all plugin libraries.
> void
> - load_plugins();
> + load_plugins(Layout* layout);
>
> // Call the plugin claim-file handlers in turn to see if any claim the file.
> Pluginobj*
> claim_file(Input_file* input_file, off_t offset, off_t filesize);
>
> + // Call the plugin inspect_unclaimed_object handlers to allow them
> + // to inspect the file.
> + void
> + inspect_unclaimed_object(Object* obj);
> +
> + Object*
> + get_unclaimed_object(unclaimed_object_handle handle);
> +
> + // True if the inspect_unclaimed_object handler of the plugins are being
> + // called.
s/are/is/
> + bool
> + in_inspect_unclaimed_object_handler()
> + { return in_inspect_unclaimed_object_handler_; }
> +
> // Let the plugin manager save an archive for later rescanning.
> // This takes ownership of the Archive pointer.
> void
> @@ -173,7 +201,7 @@ class Plugin_manager
> void
> all_symbols_read(Workqueue* workqueue, Task* task,
> Input_objects* input_objects, Symbol_table* symtab,
> - Layout* layout, Dirsearch* dirpath, Mapfile* mapfile,
> + Dirsearch* dirpath, Mapfile* mapfile,
> Task_token** last_blocker);
>
> // Tell the plugin manager that we've a new undefined symbol which
> @@ -197,6 +225,15 @@ class Plugin_manager
> (*this->current_)->set_claim_file_handler(handler);
> }
>
> + // Register a inspect_unclaimed_object handler.
> + void
> + set_inspect_unclaimed_object_handler(
> + ld_plugin_inspect_unclaimed_object_handler handler)
> + {
> + gold_assert(this->current_ != plugins_.end());
> + (*this->current_)->set_inspect_unclaimed_object_handler(handler);
> + }
> +
> // Register an all-symbols-read handler.
> void
> set_all_symbols_read_handler(ld_plugin_all_symbols_read_handler handler)
> @@ -227,6 +264,15 @@ class Plugin_manager
> return this->objects_[handle];
> }
>
> + // Return the unclaimed object associated with the given HANDLE.
> + Object*
> + unclaimed_object(unsigned int handle) const
> + {
> + if (handle >= this->unclaimed_objects_.size())
> + return NULL;
> + return this->unclaimed_objects_[handle];
> + }
> +
> // Return TRUE if any input files have been claimed by a plugin
> // and we are still in the initial input phase.
> bool
> @@ -265,6 +311,14 @@ class Plugin_manager
> in_replacement_phase() const
> { return this->in_replacement_phase_; }
>
> + Input_objects*
> + input_objects() const
> + { return input_objects_; }
Write "this->input_objects_".
> +
> + Layout*
> + layout()
> + { return layout_; }
Write "this->layout_".
> +
> private:
> Plugin_manager(const Plugin_manager&);
> Plugin_manager& operator=(const Plugin_manager&);
> @@ -294,6 +348,7 @@ class Plugin_manager
>
> typedef std::list<Plugin*> Plugin_list;
> typedef std::vector<Pluginobj*> Object_list;
> + typedef std::vector<Object*> Unclaimed_object_list;
> typedef std::vector<Relobj*> Deferred_layout_list;
> typedef std::vector<Rescannable> Rescannable_list;
> typedef std::vector<Symbol*> Undefined_symbol_list;
> @@ -315,6 +370,11 @@ class Plugin_manager
> // serves as the "handle" that we pass to the plugins.
> Object_list objects_;
>
> + // The list of unclaimed objects. The the index of an item in this
> + // in this list serves as the "handle" that we pass to the plugins
> + // in inspect_unclaimed_object_handler.
> + Unclaimed_object_list unclaimed_objects_;
> +
> // The list of regular objects whose layout has been deferred.
> Deferred_layout_list deferred_layout_objects_;
>
> @@ -340,6 +400,10 @@ class Plugin_manager
> // Whether any input files or libraries were added by a plugin.
> bool any_added_;
>
> + // Set to true when the inspect_unclaimed_object_handler of the plugins are being
> + // called.
> + bool in_inspect_unclaimed_object_handler_;
> +
> const General_options& options_;
> Workqueue* workqueue_;
> Task* task_;
> Index: readsyms.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/readsyms.cc,v
> retrieving revision 1.49
> diff -u -u -p -r1.49 readsyms.cc
> --- readsyms.cc 12 Apr 2011 00:44:48 -0000 1.49
> +++ readsyms.cc 4 Jun 2011 00:26:16 -0000
> @@ -381,6 +381,10 @@ Read_symbols::do_read_symbols(Workqueue*
> return false;
> }
>
> + // Allow the plugins to inspect the unclaimed object now.
> + if (parameters->options().has_plugins())
> + parameters->options().plugins()->inspect_unclaimed_object(obj);
> +
> Read_symbols_data* sd = new Read_symbols_data;
> obj->read_symbols(sd);
>
cvs diff: Diffing po
cvs diff: Diffing testsuite
> Index: testsuite/plugin_test.c
> ===================================================================
> RCS file: /cvs/src/src/gold/testsuite/plugin_test.c,v
> retrieving revision 1.4
> diff -u -u -p -r1.4 plugin_test.c
> --- testsuite/plugin_test.c 9 Nov 2009 16:11:34 -0000 1.4
> +++ testsuite/plugin_test.c 4 Jun 2011 00:26:17 -0000
> @@ -52,6 +52,8 @@ static struct claimed_file* first_claime
> static struct claimed_file* last_claimed_file = NULL;
>
> static ld_plugin_register_claim_file register_claim_file_hook = NULL;
> +static ld_plugin_register_inspect_unclaimed_object
> + register_inspect_unclaimed_object_hook = NULL;
> static ld_plugin_register_all_symbols_read register_all_symbols_read_hook = NULL;
> static ld_plugin_register_cleanup register_cleanup_hook = NULL;
> static ld_plugin_add_symbols add_symbols = NULL;
> @@ -60,6 +62,12 @@ static ld_plugin_add_input_file add_inpu
> static ld_plugin_message message = NULL;
> static ld_plugin_get_input_file get_input_file = NULL;
> static ld_plugin_release_input_file release_input_file = NULL;
> +static ld_plugin_get_input_section_count get_input_section_count = NULL;
> +static ld_plugin_get_input_section_type get_input_section_type = NULL;
> +static ld_plugin_get_input_section_name get_input_section_name = NULL;
> +static ld_plugin_get_input_section_contents get_input_section_contents = NULL;
> +static ld_plugin_update_section_order update_section_order = NULL;
> +static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
>
> #define MAXOPTS 10
>
> @@ -69,6 +77,7 @@ static int nopts = 0;
> enum ld_plugin_status onload(struct ld_plugin_tv *tv);
> enum ld_plugin_status claim_file_hook(const struct ld_plugin_input_file *file,
> int *claimed);
> +enum ld_plugin_status inspect_unclaimed_object_hook(void* handle);
> enum ld_plugin_status all_symbols_read_hook(void);
> enum ld_plugin_status cleanup_hook(void);
>
> @@ -101,6 +110,10 @@ onload(struct ld_plugin_tv *tv)
> case LDPT_REGISTER_CLAIM_FILE_HOOK:
> register_claim_file_hook = entry->tv_u.tv_register_claim_file;
> break;
> + case LDPT_REGISTER_INSPECT_UNCLAIMED_OBJECT_HOOK:
> + register_inspect_unclaimed_object_hook
> + = entry->tv_u.tv_register_inspect_unclaimed_object;
> + break;
> case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
> register_all_symbols_read_hook =
> entry->tv_u.tv_register_all_symbols_read;
> @@ -126,6 +139,24 @@ onload(struct ld_plugin_tv *tv)
> case LDPT_RELEASE_INPUT_FILE:
> release_input_file = entry->tv_u.tv_release_input_file;
> break;
> + case LDPT_GET_INPUT_SECTION_COUNT:
> + get_input_section_count = *entry->tv_u.tv_get_input_section_count;
> + break;
> + case LDPT_GET_INPUT_SECTION_TYPE:
> + get_input_section_type = *entry->tv_u.tv_get_input_section_type;
> + break;
> + case LDPT_GET_INPUT_SECTION_NAME:
> + get_input_section_name = *entry->tv_u.tv_get_input_section_name;
> + break;
> + case LDPT_GET_INPUT_SECTION_CONTENTS:
> + get_input_section_contents = *entry->tv_u.tv_get_input_section_contents;
> + break;
> + case LDPT_UPDATE_SECTION_ORDER:
> + update_section_order = *entry->tv_u.tv_update_section_order;
> + break;
> + case LDPT_ALLOW_SECTION_ORDERING:
> + allow_section_ordering = *entry->tv_u.tv_allow_section_ordering;
> + break;
> default:
> break;
> }
> @@ -143,6 +174,13 @@ onload(struct ld_plugin_tv *tv)
> return LDPS_ERR;
> }
>
> + if (register_inspect_unclaimed_object_hook == NULL)
> + {
> + fprintf(stderr,
> + "tv_register_inspect_unclaimed_object_hook interface missing\n");
> + return LDPS_ERR;
> + }
> +
> if (register_all_symbols_read_hook == NULL)
> {
> fprintf(stderr, "tv_register_all_symbols_read_hook interface missing\n");
> @@ -167,6 +205,14 @@ onload(struct ld_plugin_tv *tv)
> return LDPS_ERR;
> }
>
> + if ((*register_inspect_unclaimed_object_hook)
> + (inspect_unclaimed_object_hook) != LDPS_OK)
> + {
> + (*message)(LDPL_ERROR,
> + "error registering inspect unclaimed object hook");
> + return LDPS_ERR;
> + }
> +
> if ((*register_all_symbols_read_hook)(all_symbols_read_hook) != LDPS_OK)
> {
> (*message)(LDPL_ERROR, "error registering all symbols read hook");
> @@ -179,10 +225,53 @@ onload(struct ld_plugin_tv *tv)
> return LDPS_ERR;
> }
>
> + if (get_input_section_count == NULL)
> + {
> + fprintf(stderr, "tv_get_input_section_count interface missing\n");
> + return LDPS_ERR;
> + }
> +
> + if (get_input_section_type == NULL)
> + {
> + fprintf(stderr, "tv_get_input_section_type interface missing\n");
> + return LDPS_ERR;
> + }
> +
> + if (get_input_section_name == NULL)
> + {
> + fprintf(stderr, "tv_get_input_section_name interface missing\n");
> + return LDPS_ERR;
> + }
> +
> + if (get_input_section_contents == NULL)
> + {
> + fprintf(stderr, "tv_get_input_section_contents interface missing\n");
> + return LDPS_ERR;
> + }
> +
> + if (update_section_order == NULL)
> + {
> + fprintf(stderr, "tv_update_section_order interface missing\n");
> + return LDPS_ERR;
> + }
> +
> + if (allow_section_ordering == NULL)
> + {
> + fprintf(stderr, "tv_allow_section_ordering interface missing\n");
> + return LDPS_ERR;
> + }
> +
> return LDPS_OK;
> }
>
> enum ld_plugin_status
> +inspect_unclaimed_object_hook (void* handle)
> +{
> + (*message)(LDPL_INFO, "Inspect unclaimed object hook called with handle: %p",
> + handle);
> + return LDPS_OK;
> +}
> +enum ld_plugin_status
Need a blank line here.
> claim_file_hook (const struct ld_plugin_input_file* file, int* claimed)
> {
> int len;
> Index: include/plugin-api.h
> ===================================================================
> RCS file: /cvs/src/src/include/plugin-api.h,v
> retrieving revision 1.14
> diff -u -r1.14 plugin-api.h
> --- include/plugin-api.h 23 Mar 2011 14:09:48 -0000 1.14
> +++ include/plugin-api.h 4 Jun 2011 00:41:16 -0000
> @@ -157,6 +157,15 @@
> (*ld_plugin_claim_file_handler) (
> const struct ld_plugin_input_file *file, int *claimed);
>
> +/* The plugin library's unclaimed object handle. */
> +typedef void *unclaimed_object_handle;
> +
> +/* The plugin library's "inspect unclaimed object" handler. */
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_inspect_unclaimed_object_handler) (unclaimed_object_handle);
> +
> /* The plugin library's "all symbols read" handler. */
>
> typedef
> @@ -175,6 +184,14 @@
> enum ld_plugin_status
> (*ld_plugin_register_claim_file) (ld_plugin_claim_file_handler handler);
>
> +/* The linker's interface for registering the "inspect unclaimed object"
> + handler. */
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_register_inspect_unclaimed_object)
> + (ld_plugin_inspect_unclaimed_object_handler handler);
I think the open paren should be at the end of the previous line,
although I'm not sure if Gnu conventions insist on this. Still,
it would be consistent with the rest of the file.
> +
> /* The linker's interface for registering the "all symbols read" handler. */
>
> typedef
> @@ -244,6 +261,78 @@
> enum ld_plugin_status
> (*ld_plugin_message) (int level, const char *format, ...);
>
> +/* The linker's interface for retrieving the number of sections in an object.
> + The handle is obtained in the inspect_unclaimed_object_handler. This
> + interface should only be invoked in the inspect_unclaimed_object_handler.
> + This function sets *COUNT to the number of sections in the unclaimed
> + object. */
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_get_input_section_count) (unclaimed_object_handle handle,
> + unsigned int* count);
Use C-style conventions: "unsigned int *count". (Here and other
cases below.)
> +
> +/* The linker's interface for retrieving the section type of a specific
> + section in an object. The handle is obtained in the
> + inspect_unclaimed_object_handler. This interface should only be
> + invoked in the inspect_unclaimed_object_handler. This function sets
> + *TYPE to an ELF SHT_xxx value .*/
I think it would read better if you refer to the handler as the "inspect
unclaimed object" handler, like existing references to the "all symbols
read" handler. This would let you wrap lines better, and would sound
better with "the".
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_get_input_section_type) (unclaimed_object_handle handle,
> + unsigned int shndx,
> + unsigned int* type);
> +
> +/* The linker's interface for retrieving the name of specific section in
> + an object. The handle is obtained in the inspect_unclaimed_object_handler.
> + This interface should only be invoked in the
Trailing white space.
> + inspect_unclaimed_object_handler. This function sets *SECTION_NAME_PTR to
> + a null-terminated buffer allocated by malloc.*/
Need two spaces after ".". I think you should also say that the plugin
will own the pointer to that buffer, and is responsible for freeing it.
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_get_input_section_name) (unclaimed_object_handle handle,
> + unsigned int shndx,
> + char** section_name_ptr);
> +
> +/* The linker's interface for retrieving the contents of a specific section
> + in an object. The handle is obtained in the
> + inspect_unclaimed_object_handler. 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 with the size
s/with/to/
> + of the buffer. */
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_get_input_section_contents) (unclaimed_object_handle handle,
> + unsigned int shndx,
> + const unsigned char**
> + section_contents,
> + unsigned int* len);
> +
> +/* The linker's interface for specifying the desired order of sections.
> + The sections should be specifed as (handle, shndx) pairs using the arrays
> + HANDLES and SHNDX in the order in which they should appear in the final
> + layout. NUM_SECTIONS specifies the number of entries in each array.
> + This should be invoked in the all_symbols_read handler. */
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_update_section_order) (unclaimed_object_handle* handles,
> + unsigned int* shndx,
> + unsigned int num_sections);
Why use two parallel arrays instead of an array of structs?
> +
> +/* The linker's interface for specifying that reordering of sections is
> + desired so that the linker can prepare for it. This should be invoked
> + before update_section_order, preferably in the inspect_unclaimed_object
> + handler. */
> +
> +typedef
> +enum ld_plugin_status
> +(*ld_plugin_allow_section_ordering) (void);
> +
> enum ld_plugin_level
> {
> LDPL_INFO,
> @@ -262,6 +351,7 @@
> LDPT_LINKER_OUTPUT,
> LDPT_OPTION,
> LDPT_REGISTER_CLAIM_FILE_HOOK,
> + LDPT_REGISTER_INSPECT_UNCLAIMED_OBJECT_HOOK,
Adding this in the middle changes the API numbering, which would break
ABI compatibility with older plugins. Please move this to follow all the
original APIs.
> LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK,
> LDPT_REGISTER_CLEANUP_HOOK,
> LDPT_ADD_SYMBOLS,
> @@ -274,7 +364,13 @@
> LDPT_OUTPUT_NAME,
> LDPT_SET_EXTRA_LIBRARY_PATH,
> LDPT_GNU_LD_VERSION,
> - LDPT_GET_VIEW
> + LDPT_GET_VIEW,
> + LDPT_GET_INPUT_SECTION_COUNT,
> + LDPT_GET_INPUT_SECTION_TYPE,
> + LDPT_GET_INPUT_SECTION_NAME,
> + LDPT_GET_INPUT_SECTION_CONTENTS,
> + LDPT_UPDATE_SECTION_ORDER,
> + LDPT_ALLOW_SECTION_ORDERING
> };
>
> /* The plugin transfer vector. */
> @@ -287,6 +383,8 @@
> int tv_val;
> const char *tv_string;
> ld_plugin_register_claim_file tv_register_claim_file;
> + ld_plugin_register_inspect_unclaimed_object
> + tv_register_inspect_unclaimed_object;
Move this, too, to be consistent with the order in enum ld_plugin_level.
> ld_plugin_register_all_symbols_read tv_register_all_symbols_read;
> ld_plugin_register_cleanup tv_register_cleanup;
> ld_plugin_add_symbols tv_add_symbols;
> @@ -298,6 +396,12 @@
> ld_plugin_release_input_file tv_release_input_file;
> ld_plugin_add_input_library tv_add_input_library;
> ld_plugin_set_extra_library_path tv_set_extra_library_path;
> + ld_plugin_get_input_section_count tv_get_input_section_count;
> + ld_plugin_get_input_section_type tv_get_input_section_type;
> + ld_plugin_get_input_section_name tv_get_input_section_name;
> + ld_plugin_get_input_section_contents tv_get_input_section_contents;
> + ld_plugin_update_section_order tv_update_section_order;
> + ld_plugin_allow_section_ordering tv_allow_section_ordering;
> } tv_u;
> };
>