This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
[rfa:] [patch} Save/restore bug for bfd machine field in elf_object_p
- To: binutils at sources dot redhat dot com
- Subject: [rfa:] [patch} Save/restore bug for bfd machine field in elf_object_p
- From: Hans-Peter Nilsson <hp at bitrange dot com>
- Date: Wed, 20 Dec 2000 20:07:07 -0500 (EST)
Back in September, I had problems with elf_object_p munging the sections
fields in the bfd, when iterating over a set of (two) ELF targets where
only the return value from elf_backend_object_p made a difference (which
is supposed to be supported). I got an OK from Alan Modra for a patch
then, with a note of suspicion that saving and restoring the sections
fields was not enough. It wasn't.
For another target, I now see problems with the machine field. The mach
field is set-default to 0 in elf_object_p, but not restored on failure.
Since bfd_check_format_matches iterates over "live" bfd:s, not being
satisfied after finding a non-default match, this means that what was set
right in a call with a match, will later be reset in a call that signals
non-match.
Although Alan suggested that a proper fix might be to save the whole BFD
and restore on failure, that does not seem to work right. I tried a
shallow-copy (save whole structure, restore whole structure on failure),
not knowing whether there is a deep-copy. This failed as noted in the
comment: the fseek that was supposed to reset the file pointer between
calls did not happen, because the file pointer stored in the BFD was still
0 after the call. I take that as an indicator that trying to save and
restore bfd state by shallow-copy is a bad idea. ;-) I suggest to fix each
case as it is noticed: elf_object_p should only restore the specific
things that it knowingly changes. This means saving and restoring the
mach field, like in the patch below. (The names are previous_mach and
previous_arch, not preserved_*, because we're I believe we're not supposed
to know how those values are used, only that using them when calling
bfd_default_set_arch_mach makes the desrired state of state.)
Ok to commit?
* elfcode.h (elf_object_p): Also restore the bfd mach field on
error, by calling bfd_default_set_arch_mach with incoming
values.
Index: elfcode.h
===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.14
diff -p -c -r1.14 elfcode.h
*** elfcode.h 2000/09/27 07:52:24 1.14
--- elfcode.h 2000/12/21 00:31:14
*************** elf_object_p (abfd)
*** 502,507 ****
--- 502,509 ----
struct elf_obj_tdata *preserved_tdata = elf_tdata (abfd);
struct sec *preserved_sections = abfd->sections;
unsigned int preserved_section_count = abfd->section_count;
+ enum bfd_architecture previous_arch = bfd_get_arch (abfd);
+ unsigned long previous_mach = bfd_get_mach (abfd);
struct elf_obj_tdata *new_tdata = NULL;
asection *s;
*************** elf_object_p (abfd)
*** 742,747 ****
--- 744,759 ----
return (abfd->xvec);
got_wrong_format_error:
+ /* There is way too much undoing of half-known state here. The caller,
+ bfd_check_format_matches, really shouldn't iterate on live bfd's to
+ check match/no-match like it does. We have to rely on that a call to
+ bfd_default_set_arch_mach with the previously known mach, undoes what
+ was done by the first bfd_default_set_arch_mach (with mach 0) here.
+ For this to work, only elf-data and the mach may be changed by the
+ target-specific elf_backend_object_p function. Note that saving the
+ whole bfd here and restoring it would be even worse; the first thing
+ you notice is that the cached bfd file position gets out of sync. */
+ bfd_default_set_arch_mach (abfd, previous_arch, previous_mach);
bfd_set_error (bfd_error_wrong_format);
got_no_match:
if (new_tdata != NULL
brgds, H-P