From ace667e59aede65c400381f1cff704b61e8ccb0b Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Mon, 18 Jul 2016 21:00:00 +0100 Subject: [PATCH] ld: Restore file offset after a plugin fails to claim a file When using the plugin interface to claim an input file the claim method from (possible) many plugins can be called on an input file. If these claim methods read content from the input file then the file offset stored in the underlying file descriptor will change. As we share a file descriptor between the plugin interface (created with dup in ld/plugin.c:plugin_object_p) and the input bfd object, then any changes to the file offset in the file descriptor will effect the bfd object. Also, as the changes to the file offset did not originate from calls through the bfd interface, but instead came from the plugin directly, then the bfd will not be aware that the file offset has changed. This is a problem as the bfd library caches the file offset. If the plugin decides not to claim an input file then, currently, we leave the bfd in a state where the actual file offset is out of sync with the cached file offset. This problem came to light after a recent commit 7d0b9ebc1e0079271a7c7737b53bc026525eab64 (Don't include libbfd.h outside of bfd, part 6) however, I don't believe that commit actual introduces the bug, it just exposed the existing issue. This commit solves the problem by backing up and restoring the file offset for the file descriptor of the input file. The restore is only done if the plugin does not claim the input file, as it is in this case that the bfd library might be used again to try and identify the unclaimed file. ld/ChangeLog: * plugin.c (plugin_call_claim_file): Restore the file offset after an unsuccessful attempt to claim a file. * testplug.c (bytes_to_read_before_claim): New global. (record_read_length): New function, sets new global bytes_to_read_before_claim. (parse_option): Handle 'read:' option. (onclaim_file): Read file content before checking for claim. * testsuite/ld-plugin/plugin-30.d: New file. * testsuite/ld-plugin/plugin.exp: Add new test. --- ld/ChangeLog | 12 +++++++++ ld/plugin.c | 5 ++++ ld/testplug.c | 41 ++++++++++++++++++++++++++++++ ld/testsuite/ld-plugin/plugin-30.d | 26 +++++++++++++++++++ ld/testsuite/ld-plugin/plugin.exp | 7 ++++- 5 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 ld/testsuite/ld-plugin/plugin-30.d diff --git a/ld/ChangeLog b/ld/ChangeLog index 758d7adeeb7..35a2b80d57e 100644 --- a/ld/ChangeLog +++ b/ld/ChangeLog @@ -1,3 +1,15 @@ +2016-07-19 Andrew Burgess + + * plugin.c (plugin_call_claim_file): Restore the file offset after + an unsuccessful attempt to claim a file. + * testplug.c (bytes_to_read_before_claim): New global. + (record_read_length): New function, sets new global + bytes_to_read_before_claim. + (parse_option): Handle 'read:' option. + (onclaim_file): Read file content before checking for claim. + * testsuite/ld-plugin/plugin-30.d: New file. + * testsuite/ld-plugin/plugin.exp: Add new test. + 2016-07-16 Alan Modra * plugin.c: Don't include libbfd.h. Include plugin-api.h diff --git a/ld/plugin.c b/ld/plugin.c index ffa0dd39015..36094dd7c8c 100644 --- a/ld/plugin.c +++ b/ld/plugin.c @@ -1049,9 +1049,14 @@ plugin_call_claim_file (const struct ld_plugin_input_file *file, int *claimed) { if (curplug->claim_file_handler) { + off_t cur_offset; enum ld_plugin_status rv; + called_plugin = curplug; + cur_offset = lseek (file->fd, 0, SEEK_CUR); rv = (*curplug->claim_file_handler) (file, claimed); + if (!*claimed) + lseek (file->fd, cur_offset, SEEK_SET); called_plugin = NULL; if (rv != LDPS_OK) set_plugin_error (curplug->name); diff --git a/ld/testplug.c b/ld/testplug.c index cec947c51c4..684fc413839 100644 --- a/ld/testplug.c +++ b/ld/testplug.c @@ -24,6 +24,8 @@ /* For ARRAY_SIZE macro only - we don't link the library itself. */ #include "libiberty.h" +#include /* For isdigit. */ + extern enum ld_plugin_status onload (struct ld_plugin_tv *tv); static enum ld_plugin_status onclaim_file (const struct ld_plugin_input_file *file, int *claimed); @@ -139,6 +141,10 @@ 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; +/* Number of bytes read in claim file before deciding if the file can be + claimed. */ +static int bytes_to_read_before_claim = 0; + /* Add a new claimfile on the end of the chain. */ static enum ld_plugin_status record_claim_file (const char *file) @@ -159,6 +165,25 @@ record_claim_file (const char *file) return LDPS_OK; } +/* How many bytes to read before claiming (or not) an input file. */ +static enum ld_plugin_status +record_read_length (const char *length) +{ + const char *tmp; + + tmp = length; + while (*tmp != '\0' && isdigit (*tmp)) + ++tmp; + if (*tmp != '\0' || *length == '\0') + { + fprintf (stderr, "APB: Bad length string: %s\n", tmp); + return LDPS_ERR; + } + + bytes_to_read_before_claim = atoi (length); + 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) @@ -325,6 +350,8 @@ parse_option (const char *opt) return set_register_hook (opt + 10, FALSE); else if (!strncmp ("claim:", opt, 6)) return record_claim_file (opt + 6); + else if (!strncmp ("read:", opt, 5)) + return record_read_length (opt + 5); else if (!strncmp ("sym:", opt, 4)) return record_claimed_file_symbol (opt + 4); else if (!strncmp ("add:", opt, 4)) @@ -527,6 +554,20 @@ onload (struct ld_plugin_tv *tv) static enum ld_plugin_status onclaim_file (const struct ld_plugin_input_file *file, int *claimed) { + /* Possible read of some bytes out of the input file into a buffer. This + simulates a plugin that reads some file content in order to decide if + the file should be claimed or not. */ + if (bytes_to_read_before_claim > 0) + { + char *buffer = malloc (bytes_to_read_before_claim); + + if (buffer == NULL) + return LDPS_ERR; + if (read (file->fd, buffer, bytes_to_read_before_claim) < 0) + return LDPS_ERR; + free (buffer); + } + /* Let's see if we want to claim this file. */ claim_file_t *claimfile = claimfiles_list; while (claimfile) diff --git a/ld/testsuite/ld-plugin/plugin-30.d b/ld/testsuite/ld-plugin/plugin-30.d new file mode 100644 index 00000000000..eb9d4244786 --- /dev/null +++ b/ld/testsuite/ld-plugin/plugin-30.d @@ -0,0 +1,26 @@ +Hello from testplugin. +.*: LDPT_MESSAGE func@0x.* +.*: LDPT_API_VERSION value 0x1 \(1\) +.*: LDPT_GNU_LD_VERSION value 0x.* +.*: LDPT_LINKER_OUTPUT value 0x1 \(1\) +.*: LDPT_OUTPUT_NAME 'tmpdir/main.x' +.*: LDPT_REGISTER_CLAIM_FILE_HOOK func@0x.* +.*: LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK func@0x.* +.*: LDPT_REGISTER_CLEANUP_HOOK func@0x.* +.*: LDPT_ADD_SYMBOLS func@0x.* +.*: LDPT_GET_INPUT_FILE func@0x.* +.*: LDPT_GET_VIEW func@0x.* +.*: LDPT_RELEASE_INPUT_FILE func@0x.* +.*: LDPT_GET_SYMBOLS func@0x.* +.*: LDPT_GET_SYMBOLS_V2 func@0x.* +.*: LDPT_ADD_INPUT_FILE func@0x.* +.*: LDPT_ADD_INPUT_LIBRARY func@0x.* +.*: LDPT_SET_EXTRA_LIBRARY_PATH func@0x.* +.*: LDPT_OPTION 'registerclaimfile' +.*: LDPT_OPTION 'read:8' +.*: LDPT_NULL value 0x0 \(0\) +#... +hook called: claim_file tmpdir/main.o \[@0/.* not claimed +hook called: claim_file tmpdir/func.o \[@0/.* not claimed +hook called: claim_file tmpdir/text.o \[@0/.* not claimed +hook called: claim_file tmpdir/libempty.a \[@.* not claimed diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp index 4c9116b0246..a6946c47c2e 100644 --- a/ld/testsuite/ld-plugin/plugin.exp +++ b/ld/testsuite/ld-plugin/plugin.exp @@ -236,6 +236,10 @@ set plugin_lib_tests [list \ -plugin-opt sym:${_}text::0:0:0 \ -plugin-opt add:tmpdir/text.o \ $testsrcfiles_notext -Ltmpdir -ltext $libs" "" "" "" {{ld plugin-19.d}} "main.x" ] \ + [list "plugin with empty archive" \ + "-plugin $plugin_path $regclm \ + -plugin-opt read:8 \ + $testobjfiles tmpdir/libempty.a $libs" "" "" "" {{ld plugin-30.d}} "main.x" ] \ ] set plugin_extra_elf_tests [list \ @@ -289,7 +293,8 @@ if { [is_elf_format] \ run_ld_link_tests $plugin_extra_elf_tests } -if ![ar_simple_create $ar "" "tmpdir/libtext.a" "tmpdir/text.o"] { +if {![ar_simple_create $ar "" "tmpdir/libtext.a" "tmpdir/text.o"] || \ + ![ar_simple_create $ar "" "tmpdir/libempty.a" ""]} { foreach testitem $plugin_lib_tests { unresolved [lindex $testitem 0] } -- 2.30.2