From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31241 invoked by alias); 19 Nov 2012 04:48:05 -0000 Received: (qmail 31177 invoked by uid 22791); 19 Nov 2012 04:48:02 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Nov 2012 04:47:56 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 0E48A2E081; Sun, 18 Nov 2012 23:47:55 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id f9-g6H50EVhJ; Sun, 18 Nov 2012 23:47:54 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id B8E152E072; Sun, 18 Nov 2012 23:47:54 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 05D17C2D71; Sun, 18 Nov 2012 20:47:50 -0800 (PST) Date: Mon, 19 Nov 2012 04:48:00 -0000 From: Joel Brobecker To: David Albert Cc: gdb-patches Subject: Re: [PATCH] Scan for Mach-O start address in LC_MAIN and properly check bfd_mach_o_scan_start_address's return value Message-ID: <20121119044750.GB9964@adacore.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-11/txt/msg00488.txt.bz2 On Sun, Nov 18, 2012 at 11:13:21PM -0500, David Albert wrote: > Last week there was a change comitted to GDB that added support for > the new Mach-O load commands in OS X 10.8. I have two related changes > that I've included in a patch below. > > 1) On December 7th of last year, bfd_mach_o_scan_start_address was > changed to return a bfd_boolean, but no change was made to the code > that checks its return value in bfd_mach_o_scan. This means that > bfd_mach_o_scan always assumes that bfd_mach_o_scan_start_address > succeeds. This patch fixes that. > > 2) The start address can now be found in LC_MAIN in addition to > LC_THREAD and LC_UNIXTHREAD. I've updated > bfd_mach_o_scan_start_address to reflect this. I moved the call to > bfd_mach_o_flatten_sections to before bfd_mach_o_scan_start_address, > because the code that gets the start address from LC_MAIN relies on > nsects being set. > > I've never submitted a patch to GDB before, so please let me know if > there are things I can fix. I also have a few notes: > > - I used the latest version of Apple's GDB 6.3 fork (gdb-1822) as a > reference for some of my research. The changes I made are quite small > (most of the patch consists of placing a block of existing code inside > an if statement), but parts of the patch are similar to what I found > in Apple's fork. The code is simple enough that there are not really > many ways to solve the problem, but I want to make sure there aren't > issues with copyright or licensing. > > - The one thing I'm not sure of is line 4047 which reads `if > (mdata->nsects > 1)`. This is the same check found in Apple's GDB > fork, however it's unclear why we'd need two or more sections, given > that we only use the first section to calculate the start address. I > thought about changing it, but I wanted to get the opinion of someone > who knows more about Mach-O than I do. > > - I didn't run the test suite because it wasn't clear what parts of > the suite I should run. I tested it manually and didn't find any > regressions. Hi David, It's nice to see people interested in hacking on Darwin. Changes in bfd/ should be sent to binutils, however. You can also Cc: Tristan Gingold, to make sure you catch his attention. > > Best, > -David > > diff --git a/bfd/ChangeLog b/bfd/ChangeLog > index c25ceb9..4e5ff3a 100644 > --- a/bfd/ChangeLog > +++ b/bfd/ChangeLog > @@ -1,3 +1,10 @@ > +2012-11-18 David Albert > + > + * mach-o.c (bfd_mach_o_scan): Properly check return value of > + bfd_mach_o_scan_start_address. > + (bfd_mach_o_scan_start_address): Also search for start address in > + BFD_MACH_O_LC_MAIN. > + > 2012-11-16 Joey Ye > > * elf32-arm.c (elf32_arm_final_link_relocate, > diff --git a/bfd/mach-o.c b/bfd/mach-o.c > index e44cf6d..4546540 100644 > --- a/bfd/mach-o.c > +++ b/bfd/mach-o.c > @@ -3970,66 +3970,89 @@ bfd_mach_o_scan_start_address (bfd *abfd) > { > bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (abfd); > bfd_mach_o_thread_command *cmd = NULL; > + bfd_mach_o_main_command *main_cmd = NULL; > unsigned long i; > > for (i = 0; i < mdata->header.ncmds; i++) > - if ((mdata->commands[i].type == BFD_MACH_O_LC_THREAD) || > - (mdata->commands[i].type == BFD_MACH_O_LC_UNIXTHREAD)) > - { > - cmd = &mdata->commands[i].command.thread; > - break; > - } > - > - if (cmd == NULL) > - return FALSE; > + { > + if ((mdata->commands[i].type == BFD_MACH_O_LC_THREAD) || > + (mdata->commands[i].type == BFD_MACH_O_LC_UNIXTHREAD)) > + { > + cmd = &mdata->commands[i].command.thread; > + break; > + } > + else if (mdata->commands[i].type == BFD_MACH_O_LC_MAIN) > + { > + main_cmd = &mdata->commands[i].command.main; > + break; > + } > + } > > - /* FIXME: create a subtarget hook ? */ > - for (i = 0; i < cmd->nflavours; i++) > + if (cmd) > { > - if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_I386) > - && (cmd->flavours[i].flavour > - == (unsigned long) BFD_MACH_O_x86_THREAD_STATE32)) > - { > - unsigned char buf[4]; > + /* FIXME: create a subtarget hook ? */ > + for (i = 0; i < cmd->nflavours; i++) > + { > + if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_I386) > + && (cmd->flavours[i].flavour > + == (unsigned long) BFD_MACH_O_x86_THREAD_STATE32)) > + { > + unsigned char buf[4]; > > - if (bfd_seek (abfd, cmd->flavours[i].offset + 40, SEEK_SET) != 0 > - || bfd_bread (buf, 4, abfd) != 4) > - return FALSE; > + if (bfd_seek (abfd, cmd->flavours[i].offset + 40, SEEK_SET) != 0 > + || bfd_bread (buf, 4, abfd) != 4) > + return FALSE; > > - abfd->start_address = bfd_h_get_32 (abfd, buf); > - } > - else if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_POWERPC) > - && (cmd->flavours[i].flavour == BFD_MACH_O_PPC_THREAD_STATE)) > - { > - unsigned char buf[4]; > + abfd->start_address = bfd_h_get_32 (abfd, buf); > + } > + else if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_POWERPC) > + && (cmd->flavours[i].flavour == > BFD_MACH_O_PPC_THREAD_STATE)) > + { > + unsigned char buf[4]; > > - if (bfd_seek (abfd, cmd->flavours[i].offset + 0, SEEK_SET) != 0 > - || bfd_bread (buf, 4, abfd) != 4) > - return FALSE; > + if (bfd_seek (abfd, cmd->flavours[i].offset + 0, SEEK_SET) != 0 > + || bfd_bread (buf, 4, abfd) != 4) > + return FALSE; > > - abfd->start_address = bfd_h_get_32 (abfd, buf); > - } > - else if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_POWERPC_64) > - && (cmd->flavours[i].flavour == BFD_MACH_O_PPC_THREAD_STATE64)) > - { > - unsigned char buf[8]; > + abfd->start_address = bfd_h_get_32 (abfd, buf); > + } > + else if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_POWERPC_64) > + && (cmd->flavours[i].flavour == > BFD_MACH_O_PPC_THREAD_STATE64)) > + { > + unsigned char buf[8]; > > - if (bfd_seek (abfd, cmd->flavours[i].offset + 0, SEEK_SET) != 0 > - || bfd_bread (buf, 8, abfd) != 8) > - return FALSE; > + if (bfd_seek (abfd, cmd->flavours[i].offset + 0, SEEK_SET) != 0 > + || bfd_bread (buf, 8, abfd) != 8) > + return FALSE; > > - abfd->start_address = bfd_h_get_64 (abfd, buf); > - } > - else if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_X86_64) > - && (cmd->flavours[i].flavour == BFD_MACH_O_x86_THREAD_STATE64)) > - { > - unsigned char buf[8]; > + abfd->start_address = bfd_h_get_64 (abfd, buf); > + } > + else if ((mdata->header.cputype == BFD_MACH_O_CPU_TYPE_X86_64) > + && (cmd->flavours[i].flavour == > BFD_MACH_O_x86_THREAD_STATE64)) > + { > + unsigned char buf[8]; > > - if (bfd_seek (abfd, cmd->flavours[i].offset + (16 * 8), > SEEK_SET) != 0 > - || bfd_bread (buf, 8, abfd) != 8) > - return FALSE; > + if (bfd_seek (abfd, cmd->flavours[i].offset + (16 * 8), > SEEK_SET) != 0 > + || bfd_bread (buf, 8, abfd) != 8) > + return FALSE; > > - abfd->start_address = bfd_h_get_64 (abfd, buf); > + abfd->start_address = bfd_h_get_64 (abfd, buf); > + } > + } > + } > + else if (main_cmd) > + { > + bfd_vma text_address = 0; > + > + if (mdata->nsects > 1) > + { > + bfd_mach_o_section *text_sect = mdata->sections[0]; > + if (text_sect) > + { > + text_address = text_sect->addr - text_sect->offset; > + abfd->start_address = main_cmd->entryoff + text_address; > + return TRUE; > + } > } > } > > @@ -4121,10 +4144,11 @@ bfd_mach_o_scan (bfd *abfd, > } > } > > - if (bfd_mach_o_scan_start_address (abfd) < 0) > + bfd_mach_o_flatten_sections (abfd); > + > + if (!bfd_mach_o_scan_start_address (abfd)) > return FALSE; > > - bfd_mach_o_flatten_sections (abfd); > return TRUE; > } -- Joel