[Bug shlibs/13157] New: fsf gdb x86_64-apple-darwin cannot load "Mach-O fat file with 2 architectures" executables

[ehsan.akhgari at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13157

Bug #: 13157
Summary: fsf gdb x86_64-apple-darwin cannot load "Mach-O fat
file with 2 architectures" executables
Product: gdb
Version: HEAD
Status: NEW
Severity: normal
Priority: P2
Component: shlibs
AssignedTo: ***@sourceware.org
ReportedBy: ***@gmail.com
Classification: Unclassified


I grabbed the gdb trunk and built it using ./configure && make. It cannot load
Universal binaries:

ehsanakhgari:~/src/gdb/objdir (master) [02:13:23]$ file /bin/ls ./gdb/gdb
/bin/ls: Mach-O fat file with 2 architectures
./gdb/gdb: Mach-O 64-bit executable
ehsanakhgari:~/src/gdb/objdir (master) [02:16:48]$ ./gdb/gdb /bin/ls
GNU gdb (GDB) 7.3.50.20110906-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-apple-darwin10".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
"/bin/ls": not in executable format: File format not recognized
(gdb) quit
ehsanakhgari:~/src/gdb/objdir (master) [02:17:05]$ ./gdb/gdb ./gdb/gdb
GNU gdb (GDB) 7.3.50.20110906-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-apple-darwin10".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /Users/ehsanakhgari/src/gdb/objdir/gdb/gdb...
warning: can't find section '*UND*' in OSO file
/Users/ehsanakhgari/src/gdb/objdir/gdb/../libiberty/libiberty.a(safe-ctype.o)

warning: can't find section '*UND*' in OSO file
/Users/ehsanakhgari/src/gdb/objdir/gdb/libgdb.a(version.o)

warning: can't find section '*UND*' in OSO file
/Users/ehsanakhgari/src/gdb/objdir/gdb/libgdb.a(xml-builtin.o)
done.
(gdb) quit

[ehsan.akhgari at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13157

Ehsan Akhgari <ehsan.akhgari at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |josh at joshmatthews dot
| |net, jwatt at jwatt dot org

[jwatt at jwatt dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=13157

Jonathan Watt <jwatt at jwatt dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |fawzi at gmx dot ch

[dtrebbien at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Daniel Trebbien <dtrebbien at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |dtrebbien at gmail dot com

[malaperle at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Marc-Andre Laperle <malaperle at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |howarth at bromo dot med.uc.edu

--- Comment #1 from Marc-Andre Laperle <malaperle at gmail dot com> ---
*** Bug 10727 has been marked as a duplicate of this bug. ***

[jrmuizel at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Jeff Muizelaar <jrmuizel at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |jrmuizel at gmail dot com

--- Comment #2 from Jeff Muizelaar <jrmuizel at gmail dot com> ---
It looks like this was caused by bug 21787. That bug causes abfd->format to be
set to bfd_unknown which causes all subsequent archive matches to fail.

[jrmuizel at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #3 from Jeff Muizelaar <jrmuizel at gmail dot com> ---
Created attachment 11096
--> https://sourceware.org/bugzilla/attachment.cgi?id=11096&action=edit
Fix FAT binary support

None of the error cases in bfd_generic_archive_p set the format so this one
probably shouldn't either. Further setting it breaks format detection in
check_formats because SEND_FMT will use bfd_unknown on subsequent targets
instead of bfd_archive.

[jrmuizel at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Jeff Muizelaar <jrmuizel at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |tromey at sourceware dot org

[jrmuizel at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Jeff Muizelaar <jrmuizel at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #11096|Fix FAT binary support |Fix fat binary support
description| |

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
This patch didn't work for me with git master gdb.
My test case was "gdb /bin/sync", which according to "file"
is a Mach-O universal binary with 2 architectures.

Poking around a little, gdb calls bfd_check_format_matches
on the executable with bfd_object (see exec_file_attach).
This returns false.

Maybe the bfd_check_format function tables in mach-o-target.c
need to be updated to recognize a universal binary as
a bfd_object, or maybe gdb needs to have a more expansive
view of what's allowable here. I am not certain yet.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |noloader at gmail dot com

--- Comment #5 from Tom Tromey <tromey at sourceware dot org> ---
*** Bug 12870 has been marked as a duplicate of this bug. ***

[jrmuizel at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #6 from Jeff Muizelaar <jrmuizel at gmail dot com> ---
I was testing this against a non-universal binary that linked against universal
dylibs.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #7 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Jeff Muizelaar from comment #6)

Post by jrmuizel at gmail dot com
I was testing this against a non-universal binary that linked against
universal dylibs.

Ok, I see. It seems like there are two bugs then.

I found this in an updated version of the old apple fork.
https://github.com/argp/macgdb/commit/7cb63ac75766893087c004c634b3c8d205dce85c
Something like it might be needed, though that code can't land as-is.

Pedro dug this up:
https://cygwin.com/ml/binutils/2009-10/msg00201.html
So the library case is intended to be supported at least.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #8 from Tom Tromey <tromey at sourceware dot org> ---
This patch regresses bug#21787 as well so something will have to change.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #9 from Tom Tromey <tromey at sourceware dot org> ---
I think I found the problem in objdump and this BFD patch currently
seems ok.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #10 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Tom Tromey from comment #9)

Post by tromey at sourceware dot org
I think I found the problem in objdump and this BFD patch currently
seems ok.

Spoke too soon, objdump seems ok, but now this line from mach-o.c
looks fishy:

#define bfd_mach_o_close_and_cleanup _bfd_bool_bfd_true

... the issue being that closing an archive requires a bit of
extra cleanup, and this function isn't doing it.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at sourceware dot org |tromey at sourceware dot org

--- Comment #11 from Tom Tromey <tromey at sourceware dot org> ---
I have a patch I'll send soon.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #12 from Tom Tromey <tromey at sourceware dot org> ---
https://sourceware.org/ml/binutils/2018-06/msg00364.html

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #13 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=eac61af65bcd24a48633da375527eb3f36ab47ed

commit eac61af65bcd24a48633da375527eb3f36ab47ed
Author: Tom Tromey <***@tromey.com>
Date: Thu Jun 28 08:02:42 2018 -0600

Allow BFD to recognize macOS universal libraries

Bug #13157 is about a gdb regression, where previously it could handle
universal libraries, but now cannot.

gdb isn't working for me on macOS for other reasons, so I wrote this
small test program to show the problem:

#include <config.h>
#include <stdio.h>
#include <stdlib.h>
#include <bfd.h>

void
die (const char *what)
{
fprintf (stderr, "die: %s\n", what);
exit (1);
}

int
main (int argc, char **argv)
{
bfd *file = bfd_openr (argv[1], NULL);
if (file == NULL)
die ("couldn't open");

if (!bfd_check_format (file, bfd_archive))
die ("not an archive");

printf ("yay\n");

bfd_close (file);
return 0;
}

Then I built a simple universal binary. With git master BFD, I get:

$ ./doit ./universal-exe
die: not an archive

Jeff Muizelaar tracked this down to the BFD change for PR binutils/21787.
This patch changed bfd_generic_archive_p to sometimes reset the BFD's
"format" field.

However, simply changing bfd_generic_archive_p regressed the test case
in that bug.

Debugging PR binutils/21787 again, what I saw is that the mach-o
universal binary support acts like a bfd_archive but does not provide
a _close_and_cleanup function. However, if a BFD appears as an
archive member, it must always remove its own entry from its parent's
map. Otherwise, when the parent is destroyed, the already-destroyed
child BFD will be referenced. mach-o does not use the usual archive
member support, so simply using _bfd_archive_close_and_cleanup (as
other targets do) will not work.

This patch fixes the problem by introducing a new
_bfd_unlink_from_archive_parent function, then arranging for it to be
called in the mach-o case.

Ok?

bfd/ChangeLog
2018-07-02 Jeff Muizelaar <***@gmail.com>
Tom Tromey <***@tromey.com>

PR 13157
PR 21787
* mach-o.c (bfd_mach_o_fat_close_and_cleanup): New function.
(bfd_mach_o_close_and_cleanup): Redefine.
* archive.c (_bfd_unlink_from_archive_parent): New function,
extracted from..
(_bfd_archive_close_and_cleanup): ..here.
(bfd_generic_archive_p): Do not clear archive's format.
* libbfd-in.h (_bfd_unlink_from_archive_parent): Declare.
* libbfd.h: Regenerate.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

--- Comment #14 from Tom Tromey <tromey at sourceware dot org> ---
This should work ok for universal libraries, but as mentioned in
comment #4, it won't work for universal executables.

Also, it's worth noting that trying this in programs in /usr/bin
is likely to run into SIP, causing "run" to fail. So it's best
to just build your own universal executable to test.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #11096|0 |1
is obsolete| |

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13157

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target| |*-*-darwin*