* [PATCH] Add plugin interface to LD [0/4]
@ 2010-09-23 5:07 Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions Dave Korn
` (6 more replies)
0 siblings, 7 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-23 5:07 UTC (permalink / raw)
To: binutils
Hi list,
The following set of patches implements the plugin API as seen in GOLD into
GNU LD. I've done it in a set of steps that first add the infrastructure,
then fill out the functionality in logical progression, and I'll comment on
noticeable points of each patch as I post them, but first a few overall comments.
It follows the design outlined at http://gcc.gnu.org/wiki/whopr/driver, with
one significant exception so far: the GET_INPUT_FILE and RELEASE_INPUT_FILE
methods aren't implemented yet (they return errors if called).
I think it's OK to leave these unimplemented for now, because the GCC LTO
plugin doesn't use them, and that's the main target of this work. I will add
them later, or sooner in the case that anything actually starts trying to use
them!
There are two other unsupported features as well: it doesn't handle
libraries yet (in terms of offering the individual members to the claim files
hook), and it doesn't support ELF visibility settings. These two things I
would like fix with follow-on patches over the coming days, but I think that
the feature as it stands is sufficiently complete to commit now anyway, for
the following reasons:
- ELF visibility handling doesn't need to be a first priority because ELF
targets should be using GOLD for best results anyway; this patch is sufficient
for non-ELF targets that don't have the option of using GOLD and don't need
visibility support anyway.
- Linking against library files still works, but the library contents won't be
available for LTO. The rest of the program still will be, and since
LTO-compiled objects (and hence archives) contain native assembly sections as
well as LTO sections, the compiled code will still work.
- Since we only release once a year, I'd really like to have this in 2.21, for
the sake of supporting LTO on non-ELF targets when GCC 4.6.0 comes out,
otherwise it'll be another year until the whole toolchain fully supports LTO
on COFF (et al.), even if there are bugs that mean it's not fully working
until 2.21.1 (although I fully hope to have it working for 2.20.0).
- Even though it's not fully complete, it's good enough to start porting work
on the GCC LTO plugin, which will hopefully shake out any remaining bugs
fairly quickly; like before the end of GCC stage 1 next month.
- It's only conditionally compiled in by a configure-time option, so adding it
now can't break anything; but it at least gives us the chance of having LTO
support in the 2.21/4.6 combination.
I've been testing it on both i686-pc-cygwin and i686-pc-linux-gnu (with
--enable-64-bit-bfd) and it works on both, and has the testcases to show it.
I hope it's not too controversial to commit right now because it really is at
the 95% point and pretty safe.
Notes on the implementation will follow in the mails carrying the individual
patches.
What does everyone think? Ok?
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions Dave Korn
@ 2010-09-23 5:09 ` Dave Korn
2010-09-23 5:36 ` Ralf Wildenhues
` (2 more replies)
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths Dave Korn
` (4 subsequent siblings)
6 siblings, 3 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-23 5:09 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 3741 bytes --]
This patch applies on top of the previous one to add the mentioned
functionality.
- When a plugin claims an input file, we create an empty BFD from scratch and
use it to store the symbols returned by the plugin, so that they can then take
part in the link and be resolved by the linker. Later on, we'll be able to
swap the resolved symbols in the linker hash table around so that they point
at the real versions of these symbols that are in the object files that the
plugin generates and adds back to the link.
- I added a single-bit flag to the input statement struct to track whether a
file has been claimed or not. I didn't actually end up using it yet, so I
could remove it if desired, but it's only a single bit and doesn't change the
total size of the struct any.
- There's a minor bug in the definition of IRONLY_SUFFIX_LEN causing
is_ir_dummy_bfd to always return FALSE, but this function isn't used until the
next patch anyway. (During an earlier stage of development I did use it
during this patch, until I reimplemented things slightly differently, but
since I knew I'd need it for the next patch anyway, I didn't take it out; at
which time it was corrected.)
- I'd certainly appreciate a second pair of eyes looking at the logic in
asymbol_from_plugin_symbol by which I derive the fags etc. for a bfd asymbol
from the ld plugin symbol struct.
ld/ChangeLog:
* Makefile.am (libldtestplug_la_LDFLAGS): Link against libiberty.
* Makefile.in: Regenerate.
* ldfile.c (ldfile_try_open_bfd)[ENABLE_PLUGINS]: Don't return early
during compat checks if they pass, instead offer any successfully
opened and accepted file to the plugin claim file hooks chain. Create
a dummy bfd to accept symbols added by the plugin, if the plugin
claims the file.
* ldlang.c (lang_process)[ENABLE_PLUGINS]: Call plugin all symbols
read hook chain before ldemul_after_open.
* ldlang.h (struct lang_input_statement_struct): Add new single-bit
'claimed' flag.
* plugin.c: Fix order of includes.
(plugin_get_ir_dummy_bfd): New function to create the dummy bfd used
to store symbols for ir-only files.
(is_ir_dummy_bfd): New function to check if a bfd is ir-only.
(asymbol_from_plugin_symbol): New function converts symbol formats.
(add_symbols): Call it to convert plugin syms to bfd syms and add
them to the dummy bfd.
* plugin.h: Add missing include guards.
(plugin_get_ir_dummy_bfd): Add prototype.
(is_ir_dummy_bfd): Likewise.
* testplug.c: Fix order of includes.
(TV_MESSAGE): New helper macro.
(struct claim_file): New struct.
(claim_file_t): New typedef.
(tag_names[]): Make static.
(claimfiles_list): New variable.
(claimfiles_tail_chain_ptr): Likewise.
(last_claimfile): Likewise.
(record_claim_file): Record a file to claim on a singly-linked list.
(parse_symdefstr): Parse an ASCII representation of a symbol from a
plugin option into the fields of a struct ld_plugin_symbol.
(record_claimed_file_symbol): Use it to parse plugin option for
adding a symbol.
(parse_option): Parse claim file and add symbol options.
(dump_tv_tag): Use TV_MESSAGE.
(onload): Likewise.
(onclaim_file): Make static. Use TV_MESSAGE. Scan list of files to
claim and claim this file if required, adding any symbols specified.
(onall_symbols_read): Make static and use TV_MESSAGE.
(oncleanup): Likewise.
ld/testsuite/ChangeLog:
* ld-plugin/plugin-3.d: Enable regexes for new functionality.
* ld-plugin/plugin-5.d: Likewise.
* ld-plugin/plugin-6.d: New testcase.
* ld-plugin/plugin-7.d: Likewise.
* ld-plugin/plugin.exp: Use 'nm' on compiled test objects to determine
whether symbols in plugin arguments need an underscore prefix. Add
new plugin-6.d and plugin-7.d testcases.
cheers,
DaveK
[-- Attachment #2: ld-plugin-api-2-claimfiles-addsyms.diff --]
[-- Type: text/x-c, Size: 34808 bytes --]
From 9a66dc5a47e91db76b656ea72ecebcab9504269c Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Mon, 20 Sep 2010 00:18:20 +0100
Subject: [PATCH] Implement claim file and all symbols read hooks and add symbols callback.
ld/ChangeLog:
* Makefile.am (libldtestplug_la_LDFLAGS): Link against libiberty.
* Makefile.in: Regenerate.
* ldfile.c (ldfile_try_open_bfd)[ENABLE_PLUGINS]: Don't return early
during compat checks if they pass, instead offer any successfully
opened and accepted file to the plugin claim file hooks chain. Create
a dummy bfd to accept symbols added by the plugin, if the plugin claims
the file.
* ldlang.c (lang_process)[ENABLE_PLUGINS]: Call plugin all symbols read
hook chain before ldemul_after_open.
* ldlang.h (struct lang_input_statement_struct): Add new single-bit
'claimed' flag.
* plugin.c: Fix order of includes.
(plugin_get_ir_dummy_bfd): New function to create the dummy bfd used
to store symbols for ir-only files.
(is_ir_dummy_bfd): New function to check if a bfd is ir-only.
(asymbol_from_plugin_symbol): New function converts symbol formats.
(add_symbols): Call it to convert plugin syms to bfd syms and add
them to the dummy bfd.
* plugin.h: Add missing include guards.
(plugin_get_ir_dummy_bfd): Add prototype.
(is_ir_dummy_bfd): Likewise.
* testplug.c: Fix order of includes.
(TV_MESSAGE): New helper macro.
(struct claim_file): New struct.
(claim_file_t): New typedef.
(tag_names[]): Make static.
(claimfiles_list): New variable.
(claimfiles_tail_chain_ptr): Likewise.
(last_claimfile): Likewise.
(record_claim_file): Record a file to claim on a singly-linked list.
(parse_symdefstr): Parse an ASCII representation of a symbol from a
plugin option into the fields of a struct ld_plugin_symbol.
(record_claimed_file_symbol): Use it to parse plugin option for
adding a symbol.
(parse_option): Parse claim file and add symbol options.
(dump_tv_tag): Use TV_MESSAGE.
(onload): Likewise.
(onclaim_file): Make static. Use TV_MESSAGE. Scan list of files to
claim and claim this file if required, adding any symbols specified.
(onall_symbols_read): Make static and use TV_MESSAGE.
(oncleanup): Likewise.
ld/testsuite/ChangeLog:
* ld-plugin/plugin-3.d: Enable regexes for new functionality.
* ld-plugin/plugin-5.d: Likewise.
* ld-plugin/plugin-6.d: New testcase.
* ld-plugin/plugin-7.d: Likewise.
* ld-plugin/plugin.exp: Use 'nm' on compiled test objects to determine
whether symbols in plugin arguments need an underscore prefix. Add
new plugin-6.d and plugin-7.d testcases.
---
ld/Makefile.am | 2 +-
ld/Makefile.in | 2 +-
ld/ldfile.c | 72 +++++++++++-
ld/ldlang.c | 12 ++
ld/ldlang.h | 4 +
ld/plugin.c | 100 +++++++++++++++-
ld/plugin.h | 17 +++
ld/testplug.c | 240 +++++++++++++++++++++++++++++++------
ld/testsuite/ld-plugin/plugin-3.d | 5 +-
ld/testsuite/ld-plugin/plugin-5.d | 14 +--
ld/testsuite/ld-plugin/plugin-6.d | 31 +++++
ld/testsuite/ld-plugin/plugin-7.d | 30 +++++
ld/testsuite/ld-plugin/plugin.exp | 39 +++++-
13 files changed, 505 insertions(+), 63 deletions(-)
diff --git a/ld/Makefile.am b/ld/Makefile.am
index 5b0c74c..07ee3a8 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
noinst_LTLIBRARIES = libldtestplug.la
libldtestplug_la_SOURCES = testplug.c
libldtestplug_la_CFLAGS= -g -O2
-libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
+libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
endif
# DOCUMENTATION TARGETS
diff --git a/ld/ldfile.c b/ld/ldfile.c
index 4661897..d010fcb 100644
--- a/ld/ldfile.c
+++ b/ld/ldfile.c
@@ -34,6 +34,10 @@
#include "ldemul.h"
#include "libiberty.h"
#include "filenames.h"
+#ifdef ENABLE_PLUGINS
+#include "plugin-api.h"
+#include "plugin.h"
+#endif /* ENABLE_PLUGINS */
const char * ldfile_input_filename;
bfd_boolean ldfile_assumed_script = FALSE;
@@ -150,7 +154,12 @@ ldfile_try_open_bfd (const char *attempt,
compatible with the output file. If it isn't, keep searching.
If we can't open the file as an object file, stop the search
here. If we are statically linking, ensure that we don't link
- a dynamic object. */
+ a dynamic object.
+
+ In the code below, it's OK to exit early if the check fails,
+ closing the checked BFD and returning FALSE, but if the BFD
+ checks out compatible, do not exit early returning TRUE, or
+ the plugins will not get a chance to claim the file. */
if (entry->search_dirs_flag || !entry->dynamic)
{
@@ -163,6 +172,7 @@ ldfile_try_open_bfd (const char *attempt,
if (check != NULL)
{
+ bfd_boolean checked_ok = FALSE;
if (! bfd_check_format (check, bfd_object))
{
if (check == entry->the_bfd
@@ -259,10 +269,11 @@ ldfile_try_open_bfd (const char *attempt,
return FALSE;
}
}
- return TRUE;
+ checked_ok = TRUE;
}
- if (!entry->dynamic && (entry->the_bfd->flags & DYNAMIC) != 0)
+ if (!checked_ok && !entry->dynamic
+ && (entry->the_bfd->flags & DYNAMIC) != 0)
{
einfo (_("%F%P: attempted static link of dynamic object `%s'\n"),
attempt);
@@ -271,7 +282,7 @@ ldfile_try_open_bfd (const char *attempt,
return FALSE;
}
- if (entry->search_dirs_flag
+ if (!checked_ok && entry->search_dirs_flag
&& !bfd_arch_get_compatible (check, link_info.output_bfd,
command_line.accept_unknown_input_arch)
/* XCOFF archives can have 32 and 64 bit objects. */
@@ -290,6 +301,59 @@ ldfile_try_open_bfd (const char *attempt,
}
}
+#ifdef ENABLE_PLUGINS
+ /* If plugins are active, they get first chance to claim
+ any successfully-opened input file. We skip archives
+ here; the plugin wants us to offer it the individual
+ members when we enumerate them, not the whole file. We
+ also ignore corefiles, because that's just weird. It is
+ a needed side-effect of calling bfd_check_format with
+ bfd_object that it sets the bfd's arch and mach, which
+ will be needed when and if we want to bfd_create a new
+ one using this one as a template. */
+ int fildes = bfd_check_format (entry->the_bfd, bfd_object)
+ ? open (attempt, O_RDONLY
+# ifdef O_BINARY
+ | O_BINARY
+# endif /* O_BINARY */
+ )
+ : -1;
+ if (fildes >= 0)
+ {
+ struct ld_plugin_input_file file;
+
+ int claimed = 0;
+ file.name = attempt;
+ file.offset = 0;
+ file.filesize = lseek (fildes, 0, SEEK_END);
+ file.fd = fildes;
+ /* We create a dummy BFD, initially empty, to house
+ whatever symbols the plugin may want to add. */
+ file.handle = plugin_get_ir_dummy_bfd (attempt, entry->the_bfd);
+ if (plugin_call_claim_file (&file, &claimed))
+ einfo (_("%P%F: %s: plugin reported error claiming file\n"),
+ plugin_error_plugin ());
+ if (claimed)
+ {
+ /* Discard the real file's BFD and substitute the dummy one. */
+ bfd_close (entry->the_bfd);
+ entry->the_bfd = file.handle;
+ entry->claimed = TRUE;
+ bfd_make_readable (entry->the_bfd);
+ }
+ else
+ {
+ /* If plugin didn't claim the file, we don't need the fd or the
+ dummy bfd. Can't avoid speculatively creating it, alas. */
+ bfd_close_all_done (file.handle);
+ close (fildes);
+ entry->claimed = FALSE;
+ }
+ }
+#endif /* ENABLE_PLUGINS */
+
+ /* It opened OK, the format checked out, and the plugins have had
+ their chance to claim it, so this is success. */
return TRUE;
}
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 7f44445..cb1417c 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -40,6 +40,9 @@
#include "fnmatch.h"
#include "demangle.h"
#include "hashtab.h"
+#ifdef ENABLE_PLUGINS
+#include "plugin.h"
+#endif /* ENABLE_PLUGINS */
#ifndef offsetof
#define offsetof(TYPE, MEMBER) ((size_t) & (((TYPE*) 0)->MEMBER))
@@ -6340,6 +6343,15 @@ lang_process (void)
if (entry_symbol.name == NULL)
link_info.gc_sym_list = ldlang_undef_chain_list_head;
+#ifdef ENABLE_PLUGINS
+ /* Now all files are read, let the plugin(s) decide if there
+ are any more to be added to the link before we call the
+ emulation's after_open hook. */
+ if (plugin_call_all_symbols_read ())
+ einfo (_("%P%F: %s: plugin reported error after all symbols read\n"),
+ plugin_error_plugin ());
+#endif /* ENABLE_PLUGINS */
+
ldemul_after_open ();
bfd_section_already_linked_table_free ();
diff --git a/ld/ldlang.h b/ld/ldlang.h
index e9bcacf..f89eb53 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -286,6 +286,10 @@ typedef struct lang_input_statement_struct
/* Set if the file does not exist. */
unsigned int missing_file : 1;
+
+ /* Set if the file was claimed by a plugin. */
+ unsigned int claimed : 1;
+
} lang_input_statement_type;
typedef struct
diff --git a/ld/plugin.c b/ld/plugin.c
index e9f3b81..6c71978 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -18,8 +18,6 @@
Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
MA 02110-1301, USA. */
-#include <dlfcn.h>
-
#include "sysdep.h"
#include "libiberty.h"
#include "bfd.h"
@@ -30,6 +28,9 @@
#include "ldmisc.h"
#include "plugin.h"
#include "plugin-api.h"
+/* Don't include std headers until after config.h, sysdeps.h etc.,
+ or you may end up with the wrong bitsize of off_t. */
+#include <dlfcn.h>
/* Alias to shorten static function prototype lines. */
#define PLUGAPIFUNC static enum ld_plugin_status
@@ -50,6 +51,13 @@ PLUGAPIFUNC add_input_file (const char *pathname);
PLUGAPIFUNC add_input_library (const char *pathname);
PLUGAPIFUNC set_extra_library_path (const char *path);
+/* The suffix to append to the name of the real (claimed) object file
+ when generating a dummy BFD to hold the IR symbols sent from the
+ plugin. */
+#define IRONLY_SUFFIX ".ironly\004"
+/* This is sizeof an array of chars, not sizeof a const char *. */
+#define IRONLY_SUFFIX_LEN (sizeof (IRONLY_SUFFIX))
+
/* Always use this macro when invoking a plugin function. */
#define INVOKE_PLUGIN_FN(plugin, retval, fn, args) \
called_plugin = plugin; \
@@ -376,6 +384,75 @@ plugin_call_cleanup (void)
return plugin_error_p () ? -1 : 0;
}
+/* Create a dummy BFD. */
+bfd *
+plugin_get_ir_dummy_bfd (const char *name, bfd *srctemplate)
+{
+ asection *sec;
+ bfd *abfd = bfd_create (
+ concat (name, IRONLY_SUFFIX, (const char *)NULL),
+ srctemplate);
+ bfd_set_arch_info (abfd, bfd_get_arch_info (srctemplate));
+ bfd_make_writable (abfd);
+ /* Create a minimal set of sections to own the symbols. */
+ sec = bfd_make_section_old_way (abfd, ".text");
+ bfd_set_section_flags (abfd, sec,
+ SEC_CODE | SEC_HAS_CONTENTS | SEC_READONLY
+ | SEC_ALLOC | SEC_LOAD | SEC_KEEP);
+ sec->output_section = sec;
+ sec->output_offset = 0;
+ return abfd;
+}
+
+/* Check if the BFD is an IR dummy. */
+bfd_boolean
+is_ir_dummy_bfd (const bfd *abfd)
+{
+ size_t namlen = strlen (abfd->filename);
+ if (namlen < IRONLY_SUFFIX_LEN)
+ return FALSE;
+ return !strcmp (abfd->filename + namlen - IRONLY_SUFFIX_LEN, IRONLY_SUFFIX);
+}
+
+/* Helpers to convert between BFD and GOLD symbol formats. */
+static enum ld_plugin_status
+asymbol_from_plugin_symbol (bfd *abfd, asymbol *asym,
+ const struct ld_plugin_symbol *ldsym)
+{
+ asym->the_bfd = abfd;
+ asym->name = ldsym->version
+ ? concat (ldsym->name, "@", ldsym->version, NULL)
+ : ldsym->name;
+ asym->value = 0;
+ switch (ldsym->def)
+ {
+ case LDPK_DEF:
+ asym->flags = BSF_GLOBAL;
+ break;
+ case LDPK_WEAKDEF:
+ asym->flags = BSF_GLOBAL | BSF_WEAK;
+ break;
+ case LDPK_UNDEF:
+ asym->flags = BSF_NO_FLAGS;
+ break;
+ case LDPK_WEAKUNDEF:
+ asym->flags = BSF_NO_FLAGS | BSF_WEAK;
+ break;
+ case LDPK_COMMON:
+ asym->flags = BSF_GLOBAL;
+ asym->value = ldsym->size;
+ break;
+ default:
+ return LDPS_ERR;
+ }
+ asym->section = ldsym->def == LDPK_COMMON
+ ? bfd_com_section_ptr
+ : (ldsym->def == LDPK_UNDEF || ldsym->def == LDPK_WEAKUNDEF
+ ? bfd_und_section_ptr
+ : bfd_get_section_by_name (abfd, ".text"));
+ return LDPS_OK;
+}
+
/* Register a claim-file handler. */
static enum ld_plugin_status
register_claim_file (ld_plugin_claim_file_handler handler)
@@ -407,11 +484,22 @@ register_cleanup (ld_plugin_cleanup_handler handler)
static enum ld_plugin_status
add_symbols (void *handle, int nsyms, const struct ld_plugin_symbol *syms)
{
+ asymbol **symptrs;
+ bfd *abfd = handle;
+ int n;
ASSERT (called_plugin);
- handle = handle;
- nsyms = nsyms;
- syms = syms;
- return LDPS_ERR;
+ symptrs = xmalloc (nsyms * sizeof *symptrs);
+ for (n = 0; n < nsyms; n++)
+ {
+ enum ld_plugin_status rv;
+ asymbol *bfdsym = bfd_make_empty_symbol (abfd);
+ symptrs[n] = bfdsym;
+ rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms++);
+ if (rv != LDPS_OK)
+ return rv;
+ }
+ bfd_set_symtab (abfd, symptrs, nsyms);
+ return LDPS_OK;
}
/* Get the input file information with an open (possibly re-opened)
diff --git a/ld/plugin.h b/ld/plugin.h
index fe3d148..8839b8b 100644
--- a/ld/plugin.h
+++ b/ld/plugin.h
@@ -18,6 +18,10 @@
Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
MA 02110-1301, USA. */
+#ifndef GLD_PLUGIN_H
+#define GLD_PLUGIN_H
+
+
/* This is the only forward declaration we need to avoid having
to include the plugin-api.h header in order to use this file. */
struct ld_plugin_input_file;
@@ -44,3 +48,16 @@ extern int plugin_call_all_symbols_read (void);
/* Call 'cleanup' hook for all plugins. */
extern int plugin_call_cleanup (void);
+/* Generate a dummy BFD to represent an IR file, for any callers of
+ plugin_call_claim_file to use as the handle in the ld_plugin_input_file
+ struct that they build to pass in. The BFD is initially writable, so
+ that symbols can be added to it; it must be made readable after the
+ add_symbols hook has been called so that it can be read when linking. */
+extern bfd *plugin_get_ir_dummy_bfd (const char *name, bfd *template);
+
+/* Check if the BFD passed in is an IR dummy object file. */
+extern bfd_boolean is_ir_dummy_bfd (const bfd *abfd);
+
+
+#endif /* !def GLD_PLUGIN_H */
+
diff --git a/ld/testplug.c b/ld/testplug.c
index 46d7c0f..9746465 100644
--- a/ld/testplug.c
+++ b/ld/testplug.c
@@ -18,27 +18,48 @@
Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
MA 02110-1301, USA. */
-#include <stdio.h>
-#include <string.h>
#include "config.h"
#include "bfd.h"
#include "libiberty.h"
#include "plugin-api.h"
+/* Don't include std headers until after config.h, sysdeps.h etc.,
+ or you may end up with the wrong bitsize of off_t. */
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
extern enum ld_plugin_status onload (struct ld_plugin_tv *tv);
-enum ld_plugin_status onclaim_file (const struct ld_plugin_input_file *file, int *claimed);
-enum ld_plugin_status onall_symbols_read (void);
-enum ld_plugin_status oncleanup (void);
+static enum ld_plugin_status onclaim_file (
+ const struct ld_plugin_input_file *file, int *claimed);
+static enum ld_plugin_status onall_symbols_read (void);
+static enum ld_plugin_status oncleanup (void);
+
+/* Helper for calling plugin api message function. */
+#define TV_MESSAGE if (tv_message) (*tv_message)
+/* Struct for recording files to claim / files claimed. */
+typedef struct claim_file
+{
+ struct claim_file *next;
+ struct ld_plugin_input_file file;
+ bfd_boolean claimed;
+ struct ld_plugin_symbol *symbols;
+ int n_syms_allocated;
+ int n_syms_used;
+} claim_file_t;
+
+/* Helper macro for defining array of transfer vector tags and names. */
#define ADDENTRY(tag) { tag, #tag }
+/* Struct for looking up human-readable versions of tag names. */
typedef struct tag_name
{
enum ld_plugin_tag tag;
const char *name;
} tag_name_t;
-tag_name_t tag_names[] =
+/* Array of all known tags and their names. */
+static const tag_name_t tag_names[] =
{
ADDENTRY(LDPT_NULL),
ADDENTRY(LDPT_API_VERSION),
@@ -86,6 +107,128 @@ static bfd_boolean register_claimfile_hook = FALSE;
static bfd_boolean register_allsymbolsread_hook = FALSE;
static bfd_boolean register_cleanup_hook = FALSE;
+/* The master list of all claimable/claimed files. */
+static claim_file_t *claimfiles_list = NULL;
+
+/* We keep a tail pointer for easy linking on the end. */
+static claim_file_t **claimfiles_tail_chain_ptr = &claimfiles_list;
+
+/* The last claimed file added to the list, for receiving syms. */
+static claim_file_t *last_claimfile = NULL;
+
+/* Add a new claimfile on the end of the chain. */
+static enum ld_plugin_status
+record_claim_file (const char *file)
+{
+ claim_file_t *newfile;
+
+ newfile = xmalloc (sizeof *newfile);
+ memset (newfile, 0, sizeof *newfile);
+ /* Only setup for now is remembering the name to look for. */
+ newfile->file.name = file;
+ /* Chain it on the end of the list. */
+ *claimfiles_tail_chain_ptr = newfile;
+ claimfiles_tail_chain_ptr = &newfile->next;
+ /* Record it as active for receiving symbols to register. */
+ last_claimfile = newfile;
+ return LDPS_OK;
+}
+
+/* Parse a command-line argument string into a symbol definition.
+ Symbol-strings follow the colon-separated format:
+ NAME:VERSION:def:vis:size:COMDATKEY
+ where the fields in capitals are strings and those in lower
+ case are integers. We don't allow to specify a resolution as
+ doing so is not meaningful when calling the add symbols hook. */
+static enum ld_plugin_status
+parse_symdefstr (const char *str, struct ld_plugin_symbol *sym)
+{
+ int n;
+ long long size;
+ const char *colon1, *colon2, *colon5;
+
+ /* Locate the colons separating the first two strings. */
+ colon1 = strchr (str, ':');
+ if (!colon1)
+ return LDPS_ERR;
+ colon2 = strchr (colon1+1, ':');
+ if (!colon2)
+ return LDPS_ERR;
+ /* Name must not be empty (version may be). */
+ if (colon1 == str)
+ return LDPS_ERR;
+
+ /* The fifth colon and trailing comdat key string are optional,
+ but the intermediate ones must all be present. */
+ colon5 = strchr (colon2+1, ':'); /* Actually only third so far. */
+ if (!colon5)
+ return LDPS_ERR;
+ colon5 = strchr (colon5+1, ':'); /* Hopefully fourth now. */
+ if (!colon5)
+ return LDPS_ERR;
+ colon5 = strchr (colon5+1, ':'); /* Optional fifth now. */
+
+ /* Finally we'll use sscanf to parse the numeric fields, then
+ we'll split out the strings which we need to allocate separate
+ storage for anyway so that we can add nul termination. */
+ n = sscanf (colon2 + 1, "%i:%i:%lli", &sym->def, &sym->visibility, &size);
+ if (n != 3)
+ return LDPS_ERR;
+
+ /* Parsed successfully, so allocate strings and fill out fields. */
+ sym->size = size;
+ sym->resolution = LDPR_UNKNOWN;
+ sym->name = malloc (colon1 - str + 1);
+ memcpy (sym->name, str, colon1 - str);
+ sym->name[colon1 - str] = '\0';
+ if (colon2 > (colon1 + 1))
+ {
+ sym->version = malloc (colon2 - colon1);
+ memcpy (sym->version, colon1 + 1, colon2 - (colon1 + 1));
+ sym->version[colon2 - (colon1 + 1)] = '\0';
+ }
+ else
+ sym->version = NULL;
+ if (colon5 && colon5[1])
+ {
+ sym->comdat_key = malloc (strlen (colon5 + 1) + 1);
+ strcpy (sym->comdat_key, colon5 + 1);
+ }
+ else
+ sym->comdat_key = 0;
+ return LDPS_OK;
+}
+
+/* Record a symbol to be added for the last-added claimfile. */
+static enum ld_plugin_status
+record_claimed_file_symbol (const char *symdefstr)
+{
+ struct ld_plugin_symbol sym;
+
+ /* Can't add symbols except as belonging to claimed files. */
+ if (!last_claimfile)
+ return LDPS_ERR;
+
+ /* If string doesn't parse correctly, give an error. */
+ if (parse_symdefstr (symdefstr, &sym) != LDPS_OK)
+ return LDPS_ERR;
+
+ /* Check for enough space, resize array if needed, and add it. */
+ if (last_claimfile->n_syms_allocated == last_claimfile->n_syms_used)
+ {
+ int new_n_syms = last_claimfile->n_syms_allocated
+ ? 2 * last_claimfile->n_syms_allocated
+ : 10;
+ last_claimfile->symbols = xrealloc (last_claimfile->symbols,
+ new_n_syms * sizeof *last_claimfile->symbols);
+ last_claimfile->n_syms_allocated = new_n_syms;
+ }
+ last_claimfile->symbols[last_claimfile->n_syms_used++] = sym;
+
+ return LDPS_OK;
+}
+
+/* Records the status to return from one of the registered hooks. */
static enum ld_plugin_status
set_ret_val (const char *whichval, enum ld_plugin_status retval)
{
@@ -102,6 +245,7 @@ set_ret_val (const char *whichval, enum ld_plugin_status retval)
return LDPS_OK;
}
+/* Records hooks which should be registered. */
static enum ld_plugin_status
set_register_hook (const char *whichhook, bfd_boolean yesno)
{
@@ -116,6 +260,7 @@ set_register_hook (const char *whichhook, bfd_boolean yesno)
return LDPS_OK;
}
+/* Determine type of plugin option and pass to individual parsers. */
static enum ld_plugin_status
parse_option (const char *opt)
{
@@ -127,11 +272,16 @@ parse_option (const char *opt)
return set_register_hook (opt + 8, TRUE);
else if (!strncmp ("noregister", opt, 10))
return set_register_hook (opt + 10, FALSE);
+ else if (!strncmp ("claim:", opt, 6))
+ return record_claim_file (opt + 6);
+ else if (!strncmp ("sym:", opt, 4))
+ return record_claimed_file_symbol (opt + 4);
else
return LDPS_ERR;
return LDPS_OK;
}
+/* Output contents of transfer vector array entry in human-readable form. */
static void
dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
{
@@ -144,14 +294,12 @@ dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
break;
sprintf (unknownbuf, "unknown tag #%d", tv->tv_tag);
name = (tag < ARRAY_SIZE (tag_names)) ? tag_names[tag].name : unknownbuf;
- if (tv_message)
- (*tv_message) (LDPL_INFO, "tv[%d]: %s ", n, name);
+ TV_MESSAGE (LDPL_INFO, "tv[%d]: %s ", n, name);
switch (tv->tv_tag)
{
case LDPT_OPTION:
case LDPT_OUTPUT_NAME:
- if (tv_message)
- (*tv_message) (LDPL_INFO, "'%s'\n", tv->tv_u.tv_string);
+ TV_MESSAGE (LDPL_INFO, "'%s'\n", tv->tv_u.tv_string);
break;
case LDPT_REGISTER_CLAIM_FILE_HOOK:
case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
@@ -164,8 +312,7 @@ dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
case LDPT_RELEASE_INPUT_FILE:
case LDPT_ADD_INPUT_LIBRARY:
case LDPT_SET_EXTRA_LIBRARY_PATH:
- if (tv_message)
- (*tv_message) (LDPL_INFO, "func@0x%v\n",
+ TV_MESSAGE (LDPL_INFO, "func@0x%v\n",
(bfd_vma)(tv->tv_u.tv_message));
break;
case LDPT_NULL:
@@ -174,13 +321,13 @@ dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
case LDPT_LINKER_OUTPUT:
case LDPT_GNU_LD_VERSION:
default:
- if (tv_message)
- (*tv_message) (LDPL_INFO, "value %W (%d)\n",
+ TV_MESSAGE (LDPL_INFO, "value %W (%d)\n",
(bfd_vma)tv->tv_u.tv_val, tv->tv_u.tv_val);
break;
}
}
+/* Handle/record information received in a transfer vector entry. */
static enum ld_plugin_status
parse_tv_tag (struct ld_plugin_tv *tv)
{
@@ -239,6 +386,8 @@ parse_tv_tag (struct ld_plugin_tv *tv)
return LDPS_OK;
}
+/* Record any useful information in transfer vector entry and display
+ it in human-readable form using the plugin API message() callback. */
enum ld_plugin_status
parse_and_dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
{
@@ -247,6 +396,7 @@ parse_and_dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
return rv;
}
+/* Standard plugin API entry point. */
enum ld_plugin_status
onload (struct ld_plugin_tv *tv)
{
@@ -264,24 +414,21 @@ onload (struct ld_plugin_tv *tv)
tv_message = tv[0].tv_u.tv_message;
fflush (NULL);
- if (tv_message)
- (*tv_message) (LDPL_INFO, "Hello from testplugin.\n");
+ TV_MESSAGE (LDPL_INFO, "Hello from testplugin.\n");
do
if ((rv = parse_and_dump_tv_tag (n++, tv)) != LDPS_OK)
return rv;
while ((tv++)->tv_tag != LDPT_NULL);
- if (tv_message)
- (*tv_message) (LDPL_INFO, "\n");
+ TV_MESSAGE (LDPL_INFO, "\n");
/* Register hooks only if instructed by options. */
if (register_claimfile_hook)
{
if (!tv_register_claim_file)
{
- if (tv_message)
- (*tv_message) (LDPL_FATAL, "No register_claim_file hook\n");
+ TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook\n");
fflush (NULL);
return LDPS_ERR;
}
@@ -291,8 +438,7 @@ onload (struct ld_plugin_tv *tv)
{
if (!tv_register_all_symbols_read)
{
- if (tv_message)
- (*tv_message) (LDPL_FATAL, "No register_all_symbols_read hook\n");
+ TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook\n");
fflush (NULL);
return LDPS_ERR;
}
@@ -302,8 +448,7 @@ onload (struct ld_plugin_tv *tv)
{
if (!tv_register_cleanup)
{
- if (tv_message)
- (*tv_message) (LDPL_FATAL, "No register_cleanup hook\n");
+ TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook\n");
fflush (NULL);
return LDPS_ERR;
}
@@ -313,31 +458,56 @@ onload (struct ld_plugin_tv *tv)
return onload_ret;
}
-
-enum ld_plugin_status
+/* Standard plugin API registerable hook. */
+static enum ld_plugin_status
onclaim_file (const struct ld_plugin_input_file *file, int *claimed)
{
- if (tv_message)
- (*tv_message)(LDPL_INFO, "hook called: claim_file %s [@%ld/%ld]\n",
- file->name, (long)file->offset, (long)file->filesize);
+ /* Let's see if we want to claim this file. */
+ claim_file_t *claimfile = claimfiles_list;
+ while (claimfile)
+ {
+ if (!strcmp (file->name, claimfile->file.name))
+ break;
+ claimfile = claimfile->next;
+ }
+
+ /* Inform the user/testsuite. */
+ TV_MESSAGE (LDPL_INFO, "hook called: claim_file %s [@%ld/%ld] %s\n",
+ file->name, (long)file->offset, (long)file->filesize,
+ claimfile ? "CLAIMED" : "not claimed");
fflush (NULL);
+
+ /* If we decided to claim it, record that fact, and add any symbols
+ that were defined for it by plugin options. */
+ *claimed = (claimfile != 0);
+ if (claimfile)
+ {
+ claimfile->claimed = TRUE;
+ claimfile->file = *file;
+ if (claimfile->n_syms_used && !tv_add_symbols)
+ return LDPS_ERR;
+ else if (claimfile->n_syms_used)
+ return (*tv_add_symbols) (claimfile->file.handle,
+ claimfile->n_syms_used, claimfile->symbols);
+ }
+
return claim_file_ret;
}
-enum ld_plugin_status
+/* Standard plugin API registerable hook. */
+static enum ld_plugin_status
onall_symbols_read (void)
{
- if (tv_message)
- (*tv_message)(LDPL_INFO, "hook called: all symbols read.\n");
+ TV_MESSAGE (LDPL_INFO, "hook called: all symbols read.\n");
fflush (NULL);
return all_symbols_read_ret;
}
-enum ld_plugin_status
+/* Standard plugin API registerable hook. */
+static enum ld_plugin_status
oncleanup (void)
{
- if (tv_message)
- (*tv_message)(LDPL_INFO, "hook called: cleanup.\n");
+ TV_MESSAGE (LDPL_INFO, "hook called: cleanup.\n");
fflush (NULL);
return cleanup_ret;
}
diff --git a/ld/testsuite/ld-plugin/plugin-3.d b/ld/testsuite/ld-plugin/plugin-3.d
index 9014870..73aba1b 100644
--- a/ld/testsuite/ld-plugin/plugin-3.d
+++ b/ld/testsuite/ld-plugin/plugin-3.d
@@ -17,7 +17,6 @@ tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
tv\[15\]: LDPT_OPTION 'registerallsymbolsread'
tv\[16\]: LDPT_OPTION 'failallsymbolsread'
tv\[17\]: LDPT_NULL value 0x0 \(0\)
-# Enable these next two lines when all symbols read hook is being called by infrastructure.
-##...
-#.*ld.*:.*ldtestplug.*: plugin reported error after all symbols read
+#...
+.*ld.*:.*ldtestplug.*: plugin reported error after all symbols read
#...
diff --git a/ld/testsuite/ld-plugin/plugin-5.d b/ld/testsuite/ld-plugin/plugin-5.d
index b9b02ab..c0ffa66 100644
--- a/ld/testsuite/ld-plugin/plugin-5.d
+++ b/ld/testsuite/ld-plugin/plugin-5.d
@@ -18,14 +18,12 @@ tv\[15\]: LDPT_OPTION 'registerclaimfile'
tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
tv\[17\]: LDPT_OPTION 'registercleanup'
tv\[18\]: LDPT_NULL value 0x0 \(0\)
-# Enable these next four lines when claim hook is being called by infrastructure.
-##...
-#hook called: claim_file tmpdir/main.o \[@0/.*
-#hook called: claim_file tmpdir/func.o \[@0/.*
-#hook called: claim_file tmpdir/text.o \[@0/.*
-# Enable these next two lines when all symbols read hook is being called by infrastructure.
-##...
-#hook called: all symbols read.
+#...
+hook called: claim_file tmpdir/main.o \[@0/.*
+hook called: claim_file tmpdir/func.o \[@0/.*
+hook called: claim_file tmpdir/text.o \[@0/.*
+#...
+hook called: all symbols read.
#...
hook called: cleanup.
#...
diff --git a/ld/testsuite/ld-plugin/plugin-6.d b/ld/testsuite/ld-plugin/plugin-6.d
new file mode 100644
index 0000000..cb9257d
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-6.d
@@ -0,0 +1,31 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_OPTION 'claim:tmpdir/func.o'
+tv\[19\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: claim_file tmpdir/main.o \[@0/.* not claimed
+hook called: claim_file tmpdir/func.o \[@0/.* CLAIMED
+hook called: claim_file tmpdir/text.o \[@0/.* not claimed
+#...
+hook called: all symbols read.
+tmpdir/main.o: In function `main':
+.*ld/testsuite/ld-plugin/main.c:12: undefined reference to `func'
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin-7.d b/ld/testsuite/ld-plugin/plugin-7.d
new file mode 100644
index 0000000..75f25e0
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-7.d
@@ -0,0 +1,30 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_OPTION 'claim:tmpdir/func.o'
+tv\[19\]: LDPT_OPTION 'sym:_?func::0:0:0'
+tv\[20\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: claim_file tmpdir/main.o \[@0/.* not claimed
+hook called: claim_file tmpdir/func.o \[@0/.* CLAIMED
+hook called: claim_file tmpdir/text.o \[@0/.* not claimed
+#...
+hook called: all symbols read.
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index 1150d93..b983ce4 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -51,22 +51,51 @@ set regclm "-plugin-arg registerclaimfile"
set regas "-plugin-arg registerallsymbolsread"
set regcln "-plugin-arg registercleanup"
+# In order to define symbols in plugin options in the list of tests below,
+# we need to know if the platform prepends an underscore to C symbols,
+# which we find out by compiling the test objects now. If there is any
+# error compiling, we defer reporting it until after the list of tests has
+# been initialised, so that we can use the names in the list to report;
+# otherwise, we scan one of the files with 'nm' and look for a known symbol
+# in the output to see if it is prefixed or not.
+set failed_compile 0
+set _ ""
+if { ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o]
+ || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o]
+ || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/text.c tmpdir/text.o] } {
+ # Defer fail until we have list of tests set.
+ set failed_compile 1
+} else {
+ # Find out if symbols have prefix on this platform before setting tests.
+ catch "exec $NM tmpdir/func.o" nm_output
+ if { [regexp "_func" "$nm_output"] } {
+ set _ "_"
+ }
+}
+
set plugin_tests [list \
[list "load plugin" "-plugin $plugin_path \
$testobjfiles $libs" "" "" {{ld plugin-1.d}} "main.x" ] \
[list "fail plugin onload" "-plugin $plugin_path -plugin-arg failonload \
$testobjfiles $libs" "" "" {{ld plugin-2.d}} "main.x" ] \
- [list "fail plugin allsymbolsread" "-plugin $plugin_path $regas -plugin-arg failallsymbolsread \
+ [list "fail plugin allsymbolsread" "-plugin $plugin_path $regas \
+ -plugin-arg failallsymbolsread \
$testobjfiles $libs" "" "" {{ld plugin-3.d}} "main.x" ] \
- [list "fail plugin cleanup" "-plugin $plugin_path -plugin-arg failcleanup $regcln \
+ [list "fail plugin cleanup" "-plugin $plugin_path -plugin-arg failcleanup \
+ $regcln \
$testobjfiles $libs" "" "" {{ld plugin-4.d}} "main.x" ] \
[list "plugin all hooks" "-plugin $plugin_path $regclm $regas $regcln \
$testobjfiles $libs" "" "" {{ld plugin-5.d}} "main.x" ] \
+ [list "plugin claimfile lost symbol" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ $testobjfiles $libs" "" "" {{ld plugin-6.d}} "main.x" ] \
+ [list "plugin claimfile replace symbol" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ -plugin-arg sym:${_}func::0:0:0 \
+ $testobjfiles $libs" "" "" {{ld plugin-7.d}} "main.x" ] \
]
-if { ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o]
- || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o]
- || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/text.c tmpdir/text.o] } {
+if { $failed_compile != 0 } {
foreach testitem $plugin_tests {
unresolved [lindex $testitem 0]
}
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions.
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
@ 2010-09-23 5:09 ` Dave Korn
2010-10-06 22:28 ` Richard Henderson
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols Dave Korn
` (5 subsequent siblings)
6 siblings, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-09-23 5:09 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]
This one works by using the bfd linker's "notice symbol" callback to track
all symbols that are referenced by non-IR files, so that we can later
determine for prevailing defs from IR files whether or not they are IR-only.
It's pretty simple and the only minor trickiness is turing on notice_all
without inadvertently behaving like we've invoked --trace-sym on the whole world.
- As with the asymbol conversion in the previous patch, I'd appreciate extra
eyeballs looking at the logic here in get_symbols by which I determine
LDPR_xxx resolution types from the info in the linker hash table, just to
double-check my working.
ld/ChangeLog:
* ldmain.c (notice)[ENABLE_PLUGINS]: Call plugin_notice.
* plugin.c (non_ironly_hash): Add new bfd hash table.
(plugin_load_plugins): Exit early if no plugins to load. If plugins
do load successfully, set notice_all flag in link info.
(add_symbols): Clean up for-loop slightly.
(get_symbols): Implement.
(init_non_ironly_hash): Lazily init non_ironly_hash table.
(plugin_notice): Record symbols referenced from non-IR files in the
non_ironly_hash. Suppress tracing, cref generation and nocrossrefs
tracking for symbols from dummy IR bfds.
* plugin.h: Fix formatting.
(plugin_notice): Add prototype.
* testplug.c (dumpresolutions): New global var.
(parse_options): Accept "dumpresolutions".
(onall_symbols_read): Get syms and dump resolutions if it was given.
ld/testsuite/ChangeLog:
* ld-plugin/plugin-8.d: New testcase.
* ld-plugin/plugin.exp: Invoke it.
cheers,
DaveK
[-- Attachment #2: ld-plugin-api-3-get-symbols.diff --]
[-- Type: text/x-c, Size: 15915 bytes --]
From f09bc8ba9165c81d82a8a027129601867b93edd9 Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Wed, 22 Sep 2010 04:37:28 +0100
Subject: [PATCH] Implement get symbols callback.
ld/ChangeLog:
* ldmain.c (notice)[ENABLE_PLUGINS]: Call plugin_notice.
* plugin.c (IRONLY_SUFFIX_LEN): Correct off-by-one in definition.
(non_ironly_hash): Add new bfd hash table.
(plugin_load_plugins): Exit early if no plugins to load. If plugins
do load successfully, set notice_all flag in link info.
(add_symbols): Clean up for-loop slightly.
(get_symbols): Implement.
(init_non_ironly_hash): Lazily init non_ironly_hash table.
(plugin_notice): Record symbols referenced from non-IR files in the
non_ironly_hash. Suppress tracing, cref generation and nocrossrefs
tracking for symbols from dummy IR bfds.
* plugin.h: Fix formatting.
(plugin_notice): Add prototype.
* testplug.c (dumpresolutions): New global var.
(parse_options): Accept "dumpresolutions".
(onall_symbols_read): Get syms and dump resolutions if it was given.
ld/testsuite/ChangeLog:
* ld-plugin/plugin-8.d: New testcase.
* ld-plugin/plugin.exp: Invoke it.
---
ld/ldmain.c | 19 ++++-
ld/plugin.c | 149 +++++++++++++++++++++++++++++++++++--
ld/plugin.h | 7 ++-
ld/testplug.c | 36 +++++++++
ld/testsuite/ld-plugin/plugin-8.d | 34 +++++++++
ld/testsuite/ld-plugin/plugin.exp | 6 ++
6 files changed, 238 insertions(+), 13 deletions(-)
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 85236ed..ed1738e 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -1380,7 +1380,10 @@ unattached_reloc (struct bfd_link_info *info ATTRIBUTE_UNUSED,
/* This is called if link_info.notice_all is set, or when a symbol in
link_info.notice_hash is found. Symbols are put in notice_hash
- using the -y option. */
+ using the -y option, while notice_all is set if the --cref option
+ has been supplied, or if there are any NOCROSSREFS sections in the
+ linker script; and if plugins are active, since they need to monitor
+ all references from non-IR files. */
static bfd_boolean
notice (struct bfd_link_info *info,
@@ -1396,9 +1399,17 @@ notice (struct bfd_link_info *info,
return TRUE;
}
- if (! info->notice_all
- || (info->notice_hash != NULL
- && bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != NULL))
+#ifdef ENABLE_PLUGINS
+ /* We should hide symbols in the dummy IR BFDs from the nocrossrefs list
+ and let the real object files that are generated and added later trip
+ the error instead. Similarly would be better to trace the real symbol
+ from the real file than the temporary dummy. */
+ if (!plugin_notice (info, name, abfd, section, value))
+ return TRUE;
+#endif /* ENABLE_PLUGINS */
+
+ if (info->notice_hash != NULL
+ && bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != NULL)
{
if (bfd_is_und_section (section))
einfo ("%B: reference to %s\n", abfd, name);
diff --git a/ld/plugin.c b/ld/plugin.c
index 6c71978..4e8505d 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -55,8 +55,10 @@ PLUGAPIFUNC set_extra_library_path (const char *path);
when generating a dummy BFD to hold the IR symbols sent from the
plugin. */
#define IRONLY_SUFFIX ".ironly\004"
-/* This is sizeof an array of chars, not sizeof a const char *. */
-#define IRONLY_SUFFIX_LEN (sizeof (IRONLY_SUFFIX))
+
+/* This is sizeof an array of chars, not sizeof a const char *. We
+ also have to avoid inadvertently counting the trailing NUL. */
+#define IRONLY_SUFFIX_LEN (sizeof (IRONLY_SUFFIX) - 1)
/* Always use this macro when invoking a plugin function. */
#define INVOKE_PLUGIN_FN(plugin, retval, fn, args) \
@@ -110,6 +112,13 @@ static plugin_t *called_plugin = NULL;
/* Last plugin to cause an error, if any. */
static const char *error_plugin = NULL;
+/* A hash table that records symbols referenced by non-IR files. Used
+ at get_symbols time to determine whether any prevailing defs from
+ IR files are referenced only from other IR files, so tthat we can
+ we can distinguish the LDPR_PREVAILING_DEF and LDPR_PREVAILING_DEF_IRONLY
+ cases when establishing symbol resolutions. */
+static struct bfd_hash_table *non_ironly_hash = NULL;
+
/* Helper to size leading part of tv array and set it up. */
static size_t
set_tv_header (struct ld_plugin_tv *tv)
@@ -292,6 +301,10 @@ int plugin_load_plugins (void)
unsigned int max_args = 0;
plugin_t *curplug = plugins_list;
+ /* If there are no plugins, we need do nothing this run. */
+ if (!curplug)
+ return 0;
+
/* First pass over plugins to find max # args needed so that we
can size and allocate the tv array. */
while (curplug)
@@ -319,6 +332,12 @@ int plugin_load_plugins (void)
return set_plugin_error (curplug->name);
curplug = curplug->next;
}
+
+ /* Since plugin(s) inited ok, assume they're going to want symbol
+ resolutions, which needs us to track which symbols are referenced
+ by non-IR files using the linker's notice callback. */
+ link_info.notice_all = TRUE;
+
return 0;
}
@@ -489,12 +508,12 @@ add_symbols (void *handle, int nsyms, const struct ld_plugin_symbol *syms)
int n;
ASSERT (called_plugin);
symptrs = xmalloc (nsyms * sizeof *symptrs);
- for (n = 0; n < nsyms; n++)
+ for (n = 0; n < nsyms; n++, syms++)
{
enum ld_plugin_status rv;
asymbol *bfdsym = bfd_make_empty_symbol (abfd);
symptrs[n] = bfdsym;
- rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms++);
+ rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms);
if (rv != LDPS_OK)
return rv;
}
@@ -526,11 +545,71 @@ release_input_file (const void *handle)
static enum ld_plugin_status
get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
{
+ const bfd *abfd = handle;
+ int n;
ASSERT (called_plugin);
- handle = handle;
- nsyms = nsyms;
- syms = syms;
- return LDPS_ERR;
+ for (n = 0; n < nsyms; n++)
+ {
+ struct bfd_link_hash_entry *blhe;
+ blhe = bfd_link_hash_lookup (link_info.hash, syms[n].name,
+ FALSE, FALSE, TRUE);
+ if (!blhe)
+ {
+ syms[n].resolution = LDPR_UNKNOWN;
+ continue;
+ }
+ /* Determine resolution from blhe type and symbol's original type. */
+ if (blhe->type == bfd_link_hash_undefined
+ || blhe->type == bfd_link_hash_undefweak)
+ {
+ syms[n].resolution = LDPR_UNDEF;
+ continue;
+ }
+ if (blhe->type == bfd_link_hash_common)
+ {
+ /* Is this right? The COMMON section only exists in the
+ output file, so it seems reasonable. */
+ syms[n].resolution = LDPR_RESOLVED_EXEC;
+ continue;
+ }
+ if (blhe->type != bfd_link_hash_defined
+ && blhe->type != bfd_link_hash_defweak)
+ {
+ /* We should not have a new, indirect or warning symbol here. */
+ einfo ("%P%F: %s: plugin symbol table corrupt (sym type %d)",
+ called_plugin->name, blhe->type);
+ }
+ /* Defined or defweak. If it was originally undefined, then it
+ has been resolved; determine how. */
+ if (syms[n].def == LDPK_UNDEF || syms[n].def == LDPK_WEAKUNDEF)
+ {
+ if (is_ir_dummy_bfd (blhe->u.def.section->owner))
+ syms[n].resolution = LDPR_RESOLVED_IR;
+ else if (blhe->u.def.section->owner == link_info.output_bfd)
+ syms[n].resolution = LDPR_RESOLVED_EXEC;
+ else
+ syms[n].resolution = blhe->u.def.section->owner->flags
+ & DYNAMIC ? LDPR_RESOLVED_DYN : LDPR_RESOLVED_EXEC;
+ continue;
+ }
+ /* Was originally def, weakdef, or common. Does it prevail? If
+ the owner is the original dummy bfd that supplied it, then this
+ is the definition that has prevailed. */
+ if (blhe->u.def.section->owner == abfd)
+ {
+ bfd_boolean ironly = !bfd_hash_lookup (non_ironly_hash,
+ syms[n].name, FALSE, FALSE);
+ syms[n].resolution = (ironly)
+ ? LDPR_PREVAILING_DEF_IRONLY
+ : LDPR_PREVAILING_DEF;
+ continue;
+ }
+ /* Was originally def, weakdef, or common, but has been pre-empted. */
+ syms[n].resolution = is_ir_dummy_bfd (blhe->u.def.section->owner)
+ ? LDPR_PREEMPTED_IR
+ : LDPR_PREEMPTED_REG;
+ }
+ return LDPS_OK;
}
/* Add a new (real) input file generated by a plugin. */
@@ -593,3 +672,57 @@ message (int level, const char *format, ...)
return LDPS_OK;
}
+/* Lazily init the non_ironly hash table. */
+static void
+init_non_ironly_hash (void)
+{
+ if (non_ironly_hash == NULL)
+ {
+ non_ironly_hash =
+ (struct bfd_hash_table *) xmalloc (sizeof (struct bfd_hash_table));
+ if (!bfd_hash_table_init_n (non_ironly_hash,
+ bfd_hash_newfunc,
+ sizeof (struct bfd_hash_entry),
+ 61))
+ einfo (_("%P%F: bfd_hash_table_init failed: %E\n"));
+ }
+}
+
+/* To determine which symbols should be resolved LDPR_PREVAILING_DEF
+ and which LDPR_PREVAILING_DEF_IRONLY, we notice all the symbols as
+ the linker adds them to the linker hash table. If we see a symbol
+ being referenced from a non-IR file, we add it to the non_ironly hash
+ table. If we can't find it there at get_symbols time, we know that
+ it was referenced only by IR files. We have to notice_all symbols,
+ because we won't necessarily know until later which ones will be
+ contributed by IR files. */
+bfd_boolean
+plugin_notice (struct bfd_link_info *info, const char *name, bfd *abfd,
+ asection *section, bfd_vma value)
+{
+ bfd_boolean is_ref = bfd_is_und_section (section);
+ bfd_boolean is_dummy = is_ir_dummy_bfd (abfd);
+ init_non_ironly_hash ();
+ /* We only care about refs, not defs, indicated by section pointing
+ to the undefined section (according to the bfd linker notice callback
+ interface definition). */
+ if (is_ref && !is_dummy)
+ {
+ /* This is a ref from a non-IR file, so note the ref'd symbol
+ in the non-IR-only hash. */
+ if (!bfd_hash_lookup (non_ironly_hash, name, TRUE, TRUE))
+ einfo (_("%P%X: %s: hash table failure adding symbol %s"),
+ abfd->filename, name);
+ }
+ else if (!is_ref && is_dummy)
+ {
+ /* No further processing since this is a def from an IR dummy BFD. */
+ return FALSE;
+ }
+
+ /* Suppresses "unused" warnings without relying on GCC attribute. */
+ info = info;
+ value = value;
+ /* Continue with cref/nocrossref/trace-sym processing. */
+ return TRUE;
+}
diff --git a/ld/plugin.h b/ld/plugin.h
index 8839b8b..5bbe93d 100644
--- a/ld/plugin.h
+++ b/ld/plugin.h
@@ -40,7 +40,8 @@ extern int plugin_load_plugins (void);
extern const char *plugin_error_plugin (void);
/* Call 'claim file' hook for all plugins. */
-extern int plugin_call_claim_file (const struct ld_plugin_input_file *file, int *claimed);
+extern int plugin_call_claim_file (const struct ld_plugin_input_file *file,
+ int *claimed);
/* Call 'all symbols read' hook for all plugins. */
extern int plugin_call_all_symbols_read (void);
@@ -58,6 +59,10 @@ extern bfd *plugin_get_ir_dummy_bfd (const char *name, bfd *template);
/* Check if the BFD passed in is an IR dummy object file. */
extern bfd_boolean is_ir_dummy_bfd (const bfd *abfd);
+/* Notice-symbol bfd linker callback hook. */
+extern bfd_boolean plugin_notice (struct bfd_link_info *info,
+ const char *name, bfd *abfd, asection *section,
+ bfd_vma value);
#endif /* !def GLD_PLUGIN_H */
diff --git a/ld/testplug.c b/ld/testplug.c
index 9746465..ad1bdf6 100644
--- a/ld/testplug.c
+++ b/ld/testplug.c
@@ -106,6 +106,7 @@ static enum ld_plugin_status cleanup_ret = LDPS_OK;
static bfd_boolean register_claimfile_hook = FALSE;
static bfd_boolean register_allsymbolsread_hook = FALSE;
static bfd_boolean register_cleanup_hook = FALSE;
+static bfd_boolean dumpresolutions = FALSE;
/* The master list of all claimable/claimed files. */
static claim_file_t *claimfiles_list = NULL;
@@ -276,6 +277,8 @@ parse_option (const char *opt)
return record_claim_file (opt + 6);
else if (!strncmp ("sym:", opt, 4))
return record_claimed_file_symbol (opt + 4);
+ else if (!strcmp ("dumpresolutions", opt))
+ dumpresolutions = TRUE;
else
return LDPS_ERR;
return LDPS_OK;
@@ -498,7 +501,40 @@ onclaim_file (const struct ld_plugin_input_file *file, int *claimed)
static enum ld_plugin_status
onall_symbols_read (void)
{
+ static const char *resolutions[] =
+ {
+ "LDPR_UNKNOWN",
+ "LDPR_UNDEF",
+ "LDPR_PREVAILING_DEF",
+ "LDPR_PREVAILING_DEF_IRONLY",
+ "LDPR_PREEMPTED_REG",
+ "LDPR_PREEMPTED_IR",
+ "LDPR_RESOLVED_IR",
+ "LDPR_RESOLVED_EXEC",
+ "LDPR_RESOLVED_DYN",
+ };
+ claim_file_t *claimfile = dumpresolutions ? claimfiles_list : NULL;
TV_MESSAGE (LDPL_INFO, "hook called: all symbols read.\n");
+ for ( ; claimfile; claimfile = claimfile->next)
+ {
+ enum ld_plugin_status rv;
+ int n;
+ if (claimfile->n_syms_used && !tv_get_symbols)
+ return LDPS_ERR;
+ else if (!claimfile->n_syms_used)
+ continue;
+ rv = tv_get_symbols (claimfile->file.handle, claimfile->n_syms_used,
+ claimfile->symbols);
+ if (rv != LDPS_OK)
+ return rv;
+ for (n = 0; n < claimfile->n_syms_used; n++)
+ TV_MESSAGE (LDPL_INFO, "Sym: '%s%s%s' Resolution: %s\n",
+ claimfile->symbols[n].name,
+ claimfile->symbols[n].version ? "@" : "",
+ claimfile->symbols[n].version ? claimfile->symbols[n].version
+ : "",
+ resolutions[claimfile->symbols[n].resolution]);
+ }
fflush (NULL);
return all_symbols_read_ret;
}
diff --git a/ld/testsuite/ld-plugin/plugin-8.d b/ld/testsuite/ld-plugin/plugin-8.d
new file mode 100644
index 0000000..e72b039
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-8.d
@@ -0,0 +1,34 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_OPTION 'claim:tmpdir/func.o'
+tv\[19\]: LDPT_OPTION 'sym:_?func::0:0:0'
+tv\[20\]: LDPT_OPTION 'sym:_?func2::0:0:0'
+tv\[21\]: LDPT_OPTION 'dumpresolutions'
+tv\[22\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: claim_file tmpdir/main.o \[@0/.* not claimed
+hook called: claim_file tmpdir/func.o \[@0/.* CLAIMED
+hook called: claim_file tmpdir/text.o \[@0/.* not claimed
+#...
+hook called: all symbols read.
+Sym: '_?func' Resolution: LDPR_PREVAILING_DEF
+Sym: '_?func2' Resolution: LDPR_PREVAILING_DEF_IRONLY
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index b983ce4..6ed2787 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -93,6 +93,12 @@ set plugin_tests [list \
$regas $regcln -plugin-arg claim:tmpdir/func.o \
-plugin-arg sym:${_}func::0:0:0 \
$testobjfiles $libs" "" "" {{ld plugin-7.d}} "main.x" ] \
+ [list "plugin claimfile resolve symbol" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ -plugin-arg sym:${_}func::0:0:0 \
+ -plugin-arg sym:${_}func2::0:0:0 \
+ -plugin-arg dumpresolutions \
+ $testobjfiles $libs" "" "" {{ld plugin-8.d}} "main.x" ] \
]
if { $failed_compile != 0 } {
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths.
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols Dave Korn
@ 2010-09-23 5:09 ` Dave Korn
2010-09-24 0:35 ` Dave Korn
2010-10-06 22:40 ` Richard Henderson
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [1/4] Infrastructure Dave Korn
` (3 subsequent siblings)
6 siblings, 2 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-23 5:09 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 2066 bytes --]
This is the final functionality needed to start supporting the GCC LTO plugin.
- Adding new files/libs/search dirs are fairly trivial callouts to existing
functions in ld, so hopefully the added files should behave just like real
ones, with consistent lang_statements and all.
- It makes use of the bfd linker's multiple-definitions callback to spot when
a freshly-added object file contains a real symbol that was initially supplied
from an IR file, and replaces the old definition with the new one. This
should correctly cut the dummy bfd right out of the final link, I hope -
unless someone knows some reason why whipping the rug out from under BFD's
feet like this wouldn't work or might confuse it?
ld/ChangeLog:
* ldlang.c (lang_process)[ENABLE_PLUGINS]: Move invocation of
plugin_call_all_symbols_read to before setting of gc_sym_list, and
open any new input files that may have been added during it.
* ldmain.c (multiple_definition)[ENABLE_PLUGINS]: Call out to
plugin_multiple_definition and let it have first say over what to do
with the clashing definitions.
* plugin.c (no_more_claiming): New boolean variable.
(plugin_cached_allow_multiple_defs): Likewise.
(plugin_call_claim_file): Don't do anything when no_more_claiming set.
(plugin_call_all_symbols_read): Set it. Disable link info
"allow_multiple_definition" flag, but cache its value.
(add_input_file): Implement.
(add_input_library): Likewise.
(set_extra_library_path): Likewise.
(plugin_multiple_definition): New function.
* plugin.h (plugin_multiple_definition): Add prototype.
* testplug.c (addfile_enum_t): New enumerated typedef.
(add_file_t): New struct typedef.
(addfiles_list): New variable.
(addfiles_tail_chain_ptr): Likewise.
(record_add_file): New function.
(parse_option): Parse "add:", "lib:" and "dir:" options and call it.
(onall_symbols_read): Iterate the list of new files, libs and dirs,
adding them.
ld/testsuite/ChangeLog:
* testsuite/ld-plugin/plugin-9.d: New testcase.
* ld-plugin/plugin.exp: Invoke it.
cheers,
DaveK
[-- Attachment #2: ld-plugin-api-4-add-files.diff --]
[-- Type: text/x-c, Size: 15798 bytes --]
From 3e5b2e92af331115c82b9f3f943559be64ea1f4f Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Thu, 23 Sep 2010 03:23:19 +0100
Subject: [PATCH] Implement add input file, add input library and set extra library path
callbacks.
ld/ChangeLog:
* ldlang.c (lang_process)[ENABLE_PLUGINS]: Move invocation of
plugin_call_all_symbols_read to before setting of gc_sym_list, and
open any new input files that may have been added during it.
* ldmain.c (multiple_definition)[ENABLE_PLUGINS]: Call out to
plugin_multiple_definition and let it have first say over what to do
with the clashing definitions.
* plugin.c (no_more_claiming): New boolean variable.
(plugin_cached_allow_multiple_defs): Likewise.
(plugin_call_claim_file): Don't do anything when no_more_claiming set.
(plugin_call_all_symbols_read): Set it. Disable link info
"allow_multiple_definition" flag, but cache its value.
(add_input_file): Implement.
(add_input_library): Likewise.
(set_extra_library_path): Likewise.
(plugin_multiple_definition): New function.
* plugin.h (plugin_multiple_definition): Add prototype.
* testplug.c (addfile_enum_t): New enumerated typedef.
(add_file_t): New struct typedef.
(addfiles_list): New variable.
(addfiles_tail_chain_ptr): Likewise.
(record_add_file): New function.
(parse_option): Parse "add:", "lib:" and "dir:" options and call it.
(onall_symbols_read): Iterate the list of new files, libs and dirs,
adding them.
ld/testsuite/ChangeLog:
* testsuite/ld-plugin/plugin-9.d: New testcase.
* ld-plugin/plugin.exp: Invoke it.
---
ld/ldlang.c | 18 +++++++--
ld/ldmain.c | 13 ++++++-
ld/plugin.c | 73 ++++++++++++++++++++++++++++++++++---
ld/plugin.h | 6 +++
ld/testplug.c | 59 ++++++++++++++++++++++++++++++
ld/testsuite/ld-plugin/plugin-9.d | 35 ++++++++++++++++++
ld/testsuite/ld-plugin/plugin.exp | 7 ++++
7 files changed, 200 insertions(+), 11 deletions(-)
diff --git a/ld/ldlang.c b/ld/ldlang.c
index cb1417c..1310852 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6317,6 +6317,9 @@ lang_relax_sections (bfd_boolean need_layout)
void
lang_process (void)
{
+#ifdef ENABLE_PLUGINS
+ union lang_statement_union **listend;
+#endif /* ENABLE_PLUGINS */
/* Finalize dynamic list. */
if (link_info.dynamic_list)
lang_finalize_version_expr_head (&link_info.dynamic_list->head);
@@ -6339,19 +6342,26 @@ lang_process (void)
current_target = default_target;
open_input_bfds (statement_list.head, FALSE);
- link_info.gc_sym_list = &entry_symbol;
- if (entry_symbol.name == NULL)
- link_info.gc_sym_list = ldlang_undef_chain_list_head;
-
#ifdef ENABLE_PLUGINS
/* Now all files are read, let the plugin(s) decide if there
are any more to be added to the link before we call the
emulation's after_open hook. */
+ listend = statement_list.tail;
+ ASSERT (!*listend);
if (plugin_call_all_symbols_read ())
einfo (_("%P%F: %s: plugin reported error after all symbols read\n"),
plugin_error_plugin ());
+ /* If any new files were added, they will be on the end of the
+ statement list, and we can open them now by getting open_input_bfds
+ to carry on from where it ended last time. */
+ if (*listend)
+ open_input_bfds (*listend, FALSE);
#endif /* ENABLE_PLUGINS */
+ link_info.gc_sym_list = &entry_symbol;
+ if (entry_symbol.name == NULL)
+ link_info.gc_sym_list = ldlang_undef_chain_list_head;
+
ldemul_after_open ();
bfd_section_already_linked_table_free ();
diff --git a/ld/ldmain.c b/ld/ldmain.c
index ed1738e..f1d6697 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -889,7 +889,7 @@ add_archive_element (struct bfd_link_info *info,
multiple times. */
static bfd_boolean
-multiple_definition (struct bfd_link_info *info ATTRIBUTE_UNUSED,
+multiple_definition (struct bfd_link_info *info,
const char *name,
bfd *obfd,
asection *osec,
@@ -898,6 +898,17 @@ multiple_definition (struct bfd_link_info *info ATTRIBUTE_UNUSED,
asection *nsec,
bfd_vma nval)
{
+#ifdef ENABLE_PLUGINS
+ /* We may get called back even when --allow-multiple-definition is in
+ effect, as the plugin infrastructure needs to use this hook in
+ order to swap out IR-only symbols for real ones. In that case,
+ it will let us know not to continue by returning TRUE even if this
+ is not an IR-only vs. non-IR symbol conflict. */
+ if (plugin_multiple_definition (info, name, obfd, osec, oval, nbfd,
+ nsec, nval))
+ return TRUE;
+#endif /* ENABLE_PLUGINS */
+
/* If either section has the output_section field set to
bfd_abs_section_ptr, it means that the section is being
discarded, and this is not really a multiple definition at all.
diff --git a/ld/plugin.c b/ld/plugin.c
index 4e8505d..a33e907 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -26,6 +26,9 @@
#include "ld.h"
#include "ldmain.h"
#include "ldmisc.h"
+#include "ldexp.h"
+#include "ldlang.h"
+#include "ldfile.h"
#include "plugin.h"
#include "plugin-api.h"
/* Don't include std headers until after config.h, sysdeps.h etc.,
@@ -119,6 +122,16 @@ static const char *error_plugin = NULL;
cases when establishing symbol resolutions. */
static struct bfd_hash_table *non_ironly_hash = NULL;
+/* Set at all symbols read time, to avoid recursively offering the plugin
+ its own newly-added input files and libs to claim. */
+static bfd_boolean no_more_claiming = FALSE;
+
+/* If the --allow-multiple-definition command-line option is active, we
+ have to disable it so that BFD always calls our hook, and simulate the
+ effect (when not resolving IR vs. real symbols) ourselves by ensuring
+ TRUE is returned from the hook. */
+static bfd_boolean plugin_cached_allow_multiple_defs = FALSE;
+
/* Helper to size leading part of tv array and set it up. */
static size_t
set_tv_header (struct ld_plugin_tv *tv)
@@ -347,6 +360,8 @@ plugin_call_claim_file (const struct ld_plugin_input_file *file, int *claimed)
{
plugin_t *curplug = plugins_list;
*claimed = FALSE;
+ if (no_more_claiming)
+ return 0;
while (curplug && !*claimed)
{
if (curplug->claim_file_handler)
@@ -367,6 +382,17 @@ int
plugin_call_all_symbols_read (void)
{
plugin_t *curplug = plugins_list;
+
+ /* Disable any further file-claiming. */
+ no_more_claiming = TRUE;
+
+ /* If --allow-multiple-definition is in effect, we need to disable it,
+ as the plugin infrastructure relies on the multiple_definition
+ callback to swap out the dummy IR-only BFDs for new real ones
+ when it starts opening the files added during this callback. */
+ plugin_cached_allow_multiple_defs = link_info.allow_multiple_definition;
+ link_info.allow_multiple_definition = FALSE;
+
while (curplug)
{
if (curplug->all_symbols_read_handler)
@@ -617,8 +643,9 @@ static enum ld_plugin_status
add_input_file (const char *pathname)
{
ASSERT (called_plugin);
- pathname = pathname;
- return LDPS_ERR;
+ if (!lang_add_input_file (pathname, lang_input_file_is_file_enum, NULL))
+ return LDPS_ERR;
+ return LDPS_OK;
}
/* Add a new (real) library required by a plugin. */
@@ -626,8 +653,9 @@ static enum ld_plugin_status
add_input_library (const char *pathname)
{
ASSERT (called_plugin);
- pathname = pathname;
- return LDPS_ERR;
+ if (!lang_add_input_file (pathname, lang_input_file_is_l_enum, NULL))
+ return LDPS_ERR;
+ return LDPS_OK;
}
/* Set the extra library path to be used by libraries added via
@@ -636,8 +664,8 @@ static enum ld_plugin_status
set_extra_library_path (const char *path)
{
ASSERT (called_plugin);
- path = path;
- return LDPS_ERR;
+ ldfile_add_library_path (path, FALSE);
+ return LDPS_OK;
}
/* Issue a diagnostic message from a plugin. */
@@ -726,3 +754,36 @@ plugin_notice (struct bfd_link_info *info, const char *name, bfd *abfd,
/* Continue with cref/nocrossref/trace-sym processing. */
return TRUE;
}
+
+/* When we add new object files to the link at all symbols read time,
+ these contain the real code and symbols generated from the IR files,
+ and so duplicate all the definitions already supplied by the dummy
+ IR-only BFDs that we created at claim files time. We use the linker's
+ multiple-definitions callback hook to fix up the clash, discarding
+ the symbol from the IR-only BFD in favour of the symbol from the
+ real BFD. We return true if this was not-really-a-clash because
+ we've fixed it up, or anyway if --allow-multiple-definition was in
+ effect (before we disabled it to ensure we got called back). */
+bfd_boolean
+plugin_multiple_definition (struct bfd_link_info *info, const char *name,
+ bfd *obfd, asection *osec, bfd_vma oval,
+ bfd *nbfd, asection *nsec, bfd_vma nval)
+{
+ osec = osec;
+ oval = oval;
+ if (is_ir_dummy_bfd (obfd))
+ {
+ struct bfd_link_hash_entry *blhe = bfd_link_hash_lookup (info->hash,
+ name, FALSE, FALSE, FALSE);
+ if (!blhe)
+ einfo (_("%P%X: %s: can't find IR symbol '%s'"), nbfd->filename,
+ name);
+ else if (blhe->type != bfd_link_hash_defined)
+ einfo (_("%P%x: %s: bad IR symbol type %d"), name, blhe->type);
+ /* Replace it with new details. */
+ blhe->u.def.section = nsec;
+ blhe->u.def.value = nval;
+ return TRUE;
+ }
+ return plugin_cached_allow_multiple_defs;
+}
diff --git a/ld/plugin.h b/ld/plugin.h
index 5bbe93d..c21525b 100644
--- a/ld/plugin.h
+++ b/ld/plugin.h
@@ -64,5 +64,11 @@ extern bfd_boolean plugin_notice (struct bfd_link_info *info,
const char *name, bfd *abfd, asection *section,
bfd_vma value);
+/* Multiple-definition bfd linker callback hook. */
+extern bfd_boolean plugin_multiple_definition (struct bfd_link_info *info,
+ const char *name,
+ bfd *obfd, asection *osec, bfd_vma oval,
+ bfd *nbfd, asection *nsec, bfd_vma nval);
+
#endif /* !def GLD_PLUGIN_H */
diff --git a/ld/testplug.c b/ld/testplug.c
index ad1bdf6..1b775ed 100644
--- a/ld/testplug.c
+++ b/ld/testplug.c
@@ -48,6 +48,22 @@ typedef struct claim_file
int n_syms_used;
} claim_file_t;
+/* Types of things that can be added at all symbols read time. */
+typedef enum addfile_enum
+{
+ ADD_FILE,
+ ADD_LIB,
+ ADD_DIR
+} addfile_enum_t;
+
+/* Struct for recording files to add to final link. */
+typedef struct add_file
+{
+ struct add_file *next;
+ const char *name;
+ addfile_enum_t type;
+} add_file_t;
+
/* Helper macro for defining array of transfer vector tags and names. */
#define ADDENTRY(tag) { tag, #tag }
@@ -117,6 +133,12 @@ static claim_file_t **claimfiles_tail_chain_ptr = &claimfiles_list;
/* The last claimed file added to the list, for receiving syms. */
static claim_file_t *last_claimfile = NULL;
+/* The master list of all files to add to the final link. */
+static add_file_t *addfiles_list = NULL;
+
+/* We keep a tail pointer for easy linking on the end. */
+static add_file_t **addfiles_tail_chain_ptr = &addfiles_list;
+
/* Add a new claimfile on the end of the chain. */
static enum ld_plugin_status
record_claim_file (const char *file)
@@ -135,6 +157,22 @@ record_claim_file (const char *file)
return LDPS_OK;
}
+/* Add a new addfile on the end of the chain. */
+static enum ld_plugin_status
+record_add_file (const char *file, addfile_enum_t type)
+{
+ add_file_t *newfile;
+
+ newfile = xmalloc (sizeof *newfile);
+ newfile->next = NULL;
+ newfile->name = file;
+ newfile->type = type;;
+ /* Chain it on the end of the list. */
+ *addfiles_tail_chain_ptr = newfile;
+ addfiles_tail_chain_ptr = &newfile->next;
+ return LDPS_OK;
+}
+
/* Parse a command-line argument string into a symbol definition.
Symbol-strings follow the colon-separated format:
NAME:VERSION:def:vis:size:COMDATKEY
@@ -277,6 +315,12 @@ parse_option (const char *opt)
return record_claim_file (opt + 6);
else if (!strncmp ("sym:", opt, 4))
return record_claimed_file_symbol (opt + 4);
+ else if (!strncmp ("add:", opt, 4))
+ return record_add_file (opt + 4, ADD_FILE);
+ else if (!strncmp ("lib:", opt, 4))
+ return record_add_file (opt + 4, ADD_LIB);
+ else if (!strncmp ("dir:", opt, 4))
+ return record_add_file (opt + 4, ADD_DIR);
else if (!strcmp ("dumpresolutions", opt))
dumpresolutions = TRUE;
else
@@ -514,6 +558,7 @@ onall_symbols_read (void)
"LDPR_RESOLVED_DYN",
};
claim_file_t *claimfile = dumpresolutions ? claimfiles_list : NULL;
+ add_file_t *addfile = addfiles_list;
TV_MESSAGE (LDPL_INFO, "hook called: all symbols read.\n");
for ( ; claimfile; claimfile = claimfile->next)
{
@@ -535,6 +580,20 @@ onall_symbols_read (void)
: "",
resolutions[claimfile->symbols[n].resolution]);
}
+ for ( ; addfile ; addfile = addfile->next)
+ {
+ enum ld_plugin_status rv;
+ if (addfile->type == ADD_LIB && tv_add_input_library)
+ rv = (*tv_add_input_library) (addfile->name);
+ else if (addfile->type == ADD_FILE && tv_add_input_file)
+ rv = (*tv_add_input_file) (addfile->name);
+ else if (addfile->type == ADD_DIR && tv_set_extra_library_path)
+ rv = (*tv_set_extra_library_path) (addfile->name);
+ else
+ rv = LDPS_ERR;
+ if (rv != LDPS_OK)
+ return rv;
+ }
fflush (NULL);
return all_symbols_read_ret;
}
diff --git a/ld/testsuite/ld-plugin/plugin-9.d b/ld/testsuite/ld-plugin/plugin-9.d
new file mode 100644
index 0000000..b74f4a6
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-9.d
@@ -0,0 +1,35 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_OPTION 'claim:tmpdir/func.o'
+tv\[19\]: LDPT_OPTION 'sym:_?func::0:0:0'
+tv\[20\]: LDPT_OPTION 'sym:_?func2::0:0:0'
+tv\[21\]: LDPT_OPTION 'dumpresolutions'
+tv\[22\]: LDPT_OPTION 'add:tmpdir/func.o'
+tv\[23\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: claim_file tmpdir/main.o \[@0/.* not claimed
+hook called: claim_file tmpdir/func.o \[@0/.* CLAIMED
+hook called: claim_file tmpdir/text.o \[@0/.* not claimed
+#...
+hook called: all symbols read.
+Sym: '_?func' Resolution: LDPR_PREVAILING_DEF
+Sym: '_?func2' Resolution: LDPR_PREVAILING_DEF_IRONLY
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index 6ed2787..32f396c 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -99,6 +99,13 @@ set plugin_tests [list \
-plugin-arg sym:${_}func2::0:0:0 \
-plugin-arg dumpresolutions \
$testobjfiles $libs" "" "" {{ld plugin-8.d}} "main.x" ] \
+ [list "plugin claimfile replace file" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ -plugin-arg sym:${_}func::0:0:0 \
+ -plugin-arg sym:${_}func2::0:0:0 \
+ -plugin-arg dumpresolutions \
+ -plugin-arg add:tmpdir/func.o \
+ $testobjfiles $libs" "" "" {{ld plugin-9.d}} "main.x" ] \
]
if { $failed_compile != 0 } {
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
` (2 preceding siblings ...)
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths Dave Korn
@ 2010-09-23 5:09 ` Dave Korn
2010-09-23 5:41 ` Ralf Wildenhues
` (2 more replies)
2010-09-23 16:18 ` [PATCH] Add plugin interface to LD [0/4] Richard Henderson
` (2 subsequent siblings)
6 siblings, 3 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-23 5:09 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 3440 bytes --]
This is the first patch in the series: it adds the infrastructure for
parsing plugin-related command line options, and for loading, initialising,
and cleaning up the plugins. Notes, in no particular order:
- I added a --enable-plugins command to ld's configure before I remembered
that there's one in the binutils/ subdir (for nm and ar) as well. Should I
perhaps change it to --enable-ld-plugins (or something else) so that it can be
controlled separately? Should I rewrite it to use config/plugins.m4?
- Is it OK to put an unguarded "#include <stdarg.h>" in ldmisc.h?
- There are a couple of ever-so-slightly long lines and not enough comments;
both of these points get fixed in the later patches. I've split them up for
ease of review but would like to apply them either in one, or in rapid
succession, so that it'll be corrected immediately.
include/ChangeLog:
* plugin-api.h (LDPT_GNU_LD_VERSION): New ld_plugin_tag enum member.
ld/ChangeLog:
* configure.in: Add --enable-plugins option and additional AC_CHECKs
for required headers and functions.
(ENABLE_PLUGINS): Add related automake conditional.
* configure: Regenerate.
* config.in: Likewise.
* Makefile.am (PLUGIN_C): Declare plugin C source file, conditional
on ENABLE_PLUGINS being defined.
(PLUGIN_H): Likewise for header file.
(PLUGIN_OBJEXT): Likewise for object file.
(PLUGIN_CFLAGS): Likewise -D flag required to compile plugin support.
(AM_CFLAGS): Use PLUGIN_CFLAGS.
(CFILES): Use PLUGIN_C.
(HFILES): Use PLUGIN_H.
(OFILES): Use PLUGIN_OBJEXT.
(ld_new_SOURCES): Use PLUGIN_C.
(noinst_LTLIBRARIES)[ENABLE_PLUGINS]: Declare test plugin.
(libldtestplug_la_SOURCES)[ENABLE_PLUGINS]: Add automake definition
for test plugin.
(libldtestplug_la_CFLAGS)[ENABLE_PLUGINS]: Likewise.
(libldtestplug_la_LDFLAGS)[ENABLE_PLUGINS]: Likewise.
* Makefile.in: Regenerate.
* sysdep.h: Include unistd.h and one of fcntl.h or sys/file.h where
available.
(O_RDONLY): Supply default definition likewise to bfd's sysdep.h
(O_WRONLY): Likewise.
(O_RDWR): Likewise.
(O_ACCMODE): Likewise.
(SEEK_SET): Likewise.
(SEEK_CUR): Likewise.
(SEEK_END): Likewise.
* ldmisc.c (vfinfo): Make non-static.
* ldmisc.h: Include stdarg.h for va_list type.
(vfinfo): Declare extern prototype.
* lexsup.c (enum option_values)[ENABLE_PLUGINS]: Add new entries for
OPTION_PLUGIN and OPTION_PLUGIN_ARG.
(ld_options[])[ENABLE_PLUGINS]: Add option data for the above two.
(parse_args)[ENABLE_PLUGINS]: Handle them, and load all plugins once
option parsing is complete.
* ldmain.c (main)[ENABLE_PLUGINS]: Call plugin cleanup hooks just
after lang_finish.
* plugin.c: New source file.
* plugin.h: Likewise new header.
* testplug.c: New source file.
ld/testsuite/ChangeLog:
* lib/ld-lib.exp (proc regexp_diff): Extend verbose debug output.
(proc set_file_contents): Write a file with the supplied content.
(run_ld_link_tests): Add new 'ld' action to test linker output.
(proc check_plugin_api_available): Return true if linker under test
supports the plugin API.
* ld-plugin/func.c: New test source file.
* ld-plugin/main.c: Likewise.
* ld-plugin/text.c: Likewise.
* ld-plugin/plugin-1.d: New dump test output pattern script.
* ld-plugin/plugin-2.d: Likewise.
* ld-plugin/plugin-3.d: Likewise.
* ld-plugin/plugin-4.d: Likewise.
* ld-plugin/plugin-5.d: Likewise.
* ld-plugin/plugin.exp: New test control script.
cheers,
DaveK
[-- Attachment #2: ld-plugin-api-1-infra.diff --]
[-- Type: text/x-c, Size: 53715 bytes --]
From 4f6d92e075279cca74abd64cc00878197127d6e9 Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Fri, 10 Sep 2010 11:02:24 +0100
Subject: [PATCH] Add infrastructure for plugin API; functionality to follow.
include/ChangeLog:
* plugin-api.h (LDPT_GNU_LD_VERSION): New ld_plugin_tag enum member.
ld/ChangeLog:
* configure.in: Add --enable-plugins option and additional AC_CHECKs
for required headers and functions.
(ENABLE_PLUGINS): Add related automake conditional.
* configure: Regenerate.
* config.in: Likewise.
* Makefile.am (PLUGIN_C): Declare plugin C source file, conditional
on ENABLE_PLUGINS being defined.
(PLUGIN_H): Likewise for header file.
(PLUGIN_OBJEXT): Likewise for object file.
(PLUGIN_CFLAGS): Likewise -D flag required to compile plugin support.
(AM_CFLAGS): Use PLUGIN_CFLAGS.
(CFILES): Use PLUGIN_C.
(HFILES): Use PLUGIN_H.
(OFILES): Use PLUGIN_OBJEXT.
(ld_new_SOURCES): Use PLUGIN_C.
(noinst_LTLIBRARIES)[ENABLE_PLUGINS]: Declare test plugin.
(libldtestplug_la_SOURCES)[ENABLE_PLUGINS]: Add automake definition
for test plugin.
(libldtestplug_la_CFLAGS)[ENABLE_PLUGINS]: Likewise.
(libldtestplug_la_LDFLAGS)[ENABLE_PLUGINS]: Likewise.
* Makefile.in: Regenerate.
* sysdep.h: Include unistd.h and one of fcntl.h or sys/file.h where
available.
(O_RDONLY): Supply default definition likewise to bfd's sysdep.h
(O_WRONLY): Likewise.
(O_RDWR): Likewise.
(O_ACCMODE): Likewise.
(SEEK_SET): Likewise.
(SEEK_CUR): Likewise.
(SEEK_END): Likewise.
* ldmisc.c (vfinfo): Make non-static.
* ldmisc.h: Include stdarg.h for va_list type.
(vfinfo): Declare extern prototype.
* lexsup.c (enum option_values)[ENABLE_PLUGINS]: Add new entries for
OPTION_PLUGIN and OPTION_PLUGIN_ARG.
(ld_options[])[ENABLE_PLUGINS]: Add option data for the above two.
(parse_args)[ENABLE_PLUGINS]: Handle them, and load all plugins once
option parsing is complete.
* ldmain.c (main)[ENABLE_PLUGINS]: Call plugin cleanup hooks just
after lang_finish.
* plugin.c: New source file.
* plugin.h: Likewise new header.
* testplug.c: New source file.
ld/testsuite/ChangeLog:
* lib/ld-lib.exp (proc regexp_diff): Extend verbose debug output.
(proc set_file_contents): Write a file with the supplied content.
(run_ld_link_tests): Add new 'ld' action to test linker output.
(proc check_plugin_api_available): Return true if linker under test
supports the plugin API.
* ld-plugin/func.c: New test source file.
* ld-plugin/main.c: Likewise.
* ld-plugin/text.c: Likewise.
* ld-plugin/plugin-1.d: New dump test output pattern script.
* ld-plugin/plugin-2.d: Likewise.
* ld-plugin/plugin-3.d: Likewise.
* ld-plugin/plugin-4.d: Likewise.
* ld-plugin/plugin-5.d: Likewise.
* ld-plugin/plugin.exp: New test control script.
---
include/plugin-api.h | 3 +-
ld/Makefile.am | 36 +++-
ld/Makefile.in | 149 ++++++++----
ld/config.in | 18 ++
ld/configure | 59 ++++-
ld/configure.in | 12 +
ld/ldmain.c | 9 +
ld/ldmisc.c | 2 +-
ld/ldmisc.h | 3 +
ld/lexsup.c | 31 +++-
ld/plugin.c | 507 +++++++++++++++++++++++++++++++++++++
ld/plugin.h | 46 ++++
ld/sysdep.h | 35 +++
ld/testplug.c | 344 +++++++++++++++++++++++++
ld/testsuite/ld-plugin/func.c | 7 +
ld/testsuite/ld-plugin/main.c | 13 +
ld/testsuite/ld-plugin/plugin-1.d | 18 ++
ld/testsuite/ld-plugin/plugin-2.d | 21 ++
ld/testsuite/ld-plugin/plugin-3.d | 23 ++
ld/testsuite/ld-plugin/plugin-4.d | 23 ++
ld/testsuite/ld-plugin/plugin-5.d | 31 +++
ld/testsuite/ld-plugin/plugin.exp | 76 ++++++
ld/testsuite/ld-plugin/text.c | 3 +
ld/testsuite/lib/ld-lib.exp | 52 ++++-
24 files changed, 1461 insertions(+), 60 deletions(-)
diff --git a/include/plugin-api.h b/include/plugin-api.h
index a0cf5f4..9d58fef 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -268,7 +268,8 @@ enum ld_plugin_tag
LDPT_RELEASE_INPUT_FILE,
LDPT_ADD_INPUT_LIBRARY,
LDPT_OUTPUT_NAME,
- LDPT_SET_EXTRA_LIBRARY_PATH
+ LDPT_SET_EXTRA_LIBRARY_PATH,
+ LDPT_GNU_LD_VERSION
};
/* The plugin transfer vector. */
diff --git a/ld/Makefile.am b/ld/Makefile.am
index 9eaf0e2..5b0c74c 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -21,6 +21,21 @@ WARN_CFLAGS = @WARN_CFLAGS@
NO_WERROR = @NO_WERROR@
AM_CFLAGS = $(WARN_CFLAGS)
+# Conditionally enable the plugin interface.
+if ENABLE_PLUGINS
+PLUGIN_C = plugin.c
+PLUGIN_H = plugin.h
+PLUGIN_OBJEXT = plugin.@OBJEXT@
+PLUGIN_CFLAGS = -DENABLE_PLUGINS
+else
+PLUGIN_C =
+PLUGIN_H =
+PLUGIN_OBJEXT =
+PLUGIN_CFLAGS =
+endif
+
+AM_CFLAGS += $(PLUGIN_CFLAGS)
+
# We put the scripts in the directory $(scriptdir)/ldscripts.
# We can't put the scripts in $(datadir) because the SEARCH_DIR
# directives need to be different for native and cross linkers.
@@ -455,11 +470,13 @@ ALL_EMUL_EXTRA_OFILES = \
CFILES = ldctor.c ldemul.c ldexp.c ldfile.c ldlang.c \
ldmain.c ldmisc.c ldver.c ldwrite.c lexsup.c \
- mri.c ldcref.c pe-dll.c pep-dll.c ldlex-wrapper.c
+ mri.c ldcref.c pe-dll.c pep-dll.c ldlex-wrapper.c \
+ $(PLUGIN_C)
HFILES = ld.h ldctor.h ldemul.h ldexp.h ldfile.h \
ldlang.h ldlex.h ldmain.h ldmisc.h ldver.h \
- ldwrite.h mri.h deffile.h pe-dll.h pep-dll.h elf-hints-local.h
+ ldwrite.h mri.h deffile.h pe-dll.h pep-dll.h \
+ elf-hints-local.h $(PLUGIN_H)
GENERATED_CFILES = ldgram.c ldlex.c deffilep.c
GENERATED_HFILES = ldgram.h ldemul-list.h deffilep.h
@@ -468,7 +485,8 @@ GENERATED_HFILES = ldgram.h ldemul-list.h deffilep.h
# tracking will not cause them to be built beforehand.
BUILT_SOURCES = $(GENERATED_HFILES)
-OFILES = ldgram.@OBJEXT@ ldlex-wrapper.@OBJEXT@ lexsup.@OBJEXT@ ldlang.@OBJEXT@ mri.@OBJEXT@ ldctor.@OBJEXT@ ldmain.@OBJEXT@ \
+OFILES = ldgram.@OBJEXT@ ldlex-wrapper.@OBJEXT@ lexsup.@OBJEXT@ ldlang.@OBJEXT@ \
+ mri.@OBJEXT@ ldctor.@OBJEXT@ ldmain.@OBJEXT@ $(PLUGIN_OBJEXT) \
ldwrite.@OBJEXT@ ldexp.@OBJEXT@ ldemul.@OBJEXT@ ldver.@OBJEXT@ ldmisc.@OBJEXT@ \
ldfile.@OBJEXT@ ldcref.@OBJEXT@ ${EMULATION_OFILES} ${EMUL_EXTRA_OFILES}
@@ -1882,7 +1900,7 @@ EXTRA_ld_new_SOURCES = deffilep.y ldlex.l
EXTRA_ld_new_SOURCES += pep-dll.c pe-dll.c
ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmain.c \
- ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c
+ ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c $(PLUGIN_C)
ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBIBERTY) $(LIBINTL_DEP)
ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBIBERTY) $(LIBINTL)
@@ -1964,6 +1982,16 @@ bootstrap: ld3$(EXEEXT)
# END OF CHECK TARGETS
+#
+# Build a dummy plugin using libtool.
+#
+if ENABLE_PLUGINS
+noinst_LTLIBRARIES = libldtestplug.la
+libldtestplug_la_SOURCES = testplug.c
+libldtestplug_la_CFLAGS= -g -O2
+libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
+endif
+
# DOCUMENTATION TARGETS
# Manual configuration file; not usually attached to normal configuration,
# because almost all configs use "gen" version of manual.
diff --git a/ld/configure.in b/ld/configure.in
index 90baeca..cfc312b 100644
--- a/ld/configure.in
+++ b/ld/configure.in
@@ -117,6 +117,16 @@ case "${got_handling}" in
*) AC_MSG_ERROR(bad value ${got_handling} for --enable-got option) ;;
esac
+AC_ARG_ENABLE(plugins,
+[[ --enable-plugins enable plugin interface]],
+[case "${enableval}" in
+ yes | "") enable_plugins=true ;;
+ no) enable_plugins=false ;;
+ *) AC_MSG_ERROR(bad value ${enableval} for --enable-plugins option) ;;
+esac],
+[enable_plugins=false])dnl
+AM_CONDITIONAL([ENABLE_PLUGINS], [test x$enable_plugins = xtrue])
+
AM_BINUTILS_WARNINGS
AC_CONFIG_HEADERS([config.h:config.in])
@@ -159,7 +169,9 @@ AC_SUBST(HOSTING_LIBS)
AC_SUBST(NATIVE_LIB_DIRS)
AC_CHECK_HEADERS(string.h strings.h stdlib.h unistd.h elf-hints.h limits.h sys/param.h)
+AC_CHECK_HEADERS(fcntl.h sys/file.h sys/time.h sys/stat.h)
AC_CHECK_FUNCS(glob mkstemp realpath sbrk waitpid)
+AC_CHECK_FUNCS(open lseek close)
AC_HEADER_DIRENT
AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 08679ec..85236ed 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -40,6 +40,9 @@
#include "ldfile.h"
#include "ldemul.h"
#include "ldctor.h"
+#ifdef ENABLE_PLUGINS
+#include "plugin.h"
+#endif /* ENABLE_PLUGINS */
/* Somewhere above, sys/stat.h got included. */
#if !defined(S_ISDIR) && defined(S_IFDIR)
@@ -474,6 +477,12 @@ main (int argc, char **argv)
lang_finish ();
+#ifdef ENABLE_PLUGINS
+ /* Now everything is finished, we can tell the plugins to clean up. */
+ if (plugin_call_cleanup ())
+ info_msg (_("%P: %s: error in plugin cleanup (ignored)\n"), plugin_error_plugin ());
+#endif /* ENABLE_PLUGINS */
+
/* Even if we're producing relocatable output, some non-fatal errors should
be reported in the exit status. (What non-fatal errors, if any, do we
want to ignore for relocatable output?) */
diff --git a/ld/ldmisc.c b/ld/ldmisc.c
index d49cf17..31c46a9 100644
--- a/ld/ldmisc.c
+++ b/ld/ldmisc.c
@@ -62,7 +62,7 @@
%v hex bfd_vma, no leading zeros
*/
-static void
+void
vfinfo (FILE *fp, const char *fmt, va_list arg, bfd_boolean is_warning)
{
bfd_boolean fatal = FALSE;
diff --git a/ld/ldmisc.h b/ld/ldmisc.h
index ba7f0c6..b29ce5a 100644
--- a/ld/ldmisc.h
+++ b/ld/ldmisc.h
@@ -22,6 +22,9 @@
#ifndef LDMISC_H
#define LDMISC_H
+#include <stdarg.h>
+
+extern void vfinfo (FILE *fp, const char *fmt, va_list arg, bfd_boolean is_warning);
extern void einfo (const char *, ...);
extern void minfo (const char *, ...);
extern void info_msg (const char *, ...);
diff --git a/ld/lexsup.c b/ld/lexsup.c
index b992fca..b9dd3e8 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -40,6 +40,9 @@
#include "ldver.h"
#include "ldemul.h"
#include "demangle.h"
+#ifdef ENABLE_PLUGINS
+#include "plugin.h"
+#endif /* ENABLE_PLUGINS */
#ifndef PATH_SEPARATOR
#if defined (__MSDOS__) || (defined (_WIN32) && ! defined (__CYGWIN32__))
@@ -167,7 +170,11 @@ enum option_values
OPTION_WARN_SHARED_TEXTREL,
OPTION_WARN_ALTERNATE_EM,
OPTION_REDUCE_MEMORY_OVERHEADS,
- OPTION_DEFAULT_SCRIPT
+#ifdef ENABLE_PLUGINS
+ OPTION_PLUGIN,
+ OPTION_PLUGIN_ARG,
+#endif /* ENABLE_PLUGINS */
+ OPTION_DEFAULT_SCRIPT,
};
/* The long options. This structure is used for both the option
@@ -271,6 +278,12 @@ static const struct ld_option ld_options[] =
'o', N_("FILE"), N_("Set output file name"), EXACTLY_TWO_DASHES },
{ {NULL, required_argument, NULL, '\0'},
'O', NULL, N_("Optimize output file"), ONE_DASH },
+#ifdef ENABLE_PLUGINS
+ { {"plugin", required_argument, NULL, OPTION_PLUGIN},
+ '\0', N_("PLUGIN"), N_("Load named plugin"), ONE_DASH },
+ { {"plugin-arg", required_argument, NULL, OPTION_PLUGIN_ARG},
+ '\0', N_("ARG"), N_("Send arg to last-loaded plugin"), ONE_DASH },
+#endif /* ENABLE_PLUGINS */
{ {"Qy", no_argument, NULL, OPTION_IGNORE},
'\0', NULL, N_("Ignored for SVR4 compatibility"), ONE_DASH },
{ {"emit-relocs", no_argument, NULL, 'q'},
@@ -1040,6 +1053,16 @@ parse_args (unsigned argc, char **argv)
case OPTION_OFORMAT:
lang_add_output_format (optarg, NULL, NULL, 0);
break;
+#ifdef ENABLE_PLUGINS
+ case OPTION_PLUGIN:
+ if (plugin_opt_plugin (optarg))
+ einfo(_("%P%F: bad -plugin option\n"));
+ break;
+ case OPTION_PLUGIN_ARG:
+ if (plugin_opt_plugin_arg (optarg))
+ einfo(_("%P%F: bad -plugin-arg option\n"));
+ break;
+#endif /* ENABLE_PLUGINS */
case 'q':
link_info.emitrelocations = TRUE;
break;
@@ -1517,6 +1540,12 @@ parse_args (unsigned argc, char **argv)
if (link_info.unresolved_syms_in_shared_libs == RM_NOT_YET_SET)
/* FIXME: Should we allow emulations a chance to set this ? */
link_info.unresolved_syms_in_shared_libs = how_to_report_unresolved_symbols;
+
+#ifdef ENABLE_PLUGINS
+ /* Now all the plugin arguments have been gathered, we can load them. */
+ if (plugin_load_plugins ())
+ einfo (_("%P%F: %s: error loading plugin\n"), plugin_error_plugin ());
+#endif /* ENABLE_PLUGINS */
}
/* Add the (colon-separated) elements of DIRLIST_PTR to the
diff --git a/ld/plugin.c b/ld/plugin.c
new file mode 100644
index 0000000..e9f3b81
--- /dev/null
+++ b/ld/plugin.c
@@ -0,0 +1,507 @@
+/* Plugin control for the GNU linker.
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This file is part of the GNU Binutils.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+ MA 02110-1301, USA. */
+
+#include <dlfcn.h>
+
+#include "sysdep.h"
+#include "libiberty.h"
+#include "bfd.h"
+#include "bfdlink.h"
+#include "bfdver.h"
+#include "ld.h"
+#include "ldmain.h"
+#include "ldmisc.h"
+#include "plugin.h"
+#include "plugin-api.h"
+
+/* Alias to shorten static function prototype lines. */
+#define PLUGAPIFUNC static enum ld_plugin_status
+
+PLUGAPIFUNC message (int level, const char *format, ...);
+PLUGAPIFUNC register_claim_file (ld_plugin_claim_file_handler handler);
+PLUGAPIFUNC register_all_symbols_read (
+ ld_plugin_all_symbols_read_handler handler);
+PLUGAPIFUNC register_cleanup (ld_plugin_cleanup_handler handler);
+PLUGAPIFUNC add_symbols (void *handle, int nsyms,
+ const struct ld_plugin_symbol *syms);
+PLUGAPIFUNC get_input_file (const void *handle,
+ struct ld_plugin_input_file *file);
+PLUGAPIFUNC release_input_file (const void *handle);
+PLUGAPIFUNC get_symbols (const void *handle, int nsyms,
+ struct ld_plugin_symbol *syms);
+PLUGAPIFUNC add_input_file (const char *pathname);
+PLUGAPIFUNC add_input_library (const char *pathname);
+PLUGAPIFUNC set_extra_library_path (const char *path);
+
+/* Always use this macro when invoking a plugin function. */
+#define INVOKE_PLUGIN_FN(plugin, retval, fn, args) \
+ called_plugin = plugin; \
+ retval = (*fn) args; \
+ called_plugin = NULL;
+
+/* Stores a single argument passed to a plugin. */
+typedef struct plugin_arg
+{
+ struct plugin_arg *next;
+ const char *arg;
+} plugin_arg_t;
+
+/* Holds all details of a single plugin. */
+typedef struct plugin
+{
+ /* Next on the list of plugins, or NULL at end of chain. */
+ struct plugin *next;
+ /* The argument string given to --plugin. */
+ const char *name;
+ /* The shared library handle returned by dlopen. */
+ void *dlhandle;
+ /* The list of argument string given to --plugin-opt. */
+ plugin_arg_t *args;
+ /* Number of args in the list, for convenience. */
+ size_t n_args;
+ /* The plugin's event handlers. */
+ ld_plugin_claim_file_handler claim_file_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. */
+ bfd_boolean cleanup_done;
+} plugin_t;
+
+/* The master list of all plugins. */
+static plugin_t *plugins_list = NULL;
+
+/* We keep a tail pointer for easy linking on the end. */
+static plugin_t **plugins_tail_chain_ptr = &plugins_list;
+
+/* The last plugin added to the list, for receiving args. */
+static plugin_t *last_plugin = NULL;
+
+/* The tail of the arg chain of the last plugin added to the list. */
+static plugin_arg_t **last_plugin_args_tail_chain_ptr = NULL;
+
+/* The plugin which is currently having a callback executed. */
+static plugin_t *called_plugin = NULL;
+
+/* Last plugin to cause an error, if any. */
+static const char *error_plugin = NULL;
+
+/* Helper to size leading part of tv array and set it up. */
+static size_t
+set_tv_header (struct ld_plugin_tv *tv)
+{
+ size_t i;
+ /* List of tags to set in the constant leading part of the tv array. */
+ static const enum ld_plugin_tag tv_header_tags[] =
+ {
+ LDPT_MESSAGE,
+ LDPT_API_VERSION,
+ LDPT_GNU_LD_VERSION,
+ LDPT_LINKER_OUTPUT,
+ LDPT_OUTPUT_NAME,
+ LDPT_REGISTER_CLAIM_FILE_HOOK,
+ LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK,
+ LDPT_REGISTER_CLEANUP_HOOK,
+ LDPT_ADD_SYMBOLS,
+ LDPT_GET_INPUT_FILE,
+ LDPT_RELEASE_INPUT_FILE,
+ LDPT_GET_SYMBOLS,
+ LDPT_ADD_INPUT_FILE,
+ LDPT_ADD_INPUT_LIBRARY,
+ LDPT_SET_EXTRA_LIBRARY_PATH
+ };
+ /* How many entries. */
+ static const size_t tv_header_size = ARRAY_SIZE (tv_header_tags);
+
+ /* Version info. */
+ static const unsigned int major = (unsigned)(BFD_VERSION / 100000000UL);
+ static const unsigned int minor = (unsigned)(BFD_VERSION / 1000000UL) % 100;
+
+ if (!tv)
+ return tv_header_size;
+
+ for (i = 0; i < tv_header_size; i++)
+ {
+ tv[i].tv_tag = tv_header_tags[i];
+#define TVU(x) tv[i].tv_u.tv_ ## x
+ switch (tv[i].tv_tag)
+ {
+ case LDPT_MESSAGE:
+ TVU(message) = message;
+ break;
+ case LDPT_API_VERSION:
+ TVU(val) = LD_PLUGIN_API_VERSION;
+ break;
+ case LDPT_GNU_LD_VERSION:
+ TVU(val) = major * 100 + minor;
+ break;
+ case LDPT_LINKER_OUTPUT:
+ TVU(val) = link_info.relocatable ? LDPO_REL
+ : (link_info.shared ? LDPO_DYN : LDPO_EXEC);
+ break;
+ case LDPT_OUTPUT_NAME:
+ TVU(string) = output_filename;
+ break;
+ case LDPT_REGISTER_CLAIM_FILE_HOOK:
+ TVU(register_claim_file) = register_claim_file;
+ break;
+ case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
+ TVU(register_all_symbols_read) = register_all_symbols_read;
+ break;
+ case LDPT_REGISTER_CLEANUP_HOOK:
+ TVU(register_cleanup) = register_cleanup;
+ break;
+ case LDPT_ADD_SYMBOLS:
+ TVU(add_symbols) = add_symbols;
+ break;
+ case LDPT_GET_INPUT_FILE:
+ TVU(get_input_file) = get_input_file;
+ break;
+ case LDPT_RELEASE_INPUT_FILE:
+ TVU(release_input_file) = release_input_file;
+ break;
+ case LDPT_GET_SYMBOLS:
+ TVU(get_symbols) = get_symbols;
+ break;
+ case LDPT_ADD_INPUT_FILE:
+ TVU(add_input_file) = add_input_file;
+ break;
+ case LDPT_ADD_INPUT_LIBRARY:
+ TVU(add_input_library) = add_input_library;
+ break;
+ case LDPT_SET_EXTRA_LIBRARY_PATH:
+ TVU(set_extra_library_path) = set_extra_library_path;
+ break;
+ default:
+ /* Added a new entry to the array without adding
+ a new case to set up its value is a bug. */
+ FAIL ();
+ }
+#undef TVU
+ }
+ return tv_header_size;
+}
+
+/* Append the per-plugin args list and trailing LDPT_NULL to tv. */
+static void
+set_tv_plugin_args (plugin_t *plugin, struct ld_plugin_tv *tv)
+{
+ plugin_arg_t *arg = plugin->args;
+ while (arg)
+ {
+ tv->tv_tag = LDPT_OPTION;
+ tv->tv_u.tv_string = arg->arg;
+ arg = arg->next;
+ tv++;
+ }
+ tv->tv_tag = LDPT_NULL;
+ tv->tv_u.tv_val = 0;
+}
+
+/* Helper function for exiting with error status. */
+static int
+set_plugin_error (const char *plugin)
+{
+ error_plugin = plugin;
+ return -1;
+}
+
+/* Test if an error occurred. */
+static bfd_boolean
+plugin_error_p (void)
+{
+ return error_plugin != NULL;
+}
+
+/* Return name of plugin which caused an error if any. */
+const char *plugin_error_plugin (void)
+{
+ return error_plugin ? error_plugin : _("<no plugin>");
+}
+
+/* Handle -plugin arg: find and load plugin, or return error. */
+int plugin_opt_plugin (const char *plugin)
+{
+ plugin_t *newplug;
+
+ newplug = xmalloc (sizeof *newplug);
+ memset (newplug, 0, sizeof *newplug);
+ newplug->name = plugin;
+ newplug->dlhandle = dlopen (plugin, RTLD_NOW);
+ if (!newplug->dlhandle)
+ return set_plugin_error (plugin);
+
+ /* Chain on end, so when we run list it is in command-line order. */
+ *plugins_tail_chain_ptr = newplug;
+ plugins_tail_chain_ptr = &newplug->next;
+
+ /* Record it as current plugin for receiving args. */
+ last_plugin = newplug;
+ last_plugin_args_tail_chain_ptr = &newplug->args;
+ return 0;
+}
+
+/* Accumulate option arguments for last-loaded plugin, or return
+ error if none. */
+int plugin_opt_plugin_arg (const char *arg)
+{
+ plugin_arg_t *newarg;
+
+ if (!last_plugin)
+ return set_plugin_error (_("<no plugin>"));
+
+ newarg = xmalloc (sizeof *newarg);
+ newarg->arg = arg;
+ newarg->next = NULL;
+
+ /* Chain on end to preserve command-line order. */
+ *last_plugin_args_tail_chain_ptr = newarg;
+ last_plugin_args_tail_chain_ptr = &newarg->next;
+ last_plugin->n_args++;
+ return 0;
+}
+
+/* Load up and initialise all plugins after argument parsing. */
+int plugin_load_plugins (void)
+{
+ struct ld_plugin_tv *my_tv;
+ unsigned int max_args = 0;
+ plugin_t *curplug = plugins_list;
+
+ /* First pass over plugins to find max # args needed so that we
+ can size and allocate the tv array. */
+ while (curplug)
+ {
+ if (curplug->n_args > max_args)
+ max_args = curplug->n_args;
+ curplug = curplug->next;
+ }
+ /* Allocate tv array and initialise constant part. */
+ my_tv = xmalloc ((max_args + 1 + set_tv_header (NULL)) * sizeof *my_tv);
+ set_tv_header (my_tv);
+
+ curplug = plugins_list;
+ while (curplug)
+ {
+ enum ld_plugin_status rv;
+ ld_plugin_onload onloadfn = dlsym (curplug->dlhandle, "onload");
+ if (!onloadfn)
+ onloadfn = dlsym (curplug->dlhandle, "_onload");
+ if (!onloadfn)
+ return set_plugin_error (curplug->name);
+ set_tv_plugin_args (curplug, &my_tv[set_tv_header (NULL)]);
+ INVOKE_PLUGIN_FN (curplug, rv, onloadfn, (my_tv));
+ if (rv != LDPS_OK)
+ return set_plugin_error (curplug->name);
+ curplug = curplug->next;
+ }
+ return 0;
+}
+
+/* Call 'claim file' hook for all plugins. */
+int
+plugin_call_claim_file (const struct ld_plugin_input_file *file, int *claimed)
+{
+ plugin_t *curplug = plugins_list;
+ *claimed = FALSE;
+ while (curplug && !*claimed)
+ {
+ if (curplug->claim_file_handler)
+ {
+ enum ld_plugin_status rv;
+ INVOKE_PLUGIN_FN (curplug, rv, curplug->claim_file_handler, \
+ (file, claimed));
+ if (rv != LDPS_OK)
+ set_plugin_error (curplug->name);
+ }
+ curplug = curplug->next;
+ }
+ return plugin_error_p () ? -1 : 0;
+}
+
+/* Call 'all symbols read' hook for all plugins. */
+int
+plugin_call_all_symbols_read (void)
+{
+ plugin_t *curplug = plugins_list;
+ while (curplug)
+ {
+ if (curplug->all_symbols_read_handler)
+ {
+ enum ld_plugin_status rv;
+ INVOKE_PLUGIN_FN (curplug, rv, curplug->all_symbols_read_handler, \
+ ());
+ if (rv != LDPS_OK)
+ set_plugin_error (curplug->name);
+ }
+ curplug = curplug->next;
+ }
+ return plugin_error_p () ? -1 : 0;
+}
+
+/* Call 'cleanup' hook for all plugins. */
+int
+plugin_call_cleanup (void)
+{
+ plugin_t *curplug = plugins_list;
+ while (curplug)
+ {
+ if (curplug->cleanup_handler && !curplug->cleanup_done)
+ {
+ enum ld_plugin_status rv;
+ curplug->cleanup_done = TRUE;
+ INVOKE_PLUGIN_FN (curplug, rv, curplug->cleanup_handler, ());
+ if (rv != LDPS_OK)
+ set_plugin_error (curplug->name);
+ dlclose (curplug->dlhandle);
+ }
+ curplug = curplug->next;
+ }
+ return plugin_error_p () ? -1 : 0;
+}
+
+/* Register a claim-file handler. */
+static enum ld_plugin_status
+register_claim_file (ld_plugin_claim_file_handler handler)
+{
+ ASSERT (called_plugin);
+ called_plugin->claim_file_handler = handler;
+ return LDPS_OK;
+}
+
+/* Register an all-symbols-read handler. */
+static enum ld_plugin_status
+register_all_symbols_read (ld_plugin_all_symbols_read_handler handler)
+{
+ ASSERT (called_plugin);
+ called_plugin->all_symbols_read_handler = handler;
+ return LDPS_OK;
+}
+
+/* Register a cleanup handler. */
+static enum ld_plugin_status
+register_cleanup (ld_plugin_cleanup_handler handler)
+{
+ ASSERT (called_plugin);
+ called_plugin->cleanup_handler = handler;
+ return LDPS_OK;
+}
+
+/* Add symbols from a plugin-claimed input file. */
+static enum ld_plugin_status
+add_symbols (void *handle, int nsyms, const struct ld_plugin_symbol *syms)
+{
+ ASSERT (called_plugin);
+ handle = handle;
+ nsyms = nsyms;
+ syms = syms;
+ return LDPS_ERR;
+}
+
+/* Get the input file information with an open (possibly re-opened)
+ file descriptor. */
+static enum ld_plugin_status
+get_input_file (const void *handle, struct ld_plugin_input_file *file)
+{
+ ASSERT (called_plugin);
+ handle = handle;
+ file = file;
+ return LDPS_ERR;
+}
+
+/* Release the input file. */
+static enum ld_plugin_status
+release_input_file (const void *handle)
+{
+ ASSERT (called_plugin);
+ handle = handle;
+ return LDPS_ERR;
+}
+
+/* Get the symbol resolution info for a plugin-claimed input file. */
+static enum ld_plugin_status
+get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
+{
+ ASSERT (called_plugin);
+ handle = handle;
+ nsyms = nsyms;
+ syms = syms;
+ return LDPS_ERR;
+}
+
+/* Add a new (real) input file generated by a plugin. */
+static enum ld_plugin_status
+add_input_file (const char *pathname)
+{
+ ASSERT (called_plugin);
+ pathname = pathname;
+ return LDPS_ERR;
+}
+
+/* Add a new (real) library required by a plugin. */
+static enum ld_plugin_status
+add_input_library (const char *pathname)
+{
+ ASSERT (called_plugin);
+ pathname = pathname;
+ return LDPS_ERR;
+}
+
+/* Set the extra library path to be used by libraries added via
+ add_input_library. */
+static enum ld_plugin_status
+set_extra_library_path (const char *path)
+{
+ ASSERT (called_plugin);
+ path = path;
+ return LDPS_ERR;
+}
+
+/* Issue a diagnostic message from a plugin. */
+static enum ld_plugin_status
+message (int level, const char *format, ...)
+{
+ va_list args;
+ va_start (args, format);
+
+ switch (level)
+ {
+ case LDPL_INFO:
+ vfinfo (stdout, format, args, FALSE);
+ break;
+ case LDPL_WARNING:
+ vfinfo (stdout, format, args, TRUE);
+ break;
+ case LDPL_FATAL:
+ case LDPL_ERROR:
+ default:
+ {
+ char *newfmt = xmalloc (strlen (format) + 3);
+ newfmt[0] = '%';
+ newfmt[1] = (level == LDPL_FATAL) ? 'F' : 'X';
+ strcpy (&newfmt[2], format);
+ vfinfo (stderr, newfmt, args, TRUE);
+ }
+ break;
+ }
+
+ va_end (args);
+ return LDPS_OK;
+}
+
diff --git a/ld/plugin.h b/ld/plugin.h
new file mode 100644
index 0000000..fe3d148
--- /dev/null
+++ b/ld/plugin.h
@@ -0,0 +1,46 @@
+/* Plugin control for the GNU linker.
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This file is part of the GNU Binutils.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+ MA 02110-1301, USA. */
+
+/* This is the only forward declaration we need to avoid having
+ to include the plugin-api.h header in order to use this file. */
+struct ld_plugin_input_file;
+
+/* Handle -plugin arg: find and load plugin, or return error. */
+extern int plugin_opt_plugin (const char *plugin);
+
+/* Accumulate option arguments for last-loaded plugin, or return
+ error if none. */
+extern int plugin_opt_plugin_arg (const char *arg);
+
+/* Load up and initialise all plugins after argument parsing. */
+extern int plugin_load_plugins (void);
+
+/* Return name of plugin which caused an error in any of the above. */
+extern const char *plugin_error_plugin (void);
+
+/* Call 'claim file' hook for all plugins. */
+extern int plugin_call_claim_file (const struct ld_plugin_input_file *file, int *claimed);
+
+/* Call 'all symbols read' hook for all plugins. */
+extern int plugin_call_all_symbols_read (void);
+
+/* Call 'cleanup' hook for all plugins. */
+extern int plugin_call_cleanup (void);
+
diff --git a/ld/sysdep.h b/ld/sysdep.h
index 97fe17d..929a5b4 100644
--- a/ld/sysdep.h
+++ b/ld/sysdep.h
@@ -71,12 +71,47 @@ extern char *strrchr ();
# define REALPATH(a,b) NULL
#endif
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
#ifdef USE_BINARY_FOPEN
#include "fopen-bin.h"
#else
#include "fopen-same.h"
#endif
+#ifdef HAVE_FCNTL_H
+#include <fcntl.h>
+#else
+#ifdef HAVE_SYS_FILE_H
+#include <sys/file.h>
+#endif
+#endif
+
+#ifndef O_RDONLY
+#define O_RDONLY 0
+#endif
+#ifndef O_WRONLY
+#define O_WRONLY 1
+#endif
+#ifndef O_RDWR
+#define O_RDWR 2
+#endif
+#ifndef O_ACCMODE
+#define O_ACCMODE (O_RDONLY | O_WRONLY | O_RDWR)
+#endif
+
+#ifndef SEEK_SET
+#define SEEK_SET 0
+#endif
+#ifndef SEEK_CUR
+#define SEEK_CUR 1
+#endif
+#ifndef SEEK_END
+#define SEEK_END 2
+#endif
+
#if !HAVE_DECL_STRSTR
extern char *strstr ();
#endif
diff --git a/ld/testplug.c b/ld/testplug.c
new file mode 100644
index 0000000..46d7c0f
--- /dev/null
+++ b/ld/testplug.c
@@ -0,0 +1,344 @@
+/* Test plugin for the GNU linker.
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This file is part of the GNU Binutils.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+ MA 02110-1301, USA. */
+
+#include <stdio.h>
+#include <string.h>
+#include "config.h"
+#include "bfd.h"
+#include "libiberty.h"
+#include "plugin-api.h"
+
+extern enum ld_plugin_status onload (struct ld_plugin_tv *tv);
+enum ld_plugin_status onclaim_file (const struct ld_plugin_input_file *file, int *claimed);
+enum ld_plugin_status onall_symbols_read (void);
+enum ld_plugin_status oncleanup (void);
+
+#define ADDENTRY(tag) { tag, #tag }
+
+typedef struct tag_name
+{
+ enum ld_plugin_tag tag;
+ const char *name;
+} tag_name_t;
+
+tag_name_t tag_names[] =
+{
+ ADDENTRY(LDPT_NULL),
+ ADDENTRY(LDPT_API_VERSION),
+ ADDENTRY(LDPT_GOLD_VERSION),
+ ADDENTRY(LDPT_LINKER_OUTPUT),
+ ADDENTRY(LDPT_OPTION),
+ ADDENTRY(LDPT_REGISTER_CLAIM_FILE_HOOK),
+ ADDENTRY(LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK),
+ ADDENTRY(LDPT_REGISTER_CLEANUP_HOOK),
+ ADDENTRY(LDPT_ADD_SYMBOLS),
+ ADDENTRY(LDPT_GET_SYMBOLS),
+ ADDENTRY(LDPT_ADD_INPUT_FILE),
+ ADDENTRY(LDPT_MESSAGE),
+ ADDENTRY(LDPT_GET_INPUT_FILE),
+ ADDENTRY(LDPT_RELEASE_INPUT_FILE),
+ ADDENTRY(LDPT_ADD_INPUT_LIBRARY),
+ ADDENTRY(LDPT_OUTPUT_NAME),
+ ADDENTRY(LDPT_SET_EXTRA_LIBRARY_PATH),
+ ADDENTRY(LDPT_GNU_LD_VERSION)
+};
+
+/* Function pointers to cache hooks passed at onload time. */
+static ld_plugin_register_claim_file tv_register_claim_file = 0;
+static ld_plugin_register_all_symbols_read tv_register_all_symbols_read = 0;
+static ld_plugin_register_cleanup tv_register_cleanup = 0;
+static ld_plugin_add_symbols tv_add_symbols = 0;
+static ld_plugin_get_symbols tv_get_symbols = 0;
+static ld_plugin_add_input_file tv_add_input_file = 0;
+static ld_plugin_message tv_message = 0;
+static ld_plugin_get_input_file tv_get_input_file = 0;
+static ld_plugin_release_input_file tv_release_input_file = 0;
+static ld_plugin_add_input_library tv_add_input_library = 0;
+static ld_plugin_set_extra_library_path tv_set_extra_library_path = 0;
+
+/* Other cached info from the transfer vector. */
+static enum ld_plugin_output_file_type linker_output;
+static const char *output_name;
+
+/* Behaviour control flags set by plugin options. */
+static enum ld_plugin_status onload_ret = LDPS_OK;
+static enum ld_plugin_status claim_file_ret = LDPS_OK;
+static enum ld_plugin_status all_symbols_read_ret = LDPS_OK;
+static enum ld_plugin_status cleanup_ret = LDPS_OK;
+static bfd_boolean register_claimfile_hook = FALSE;
+static bfd_boolean register_allsymbolsread_hook = FALSE;
+static bfd_boolean register_cleanup_hook = FALSE;
+
+static enum ld_plugin_status
+set_ret_val (const char *whichval, enum ld_plugin_status retval)
+{
+ if (!strcmp ("onload", whichval))
+ onload_ret = retval;
+ else if (!strcmp ("claimfile", whichval))
+ claim_file_ret = retval;
+ else if (!strcmp ("allsymbolsread", whichval))
+ all_symbols_read_ret = retval;
+ else if (!strcmp ("cleanup", whichval))
+ cleanup_ret = retval;
+ else
+ return LDPS_ERR;
+ return LDPS_OK;
+}
+
+static enum ld_plugin_status
+set_register_hook (const char *whichhook, bfd_boolean yesno)
+{
+ if (!strcmp ("claimfile", whichhook))
+ register_claimfile_hook = yesno;
+ else if (!strcmp ("allsymbolsread", whichhook))
+ register_allsymbolsread_hook = yesno;
+ else if (!strcmp ("cleanup", whichhook))
+ register_cleanup_hook = yesno;
+ else
+ return LDPS_ERR;
+ return LDPS_OK;
+}
+
+static enum ld_plugin_status
+parse_option (const char *opt)
+{
+ if (!strncmp ("fail", opt, 4))
+ return set_ret_val (opt + 4, LDPS_ERR);
+ else if (!strncmp ("pass", opt, 4))
+ return set_ret_val (opt + 4, LDPS_OK);
+ else if (!strncmp ("register", opt, 8))
+ return set_register_hook (opt + 8, TRUE);
+ else if (!strncmp ("noregister", opt, 10))
+ return set_register_hook (opt + 10, FALSE);
+ else
+ return LDPS_ERR;
+ return LDPS_OK;
+}
+
+static void
+dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
+{
+ size_t tag;
+ char unknownbuf[40];
+ const char *name;
+
+ for (tag = 0; tag < ARRAY_SIZE (tag_names); tag++)
+ if (tag_names[tag].tag == tv->tv_tag)
+ break;
+ sprintf (unknownbuf, "unknown tag #%d", tv->tv_tag);
+ name = (tag < ARRAY_SIZE (tag_names)) ? tag_names[tag].name : unknownbuf;
+ if (tv_message)
+ (*tv_message) (LDPL_INFO, "tv[%d]: %s ", n, name);
+ switch (tv->tv_tag)
+ {
+ case LDPT_OPTION:
+ case LDPT_OUTPUT_NAME:
+ if (tv_message)
+ (*tv_message) (LDPL_INFO, "'%s'\n", tv->tv_u.tv_string);
+ break;
+ case LDPT_REGISTER_CLAIM_FILE_HOOK:
+ case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
+ case LDPT_REGISTER_CLEANUP_HOOK:
+ case LDPT_ADD_SYMBOLS:
+ case LDPT_GET_SYMBOLS:
+ case LDPT_ADD_INPUT_FILE:
+ case LDPT_MESSAGE:
+ case LDPT_GET_INPUT_FILE:
+ case LDPT_RELEASE_INPUT_FILE:
+ case LDPT_ADD_INPUT_LIBRARY:
+ case LDPT_SET_EXTRA_LIBRARY_PATH:
+ if (tv_message)
+ (*tv_message) (LDPL_INFO, "func@0x%v\n",
+ (bfd_vma)(tv->tv_u.tv_message));
+ break;
+ case LDPT_NULL:
+ case LDPT_API_VERSION:
+ case LDPT_GOLD_VERSION:
+ case LDPT_LINKER_OUTPUT:
+ case LDPT_GNU_LD_VERSION:
+ default:
+ if (tv_message)
+ (*tv_message) (LDPL_INFO, "value %W (%d)\n",
+ (bfd_vma)tv->tv_u.tv_val, tv->tv_u.tv_val);
+ break;
+ }
+}
+
+static enum ld_plugin_status
+parse_tv_tag (struct ld_plugin_tv *tv)
+{
+#define SETVAR(x) x = tv->tv_u.x
+ switch (tv->tv_tag)
+ {
+ case LDPT_OPTION:
+ return parse_option (tv->tv_u.tv_string);
+ case LDPT_NULL:
+ case LDPT_GOLD_VERSION:
+ case LDPT_GNU_LD_VERSION:
+ case LDPT_API_VERSION:
+ default:
+ break;
+ case LDPT_OUTPUT_NAME:
+ output_name = tv->tv_u.tv_string;
+ break;
+ case LDPT_LINKER_OUTPUT:
+ linker_output = tv->tv_u.tv_val;
+ break;
+ case LDPT_REGISTER_CLAIM_FILE_HOOK:
+ SETVAR(tv_register_claim_file);
+ break;
+ case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
+ SETVAR(tv_register_all_symbols_read);
+ break;
+ case LDPT_REGISTER_CLEANUP_HOOK:
+ SETVAR(tv_register_cleanup);
+ break;
+ case LDPT_ADD_SYMBOLS:
+ SETVAR(tv_add_symbols);
+ break;
+ case LDPT_GET_SYMBOLS:
+ SETVAR(tv_get_symbols);
+ break;
+ case LDPT_ADD_INPUT_FILE:
+ SETVAR(tv_add_input_file);
+ break;
+ case LDPT_MESSAGE:
+ SETVAR(tv_message);
+ break;
+ case LDPT_GET_INPUT_FILE:
+ SETVAR(tv_get_input_file);
+ break;
+ case LDPT_RELEASE_INPUT_FILE:
+ SETVAR(tv_release_input_file);
+ break;
+ case LDPT_ADD_INPUT_LIBRARY:
+ SETVAR(tv_add_input_library);
+ break;
+ case LDPT_SET_EXTRA_LIBRARY_PATH:
+ SETVAR(tv_set_extra_library_path);
+ break;
+ }
+#undef SETVAR
+ return LDPS_OK;
+}
+
+enum ld_plugin_status
+parse_and_dump_tv_tag (size_t n, struct ld_plugin_tv *tv)
+{
+ enum ld_plugin_status rv = parse_tv_tag (tv);
+ dump_tv_tag (n, tv);
+ return rv;
+}
+
+enum ld_plugin_status
+onload (struct ld_plugin_tv *tv)
+{
+ size_t n = 0;
+ enum ld_plugin_status rv;
+
+ /* This plugin does nothing but dump the tv array. It would
+ be an error if this function was called without one. */
+ if (!tv)
+ return LDPS_ERR;
+
+ /* First entry should always be LDPT_MESSAGE, letting us get
+ hold of it easily so we can send output straight away. */
+ if (tv[0].tv_tag == LDPT_MESSAGE)
+ tv_message = tv[0].tv_u.tv_message;
+
+ fflush (NULL);
+ if (tv_message)
+ (*tv_message) (LDPL_INFO, "Hello from testplugin.\n");
+
+ do
+ if ((rv = parse_and_dump_tv_tag (n++, tv)) != LDPS_OK)
+ return rv;
+ while ((tv++)->tv_tag != LDPT_NULL);
+
+ if (tv_message)
+ (*tv_message) (LDPL_INFO, "\n");
+
+ /* Register hooks only if instructed by options. */
+ if (register_claimfile_hook)
+ {
+ if (!tv_register_claim_file)
+ {
+ if (tv_message)
+ (*tv_message) (LDPL_FATAL, "No register_claim_file hook\n");
+ fflush (NULL);
+ return LDPS_ERR;
+ }
+ (*tv_register_claim_file) (onclaim_file);
+ }
+ if (register_allsymbolsread_hook)
+ {
+ if (!tv_register_all_symbols_read)
+ {
+ if (tv_message)
+ (*tv_message) (LDPL_FATAL, "No register_all_symbols_read hook\n");
+ fflush (NULL);
+ return LDPS_ERR;
+ }
+ (*tv_register_all_symbols_read) (onall_symbols_read);
+ }
+ if (register_cleanup_hook)
+ {
+ if (!tv_register_cleanup)
+ {
+ if (tv_message)
+ (*tv_message) (LDPL_FATAL, "No register_cleanup hook\n");
+ fflush (NULL);
+ return LDPS_ERR;
+ }
+ (*tv_register_cleanup) (oncleanup);
+ }
+ fflush (NULL);
+ return onload_ret;
+}
+
+
+enum ld_plugin_status
+onclaim_file (const struct ld_plugin_input_file *file, int *claimed)
+{
+ if (tv_message)
+ (*tv_message)(LDPL_INFO, "hook called: claim_file %s [@%ld/%ld]\n",
+ file->name, (long)file->offset, (long)file->filesize);
+ fflush (NULL);
+ return claim_file_ret;
+}
+
+enum ld_plugin_status
+onall_symbols_read (void)
+{
+ if (tv_message)
+ (*tv_message)(LDPL_INFO, "hook called: all symbols read.\n");
+ fflush (NULL);
+ return all_symbols_read_ret;
+}
+
+enum ld_plugin_status
+oncleanup (void)
+{
+ if (tv_message)
+ (*tv_message)(LDPL_INFO, "hook called: cleanup.\n");
+ fflush (NULL);
+ return cleanup_ret;
+}
+
diff --git a/ld/testsuite/ld-plugin/func.c b/ld/testsuite/ld-plugin/func.c
new file mode 100644
index 0000000..8c668db
--- /dev/null
+++ b/ld/testsuite/ld-plugin/func.c
@@ -0,0 +1,7 @@
+
+extern int retval;
+
+int func (void)
+{
+ return retval;
+}
diff --git a/ld/testsuite/ld-plugin/main.c b/ld/testsuite/ld-plugin/main.c
new file mode 100644
index 0000000..2d64617
--- /dev/null
+++ b/ld/testsuite/ld-plugin/main.c
@@ -0,0 +1,13 @@
+
+extern int printf (const char *fmt, ...);
+
+extern const char *text;
+extern int func (void);
+
+int retval = 0;
+
+int main (int argc, const char **argv)
+{
+ printf ("%s\n", text);
+ return func ();
+}
diff --git a/ld/testsuite/ld-plugin/plugin-1.d b/ld/testsuite/ld-plugin/plugin-1.d
new file mode 100644
index 0000000..0ce0794
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-1.d
@@ -0,0 +1,18 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_NULL value 0x0 \(0\)
+#...
diff --git a/ld/testsuite/ld-plugin/plugin-2.d b/ld/testsuite/ld-plugin/plugin-2.d
new file mode 100644
index 0000000..677f8fb
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-2.d
@@ -0,0 +1,21 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'failonload'
+tv\[16\]: LDPT_NULL value 0x0 \(0\)
+#...
+.*ld.*:.*ldtestplug.*: error loading plugin
+#...
diff --git a/ld/testsuite/ld-plugin/plugin-3.d b/ld/testsuite/ld-plugin/plugin-3.d
new file mode 100644
index 0000000..9014870
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-3.d
@@ -0,0 +1,23 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[16\]: LDPT_OPTION 'failallsymbolsread'
+tv\[17\]: LDPT_NULL value 0x0 \(0\)
+# Enable these next two lines when all symbols read hook is being called by infrastructure.
+##...
+#.*ld.*:.*ldtestplug.*: plugin reported error after all symbols read
+#...
diff --git a/ld/testsuite/ld-plugin/plugin-4.d b/ld/testsuite/ld-plugin/plugin-4.d
new file mode 100644
index 0000000..580cbac
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-4.d
@@ -0,0 +1,23 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'failcleanup'
+tv\[16\]: LDPT_OPTION 'registercleanup'
+tv\[17\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: cleanup.
+.*ld.*:.*ldtestplug.*: error in plugin cleanup \(ignored\)
+#...
diff --git a/ld/testsuite/ld-plugin/plugin-5.d b/ld/testsuite/ld-plugin/plugin-5.d
new file mode 100644
index 0000000..b9b02ab
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-5.d
@@ -0,0 +1,31 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_NULL value 0x0 \(0\)
+# Enable these next four lines when claim hook is being called by infrastructure.
+##...
+#hook called: claim_file tmpdir/main.o \[@0/.*
+#hook called: claim_file tmpdir/func.o \[@0/.*
+#hook called: claim_file tmpdir/text.o \[@0/.*
+# Enable these next two lines when all symbols read hook is being called by infrastructure.
+##...
+#hook called: all symbols read.
+#...
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
new file mode 100644
index 0000000..1150d93
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -0,0 +1,76 @@
+# Expect script for ld-plugin tests
+# Copyright 2010
+# Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# These tests require the plugin API to be configured in.
+if ![check_plugin_api_available] {
+ return
+}
+
+pass "plugin API enabled"
+
+global base_dir
+
+# Look for the name we can dlopen in the test plugin's libtool control script.
+set plugin_name [file_contents "$base_dir/libldtestplug.la"]
+set plugin_name [regsub "'.*" [regsub ".*dlname='" "$plugin_name" ""] ""]
+verbose "plugin name is '$plugin_name'"
+
+# Use libtool to find full path to plugin rather than worrying
+# about run paths or anything like that.
+catch "exec $base_dir/libtool --config" lt_config
+verbose "Full lt config: $lt_config" 3
+# Look for "objdir=.libs"
+regexp -line "^objdir=.*$" "$lt_config" lt_objdir
+verbose "lt_objdir line is '$lt_objdir'" 3
+set lt_objdir [regsub "objdir=" "$lt_objdir" ""]
+set plugin_path "$base_dir/$lt_objdir/$plugin_name"
+verbose "Full plugin path $plugin_path" 2
+
+set testobjfiles "$HOSTING_CRT0 tmpdir/main.o tmpdir/func.o tmpdir/text.o"
+set libs "$LIBS $HOSTING_LIBS"
+
+set regclm "-plugin-arg registerclaimfile"
+set regas "-plugin-arg registerallsymbolsread"
+set regcln "-plugin-arg registercleanup"
+
+set plugin_tests [list \
+ [list "load plugin" "-plugin $plugin_path \
+ $testobjfiles $libs" "" "" {{ld plugin-1.d}} "main.x" ] \
+ [list "fail plugin onload" "-plugin $plugin_path -plugin-arg failonload \
+ $testobjfiles $libs" "" "" {{ld plugin-2.d}} "main.x" ] \
+ [list "fail plugin allsymbolsread" "-plugin $plugin_path $regas -plugin-arg failallsymbolsread \
+ $testobjfiles $libs" "" "" {{ld plugin-3.d}} "main.x" ] \
+ [list "fail plugin cleanup" "-plugin $plugin_path -plugin-arg failcleanup $regcln \
+ $testobjfiles $libs" "" "" {{ld plugin-4.d}} "main.x" ] \
+ [list "plugin all hooks" "-plugin $plugin_path $regclm $regas $regcln \
+ $testobjfiles $libs" "" "" {{ld plugin-5.d}} "main.x" ] \
+]
+
+if { ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o]
+ || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o]
+ || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/text.c tmpdir/text.o] } {
+ foreach testitem $plugin_tests {
+ unresolved [lindex $testitem 0]
+ }
+ return
+}
+
+run_ld_link_tests $plugin_tests
diff --git a/ld/testsuite/ld-plugin/text.c b/ld/testsuite/ld-plugin/text.c
new file mode 100644
index 0000000..6d02114
--- /dev/null
+++ b/ld/testsuite/ld-plugin/text.c
@@ -0,0 +1,3 @@
+
+const char *text = "Hello world!\n";
+
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index 709702a..9803076 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -1125,6 +1125,7 @@ proc regexp_diff { file_1 file_2 } {
} else {
verbose "regexp \"^$line_b$\"\nline \"$line_a\"" 3
if ![regexp "^$line_b$" "$line_a"] {
+ verbose "regexp_diff match failure\n" 3
send_log "regexp_diff match failure\n"
send_log "regexp \"^$line_b$\"\nline \"$line_a\"\n"
set differences 1
@@ -1159,6 +1160,12 @@ proc file_contents { filename } {
return $contents
}
+proc set_file_contents { filename contents } {
+ set file [open $filename w]
+ puts $file "$contents"
+ close $file
+}
+
# Create an archive using ar
#
proc ar_simple_create { ar aropts target objects } {
@@ -1185,6 +1192,10 @@ proc ar_simple_create { ar aropts target objects } {
# objdump: Apply objdump options on result. Compare with regex (last arg).
# nm: Apply nm options on result. Compare with regex (last arg).
# readelf: Apply readelf options on result. Compare with regex (last arg).
+# ld: Don't apply anything on result. Compare output during linking with
+# regex (second arg). Note that this *must* be the first action if it
+# is to be used at all; in all other cases, any output from the linker
+# during linking is treated as a sign of an error and FAILs the test.
#
proc run_ld_link_tests { ldtests } {
global ld
@@ -1199,6 +1210,7 @@ proc run_ld_link_tests { ldtests } {
global CC
global CFLAGS
global runtests
+ global exec_output
foreach testitem $ldtests {
set testname [lindex $testitem 0]
@@ -1216,6 +1228,8 @@ proc run_ld_link_tests { ldtests } {
set objfiles {}
set is_unresolved 0
set failed 0
+ set maybe_failed 0
+ set ld_output ""
# verbose -log "Testname is $testname"
# verbose -log "ld_options is $ld_options"
@@ -1258,8 +1272,8 @@ proc run_ld_link_tests { ldtests } {
set failed 0
}
} elseif { ![ld_simple_link $ld $binfile "-L$srcdir/$subdir $ld_options $objfiles"] } {
- fail $testname
- set failed 1
+ set maybe_failed 1
+ set ld_output "$exec_output"
} else {
set failed 0
}
@@ -1280,6 +1294,8 @@ proc run_ld_link_tests { ldtests } {
{ set dump_prog $nm }
readelf
{ set dump_prog $READELF }
+ ld
+ { set dump_prog "ld" }
default
{
perror "Unrecognized action $action"
@@ -1288,7 +1304,21 @@ proc run_ld_link_tests { ldtests } {
}
}
- if { $dump_prog != "" } {
+ if { $action == "ld" } {
+ set dumpfile [lindex $actionlist 1]
+ verbose "dumpfile is $dumpfile"
+ set_file_contents "tmpdir/ld.messages" "$ld_output"
+ verbose "ld.messages has '[file_contents tmpdir/ld.messages]'"
+ if { [regexp_diff "tmpdir/ld.messages" "$srcdir/$subdir/$dumpfile"] } then {
+ verbose "output is $ld_output" 2
+ set failed 1
+ break
+ }
+ set maybe_failed 0
+ } elseif { $maybe_failed != 0 } {
+ set failed 1
+ break
+ } elseif { $dump_prog != "" } {
set dumpfile [lindex $actionlist 2]
set binary $dump_prog
@@ -1684,6 +1714,22 @@ proc check_gc_sections_available { } {
return $gc_sections_available_saved
}
+# Returns true if the target ld supports the plugin API.
+proc check_plugin_api_available { } {
+ global plugin_api_available_saved
+ global ld
+ if {![info exists plugin_api_available_saved]} {
+ # Check if the ld used by gcc supports --plugin.
+ set ld_output [remote_exec host $ld "--help"]
+ if { [ string first "-plugin" $ld_output ] >= 0 } {
+ set plugin_api_available_saved 1
+ } else {
+ set plugin_api_available_saved 0
+ }
+ }
+ return $plugin_api_available_saved
+}
+
# Check if the assembler supports CFI statements.
proc check_as_cfi { } {
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols Dave Korn
@ 2010-09-23 5:36 ` Ralf Wildenhues
2010-09-23 6:03 ` Dave Korn
2010-09-24 1:23 ` Dave Korn
2010-10-06 22:14 ` Richard Henderson
2 siblings, 1 reply; 39+ messages in thread
From: Ralf Wildenhues @ 2010-09-23 5:36 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
Hi Dave,
* Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
> --- a/ld/Makefile.am
> +++ b/ld/Makefile.am
> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
> noinst_LTLIBRARIES = libldtestplug.la
> libldtestplug_la_SOURCES = testplug.c
> libldtestplug_la_CFLAGS= -g -O2
> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
Why the '-Wl,' prefix? I'd have expected $(LIBIBERTY) to be added to
libldtestplug_la_LIBADD.
If you need to work around libtool warning about adding static library
deps to a shared library, then I suggest at least -Wc, rather than -Wl,
but still I think you will run into troubles on static-only systems.
> endif
>
> # DOCUMENTATION TARGETS
Cheers,
Ralf
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [1/4] Infrastructure Dave Korn
@ 2010-09-23 5:41 ` Ralf Wildenhues
2010-09-23 6:01 ` Dave Korn
2010-10-04 4:35 ` Dave Korn
2010-10-06 20:57 ` Richard Henderson
2 siblings, 1 reply; 39+ messages in thread
From: Ralf Wildenhues @ 2010-09-23 5:41 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
Hi Dave,
* Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:17AM CEST:
> --- a/ld/Makefile.am
> +++ b/ld/Makefile.am
> @@ -21,6 +21,21 @@ WARN_CFLAGS = @WARN_CFLAGS@
> NO_WERROR = @NO_WERROR@
> AM_CFLAGS = $(WARN_CFLAGS)
>
> +# Conditionally enable the plugin interface.
> +if ENABLE_PLUGINS
> +PLUGIN_C = plugin.c
> +PLUGIN_H = plugin.h
> +PLUGIN_OBJEXT = plugin.@OBJEXT@
Maybe rather PLUGIN_OBJECT, because it's not only an extension?
> +PLUGIN_CFLAGS = -DENABLE_PLUGINS
> +else
> +PLUGIN_C =
> +PLUGIN_H =
> +PLUGIN_OBJEXT =
> +PLUGIN_CFLAGS =
> +endif
> +
> +AM_CFLAGS += $(PLUGIN_CFLAGS)
AM_CPPFLAGS for -D flags, I'd say.
Cheers,
Ralf
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-09-23 5:41 ` Ralf Wildenhues
@ 2010-09-23 6:01 ` Dave Korn
0 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-23 6:01 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: binutils
On 23/09/2010 06:41, Ralf Wildenhues wrote:
> Maybe rather PLUGIN_OBJECT, because it's not only an extension?
> AM_CPPFLAGS for -D flags, I'd say.
Thanks, will do!
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 5:36 ` Ralf Wildenhues
@ 2010-09-23 6:03 ` Dave Korn
2010-09-23 6:12 ` Dave Korn
2010-09-23 6:13 ` Ralf Wildenhues
0 siblings, 2 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-23 6:03 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: binutils
On 23/09/2010 06:36, Ralf Wildenhues wrote:
> Hi Dave,
>
> * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
>> --- a/ld/Makefile.am
>> +++ b/ld/Makefile.am
>> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>> noinst_LTLIBRARIES = libldtestplug.la
>> libldtestplug_la_SOURCES = testplug.c
>> libldtestplug_la_CFLAGS= -g -O2
>> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
>> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
>
> Why the '-Wl,' prefix? I'd have expected $(LIBIBERTY) to be added to
> libldtestplug_la_LIBADD.
libiberty isn't built under libtool control at all, AFAIK, and is only ever
built as a static archive, so I was under the impression I've got to pass it
around behind libtool's back like this.
> If you need to work around libtool warning about adding static library
> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
> but still I think you will run into troubles on static-only systems.
What kind of problems?
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 6:03 ` Dave Korn
@ 2010-09-23 6:12 ` Dave Korn
2010-09-23 6:14 ` Ralf Wildenhues
2010-09-23 6:13 ` Ralf Wildenhues
1 sibling, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-09-23 6:12 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: binutils
On 23/09/2010 07:25, Dave Korn wrote:
> On 23/09/2010 06:36, Ralf Wildenhues wrote:
>> If you need to work around libtool warning about adding static library
>> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
>> but still I think you will run into troubles on static-only systems.
>
> What kind of problems?
Uh, wait a minute, what are we even talking about? Plugins only exist as
DSOs. There aren't going to be any plugins on static-only systems, right?
I'm going to go get a few hours sleep!
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 6:03 ` Dave Korn
2010-09-23 6:12 ` Dave Korn
@ 2010-09-23 6:13 ` Ralf Wildenhues
2010-09-30 13:13 ` Dave Korn
1 sibling, 1 reply; 39+ messages in thread
From: Ralf Wildenhues @ 2010-09-23 6:13 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
* Dave Korn wrote on Thu, Sep 23, 2010 at 08:25:42AM CEST:
> On 23/09/2010 06:36, Ralf Wildenhues wrote:
> > * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
> >> --- a/ld/Makefile.am
> >> +++ b/ld/Makefile.am
> >> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
> >> noinst_LTLIBRARIES = libldtestplug.la
> >> libldtestplug_la_SOURCES = testplug.c
> >> libldtestplug_la_CFLAGS= -g -O2
> >> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
> >> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
> >
> > Why the '-Wl,' prefix? I'd have expected $(LIBIBERTY) to be added to
> > libldtestplug_la_LIBADD.
>
> libiberty isn't built under libtool control at all, AFAIK, and is only ever
> built as a static archive, so I was under the impression I've got to pass it
> around behind libtool's back like this.
Not because of that, no.
> > If you need to work around libtool warning about adding static library
> > deps to a shared library, then I suggest at least -Wc, rather than -Wl,
> > but still I think you will run into troubles on static-only systems.
>
> What kind of problems?
Ordering. You can't rely on libtool expanding -Wl, link flags after the
objects, meaning that libiberty may not be picked up at all. (With
static linking, objects and libraries are evaluated in a strict command
line order, so all needed libraries must come after objects that need
them.)
Cheers,
Ralf
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 6:12 ` Dave Korn
@ 2010-09-23 6:14 ` Ralf Wildenhues
0 siblings, 0 replies; 39+ messages in thread
From: Ralf Wildenhues @ 2010-09-23 6:14 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
* Dave Korn wrote on Thu, Sep 23, 2010 at 08:35:26AM CEST:
> On 23/09/2010 07:25, Dave Korn wrote:
> > On 23/09/2010 06:36, Ralf Wildenhues wrote:
>
> >> If you need to work around libtool warning about adding static library
> >> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
> >> but still I think you will run into troubles on static-only systems.
> >
> > What kind of problems?
>
> Uh, wait a minute, what are we even talking about? Plugins only exist as
> DSOs. There aren't going to be any plugins on static-only systems, right?
OK but ordering issues exist elsewhere too.
For static-only systems it might be sufficient to just not build this
thing when $enable_shared is not true in configure after
AC_PROG_LIBTOOL.
Cheers,
Ralf
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [0/4]
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
` (3 preceding siblings ...)
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [1/4] Infrastructure Dave Korn
@ 2010-09-23 16:18 ` Richard Henderson
2010-09-24 0:30 ` [PATCH] Add plugin interface to LD [0/4] Support ELF symbol visibility Dave Korn
2010-09-27 0:21 ` [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface Dave Korn
2010-10-02 15:52 ` [PING] Re: [PATCH] Add plugin interface to LD [0/4] Dave Korn
6 siblings, 1 reply; 39+ messages in thread
From: Richard Henderson @ 2010-09-23 16:18 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/22/2010 10:29 PM, Dave Korn wrote:
> - ELF visibility handling doesn't need to be a first priority because ELF
> targets should be using GOLD for best results anyway; this patch is sufficient
> for non-ELF targets that don't have the option of using GOLD and don't need
> visibility support anyway.
"Should be", but there's a whole collection of elf targets that
are unlikely to ever be re-written for GOLD.
But you're right that it needn't be a first priority.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Add plugin interface to LD [0/4] Support ELF symbol visibility
2010-09-23 16:18 ` [PATCH] Add plugin interface to LD [0/4] Richard Henderson
@ 2010-09-24 0:30 ` Dave Korn
2010-09-24 0:38 ` [PATCH] Add plugin interface to LD [5/4] " Dave Korn
2010-10-06 22:42 ` Richard Henderson
0 siblings, 2 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-24 0:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 997 bytes --]
On 23/09/2010 17:18, Richard Henderson wrote:
> On 09/22/2010 10:29 PM, Dave Korn wrote:
>> - ELF visibility handling doesn't need to be a first priority because ELF
>> targets should be using GOLD for best results anyway; this patch is sufficient
>> for non-ELF targets that don't have the option of using GOLD and don't need
>> visibility support anyway.
>
> "Should be", but there's a whole collection of elf targets that
> are unlikely to ever be re-written for GOLD.
>
> But you're right that it needn't be a first priority.
>
>
> r~
Yeah, but it's like about /four/ extra statements or so... so here it is!
ld/ChangeLog:
* plugin.c (asymbol_from_plugin_symbol): If the bfd is an ELF bfd,
find the elf symbol data and set the visibility in the st_other field.
ld/testsuite/ChangeLog:
* plugin-ignore.d: New dump test control script.
* plugin-vis-1.d: Likewise.
* plugin.exp: Add list of ELF-only tests and run them if testing on
an ELF target.
cheers,
DaveK
[-- Attachment #2: ld-plugin-api-5-elf-vis.diff --]
[-- Type: text/x-c, Size: 3690 bytes --]
From 34fd3199e5edc9a3f283741a21bd7e832e5a56c6 Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Fri, 24 Sep 2010 01:41:15 +0100
Subject: [PATCH] Add ELF symbol visibility support to plugin interface.
ld/ChangeLog:
* plugin.c (asymbol_from_plugin_symbol): If the bfd is an ELF bfd,
find the elf symbol data and set the visibility in the st_other field.
ld/testsuite/ChangeLog:
* plugin-ignore.d: New dump test control script.
* plugin-vis-1.d: Likewise.
* plugin.exp: Add list of ELF-only tests and run them if testing on
an ELF target.
---
ld/plugin.c | 10 ++++++++++
ld/testsuite/ld-plugin/plugin-ignore.d | 1 +
ld/testsuite/ld-plugin/plugin-vis-1.d | 9 +++++++++
ld/testsuite/ld-plugin/plugin.exp | 22 ++++++++++++++++++++++
4 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/ld/plugin.c b/ld/plugin.c
index a33e907..e0177f1 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -31,6 +31,7 @@
#include "ldfile.h"
#include "plugin.h"
#include "plugin-api.h"
+#include "elf-bfd.h"
/* Don't include std headers until after config.h, sysdeps.h etc.,
or you may end up with the wrong bitsize of off_t. */
#include <dlfcn.h>
@@ -495,6 +496,15 @@ asymbol_from_plugin_symbol (bfd *abfd, asymbol *asym,
: (ldsym->def == LDPK_UNDEF || ldsym->def == LDPK_WEAKUNDEF
? bfd_und_section_ptr
: bfd_get_section_by_name (abfd, ".text"));
+ /* Visibility only applies on ELF targets. */
+ if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+ {
+ elf_symbol_type *elfsym = elf_symbol_from (abfd, asym);
+ if (!elfsym)
+ einfo (_("%P%F: %s: non-ELF symbol in ELF BFD!"), asym->name);
+ elfsym->internal_elf_sym.st_other &= ~3;
+ elfsym->internal_elf_sym.st_other |= ldsym->visibility;
+ }
return LDPS_OK;
}
diff --git a/ld/testsuite/ld-plugin/plugin-ignore.d b/ld/testsuite/ld-plugin/plugin-ignore.d
new file mode 100644
index 0000000..875ac51
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-ignore.d
@@ -0,0 +1 @@
+#pass
diff --git a/ld/testsuite/ld-plugin/plugin-vis-1.d b/ld/testsuite/ld-plugin/plugin-vis-1.d
new file mode 100644
index 0000000..41f4971
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-vis-1.d
@@ -0,0 +1,9 @@
+#...
+.*(PROTECTED.*func3|DEFAULT.*func|HIDDEN.*func2|INTERNAL.*func1)
+#...
+.*(PROTECTED.*func3|DEFAULT.*func|HIDDEN.*func2|INTERNAL.*func1)
+#...
+.*(PROTECTED.*func3|DEFAULT.*func|HIDDEN.*func2|INTERNAL.*func1)
+#...
+.*(PROTECTED.*func3|DEFAULT.*func|HIDDEN.*func2|INTERNAL.*func1)
+#...
diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index 32f396c..6072e3b 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -108,11 +108,33 @@ set plugin_tests [list \
$testobjfiles $libs" "" "" {{ld plugin-9.d}} "main.x" ] \
]
+set plugin_extra_elf_tests [list \
+ [list "plugin set symbol visibility" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ -plugin-arg sym:${_}func::0:0:0 \
+ -plugin-arg sym:${_}func1::0:1:0 \
+ -plugin-arg sym:${_}func2::0:2:0 \
+ -plugin-arg sym:${_}func3::0:3:0 \
+ -plugin-arg dumpresolutions \
+ $testobjfiles $libs" "" "" {{ld plugin-ignore.d} \
+ {readelf -s plugin-vis-1.d}} "main.x" ] \
+]
+
if { $failed_compile != 0 } {
foreach testitem $plugin_tests {
unresolved [lindex $testitem 0]
}
+ if { [is_elf_format] } {
+ foreach testitem $plugin_extra_elf_tests {
+ unresolved [lindex $testitem 0]
+ }
+ }
return
}
run_ld_link_tests $plugin_tests
+
+if { [is_elf_format] } {
+ run_ld_link_tests $plugin_extra_elf_tests
+}
+
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths Dave Korn
@ 2010-09-24 0:35 ` Dave Korn
2010-10-06 22:40 ` Richard Henderson
1 sibling, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-24 0:35 UTC (permalink / raw)
To: binutils
On 23/09/2010 06:32, Dave Korn wrote:
Spot of self-review here:
> --- a/ld/ldmain.c
> +++ b/ld/ldmain.c
> @@ -889,7 +889,7 @@ add_archive_element (struct bfd_link_info *info,
> multiple times. */
>
> static bfd_boolean
> -multiple_definition (struct bfd_link_info *info ATTRIBUTE_UNUSED,
> +multiple_definition (struct bfd_link_info *info,
> const char *name,
> bfd *obfd,
> asection *osec,
Don't do that, because it's only used when ENABLE_PLUGINS. Will remember
not to commit with this change.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [5/4] Support ELF symbol visibility
2010-09-24 0:30 ` [PATCH] Add plugin interface to LD [0/4] Support ELF symbol visibility Dave Korn
@ 2010-09-24 0:38 ` Dave Korn
2010-10-06 22:42 ` Richard Henderson
1 sibling, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-24 0:38 UTC (permalink / raw)
To: binutils
On 24/09/2010 01:53, Dave Korn wrote:
> Subject: Re: [PATCH] Add plugin interface to LD [0/4] Support ELF symbol
> visibility
Oops. That was, of course, patch five. Subject line updated.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols Dave Korn
2010-09-23 5:36 ` Ralf Wildenhues
@ 2010-09-24 1:23 ` Dave Korn
2010-10-06 22:14 ` Richard Henderson
2 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-09-24 1:23 UTC (permalink / raw)
To: binutils
On 23/09/2010 06:31, Dave Korn wrote:
More self-review:
> @@ -290,6 +301,59 @@ ldfile_try_open_bfd (const char *attempt,
> }
> }
>
> +#ifdef ENABLE_PLUGINS
> + /* If plugins are active, they get first chance to claim
> + any successfully-opened input file. We skip archives
> + here; the plugin wants us to offer it the individual
> + members when we enumerate them, not the whole file. We
> + also ignore corefiles, because that's just weird. It is
> + a needed side-effect of calling bfd_check_format with
> + bfd_object that it sets the bfd's arch and mach, which
> + will be needed when and if we want to bfd_create a new
> + one using this one as a template. */
> + int fildes = bfd_check_format (entry->the_bfd, bfd_object)
> + ? open (attempt, O_RDONLY
Oops. C99-style definition mingling amongst code; will have to refactor it
before commit.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface.
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
` (4 preceding siblings ...)
2010-09-23 16:18 ` [PATCH] Add plugin interface to LD [0/4] Richard Henderson
@ 2010-09-27 0:21 ` Dave Korn
2010-10-06 22:57 ` Richard Henderson
2010-10-02 15:52 ` [PING] Re: [PATCH] Add plugin interface to LD [0/4] Dave Korn
6 siblings, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-09-27 0:21 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 4145 bytes --]
Hello everyone,
And here is the final part of this patch series (for now, anyway - bugs will
probably need fixing later, but I won't discover them until I start playing
with the LTO plugin). It allows plugins to deal with library archives, by
offering each of the individual archive members chosen for adding into the
link to the plugins via the API "claim files" hook.
This one isn't as neat as the other patches. I couldn't see any way to use
the existing functionality of the linker "add_archive_element" callback to
supply an alternative BFD with its own set of symbols from the IR, so I had to
extend BFD to make this possible. And I didn't want to change the interface
of add_archive_element, since it's one of the few actual public interfaces of
libbfd, and we don't have any versioning.
So that led me to take a somewhat ugly solution: I added a new member to
struct areltdata, and use that to communicate between libbfd and the linker
callback. The member gets zeroed immediately before calling the callback, and
if on return it has been set to point to a BFD, the symbols from that BFD are
added to the linker hash table instead of the archive element's own symbols.
It's not ideal, but I think it's OK enough; there are only so many places from
which BFD calls the hook. If anyone has a better idea how to implement this,
I'm all ears. Otherwise, OK?
bfd/ChangeLog:
* aoutx.h (aout_link_check_ar_symbols): Clear arelt_substitute_bfd
before calling add_archive_element callback.
(aout_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
* cofflink.c (coff_link_check_ar_symbols): Clear arelt_substitute_bfd
before calling add_archive_element callback.
(coff_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
* ecoff.c (read_ext_syms_and_strs): New function holds symbol-reading
code factored-out from ecoff_link_check_archive_element.
(reread_ext_syms_and_strs): Clear old symbols and call it.
(ecoff_link_check_archive_element): Clear arelt_substitute_bfd before
calling add_archive_element callback and handle the substitution if
the callback sets it.
(ecoff_link_add_archive_symbols): Likewise.
* elflink.c (elf_link_add_archive_symbols): Likewise.
* libbfd-in.h (struct areltdata): Add new substitute_bfd member.
(arelt_substitute_bfd): Add new accessor macro for it.
* libbfd.h: Regenerate.
* linker.c (generic_link_check_archive_element): Clear
arelt_substitute_bfd before calling add_archive_element callback and
handle the substitution if the callback sets it.
* pdp11.c (aout_link_check_ar_symbols): Clear arelt_substitute_bfd
before calling add_archive_element callback.
(aout_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
* vms-alpha.c (alpha_vms_link_add_archive_symbols): Clear
arelt_substitute_bfd before calling add_archive_element callback and
handle the substitution if the callback sets it.
* xcofflink.c (xcoff_link_check_dynamic_ar_symbols): Clear
arelt_substitute_bfd before calling add_archive_element callback.
(xcoff_link_check_ar_symbols): Likewise.
(xcoff_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
ld/ChangeLog:
* ldlang.c (load_symbols): Clear arelt_substitute_bfd before calling
add_archive_element callback and handle the substitution if the
callback sets it.
* ldmain.c (add_archive_element)[ENABLE_PLUGINS]: Offer the archive
member to the plugins and if claimed set arelt_substitute_bfd to point
to the dummy IR-only BFD.
ld/testsuite/ChangeLog:
* ld-plugin/plugin-10.d: New dump test control script.
* ld-plugin/plugin-11.d: Likewise.
* ld-plugin/plugin.exp: Run them.
include/ChangeLog:
* include/bfdlink.h (struct_bfd_link_callbacks): Document new
facility for add_archive_element callback to indicate a replacement
bfd to be added to the hash table in place of the original element.
(As before, tested on i686-pc-cygwin and i686-pc-linux-gnu.)
cheers,
DaveK
[-- Attachment #2: ld-plugin-api-6-libs.diff --]
[-- Type: text/x-c, Size: 31715 bytes --]
From afb802cf1036582f77f0560f0c39453e1237e593 Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Mon, 27 Sep 2010 00:25:28 +0100
Subject: [PATCH] Add archive support to plugin interface.
bfd/ChangeLog:
* aoutx.h (aout_link_check_ar_symbols): Clear arelt_substitute_bfd
before calling add_archive_element callback.
(aout_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
* cofflink.c (coff_link_check_ar_symbols): Clear arelt_substitute_bfd
before calling add_archive_element callback.
(coff_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
* ecoff.c (read_ext_syms_and_strs): New function holds symbol-reading
code factored-out from ecoff_link_check_archive_element.
(reread_ext_syms_and_strs): Clear old symbols and call it.
(ecoff_link_check_archive_element): Clear arelt_substitute_bfd before
calling add_archive_element callback and handle the substitution if
the callback sets it.
(ecoff_link_add_archive_symbols): Likewise.
* elflink.c (elf_link_add_archive_symbols): Likewise.
* libbfd-in.h (struct areltdata): Add new substitute_bfd member.
(arelt_substitute_bfd): Add new accessor macro for it.
* libbfd.h: Regenerate.
* linker.c (generic_link_check_archive_element): Clear
arelt_substitute_bfd before calling add_archive_element callback and
handle the substitution if the callback sets it.
* pdp11.c (aout_link_check_ar_symbols): Clear arelt_substitute_bfd
before calling add_archive_element callback.
(aout_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
* vms-alpha.c (alpha_vms_link_add_archive_symbols): Clear
arelt_substitute_bfd before calling add_archive_element callback and
handle the substitution if the callback sets it.
* xcofflink.c (xcoff_link_check_dynamic_ar_symbols): Clear
arelt_substitute_bfd before calling add_archive_element callback.
(xcoff_link_check_ar_symbols): Likewise.
(xcoff_link_check_archive_element): Handle arelt_substitute_bfd if it
was set during add_archive_element callback.
ld/ChangeLog:
* ldlang.c (load_symbols): Clear arelt_substitute_bfd before calling
add_archive_element callback and handle the substitution if the
callback sets it.
* ldmain.c (add_archive_element)[ENABLE_PLUGINS]: Offer the archive
member to the plugins and if claimed set arelt_substitute_bfd to point
to the dummy IR-only BFD.
ld/testsuite/ChangeLog:
* ld-plugin/plugin-10.d: New dump test control script.
* ld-plugin/plugin-11.d: Likewise.
* ld-plugin/plugin.exp: Run them.
include/ChangeLog:
* include/bfdlink.h (struct_bfd_link_callbacks): Document new
facility for add_archive_element callback to indicate a replacement
bfd to be added to the hash table in place of the original element.
---
bfd/aoutx.h | 11 ++++-
bfd/cofflink.c | 11 ++++-
bfd/ecoff.c | 96 ++++++++++++++++++++++++++---------
bfd/elflink.c | 6 ++-
bfd/libbfd-in.h | 3 +
bfd/libbfd.h | 3 +
bfd/linker.c | 46 ++++++++++++++---
bfd/pdp11.c | 10 +++-
bfd/vms-alpha.c | 6 ++-
bfd/xcofflink.c | 10 +++-
include/bfdlink.h | 4 +-
ld/ldlang.c | 7 ++-
ld/ldmain.c | 62 ++++++++++++++++++++++-
ld/testsuite/ld-plugin/plugin-10.d | 36 +++++++++++++
ld/testsuite/ld-plugin/plugin-11.d | 40 +++++++++++++++
ld/testsuite/ld-plugin/plugin.exp | 28 ++++++++++
16 files changed, 336 insertions(+), 43 deletions(-)
diff --git a/bfd/aoutx.h b/bfd/aoutx.h
index 11598a0..41fda49 100644
--- a/bfd/aoutx.h
+++ b/bfd/aoutx.h
@@ -3304,6 +3304,7 @@ aout_link_check_ar_symbols (bfd *abfd,
continue;
}
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
return FALSE;
*pneeded = TRUE;
@@ -3331,6 +3332,7 @@ aout_link_check_ar_symbols (bfd *abfd,
outside BFD. We assume that we should link
in the object file. This is done for the -u
option in the linker. */
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info,
abfd,
name))
@@ -3381,6 +3383,7 @@ aout_link_check_ar_symbols (bfd *abfd,
it if the current link symbol is common. */
if (h->type == bfd_link_hash_undefined)
{
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
return FALSE;
*pneeded = TRUE;
@@ -3410,7 +3413,13 @@ aout_link_check_archive_element (bfd *abfd,
if (*pneeded)
{
- if (! aout_link_add_symbols (abfd, info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (arelt_substitute_bfd (abfd)
+ && !aout_get_external_symbols (arelt_substitute_bfd (abfd)))
+ return FALSE;
+ if (! aout_link_add_symbols (arelt_substitute_bfd (abfd)
+ ? arelt_substitute_bfd (abfd) : abfd, info))
return FALSE;
}
diff --git a/bfd/cofflink.c b/bfd/cofflink.c
index a29b687..b6e095c 100644
--- a/bfd/cofflink.c
+++ b/bfd/cofflink.c
@@ -243,6 +243,7 @@ coff_link_check_ar_symbols (bfd *abfd,
if (h != (struct bfd_link_hash_entry *) NULL
&& h->type == bfd_link_hash_undefined)
{
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
return FALSE;
*pneeded = TRUE;
@@ -273,8 +274,16 @@ coff_link_check_archive_element (bfd *abfd,
if (! coff_link_check_ar_symbols (abfd, info, pneeded))
return FALSE;
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
if (*pneeded
- && ! coff_link_add_symbols (abfd, info))
+ && arelt_substitute_bfd (abfd)
+ && ! _bfd_coff_get_external_symbols (arelt_substitute_bfd (abfd)))
+ return FALSE;
+
+ if (*pneeded
+ && ! coff_link_add_symbols (arelt_substitute_bfd (abfd)
+ ? arelt_substitute_bfd (abfd) : abfd, info))
return FALSE;
if ((! info->keep_memory || ! *pneeded)
diff --git a/bfd/ecoff.c b/bfd/ecoff.c
index af9d7d6..ec36734 100644
--- a/bfd/ecoff.c
+++ b/bfd/ecoff.c
@@ -3509,6 +3509,58 @@ ecoff_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
return FALSE;
}
+/* Factored out from ecoff_link_check_archive_element. */
+
+static bfd_boolean
+read_ext_syms_and_strs (HDRR **symhdr, bfd_size_type *external_ext_size,
+ bfd_size_type *esize, void **external_ext, char **ssext, bfd *abfd,
+ const struct ecoff_backend_data * const backend)
+{
+ if (! ecoff_slurp_symbolic_header (abfd))
+ return FALSE;
+
+ /* If there are no symbols, we don't want it. */
+ if (bfd_get_symcount (abfd) == 0)
+ return TRUE;
+
+ *symhdr = &ecoff_data (abfd)->debug_info.symbolic_header;
+
+ *external_ext_size = backend->debug_swap.external_ext_size;
+ *esize = (*symhdr)->iextMax * *external_ext_size;
+ *external_ext = bfd_malloc (*esize);
+ if (*external_ext == NULL && *esize != 0)
+ return FALSE;
+
+ if (bfd_seek (abfd, (file_ptr) (*symhdr)->cbExtOffset, SEEK_SET) != 0
+ || bfd_bread (*external_ext, *esize, abfd) != *esize)
+ return FALSE;
+
+ *ssext = (char *) bfd_malloc ((bfd_size_type) (*symhdr)->issExtMax);
+ if (*ssext == NULL && (*symhdr)->issExtMax != 0)
+ return FALSE;
+
+ if (bfd_seek (abfd, (file_ptr) (*symhdr)->cbSsExtOffset, SEEK_SET) != 0
+ || (bfd_bread (*ssext, (bfd_size_type) (*symhdr)->issExtMax, abfd)
+ != (bfd_size_type) (*symhdr)->issExtMax))
+ return FALSE;
+ return TRUE;
+}
+
+static bfd_boolean
+reread_ext_syms_and_strs (HDRR **symhdr, bfd_size_type *external_ext_size,
+ bfd_size_type *esize, void **external_ext, char **ssext, bfd *abfd,
+ const struct ecoff_backend_data * const backend)
+{
+ if (*external_ext != NULL)
+ free (*external_ext);
+ *external_ext = NULL;
+ if (*ssext != NULL)
+ free (*ssext);
+ *ssext = NULL;
+ return read_ext_syms_and_strs (symhdr, external_ext_size, esize,
+ external_ext, ssext, abfd, backend);
+}
+
/* This is called if we used _bfd_generic_link_add_archive_symbols
because we were not dealing with an ECOFF archive. */
@@ -3530,35 +3582,15 @@ ecoff_link_check_archive_element (bfd *abfd,
*pneeded = FALSE;
- if (! ecoff_slurp_symbolic_header (abfd))
+ /* Read in the external symbols and external strings. */
+ if (!read_ext_syms_and_strs (&symhdr, &external_ext_size, &esize,
+ &external_ext, &ssext, abfd, backend))
goto error_return;
/* If there are no symbols, we don't want it. */
if (bfd_get_symcount (abfd) == 0)
goto successful_return;
- symhdr = &ecoff_data (abfd)->debug_info.symbolic_header;
-
- /* Read in the external symbols and external strings. */
- external_ext_size = backend->debug_swap.external_ext_size;
- esize = symhdr->iextMax * external_ext_size;
- external_ext = bfd_malloc (esize);
- if (external_ext == NULL && esize != 0)
- goto error_return;
-
- if (bfd_seek (abfd, (file_ptr) symhdr->cbExtOffset, SEEK_SET) != 0
- || bfd_bread (external_ext, esize, abfd) != esize)
- goto error_return;
-
- ssext = (char *) bfd_malloc ((bfd_size_type) symhdr->issExtMax);
- if (ssext == NULL && symhdr->issExtMax != 0)
- goto error_return;
-
- if (bfd_seek (abfd, (file_ptr) symhdr->cbSsExtOffset, SEEK_SET) != 0
- || (bfd_bread (ssext, (bfd_size_type) symhdr->issExtMax, abfd)
- != (bfd_size_type) symhdr->issExtMax))
- goto error_return;
-
/* Look through the external symbols to see if they define some
symbol that is currently undefined. */
ext_ptr = (char *) external_ext;
@@ -3612,9 +3644,19 @@ ecoff_link_check_archive_element (bfd *abfd,
continue;
/* Include this element. */
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
goto error_return;
- if (! ecoff_link_add_externals (abfd, info, external_ext, ssext))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. However
+ I don't think that can work here without reloading the
+ external symbols. */
+ if (arelt_substitute_bfd (abfd)
+ && !reread_ext_syms_and_strs (&symhdr, &external_ext_size, &esize,
+ &external_ext, &ssext, abfd, backend))
+ goto error_return;
+ if (! ecoff_link_add_externals (arelt_substitute_bfd (abfd)
+ ? arelt_substitute_bfd (abfd) : abfd, info, external_ext, ssext))
goto error_return;
*pneeded = TRUE;
@@ -3777,9 +3819,13 @@ ecoff_link_add_archive_symbols (bfd *abfd, struct bfd_link_info *info)
/* Unlike the generic linker, we know that this element provides
a definition for an undefined symbol and we know that we want
to include it. We don't need to check anything. */
+ arelt_substitute_bfd (element) = NULL;
if (! (*info->callbacks->add_archive_element) (info, element, name))
return FALSE;
- if (! ecoff_link_add_object_symbols (element, info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (! ecoff_link_add_object_symbols (arelt_substitute_bfd (element)
+ ? arelt_substitute_bfd (element) : element, info))
return FALSE;
pundef = &(*pundef)->u.undef.next;
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 1446885..b3a12ee 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5088,10 +5088,14 @@ elf_link_add_archive_symbols (bfd *abfd, struct bfd_link_info *info)
undefs_tail = info->hash->undefs_tail;
+ arelt_substitute_bfd (element) = NULL;
if (! (*info->callbacks->add_archive_element) (info, element,
symdef->name))
goto error_return;
- if (! bfd_link_add_symbols (element, info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (! bfd_link_add_symbols (arelt_substitute_bfd (element)
+ ? arelt_substitute_bfd (element) : element, info))
goto error_return;
/* If there are any new undefined symbols, we need to make
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index b5b614c..e097aaa 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -94,9 +94,12 @@ struct areltdata {
unsigned int extra_size; /* BSD4.4: extra bytes after the header. */
char *filename; /* null-terminated */
file_ptr origin; /* for element of a thin archive */
+ bfd *substitute_bfd; /* used by linker's add_archive_element hook. */
};
#define arelt_size(bfd) (((struct areltdata *)((bfd)->arelt_data))->parsed_size)
+#define arelt_substitute_bfd(bfd) \
+ (((struct areltdata *)((bfd)->arelt_data))->substitute_bfd)
extern void *bfd_malloc
(bfd_size_type);
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index fb95c40..82a3d4d 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -99,9 +99,12 @@ struct areltdata {
unsigned int extra_size; /* BSD4.4: extra bytes after the header. */
char *filename; /* null-terminated */
file_ptr origin; /* for element of a thin archive */
+ bfd *substitute_bfd; /* used by linker's add_archive_element hook. */
};
#define arelt_size(bfd) (((struct areltdata *)((bfd)->arelt_data))->parsed_size)
+#define arelt_substitute_bfd(bfd) \
+ (((struct areltdata *)((bfd)->arelt_data))->substitute_bfd)
extern void *bfd_malloc
(bfd_size_type);
diff --git a/bfd/linker.c b/bfd/linker.c
index 76bc70a..a43e1e5 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -223,7 +223,10 @@ SUBSUBSECTION
archive and decide which elements of the archive should be
included in the link. For each such element it must call the
<<add_archive_element>> linker callback, and it must add the
- symbols from the object file to the linker hash table.
+ symbols from the object file to the linker hash table. (The
+ callback may in fact indicate that a replacement BFD should be
+ used, in which case they symbols from that BFD should be added
+ to the linker hash table instead.)
@findex _bfd_generic_link_add_archive_symbols
In most cases the work of looking through the symbols in the
@@ -241,11 +244,18 @@ SUBSUBSECTION
<<_bfd_generic_link_add_archive_symbols>> must read the
symbols of the archive element and decide whether the archive
element should be included in the link. If the element is to
- be included, the <<add_archive_element>> linker callback
- routine must be called with the element as an argument, and
- the elements symbols must be added to the linker hash table
- just as though the element had itself been passed to the
- <<_bfd_link_add_symbols>> function.
+ be included, it must clear the <<substitute_bfd>> field in the
+ element's BFD's <<struct areltdata>> (by using the accessor
+ macro <<arelt_substitute_bfd>>) and then call out to the
+ <<add_archive_element>> linker callback. This callback is
+ called with the element as an argument, and if it indicates no
+ error on returning, the element's symbols must be added to the
+ linker hash table just as though the element had itself been
+ passed to the <<_bfd_link_add_symbols>> function. Optionally,
+ the callback may set the <<substitute_bfd>> field to point to
+ a replacement BFD, in which case the symbols from that BFD are
+ to be added to the linker hash table in place of the original
+ element's symbols.
When the a.out <<_bfd_link_add_symbols>> function receives an
archive, it calls <<_bfd_generic_link_add_archive_symbols>>
@@ -957,8 +967,14 @@ archive_hash_table_init
included. CHECKFN should set *PNEEDED to TRUE if the object file
should be included, and must also call the bfd_link_info
add_archive_element callback function and handle adding the symbols
- to the global hash table. CHECKFN should only return FALSE if some
- sort of error occurs.
+ to the global hash table. Before calling add_archive_element,
+ CHECKFN must set arelt_substitute_bfd to NULL on the BFD being
+ checked, and if it is non-NULL after the callback returns, CHECKFN
+ must add the symbols from the replacement BFD that it now points to
+ into the global hash table, rather than the object file's original
+ symbols. (This functionality is used to support the linker plugin
+ architecture.) CHECKFN should only return FALSE if some sort of
+ error occurs.
For some formats, such as a.out, it is possible to look through an
object file but not actually include it in the link. The
@@ -1215,9 +1231,18 @@ generic_link_check_archive_element (bfd *abfd,
asymbol **symbols;
/* This object file defines this symbol, so pull it in. */
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd,
bfd_asymbol_name (p)))
return FALSE;
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (arelt_substitute_bfd (abfd))
+ {
+ abfd = arelt_substitute_bfd (abfd);
+ if (!bfd_generic_link_read_symbols (abfd))
+ return FALSE;
+ }
symcount = _bfd_generic_link_get_symcount (abfd);
symbols = _bfd_generic_link_get_symbols (abfd);
if (! generic_link_add_symbol_list (abfd, info, symcount,
@@ -1241,9 +1266,14 @@ generic_link_check_archive_element (bfd *abfd,
/* This symbol was created as undefined from outside
BFD. We assume that we should link in the object
file. This is for the -u option in the linker. */
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element)
(info, abfd, bfd_asymbol_name (p)))
return FALSE;
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. But no
+ symbols are going to get registered by anything we're
+ returning to from here. */
*pneeded = TRUE;
return TRUE;
}
diff --git a/bfd/pdp11.c b/bfd/pdp11.c
index 1a7694c..9983020 100644
--- a/bfd/pdp11.c
+++ b/bfd/pdp11.c
@@ -2600,6 +2600,7 @@ aout_link_check_ar_symbols (bfd *abfd,
but not if it is defined in the .text section. That
seems a bit crazy to me, and I haven't implemented it.
However, it might be correct. */
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
return FALSE;
*pneeded = TRUE;
@@ -2627,6 +2628,7 @@ aout_link_check_ar_symbols (bfd *abfd,
outside BFD. We assume that we should link
in the object file. This is done for the -u
option in the linker. */
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element)
(info, abfd, name))
return FALSE;
@@ -2688,7 +2690,13 @@ aout_link_check_archive_element (bfd *abfd,
if (*pneeded)
{
- if (! aout_link_add_symbols (abfd, info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (arelt_substitute_bfd (abfd)
+ && ! aout_get_external_symbols (arelt_substitute_bfd (abfd)))
+ return FALSE;
+ if (! aout_link_add_symbols (arelt_substitute_bfd (abfd)
+ ? arelt_substitute_bfd (abfd) : abfd, info))
return FALSE;
}
diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c
index 5d38d1d..e736fe7 100644
--- a/bfd/vms-alpha.c
+++ b/bfd/vms-alpha.c
@@ -8277,10 +8277,14 @@ alpha_vms_link_add_archive_symbols (bfd *abfd, struct bfd_link_info *info)
/* Unlike the generic linker, we know that this element provides
a definition for an undefined symbol and we know that we want
to include it. We don't need to check anything. */
+ arelt_substitute_bfd (element) = NULL;
if (! (*info->callbacks->add_archive_element) (info, element,
h->root.string))
return FALSE;
- if (! alpha_vms_link_add_object_symbols (element, info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (! alpha_vms_link_add_object_symbols (arelt_substitute_bfd (element)
+ ? arelt_substitute_bfd (element) : element, info))
return FALSE;
orig_element->archive_pass = pass;
diff --git a/bfd/xcofflink.c b/bfd/xcofflink.c
index 30923cf..cf2cd01 100644
--- a/bfd/xcofflink.c
+++ b/bfd/xcofflink.c
@@ -2291,6 +2291,7 @@ xcoff_link_check_dynamic_ar_symbols (bfd *abfd,
&& (((struct xcoff_link_hash_entry *) h)->flags
& XCOFF_DEF_DYNAMIC) == 0)
{
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
return FALSE;
*pneeded = TRUE;
@@ -2361,6 +2362,7 @@ xcoff_link_check_ar_symbols (bfd *abfd,
|| (((struct xcoff_link_hash_entry *) h)->flags
& XCOFF_DEF_DYNAMIC) == 0))
{
+ arelt_substitute_bfd (abfd) = NULL;
if (! (*info->callbacks->add_archive_element) (info, abfd, name))
return FALSE;
*pneeded = TRUE;
@@ -2396,7 +2398,13 @@ xcoff_link_check_archive_element (bfd *abfd,
if (*pneeded)
{
- if (! xcoff_link_add_symbols (abfd, info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (arelt_substitute_bfd (abfd)
+ && !_bfd_coff_get_external_symbols (arelt_substitute_bfd (abfd)))
+ return FALSE;
+ if (! xcoff_link_add_symbols (arelt_substitute_bfd (abfd)
+ ? arelt_substitute_bfd (abfd) : abfd, info))
return FALSE;
if (info->keep_memory)
keep_syms_p = TRUE;
diff --git a/include/bfdlink.h b/include/bfdlink.h
index af3c5c4..9154cc5 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -481,7 +481,9 @@ struct bfd_link_callbacks
/* A function which is called when an object is added from an
archive. ABFD is the archive element being added. NAME is the
name of the symbol which caused the archive element to be pulled
- in. */
+ in. This function may set arelt_substitute_bfd(ABFD) to point to
+ an alternative BFD from which symbols should in fact be added in
+ place of the original BFD's symbols. */
bfd_boolean (*add_archive_element)
(struct bfd_link_info *, bfd *abfd, const char *name);
/* A function which is called when a symbol is found with multiple
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 1310852..b161eb0 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -40,6 +40,7 @@
#include "fnmatch.h"
#include "demangle.h"
#include "hashtab.h"
+#include "libbfd.h"
#ifdef ENABLE_PLUGINS
#include "plugin.h"
#endif /* ENABLE_PLUGINS */
@@ -2706,11 +2707,15 @@ load_symbols (lang_input_statement_type *entry,
loaded = FALSE;
}
+ arelt_substitute_bfd (member) = NULL;
if (! ((*link_info.callbacks->add_archive_element)
(&link_info, member, "--whole-archive")))
abort ();
- if (! bfd_link_add_symbols (member, &link_info))
+ /* Potentially, the add_archive_element hook may have set a
+ substitute BFD for us in the areltdata struct. */
+ if (! bfd_link_add_symbols (arelt_substitute_bfd (member)
+ ? arelt_substitute_bfd (member) : member, &link_info))
{
einfo (_("%F%B: could not read symbols: %E\n"), member);
loaded = FALSE;
diff --git a/ld/ldmain.c b/ld/ldmain.c
index f1d6697..0cc0ef9 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -42,6 +42,8 @@
#include "ldctor.h"
#ifdef ENABLE_PLUGINS
#include "plugin.h"
+#include "plugin-api.h"
+#include "libbfd.h"
#endif /* ENABLE_PLUGINS */
/* Somewhere above, sys/stat.h got included. */
@@ -793,6 +795,10 @@ add_archive_element (struct bfd_link_info *info,
const char *name)
{
lang_input_statement_type *input;
+#ifdef ENABLE_PLUGINS
+ lang_input_statement_type orig_input;
+ int fildes;
+#endif /* ENABLE_PLUGINS */
input = (lang_input_statement_type *)
xcalloc (1, sizeof (lang_input_statement_type));
@@ -800,6 +806,53 @@ add_archive_element (struct bfd_link_info *info,
input->local_sym_name = abfd->filename;
input->the_bfd = abfd;
+#ifdef ENABLE_PLUGINS
+ /* Save the original data for trace files/tries below. */
+ orig_input = *input;
+ /* We must offer this archive member to the plugins to claim. */
+ fildes = bfd_my_archive (abfd) != NULL
+ ? open (bfd_my_archive (abfd)->filename, O_RDONLY
+# ifdef O_BINARY
+ | O_BINARY
+# endif /* O_BINARY */
+ )
+ : -1;
+ if (fildes >= 0)
+ {
+ struct ld_plugin_input_file file;
+ int claimed = 0;
+ /* Offset and filesize must refer to the individual archive
+ member, not the whole file, and must exclude the header.
+ Fortunately for us, that is how the data is stored in the
+ origin field of the bfd and in the arelt_data. */
+ file.name = bfd_my_archive (abfd)->filename;
+ file.offset = abfd->origin;
+ file.filesize = arelt_size (abfd);
+ file.fd = fildes;
+ /* We create a dummy BFD, initially empty, to house
+ whatever symbols the plugin may want to add. */
+ file.handle = plugin_get_ir_dummy_bfd (abfd->filename, abfd);
+ if (plugin_call_claim_file (&file, &claimed))
+ einfo (_("%P%F: %s: plugin reported error claiming file\n"),
+ plugin_error_plugin ());
+ if (claimed)
+ {
+ /* Substitute the dummy BFD. */
+ input->the_bfd = file.handle;
+ input->claimed = TRUE;
+ bfd_make_readable (input->the_bfd);
+ arelt_substitute_bfd (abfd) = input->the_bfd;
+ }
+ else
+ {
+ /* Abandon the dummy BFD. */
+ bfd_close_all_done (file.handle);
+ close (fildes);
+ input->claimed = FALSE;
+ }
+ }
+#endif /* ENABLE_PLUGINS */
+
ldlang_add_file (input);
if (config.map_file != NULL)
@@ -880,8 +933,13 @@ add_archive_element (struct bfd_link_info *info,
}
if (trace_files || trace_file_tries)
- info_msg ("%I\n", input);
-
+ info_msg ("%I\n",
+#ifdef ENABLE_PLUGINS
+ &orig_input
+#else
+ input
+#endif /* ENABLE_PLUGINS */
+ );
return TRUE;
}
diff --git a/ld/testsuite/ld-plugin/plugin-10.d b/ld/testsuite/ld-plugin/plugin-10.d
new file mode 100644
index 0000000..7e3c3bb
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-10.d
@@ -0,0 +1,36 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_OPTION 'claim:tmpdir/func.o'
+tv\[19\]: LDPT_OPTION 'sym:_?func::0:0:0'
+tv\[20\]: LDPT_OPTION 'sym:_?func2::0:0:0'
+tv\[21\]: LDPT_OPTION 'dumpresolutions'
+tv\[22\]: LDPT_OPTION 'add:tmpdir/func.o'
+tv\[23\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: claim_file tmpdir/main.o \[@0/.* not claimed
+hook called: claim_file tmpdir/func.o \[@0/.* CLAIMED
+#...
+hook called: claim_file tmpdir/libtext.a \[@.* not claimed
+#...
+hook called: all symbols read.
+Sym: '_?func' Resolution: LDPR_PREVAILING_DEF
+Sym: '_?func2' Resolution: LDPR_PREVAILING_DEF_IRONLY
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin-11.d b/ld/testsuite/ld-plugin/plugin-11.d
new file mode 100644
index 0000000..927cffd
--- /dev/null
+++ b/ld/testsuite/ld-plugin/plugin-11.d
@@ -0,0 +1,40 @@
+Hello from testplugin.
+tv\[0\]: LDPT_MESSAGE func@0x.*
+tv\[1\]: LDPT_API_VERSION value 0x1 \(1\)
+tv\[2\]: LDPT_GNU_LD_VERSION value 0x.*
+tv\[3\]: LDPT_LINKER_OUTPUT value 0x1 \(1\)
+tv\[4\]: LDPT_OUTPUT_NAME 'tmpdir/main.x'
+tv\[5\]: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.*
+tv\[6\]: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.*
+tv\[7\]: LDPT_REGISTER_CLEANUP_HOOK func@0x.*
+tv\[8\]: LDPT_ADD_SYMBOLS func@0x.*
+tv\[9\]: LDPT_GET_INPUT_FILE func@0x.*
+tv\[10\]: LDPT_RELEASE_INPUT_FILE func@0x.*
+tv\[11\]: LDPT_GET_SYMBOLS func@0x.*
+tv\[12\]: LDPT_ADD_INPUT_FILE func@0x.*
+tv\[13\]: LDPT_ADD_INPUT_LIBRARY func@0x.*
+tv\[14\]: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.*
+tv\[15\]: LDPT_OPTION 'registerclaimfile'
+tv\[16\]: LDPT_OPTION 'registerallsymbolsread'
+tv\[17\]: LDPT_OPTION 'registercleanup'
+tv\[18\]: LDPT_OPTION 'claim:tmpdir/func.o'
+tv\[19\]: LDPT_OPTION 'sym:_?func::0:0:0'
+tv\[20\]: LDPT_OPTION 'sym:_?func2::0:0:0'
+tv\[21\]: LDPT_OPTION 'dumpresolutions'
+tv\[22\]: LDPT_OPTION 'add:tmpdir/func.o'
+tv\[23\]: LDPT_OPTION 'claim:tmpdir/libtext.a'
+tv\[24\]: LDPT_OPTION 'sym:_?text::0:0:0'
+tv\[25\]: LDPT_OPTION 'add:tmpdir/text.o'
+tv\[26\]: LDPT_NULL value 0x0 \(0\)
+#...
+hook called: claim_file tmpdir/main.o \[@0/.* not claimed
+hook called: claim_file tmpdir/func.o \[@0/.* CLAIMED
+#...
+hook called: claim_file tmpdir/libtext.a \[@.* CLAIMED
+#...
+hook called: all symbols read.
+Sym: '_?func' Resolution: LDPR_PREVAILING_DEF
+Sym: '_?func2' Resolution: LDPR_PREVAILING_DEF_IRONLY
+Sym: '_?text' Resolution: LDPR_PREVAILING_DEF
+hook called: cleanup.
+#...
diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index 6072e3b..ab8cbe1 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -45,6 +45,7 @@ set plugin_path "$base_dir/$lt_objdir/$plugin_name"
verbose "Full plugin path $plugin_path" 2
set testobjfiles "$HOSTING_CRT0 tmpdir/main.o tmpdir/func.o tmpdir/text.o"
+set testobjfiles_notext "$HOSTING_CRT0 tmpdir/main.o tmpdir/func.o"
set libs "$LIBS $HOSTING_LIBS"
set regclm "-plugin-arg registerclaimfile"
@@ -108,6 +109,26 @@ set plugin_tests [list \
$testobjfiles $libs" "" "" {{ld plugin-9.d}} "main.x" ] \
]
+set plugin_lib_tests [list \
+ [list "plugin ignore lib" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ -plugin-arg sym:${_}func::0:0:0 \
+ -plugin-arg sym:${_}func2::0:0:0 \
+ -plugin-arg dumpresolutions \
+ -plugin-arg add:tmpdir/func.o \
+ $testobjfiles_notext -Ltmpdir -ltext $libs" "" "" {{ld plugin-10.d}} "main.x" ] \
+ [list "plugin claimfile replace lib" "-plugin $plugin_path $regclm \
+ $regas $regcln -plugin-arg claim:tmpdir/func.o \
+ -plugin-arg sym:${_}func::0:0:0 \
+ -plugin-arg sym:${_}func2::0:0:0 \
+ -plugin-arg dumpresolutions \
+ -plugin-arg add:tmpdir/func.o \
+ -plugin-arg claim:tmpdir/libtext.a \
+ -plugin-arg sym:${_}text::0:0:0 \
+ -plugin-arg add:tmpdir/text.o \
+ $testobjfiles_notext -Ltmpdir -ltext $libs" "" "" {{ld plugin-11.d}} "main.x" ] \
+]
+
set plugin_extra_elf_tests [list \
[list "plugin set symbol visibility" "-plugin $plugin_path $regclm \
$regas $regcln -plugin-arg claim:tmpdir/func.o \
@@ -138,3 +159,10 @@ if { [is_elf_format] } {
run_ld_link_tests $plugin_extra_elf_tests
}
+if ![ar_simple_create $ar "" "tmpdir/libtext.a" "tmpdir/text.o"] {
+ foreach testitem $plugin_lib_tests {
+ unresolved [lindex $testitem 0]
+ }
+} else {
+ run_ld_link_tests $plugin_lib_tests
+}
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 6:13 ` Ralf Wildenhues
@ 2010-09-30 13:13 ` Dave Korn
2010-09-30 13:20 ` Dave Korn
0 siblings, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-09-30 13:13 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: binutils
[ Sorry to return to this issue, but I still haven't fixed it yet. ]
On 23/09/2010 07:13, Ralf Wildenhues wrote:
> * Dave Korn wrote on Thu, Sep 23, 2010 at 08:25:42AM CEST:
>> On 23/09/2010 06:36, Ralf Wildenhues wrote:
>>> * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
>>>> --- a/ld/Makefile.am
>>>> +++ b/ld/Makefile.am
>>>> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>>>> noinst_LTLIBRARIES = libldtestplug.la
>>>> libldtestplug_la_SOURCES = testplug.c
>>>> libldtestplug_la_CFLAGS= -g -O2
>>>> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
>>>> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
>>> Why the '-Wl,' prefix? I'd have expected $(LIBIBERTY) to be added to
>>> libldtestplug_la_LIBADD.
>> libiberty isn't built under libtool control at all, AFAIK, and is only ever
>> built as a static archive, so I was under the impression I've got to pass it
>> around behind libtool's back like this.
>
> Not because of that, no.
>
>>> If you need to work around libtool warning about adding static library
>>> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
>>> but still I think you will run into troubles on static-only systems.
>> What kind of problems?
>
> Ordering. You can't rely on libtool expanding -Wl, link flags after the
> objects, meaning that libiberty may not be picked up at all. (With
> static linking, objects and libraries are evaluated in a strict command
> line order, so all needed libraries must come after objects that need
> them.)
Seems I'm not the only one who has this problem. The makefile for GCC's
lto-plugin links libiberty like so:
> liblto_plugin_la_LIBADD = $(LIBELFLIBS) ../libiberty/pic/libiberty.a
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-30 13:13 ` Dave Korn
@ 2010-09-30 13:20 ` Dave Korn
2010-09-30 20:25 ` Ralf Wildenhues
0 siblings, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-09-30 13:20 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: binutils
[ groan, fat-fingered the send key prematurely... ]
[ Sorry to return to this issue, but I still haven't fixed it yet. ]
On 23/09/2010 07:13, Ralf Wildenhues wrote:
> * Dave Korn wrote on Thu, Sep 23, 2010 at 08:25:42AM CEST:
>> On 23/09/2010 06:36, Ralf Wildenhues wrote:
>>> * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
>>>> --- a/ld/Makefile.am
>>>> +++ b/ld/Makefile.am
>>>> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>>>> noinst_LTLIBRARIES = libldtestplug.la
>>>> libldtestplug_la_SOURCES = testplug.c
>>>> libldtestplug_la_CFLAGS= -g -O2
>>>> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
>>>> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
>>> Why the '-Wl,' prefix? I'd have expected $(LIBIBERTY) to be added to
>>> libldtestplug_la_LIBADD.
>> libiberty isn't built under libtool control at all, AFAIK, and is only ever
>> built as a static archive, so I was under the impression I've got to pass it
>> around behind libtool's back like this.
>
> Not because of that, no.
>
>>> If you need to work around libtool warning about adding static library
>>> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
>>> but still I think you will run into troubles on static-only systems.
>> What kind of problems?
>
> Ordering. You can't rely on libtool expanding -Wl, link flags after the
> objects, meaning that libiberty may not be picked up at all. (With
> static linking, objects and libraries are evaluated in a strict command
> line order, so all needed libraries must come after objects that need
> them.)
Seems I'm not the only one who has this problem. The makefile for GCC's
lto-plugin links libiberty like so:
> liblto_plugin_la_LIBADD = $(LIBELFLIBS) ../libiberty/pic/libiberty.a
which produces this warning on Linux:
> /bin/sh ./libtool --tag=CC --mode=link /gnu/gcc/obj.clean/./prev-gcc/xgcc -B/gnu/gcc/obj.clean/./prev-gcc/ -B/opt/gold/i686-pc-linux-gnu/bin/ -B/opt/gold/i686-pc-linux-gnu/bin/ -B/opt/gold/i686-pc-linux-gnu/lib/ -isystem /opt/gold/i686-pc-linux-gnu/include -isystem /opt/gold/i686-pc-linux-gnu/sys-include -Wall -Werror -g -O2 -fomit-frame-pointer -gtoggle -o liblto_plugin.la -rpath /opt/gold/libexec/gcc/i686-pc-linux-gnu/4.6.0 lto-plugin.lo -lelf ../libiberty/pic/libiberty.a
>
> *** Warning: Linking the shared library liblto_plugin.la against the
> *** static library ../libiberty/pic/libiberty.a is not portable!
When I try using LIBADD to link a static archive into a shared library on
Cygwin, such as like so:
> libldtestplug_la_LIBADD = $(LIBIBERTY)
... that warning becomes an error:
> *** Warning: Trying to link with static lib archive ../libiberty/libiberty.a.
> *** I have the capability to make that library automatically link in when
> *** you link to this library. But I can only do this if you have a
> *** shared version of the library, which you do not appear to have
> *** because the file extensions .a of this argument makes me believe
> *** that it is just a static archive that I should not use here.
... and the link fails with unresolved symbols:
> Creating library file: .libs/libldtestplug.dll.a.libs/libldtestplug_la-testplug.
> o: In function `record_claim_file':
> /gnu/binutils/git.repo/binutils/ld/testplug.c:148: undefined reference to `_xmalloc'
> .libs/libldtestplug_la-testplug.o: In function `record_claimed_file_symbol':
> /gnu/binutils/git.repo/binutils/ld/testplug.c:261: undefined reference to `_xrealloc'
> .libs/libldtestplug_la-testplug.o: In function `record_add_file':
> /gnu/binutils/git.repo/binutils/ld/testplug.c:166: undefined reference to `_xmalloc'
> /gnu/binutils/git.repo/binutils/ld/testplug.c:166: undefined reference to `_xmalloc'
> /gnu/binutils/git.repo/binutils/ld/testplug.c:166: undefined reference to `_xmalloc'
> collect2: ld returned 1 exit status
>
> make[2]: *** [libldtestplug.la] Error 1
So if -Wl won't work, what can I do? Do I have to build a libtool
convenience library out of the libiberty object files?
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-30 13:20 ` Dave Korn
@ 2010-09-30 20:25 ` Ralf Wildenhues
0 siblings, 0 replies; 39+ messages in thread
From: Ralf Wildenhues @ 2010-09-30 20:25 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
Hi Dave,
* Dave Korn wrote on Thu, Sep 30, 2010 at 03:43:03PM CEST:
> On 23/09/2010 07:13, Ralf Wildenhues wrote:
> > Ordering. You can't rely on libtool expanding -Wl, link flags after the
> > objects, meaning that libiberty may not be picked up at all. (With
> > static linking, objects and libraries are evaluated in a strict command
> > line order, so all needed libraries must come after objects that need
> > them.)
>
> Seems I'm not the only one who has this problem. The makefile for GCC's
> lto-plugin links libiberty like so:
>
> > liblto_plugin_la_LIBADD = $(LIBELFLIBS) ../libiberty/pic/libiberty.a
>
> which produces this warning on Linux:
[...]
> When I try using LIBADD to link a static archive into a shared library on
> Cygwin, such as like so:
>
> > libldtestplug_la_LIBADD = $(LIBIBERTY)
>
> ... that warning becomes an error:
Ouch.
> So if -Wl won't work, what can I do? Do I have to build a libtool
> convenience library out of the libiberty object files?
Then use -Wc, please, so at least it is passed through to the compiler
driver without -Wl,. A convenience archive would be cleaner from a
libtool POV, yes; I think I suggested that at one time but wasn't
received too well.
Cheers,
Ralf
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PING] Re: [PATCH] Add plugin interface to LD [0/4]
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
` (5 preceding siblings ...)
2010-09-27 0:21 ` [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface Dave Korn
@ 2010-10-02 15:52 ` Dave Korn
6 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-02 15:52 UTC (permalink / raw)
To: binutils
On 23/09/2010 06:29, Dave Korn wrote:
> Hi list,
>
> The following set of patches implements the plugin API as seen in GOLD into
> GNU LD.
Sorry to be pinging after only 9 days, but I figure Cary would like to get
on with cutting the release branch fairly pronto, so I'm asking a bit earlier
than I otherwise might.
I've incorporated the tweaks Ralf suggested to the makefiles, and added the
missing features from the initial set of posts. Is there anything else anyone
thinks I should do?
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [1/4] Infrastructure Dave Korn
2010-09-23 5:41 ` Ralf Wildenhues
@ 2010-10-04 4:35 ` Dave Korn
2010-10-06 20:57 ` Richard Henderson
2 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-04 4:35 UTC (permalink / raw)
To: binutils
On 23/09/2010 06:31, Dave Korn wrote:
> This is the first patch in the series: it adds the infrastructure for
> parsing plugin-related command line options, and for loading, initialising,
> and cleaning up the plugins.
One more thing I already noticed and fixed, to save anyone else the trouble
of pointing it out:
> +#ifdef ENABLE_PLUGINS
> + { {"plugin", required_argument, NULL, OPTION_PLUGIN},
> + '\0', N_("PLUGIN"), N_("Load named plugin"), ONE_DASH },
> + { {"plugin-arg", required_argument, NULL, OPTION_PLUGIN_ARG},
> + '\0', N_("ARG"), N_("Send arg to last-loaded plugin"), ONE_DASH },
> +#endif /* ENABLE_PLUGINS */
That's meant to be -plugin-opt, not -plugin-arg. Fixed in current respin.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [1/4] Infrastructure Dave Korn
2010-09-23 5:41 ` Ralf Wildenhues
2010-10-04 4:35 ` Dave Korn
@ 2010-10-06 20:57 ` Richard Henderson
2010-10-07 21:38 ` Dave Korn
2010-10-12 8:32 ` Richard Sandiford
2 siblings, 2 replies; 39+ messages in thread
From: Richard Henderson @ 2010-10-06 20:57 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/22/2010 10:31 PM, Dave Korn wrote:
> - I added a --enable-plugins command to ld's configure before I remembered
> that there's one in the binutils/ subdir (for nm and ar) as well. Should I
> perhaps change it to --enable-ld-plugins (or something else) so that it can be
> controlled separately? Should I rewrite it to use config/plugins.m4?
I honestly don't have any idea. I didn't realize that we had a plugin
interface to bfd. I almost suggested renaming the existing option to
--enable-bfd-plugins, but I don't see that actually reducing the
confusion at all.
Personally I think the least amount of confusion would be generated
by having the ld plugin be non-optional. That way for some minimal
binutils version gcc can simply assume its availability.
> - Is it OK to put an unguarded "#include <stdarg.h>" in ldmisc.h?
We assume C90, so technically ok, but why do you want it in
that header file? Probably sysdep.h would be better for
consistency.
[ Ignoring most of the configury and makefile hackery ]
> +/* Alias to shorten static function prototype lines. */
> +#define PLUGAPIFUNC static enum ld_plugin_status
That just seems odd. I'd prefer formatting like
static enum ld_plugin_status message
(int level, const char *format, ...);
and I'd prefer even more if those declarations weren't needed.
I.e. move the uses of those functions to the end of the file.
> + /* Allocate tv array and initialise constant part. */
> + my_tv = xmalloc ((max_args + 1 + set_tv_header (NULL)) * sizeof *my_tv);
> + set_tv_header (my_tv);
I think this interface to set_tv_header is unnecessarily ugly.
Move tv_header_tags and tv_header_size out of set_tv_header
and you no longer need those silly NULL calls.
> + newplug->dlhandle = dlopen (plugin, RTLD_NOW);
While I like avoiding libtool as much as possible, don't we
have to use lt_dlopen on some targets? I suppose cygwin
already does the appropriate mapping to LoadLibrary...
> +/* Always use this macro when invoking a plugin function. */
> +#define INVOKE_PLUGIN_FN(plugin, retval, fn, args) \
> + called_plugin = plugin; \
> + retval = (*fn) args; \
> + called_plugin = NULL;
While this does manage called_plugin effectively, this makes
the actual invocations unnecessarily ugly. Do you expect
more code to go here, or couldn't we just expand this in
place in the 4 places its used?
> + {
> + char *newfmt = xmalloc (strlen (format) + 3);
> + newfmt[0] = '%';
> + newfmt[1] = (level == LDPL_FATAL) ? 'F' : 'X';
> + strcpy (&newfmt[2], format);
> + vfinfo (stderr, newfmt, args, TRUE);
> + }
Probably better as
newfmt = concat ((level == LDPL_FATAL ? "%F" : "%X"),
format, NULL);
Also missing a free.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols Dave Korn
2010-09-23 5:36 ` Ralf Wildenhues
2010-09-24 1:23 ` Dave Korn
@ 2010-10-06 22:14 ` Richard Henderson
2010-10-07 21:42 ` Dave Korn
2010-10-09 3:04 ` Dave Korn
2 siblings, 2 replies; 39+ messages in thread
From: Richard Henderson @ 2010-10-06 22:14 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/22/2010 10:31 PM, Dave Korn wrote:
> - I'd certainly appreciate a second pair of eyes looking at the logic in
> asymbol_from_plugin_symbol by which I derive the fags etc. for a bfd asymbol
> from the ld plugin symbol struct.
It does seem to be broken. I see no way you're going to
be able to generate cigarettes from that code.
> - return TRUE;
> + checked_ok = TRUE;
I wonder if a "goto success" wouldn't be cleaner.
> + int fildes = bfd_check_format (entry->the_bfd, bfd_object)
> + ? open (attempt, O_RDONLY
> +# ifdef O_BINARY
> + | O_BINARY
> +# endif /* O_BINARY */
> + )
> + : -1;
You already caught the c99 issue yourself. As for O_BINARY,
better to put that in sysdeps.h with the rest of the O_FLAGS.
> +/* Don't include std headers until after config.h, sysdeps.h etc.,
> + or you may end up with the wrong bitsize of off_t. */
> +#include <dlfcn.h>
Better to do this in sysdeps.h, with ifdef protection. I like
having everything system related done in the one place.
> + asym->section = ldsym->def == LDPK_COMMON
> + ? bfd_com_section_ptr
> + : (ldsym->def == LDPK_UNDEF || ldsym->def == LDPK_WEAKUNDEF
> + ? bfd_und_section_ptr
> + : bfd_get_section_by_name (abfd, ".text"));
This is pretty hard to read as-is. Better as
flagword flags = BSF_NO_FLAGS;
struct bfd_section *section;
switch (ldsym->def)
{
case LDPK_WEAKDEF:
flags = BSF_WEAK;
/* FALLTHRU */
case LDPK_DEF:
flags |= BSF_GLOBAL;
section = bfd_get_section_by_name (abfd, ".text");
break;
case LDPK_WEAKUNDEF:
flags = BSF_WEAK;
/* FALLTHRU */
case LDPK_UNDEF:
section = bfd_und_section_ptr;
break;
case LDPK_COMMON:
flags = BSF_GLOBAL;
section = bfd_com_section_ptr;
break;
default:
return LDPS_ERR;
}
asym->flags = flags;
asym->section = section;
> + rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms++);
Better I think just as syms + n. I prefer not to increment
induction variables at non-obvious spots inside the loop.
In this case we can rely on the compiler producing the ++
if it really turns out to be useful.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions Dave Korn
@ 2010-10-06 22:28 ` Richard Henderson
2010-10-07 21:55 ` Dave Korn
0 siblings, 1 reply; 39+ messages in thread
From: Richard Henderson @ 2010-10-06 22:28 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/22/2010 10:31 PM, Dave Korn wrote:
> + if (blhe->type == bfd_link_hash_common)
> + {
> + /* Is this right? The COMMON section only exists in the
> + output file, so it seems reasonable. */
> + syms[n].resolution = LDPR_RESOLVED_EXEC;
> + continue;
Common symbols can be exported or not exported when generating a
shared library. Another check is needed here. In fact, I don't
see that you're doing any shared library preemption checking in
this function.
... that said, all that is elf-only; windows doesn't do library
symbol preemption.
> + /* Suppresses "unused" warnings without relying on GCC attribute. */
> + info = info;
> + value = value;
Er, I presume this is going away in a future patch?
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths.
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths Dave Korn
2010-09-24 0:35 ` Dave Korn
@ 2010-10-06 22:40 ` Richard Henderson
2010-10-07 21:48 ` Dave Korn
1 sibling, 1 reply; 39+ messages in thread
From: Richard Henderson @ 2010-10-06 22:40 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/22/2010 10:32 PM, Dave Korn wrote:
> +#ifdef ENABLE_PLUGINS
> + union lang_statement_union **listend;
> +#endif /* ENABLE_PLUGINS */
...
> #ifdef ENABLE_PLUGINS
> /* Now all files are read, let the plugin(s) decide if there
> are any more to be added to the link before we call the
> emulation's after_open hook. */
> + listend = statement_list.tail;
> + ASSERT (!*listend);
Better to merge these two with a set of braces inside
the second ENABLE_PLUGINS ifdef.
> + osec = osec;
> + oval = oval;
Please just use ATTRIBUTE_UNUSED.
Otherwise ok.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [5/4] Support ELF symbol visibility
2010-09-24 0:30 ` [PATCH] Add plugin interface to LD [0/4] Support ELF symbol visibility Dave Korn
2010-09-24 0:38 ` [PATCH] Add plugin interface to LD [5/4] " Dave Korn
@ 2010-10-06 22:42 ` Richard Henderson
1 sibling, 0 replies; 39+ messages in thread
From: Richard Henderson @ 2010-10-06 22:42 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/23/2010 05:53 PM, Dave Korn wrote:
> ld/ChangeLog:
>
> * plugin.c (asymbol_from_plugin_symbol): If the bfd is an ELF bfd,
> find the elf symbol data and set the visibility in the st_other field.
>
> ld/testsuite/ChangeLog:
>
> * plugin-ignore.d: New dump test control script.
> * plugin-vis-1.d: Likewise.
> * plugin.exp: Add list of ELF-only tests and run them if testing on
> an ELF target.
Ok.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface.
2010-09-27 0:21 ` [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface Dave Korn
@ 2010-10-06 22:57 ` Richard Henderson
2010-10-07 21:52 ` Dave Korn
0 siblings, 1 reply; 39+ messages in thread
From: Richard Henderson @ 2010-10-06 22:57 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 09/26/2010 05:43 PM, Dave Korn wrote:
> This one isn't as neat as the other patches. I couldn't see any way to use
> the existing functionality of the linker "add_archive_element" callback to
> supply an alternative BFD with its own set of symbols from the IR, so I had to
> extend BFD to make this possible. And I didn't want to change the interface
> of add_archive_element, since it's one of the few actual public interfaces of
> libbfd, and we don't have any versioning.
Nothing in libbfd pretends to have a stable ABI, so I don't think you
need to worry about that.
> So that led me to take a somewhat ugly solution: I added a new member to
> struct areltdata, and use that to communicate between libbfd and the linker
> callback. The member gets zeroed immediately before calling the callback, and
> if on return it has been set to point to a BFD, the symbols from that BFD are
> added to the linker hash table instead of the archive element's own symbols.
> It's not ideal, but I think it's OK enough; there are only so many places from
> which BFD calls the hook. If anyone has a better idea how to implement this,
> I'm all ears. Otherwise, OK?
I think I'd prefer a fourth argument for add_archive_element which, if
non-null, receives the replacement. Thus for a.out we can simply not
change anything except add the NULL 4th argument. (Recall that a.out
cannot represent LTO data at all, and so needn't worry about this whole
plugin business.)
Honestly, I think it would clean up some of your arelt_substitute_bfd
checks as well.
> +#ifdef ENABLE_PLUGINS
> + lang_input_statement_type orig_input;
> + int fildes;
> +#endif /* ENABLE_PLUGINS */
See previous comments re keeping decls near code.
> + info_msg ("%I\n",
> +#ifdef ENABLE_PLUGINS
> + &orig_input
> +#else
> + input
> +#endif /* ENABLE_PLUGINS */
In this case I don't think you need to conditionalize orig_input.
Just do it all the time and be done with it.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-10-06 20:57 ` Richard Henderson
@ 2010-10-07 21:38 ` Dave Korn
2010-10-07 21:48 ` Richard Henderson
2010-10-12 8:32 ` Richard Sandiford
1 sibling, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-10-07 21:38 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
On 06/10/2010 21:57, Richard Henderson wrote:
> On 09/22/2010 10:31 PM, Dave Korn wrote:
>> - I added a --enable-plugins command to ld's configure before I remembered
>> that there's one in the binutils/ subdir (for nm and ar) as well. Should I
>> perhaps change it to --enable-ld-plugins (or something else) so that it can be
>> controlled separately? Should I rewrite it to use config/plugins.m4?
>
> I honestly don't have any idea. I didn't realize that we had a plugin
> interface to bfd.
:) I wasn't aware of it either until I started on this, but looking back I
see we've had it for quite some time!
> Personally I think the least amount of confusion would be generated
> by having the ld plugin be non-optional. That way for some minimal
> binutils version gcc can simply assume its availability.
I was thinking mostly in terms of having the ability to disable it as a
get-out-of-jail-free card if it broke anything. I'm pretty confident that
none of the regular code paths will be affected if the user simply doesn't use
the -plugin option, but some platforms might not have dlopen, so I'll have to
add some AC_FUNC/AC_HEADERS tests for those, and then I'll still need some way
of disabling the plugin code from building on platforms that don't support the
dl-* interfaces, so since I'm going to have all the mechanism anyway, why not
provide command-line control over it?
I could leave the option there but default it to on for platforms where the
dl-* headers and funcs are found, or I could remove the command-line option
altogether and make it depend only on the presence of the libs and headers;
please let me know which you'd prefer.
>> - Is it OK to put an unguarded "#include <stdarg.h>" in ldmisc.h?
>
> We assume C90, so technically ok, but why do you want it in
> that header file? Probably sysdep.h would be better for
> consistency.
I just put it there because that was what depended on it. I'll move it to
sysdep.h as you suggest.
>> +/* Alias to shorten static function prototype lines. */
>> +#define PLUGAPIFUNC static enum ld_plugin_status
>
> That just seems odd. I'd prefer formatting like
>
> static enum ld_plugin_status message
> (int level, const char *format, ...);
>
> and I'd prefer even more if those declarations weren't needed.
> I.e. move the uses of those functions to the end of the file.
OK, will do.
>> + /* Allocate tv array and initialise constant part. */
>> + my_tv = xmalloc ((max_args + 1 + set_tv_header (NULL)) * sizeof *my_tv);
>> + set_tv_header (my_tv);
>
> I think this interface to set_tv_header is unnecessarily ugly.
> Move tv_header_tags and tv_header_size out of set_tv_header
> and you no longer need those silly NULL calls.
OK, will do.
>> + newplug->dlhandle = dlopen (plugin, RTLD_NOW);
>
> While I like avoiding libtool as much as possible, don't we
> have to use lt_dlopen on some targets? I suppose cygwin
> already does the appropriate mapping to LoadLibrary...
I copied what both GOLD and BFD do here. As far as I know, the main thing
that using libltdl would give us is dlpreopening, which in theory would allow
linking plugins statically into ld on platforms that don't support DSOs; but
IIUC, that would require the plugin to be built as part of the ld build and
modifications to the ld makefile to link it against the plugin, which I don't
think is going to be a significant use case; I think most plugins will be
built as part of separate projects, like GCC's lto-plugin.
So, I'd rather just punt on that. We can add it later if it turns out to
actually be needed for anything?
>> +/* Always use this macro when invoking a plugin function. */
>> +#define INVOKE_PLUGIN_FN(plugin, retval, fn, args) \
>> + called_plugin = plugin; \
>> + retval = (*fn) args; \
>> + called_plugin = NULL;
>
> While this does manage called_plugin effectively, this makes
> the actual invocations unnecessarily ugly. Do you expect
> more code to go here, or couldn't we just expand this in
> place in the 4 places its used?
Well, I just didn't like cut'n'pasting the same code repeatedly, but I guess
if we don't expect a whole load more interfaces to be added to the API in
future then I'll just expand it inline.
>> + {
>> + char *newfmt = xmalloc (strlen (format) + 3);
>> + newfmt[0] = '%';
>> + newfmt[1] = (level == LDPL_FATAL) ? 'F' : 'X';
>> + strcpy (&newfmt[2], format);
>> + vfinfo (stderr, newfmt, args, TRUE);
>> + }
>
> Probably better as
>
> newfmt = concat ((level == LDPL_FATAL ? "%F" : "%X"),
> format, NULL);
>
> Also missing a free.
Right, thanks, will fix.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-10-06 22:14 ` Richard Henderson
@ 2010-10-07 21:42 ` Dave Korn
2010-10-09 3:04 ` Dave Korn
1 sibling, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-07 21:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
On 06/10/2010 23:14, Richard Henderson wrote:
> On 09/22/2010 10:31 PM, Dave Korn wrote:
>> - I'd certainly appreciate a second pair of eyes looking at the logic in
>> asymbol_from_plugin_symbol by which I derive the fags etc. for a bfd asymbol
>> from the ld plugin symbol struct.
>
> It does seem to be broken. I see no way you're going to
> be able to generate cigarettes from that code.
LOL, only thing broken there was my brain... I meant flags of course!
>> - return TRUE;
>> + checked_ok = TRUE;
>
> I wonder if a "goto success" wouldn't be cleaner.
Will do.
>> + int fildes = bfd_check_format (entry->the_bfd, bfd_object)
>> + ? open (attempt, O_RDONLY
>> +# ifdef O_BINARY
>> + | O_BINARY
>> +# endif /* O_BINARY */
>> + )
>> + : -1;
>
> You already caught the c99 issue yourself. As for O_BINARY,
> better to put that in sysdeps.h with the rest of the O_FLAGS.
Will do.
>
>> +/* Don't include std headers until after config.h, sysdeps.h etc.,
>> + or you may end up with the wrong bitsize of off_t. */
>> +#include <dlfcn.h>
>
> Better to do this in sysdeps.h, with ifdef protection. I like
> having everything system related done in the one place.
OK. Leaving it here worked out when --enable-plugins was not the default
and there wasn't autoconfigury to detect the dl* functions/headers, but since
I'm adding that (see previous reply) I can easily move this now that there'll
be a HAVE_DLFCN_H for it.
>
>> + asym->section = ldsym->def == LDPK_COMMON
>> + ? bfd_com_section_ptr
>> + : (ldsym->def == LDPK_UNDEF || ldsym->def == LDPK_WEAKUNDEF
>> + ? bfd_und_section_ptr
>> + : bfd_get_section_by_name (abfd, ".text"));
>
> This is pretty hard to read as-is. Better as
Thanks, will refactor it into a switch as you suggest.
>> + rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms++);
>
> Better I think just as syms + n. I prefer not to increment
> induction variables at non-obvious spots inside the loop.
> In this case we can rely on the compiler producing the ++
> if it really turns out to be useful.
OK, will do. Thanks for reviewing!
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths.
2010-10-06 22:40 ` Richard Henderson
@ 2010-10-07 21:48 ` Dave Korn
0 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-07 21:48 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
On 06/10/2010 23:40, Richard Henderson wrote:
> On 09/22/2010 10:32 PM, Dave Korn wrote:
> Better to merge these two with a set of braces inside
> the second ENABLE_PLUGINS ifdef.
>
> Please just use ATTRIBUTE_UNUSED.
Will change both as you say.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-10-07 21:38 ` Dave Korn
@ 2010-10-07 21:48 ` Richard Henderson
0 siblings, 0 replies; 39+ messages in thread
From: Richard Henderson @ 2010-10-07 21:48 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 10/07/2010 03:00 PM, Dave Korn wrote:
> I could leave the option there but default it to on for platforms where the
> dl-* headers and funcs are found, or I could remove the command-line option
> altogether and make it depend only on the presence of the libs and headers;
> please let me know which you'd prefer.
Availability of libs and headers sounds good to me.
> So, I'd rather just punt on that. We can add it later if it turns out to
> actually be needed for anything?
I'd been thinking mostly of a mingw host, which won't have dlopen
but will have LoadLibrary. But we can put that off, sure.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface.
2010-10-06 22:57 ` Richard Henderson
@ 2010-10-07 21:52 ` Dave Korn
0 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-07 21:52 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
On 06/10/2010 23:57, Richard Henderson wrote:
> On 09/26/2010 05:43 PM, Dave Korn wrote:
>> I didn't want to change the interface
>> of add_archive_element, since it's one of the few actual public interfaces of
>> libbfd, and we don't have any versioning.
>
> Nothing in libbfd pretends to have a stable ABI, so I don't think you
> need to worry about that.
> I think I'd prefer a fourth argument for add_archive_element which, if
> non-null, receives the replacement. Thus for a.out we can simply not
> change anything except add the NULL 4th argument. (Recall that a.out
> cannot represent LTO data at all, and so needn't worry about this whole
> plugin business.)
>
> Honestly, I think it would clean up some of your arelt_substitute_bfd
> checks as well.
OK, I was erring on the side of caution, but on most platforms, it won't
even hurt if there's a fourth argument provided by the caller that the callee
doesn't know about; will change it as you suggest.
>> +#ifdef ENABLE_PLUGINS
>> + lang_input_statement_type orig_input;
>> + int fildes;
>> +#endif /* ENABLE_PLUGINS */
>
> See previous comments re keeping decls near code.
>
>> + info_msg ("%I\n",
>> +#ifdef ENABLE_PLUGINS
>> + &orig_input
>> +#else
>> + input
>> +#endif /* ENABLE_PLUGINS */
>
> In this case I don't think you need to conditionalize orig_input.
> Just do it all the time and be done with it.
Right you are. Thanks for the comments.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions.
2010-10-06 22:28 ` Richard Henderson
@ 2010-10-07 21:55 ` Dave Korn
0 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-07 21:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
On 06/10/2010 23:28, Richard Henderson wrote:
> On 09/22/2010 10:31 PM, Dave Korn wrote:
>> + if (blhe->type == bfd_link_hash_common)
>> + {
>> + /* Is this right? The COMMON section only exists in the
>> + output file, so it seems reasonable. */
>> + syms[n].resolution = LDPR_RESOLVED_EXEC;
>> + continue;
>
> Common symbols can be exported or not exported when generating a
> shared library. Another check is needed here. In fact, I don't
> see that you're doing any shared library preemption checking in
> this function.
>
> ... that said, all that is elf-only; windows doesn't do library
> symbol preemption.
Yep. Or visibility for that matter. And library symbols typically get
resolved in practice by linking an import stub into the executable.
>
>> + /* Suppresses "unused" warnings without relying on GCC attribute. */
>> + info = info;
>> + value = value;
>
> Er, I presume this is going away in a future patch?
Well I had to do something to suppress the unused warnings, those parameters
are passed by the BFD callback interface whether I need to use them or not.
We've got ATTRIBUTE_UNUSED, I know, but I thought "Why not use something
completely generic". I see you've asked for ATTRIBUTE_UNUSED in one of the
other posts, so I'll do that.
Need to think more about the common symbols, but have to go AFK for a while
now. Thanks for all the help, I'll get back on this later.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-10-06 22:14 ` Richard Henderson
2010-10-07 21:42 ` Dave Korn
@ 2010-10-09 3:04 ` Dave Korn
2010-10-09 17:59 ` Richard Henderson
1 sibling, 1 reply; 39+ messages in thread
From: Dave Korn @ 2010-10-09 3:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: binutils
On 06/10/2010 23:14, Richard Henderson wrote:
Re:
>> + asym->section = ldsym->def == LDPK_COMMON
>> + ? bfd_com_section_ptr
>> + : (ldsym->def == LDPK_UNDEF || ldsym->def == LDPK_WEAKUNDEF
>> + ? bfd_und_section_ptr
>> + : bfd_get_section_by_name (abfd, ".text"));
>
> This is pretty hard to read as-is. Better as
>
> flagword flags = BSF_NO_FLAGS;
> struct bfd_section *section;
>
> switch (ldsym->def)
> {
> case LDPK_WEAKDEF:
> flags = BSF_WEAK;
> /* FALLTHRU */
> case LDPK_DEF:
> flags |= BSF_GLOBAL;
> section = bfd_get_section_by_name (abfd, ".text");
> break;
>
> case LDPK_WEAKUNDEF:
> flags = BSF_WEAK;
> /* FALLTHRU */
> case LDPK_UNDEF:
> section = bfd_und_section_ptr;
> break;
>
> case LDPK_COMMON:
> flags = BSF_GLOBAL;
> section = bfd_com_section_ptr;
> break;
>
> default:
> return LDPS_ERR;
> }
> asym->flags = flags;
> asym->section = section;
I think you didn't intend to remove the "asym->value = ldsym->size;" for
common symbols in the original switch statement, and I think it's still the
right thing to do? That's how I'm respinning it to start with, let me know if
I assumed wrong.
>> + rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms++);
>
> Better I think just as syms + n. I prefer not to increment
> induction variables at non-obvious spots inside the loop.
> In this case we can rely on the compiler producing the ++
> if it really turns out to be useful.
Looking back at my source tree, I'd already removed the ++ and put syms++
into the for loop-increment clause, but I'm sure the compiler will optimise it
for us, and I also think it's a bit cleaner to not modify your formal
parameters if you really don't have to, so I redid it this way anyway.
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.
2010-10-09 3:04 ` Dave Korn
@ 2010-10-09 17:59 ` Richard Henderson
0 siblings, 0 replies; 39+ messages in thread
From: Richard Henderson @ 2010-10-09 17:59 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On 10/08/2010 08:27 PM, Dave Korn wrote:
> I think you didn't intend to remove the "asym->value = ldsym->size;" for
> common symbols in the original switch statement, and I think it's still the
> right thing to do?
Quite right.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-10-06 20:57 ` Richard Henderson
2010-10-07 21:38 ` Dave Korn
@ 2010-10-12 8:32 ` Richard Sandiford
2010-10-12 11:12 ` Dave Korn
1 sibling, 1 reply; 39+ messages in thread
From: Richard Sandiford @ 2010-10-12 8:32 UTC (permalink / raw)
To: Richard Henderson; +Cc: Dave Korn, binutils
Richard Henderson <rth@redhat.com> writes:
>> + {
>> + char *newfmt = xmalloc (strlen (format) + 3);
>> + newfmt[0] = '%';
>> + newfmt[1] = (level == LDPL_FATAL) ? 'F' : 'X';
>> + strcpy (&newfmt[2], format);
>> + vfinfo (stderr, newfmt, args, TRUE);
>> + }
>
> Probably better as
>
> newfmt = concat ((level == LDPL_FATAL ? "%F" : "%X"),
> format, NULL);
Plugging my favourite libiberty macro, there's also ACONCAT ((...))
(which avoids the malloc and free). Should be safe for the kinds
of string lengths we're talking about here.
Richard
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.
2010-10-12 8:32 ` Richard Sandiford
@ 2010-10-12 11:12 ` Dave Korn
0 siblings, 0 replies; 39+ messages in thread
From: Dave Korn @ 2010-10-12 11:12 UTC (permalink / raw)
To: Richard Henderson, Dave Korn, binutils, richard.sandiford
On 12/10/2010 09:32, Richard Sandiford wrote:
> Richard Henderson <rth@redhat.com> writes:
>>> + {
>>> + char *newfmt = xmalloc (strlen (format) + 3);
>>> + newfmt[0] = '%';
>>> + newfmt[1] = (level == LDPL_FATAL) ? 'F' : 'X';
>>> + strcpy (&newfmt[2], format);
>>> + vfinfo (stderr, newfmt, args, TRUE);
>>> + }
>> Probably better as
>>
>> newfmt = concat ((level == LDPL_FATAL ? "%F" : "%X"),
>> format, NULL);
>
> Plugging my favourite libiberty macro, there's also ACONCAT ((...))
> (which avoids the malloc and free). Should be safe for the kinds
> of string lengths we're talking about here.
It's been years since we had to worry about alloca portability, hasn't it?
I'll use it!
cheers,
DaveK
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2010-10-12 11:12 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-23 5:07 [PATCH] Add plugin interface to LD [0/4] Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions Dave Korn
2010-10-06 22:28 ` Richard Henderson
2010-10-07 21:55 ` Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols Dave Korn
2010-09-23 5:36 ` Ralf Wildenhues
2010-09-23 6:03 ` Dave Korn
2010-09-23 6:12 ` Dave Korn
2010-09-23 6:14 ` Ralf Wildenhues
2010-09-23 6:13 ` Ralf Wildenhues
2010-09-30 13:13 ` Dave Korn
2010-09-30 13:20 ` Dave Korn
2010-09-30 20:25 ` Ralf Wildenhues
2010-09-24 1:23 ` Dave Korn
2010-10-06 22:14 ` Richard Henderson
2010-10-07 21:42 ` Dave Korn
2010-10-09 3:04 ` Dave Korn
2010-10-09 17:59 ` Richard Henderson
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths Dave Korn
2010-09-24 0:35 ` Dave Korn
2010-10-06 22:40 ` Richard Henderson
2010-10-07 21:48 ` Dave Korn
2010-09-23 5:09 ` [PATCH] Add plugin interface to LD [1/4] Infrastructure Dave Korn
2010-09-23 5:41 ` Ralf Wildenhues
2010-09-23 6:01 ` Dave Korn
2010-10-04 4:35 ` Dave Korn
2010-10-06 20:57 ` Richard Henderson
2010-10-07 21:38 ` Dave Korn
2010-10-07 21:48 ` Richard Henderson
2010-10-12 8:32 ` Richard Sandiford
2010-10-12 11:12 ` Dave Korn
2010-09-23 16:18 ` [PATCH] Add plugin interface to LD [0/4] Richard Henderson
2010-09-24 0:30 ` [PATCH] Add plugin interface to LD [0/4] Support ELF symbol visibility Dave Korn
2010-09-24 0:38 ` [PATCH] Add plugin interface to LD [5/4] " Dave Korn
2010-10-06 22:42 ` Richard Henderson
2010-09-27 0:21 ` [PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface Dave Korn
2010-10-06 22:57 ` Richard Henderson
2010-10-07 21:52 ` Dave Korn
2010-10-02 15:52 ` [PING] Re: [PATCH] Add plugin interface to LD [0/4] Dave Korn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).