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
On Mon, Nov 19, 2012 at 12:06 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> 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.
Ah, good to know. Serves me right for using the git mirror then.
Thanks for the info.
Best,
-David
>
> 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