Re: [patch][rfc] Giving plugins access to gold's file views

[Rafael Avila de Espindola <respindola at mozilla dot 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);