This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Scan for Mach-O start address in LC_MAIN and properly check bfd_mach_o_scan_start_address's return value
- From: Joel Brobecker <brobecker at adacore dot com>
- To: David Albert <davidbalbert at gmail dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Sun, 18 Nov 2012 20:47:50 -0800
- Subject: Re: [PATCH] Scan for Mach-O start address in LC_MAIN and properly check bfd_mach_o_scan_start_address's return value
- References: <CAKYxTUadUr9GUqMHAA3JdASDfwuEFzS9txT+aXQSQjro+iwx8w@mail.gmail.com>
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 <davidbalbert@gmail.com>
> +
> + * 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 <joey.ye@arm.com>
>
> * 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