[PATCH] Scan for Mach-O start address in LC_MAIN and properly check bfd_mach_o_scan_start_address's return value
Andrew Pinski
pinskia@gmail.com
Mon Nov 19 05:06:00 GMT 2012
On Sun, Nov 18, 2012 at 9:03 PM, David Albert <davidbalbert@gmail.com> wrote:
> On Sun, Nov 18, 2012 at 11:47 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> 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.
>
> Thanks, Joel. I'll do this in the morning. If this patch gets pulled
> into binutils, what's the process for getting it in GDB? Does the
> latest BFD just get merged into GDB every now and then or does it have
> to be shepherded through by someone?
Right now gdb and binutils share the same cvs repo so there is nothing
special to do.
Thanks,
Andrew
>
> Thanks,
> -David
>
>>
>>
>>>
>>> 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
More information about the Gdb-patches
mailing list