This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch][rfc] Giving plugins access to gold's file views
- From: Rafael Avila de Espindola <respindola at mozilla dot com>
- To: Ian Lance Taylor <iant at google dot com>
- Cc: binutils at sourceware dot org, ccoutant at google dot com
- Date: Tue, 22 Mar 2011 10:57:06 -0400
- Subject: Re: [patch][rfc] Giving plugins access to gold's file views
- References: <4D826BCA.5050909@mozilla.com> <mcr4o6wdpog.fsf@google.com>
On 11-03-21 07:22 PM, Ian Lance Taylor wrote:
Rafael Avila de Espindola<respindola@mozilla.com> writes:
*) It is only possible to ask for views of the entire file. The reason
is that during LTO it is more efficient to merge files as we read
them, so I didn't see a lot of reason to add support to mapping only
parts of a file. In any case, I can add an offset and size argument if
you want.
*) The call works when called from the claim_file hook or from the
all_symbols_read hook. I only tested the first one, but making it work
on both cases looked easy.
*) The plugin has no guarantees of how long the view will be valid
for. It it wants to see the view in a subsequent callback, it has to
ask for it again.
*) The "aligned" and "cache" arguments are not exposed. Since the
behaviour with aligned set to true is the copy in into anonymous
memory, the plugin can just read the file itself. The "cache" value is
always set to false. If the plugin claims the file, it will probably
use it all at once. If it doesn't and gold then asks for a cached
view, find_or_make_view correctly calls set_cache.
If you are OK with this, I will finish it up (comments, interface
documentation and ChangeLog), split the bits that have to be committed
in gcc first and send an updated version for review.
I'm OK with this approach.
We already expose the file descriptor in the plugin interface, so this
is just for efficiency, right?
Correct. Running strace on a link I noticed many consecutive calls to
mmap on the same fd, one from gold, one from the plugin.
+ if (this->objects_.size() == handle) {
+ // We are being called from the claim_file hook.
+ const struct ld_plugin_input_file&f = this->plugin_input_file_;
+ offset = f.offset;
+ filesize = f.filesize;
+ input_file = this->input_file_;
+ } else {
+ // An already claimed file.
+ Pluginobj* obj = this->object(handle);
+ if (obj == NULL)
+ return LDPS_BAD_HANDLE;
+ offset = obj->offset();
+ filesize = obj->filesize();
+ input_file = obj->input_file();
+ }
Braces need to be GNU style.
Fixed. The two patches are attached.
2010-03-22 Rafael Ávila de Espíndola <respindola@mozilla.com>
* plugin-api.h (ld_plugin_get_view): New.
(ld_plugin_tag): Add LDPT_GET_VIEW.
(ld_plugin_tv): Add tv_get_view.
2010-03-22 Rafael Ávila de Espíndola <respindola@mozilla.com>
* plugin.cc (get_view): New.
(Plugin::load): Pass get_view to the plugin.
(Plugin_manager::get_view): New.
Ian
Cheers,
Rafael
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 956df00..7450a9e 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -203,6 +203,10 @@ enum ld_plugin_status
(*ld_plugin_get_input_file) (const void *handle,
struct ld_plugin_input_file *file);
+typedef
+enum ld_plugin_status
+(*ld_plugin_get_view) (const void *handle, const void **viewp);
+
/* The linker's interface for releasing the input file. */
typedef
@@ -269,7 +273,8 @@ enum ld_plugin_tag
LDPT_ADD_INPUT_LIBRARY,
LDPT_OUTPUT_NAME,
LDPT_SET_EXTRA_LIBRARY_PATH,
- LDPT_GNU_LD_VERSION
+ LDPT_GNU_LD_VERSION,
+ LDPT_GET_VIEW
};
/* The plugin transfer vector. */
@@ -289,6 +294,7 @@ struct ld_plugin_tv
ld_plugin_add_input_file tv_add_input_file;
ld_plugin_message tv_message;
ld_plugin_get_input_file tv_get_input_file;
+ ld_plugin_get_view tv_get_view;
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;
diff --git a/gold/plugin.cc b/gold/plugin.cc
index 214eff3..7e259e0 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -69,6 +69,9 @@ static enum ld_plugin_status
get_input_file(const void *handle, struct ld_plugin_input_file *file);
static enum ld_plugin_status
+get_view(const void *handle, const void **viewp);
+
+static enum ld_plugin_status
release_input_file(const void *handle);
static enum ld_plugin_status
@@ -130,7 +133,7 @@ Plugin::load()
sscanf(ver, "%d.%d", &major, &minor);
// Allocate and populate a transfer vector.
- const int tv_fixed_size = 16;
+ const int tv_fixed_size = 17;
int tv_size = this->args_.size() + tv_fixed_size;
ld_plugin_tv* tv = new ld_plugin_tv[tv_size];
@@ -189,6 +192,10 @@ Plugin::load()
tv[i].tv_u.tv_get_input_file = get_input_file;
++i;
+ tv[i].tv_tag = LDPT_GET_VIEW;
+ tv[i].tv_u.tv_get_view = get_view;
+
+ ++i;
tv[i].tv_tag = LDPT_RELEASE_INPUT_FILE;
tv[i].tv_u.tv_release_input_file = release_input_file;
@@ -635,6 +642,35 @@ Plugin_manager::release_input_file(unsigned int handle)
return LDPS_OK;
}
+ld_plugin_status
+Plugin_manager::get_view(unsigned int handle, const void **viewp)
+{
+ off_t offset;
+ size_t filesize;
+ Input_file *input_file;
+ if (this->objects_.size() == handle)
+ {
+ // We are being called from the claim_file hook.
+ const struct ld_plugin_input_file &f = this->plugin_input_file_;
+ offset = f.offset;
+ filesize = f.filesize;
+ input_file = this->input_file_;
+ }
+ else
+ {
+ // An already claimed file.
+ Pluginobj* obj = this->object(handle);
+ if (obj == NULL)
+ return LDPS_BAD_HANDLE;
+ offset = obj->offset();
+ filesize = obj->filesize();
+ input_file = obj->input_file();
+ }
+ *viewp = (void*) input_file->file().get_view(offset, 0, filesize, false,
+ false);
+ return LDPS_OK;
+}
+
// Add a new library path.
ld_plugin_status
@@ -1247,6 +1283,15 @@ release_input_file(const void* handle)
return parameters->options().plugins()->release_input_file(obj_index);
}
+static enum ld_plugin_status
+get_view(const void *handle, const void **viewp)
+{
+ gold_assert(parameters->options().has_plugins());
+ unsigned int obj_index =
+ static_cast<unsigned int>(reinterpret_cast<intptr_t>(handle));
+ return parameters->options().plugins()->get_view(obj_index, viewp);
+}
+
// Get the symbol resolution info for a plugin-claimed input file.
static enum ld_plugin_status
diff --git a/gold/plugin.h b/gold/plugin.h
index c26414d..87747bf 100644
--- a/gold/plugin.h
+++ b/gold/plugin.h
@@ -245,6 +245,9 @@ class Plugin_manager
ld_plugin_status
get_input_file(unsigned int handle, struct ld_plugin_input_file* file);
+ ld_plugin_status
+ get_view(unsigned int handle, const void **viewp);
+
// Release an input file.
ld_plugin_status
release_input_file(unsigned int handle);