This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[rfa:] [patch} Save/restore bug for bfd machine field in elf_object_p


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]