Fixes for Mac OS X/x86 building/running

Fixes for Mac OS X/x86 building/running

[Jonas Maebe]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

Hello,

Archer (archer branch) is currently broken on Mac OS X/x86. The  
attached (minor) patches fix this:

* darwin-nat.c.patch fixes compilation errors due to some missing  
"struct target_ops *" parameters. Probably caused by:

commit be44ac148163ced0dde7b8ff23c8f1cc49efa106
Author: Pedro Alves <pedro@codesourcery.com>
Date:   Mon Feb 23 00:03:47 2009 +0000

* machoread.c.patch fixes a crash caused by a missing function pointer  
field (sym_read_psymbols) initializer in the middle of macho's struct  
sym_fns (so the subsequent function pointer initializers were all  
shifted). I've simply copied the elfread.c implementation of  
read_psyms (because it appeared to be generic) and it seems to work  
fine (if it's called at all). I guess this needs to be solved in a  
better way though (maybe a NULL pointer would be enough). I'm not sure  
why I didn't see this crash when I built gdb last time (on January 17  
from the archer-jankratochvil-vla banch), as this field was already  
added on 2008-09-09 (by Tom Tromey) according to git.

With these two patches applied, the Mac OS X build and works fine  
again on i386 (at least in my cursory testing).


Jonas

[-- Attachment #2: machoread.c.patch --]
[-- Type: application/octet-stream, Size: 1085 bytes --]

diff --git a/gdb/machoread.c b/gdb/machoread.c
index d8d3bd2..bed7f86 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -675,11 +675,26 @@ macho_symfile_offsets (struct objfile *objfile,
     }
 }
 
+static void
+read_psyms (struct objfile *objfile)
+{
+  if (dwarf2_has_info (objfile))
+    {
+      /* DWARF 2 sections */
+      dwarf2_build_psymtabs (objfile, 0);
+    }
+
+  /* FIXME: kettenis/20030504: This still needs to be integrated with
+     dwarf2read.c in a better way.  */
+  dwarf2_build_frame_info (objfile);
+}
+
 static struct sym_fns macho_sym_fns = {
   bfd_target_mach_o_flavour,
 
   macho_new_init,               /* sym_new_init: init anything gbl to entire symtab */
   macho_symfile_init,           /* sym_init: read initial info, setup for sym_read() */
+  read_psyms,                   /* sym_read_psymbols */
   macho_symfile_read,           /* sym_read: read a symbol file into symtab */
   macho_symfile_finish,         /* sym_finish: finished with file, cleanup */
   macho_symfile_offsets,        /* sym_offsets:  xlate external to internal form */

[-- Attachment #3: darwin-nat.c.patch --]
[-- Type: application/octet-stream, Size: 1897 bytes --]

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 15a4b55..cabd4e2 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -90,7 +90,7 @@ static void darwin_mourn_inferior (struct target_ops *ops);
 
 static int darwin_lookup_task (char *args, task_t * ptask, int *ppid);
 
-static void darwin_kill_inferior (void);
+static void darwin_kill_inferior (struct target_ops *ops);
 
 static void darwin_ptrace_me (void);
 
@@ -668,7 +668,7 @@ darwin_mourn_inferior (struct target_ops *ops)
 }
 
 static void
-darwin_stop_inferior (darwin_inferior *inf)
+darwin_stop_inferior (struct target_ops *ops, darwin_inferior *inf)
 {
   struct target_waitstatus wstatus;
   ptid_t ptid;
@@ -688,7 +688,7 @@ darwin_stop_inferior (darwin_inferior *inf)
   if (res != 0)
     warning (_("cannot kill: %s\n"), strerror (errno));
 
-  ptid = darwin_wait (inferior_ptid, &wstatus);
+  ptid = darwin_wait (ops, inferior_ptid, &wstatus);
   gdb_assert (wstatus.kind = TARGET_WAITKIND_STOPPED);
 }
 
@@ -706,7 +706,7 @@ darwin_kill_inferior (struct target_ops *ops)
   if (ptid_equal (inferior_ptid, null_ptid))
     return;
 
-  darwin_stop_inferior (darwin_inf);
+  darwin_stop_inferior (ops, darwin_inf);
 
   res = PTRACE (PT_KILL, darwin_inf->pid, 0, 0);
   gdb_assert (res == 0);
@@ -720,7 +720,7 @@ darwin_kill_inferior (struct target_ops *ops)
   kret = task_resume (darwin_inf->task);
   MACH_CHECK_ERROR (kret);
 
-  ptid = darwin_wait (inferior_ptid, &wstatus);
+  ptid = darwin_wait (ops, inferior_ptid, &wstatus);
 
   /* This double wait seems required...  */
   res = waitpid (darwin_inf->pid, &status, 0);
@@ -1011,7 +1011,7 @@ darwin_detach (struct target_ops *ops, char *args, int from_tty)
       gdb_flush (gdb_stdout);
     }
 
-  darwin_stop_inferior (darwin_inf);
+  darwin_stop_inferior (ops, darwin_inf);
 
   kret = darwin_restore_exception_ports (darwin_inf);
   MACH_CHECK_ERROR (kret);

Re: Fixes for Mac OS X/x86 building/running

[Jonas Maebe]

On 11 Mar 2009, at 20:41, Jonas Maebe wrote:

> * machoread.c.patch fixes a crash caused by a missing function  
> pointer field (sym_read_psymbols) initializer in the middle of  
> macho's struct sym_fns (so the subsequent function pointer  
> initializers were all shifted). I've simply copied the elfread.c  
> implementation of read_psyms (because it appeared to be generic) and  
> it seems to work fine (if it's called at all). I guess this needs to  
> be solved in a better way though (maybe a NULL pointer would be  
> enough). I'm not sure why I didn't see this crash when I built gdb  
> last time (on January 17 from the archer-jankratochvil-vla banch),  
> as this field was already added on 2008-09-09 (by Tom Tromey)  
> according to git.
>
> With these two patches applied, the Mac OS X build and works fine  
> again on i386 (at least in my cursory testing).

Less cursory testing shows that it doesn't work fine. The problem is  
that the addresses for variables are read wrongly from the DWARF2  
sections (code addresses are correct). I guess it's related to this  
comment in machoread.c:

   /* Do not try to read .eh_frame/.debug_frame as they are not  
relocated
      and dwarf2_build_frame_info cannot deal with unrelocated  
sections.  */

in combination with the new sym_read_psymbols()/read_psyms() call  
(which also calls dwarf2_build_frame_info). Commenting it out  
everything in the read_psyms() function I added doesn't solve it  
however. Anyway, I'm afraid that's for someone else to solve...


Jonas

Re: Fixes for Mac OS X/x86 building/running

[Joel Brobecker]

> Less cursory testing shows that it doesn't work fine. The problem is  
> that[...]

I recommend that you discuss this issue on gdb@ rather than the Archer
list, because the port originates from GDB, not Archer: I think you have
more chances of reaching some developers that know a little more about
MacOS. The author of the MacOS port is Tristan Gingold, and I'm not sure
he's either on gdb@ or archer@, so I'd recommend CC'ing him in your
report (gingold /AT/ adacore.com). The reviewer was Stan Shebs, who used
to work at Apple...

PS: I'll take a look at your first patch in a moment, and I expect to
be able to approve it for inclusion in the FSF tree.

-- 
Joel

Re: Fixes for Mac OS X/x86 building/running

[Jonas Maebe]

On 12 Mar 2009, at 19:40, Joel Brobecker wrote:

>> Less cursory testing shows that it doesn't work fine. The problem is
>> that[...]
>
> I recommend that you discuss this issue on gdb@ rather than the Archer
> list, because the port originates from GDB, not Archer:

I checked my GDB cvs checkout and could not find any trace of gdb/ 
machoread.c nor of gdb/darwin-nat.c. That's why I thought it was  
Archer-specific. But now I do see that it's mentioned in the Changelog  
file. Apparently, because I started my cvs checkout from a tbz source  
snapshot, I had some kind of sticky tag. I've now done a "cvs up -A",  
and now I have the file. Sorry for the trouble.

> I think you have
> more chances of reaching some developers that know a little more about
> MacOS. The author of the MacOS port is Tristan Gingold, and I'm not  
> sure
> he's either on gdb@ or archer@, so I'd recommend CC'ing him in your
> report (gingold /AT/ adacore.com).

Thanks, I'll keep that in mind.

> The reviewer was Stan Shebs, who used
> to work at Apple...

Yes, I remember him. He used to their main GCC guy, if I remember  
correctly (or at least the most vocal one :)

> PS: I'll take a look at your first patch in a moment, and I expect to
> be able to approve it for inclusion in the FSF tree.

Thanks!


Jonas

Re: Fixes for Mac OS X/x86 building/running

[Tom Tromey]

>>>>> "Jonas" == Jonas Maebe <jonas.maebe@elis.ugent.be> writes:

Jonas> Archer (archer branch) is currently broken on Mac OS X/x86. The
Jonas> attached (minor) patches fix this:

Thanks.

Do you have a gdb copyright assignment in place?
For non-trivial patches we have to get this... I can get you started
on the process if you aren't already set up.

Jonas> * darwin-nat.c.patch fixes compilation errors due to some missing
Jonas> "struct target_ops *" parameters. Probably caused by:

This patch should go to gdb-patches, like Joel said.

Jonas> * machoread.c.patch fixes a crash caused by a missing function pointer
Jonas> field (sym_read_psymbols) initializer in the middle of macho's struct
Jonas> sym_fns (so the subsequent function pointer initializers were all
Jonas> shifted). I've simply copied the elfread.c implementation of
Jonas> read_psyms (because it appeared to be generic) and it seems to work
Jonas> fine (if it's called at all).

I've appended the corresponding change from elfread.c.
You probably want something a bit more like this.  The delayed symfile
stuff only works if you also arrange to create the quick addrmap.

Or, you can just set the new field to NULL.  All this means is that
you won't get lazy loading of debug info.

Tom

diff --git a/gdb/elfread.c b/gdb/elfread.c
index ff220a2..13158f4 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -727,10 +727,18 @@ elf_symfile_read (struct objfile *objfile, int mainline)
 				str_sect->filepos,
 				bfd_section_size (abfd, str_sect));
     }
+
+  if (dwarf2_has_info (objfile))
+    dwarf2_create_quick_addrmap (objfile);
+}
+
+static void
+read_psyms (struct objfile *objfile)
+{
   if (dwarf2_has_info (objfile))
     {
       /* DWARF 2 sections */
-      dwarf2_build_psymtabs (objfile, mainline);
+      dwarf2_build_psymtabs (objfile, 0);
     }
 
   /* FIXME: kettenis/20030504: This still needs to be integrated with

Re: Fixes for Mac OS X/x86 building/running

[Jonas Maebe]

On 12 Mar 2009, at 21:54, Tom Tromey wrote:

>>>>>> "Jonas" == Jonas Maebe <jonas.maebe@elis.ugent.be> writes:
>
> Do you have a gdb copyright assignment in place?
> For non-trivial patches we have to get this... I can get you started
> on the process if you aren't already set up.

No, I don't have one. I never needed it because until now I've only  
written fairly small patches to FSF source code. However, it indeed  
might not be a bad idea to finally go through with it, because I may  
start getting more involved in brushing up GDB's support for Pascal.  
So yes, please.

Does starting the process entail anything besides filling out http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future 
  and mailing it in?

> Jonas> * machoread.c.patch fixes a crash caused by a missing  
> function pointer
> Jonas> field (sym_read_psymbols) initializer in the middle of  
> macho's struct
> Jonas> sym_fns (so the subsequent function pointer initializers were  
> all
> Jonas> shifted). I've simply copied the elfread.c implementation of
> Jonas> read_psyms (because it appeared to be generic) and it seems  
> to work
> Jonas> fine (if it's called at all).
>
> I've appended the corresponding change from elfread.c.
> You probably want something a bit more like this.  The delayed symfile
> stuff only works if you also arrange to create the quick addrmap.

Thanks. I've now adjusted the machoread.c code in a similar way, but  
there's no change in the behaviour. I'll take it to the main gdb  
mailing list, with a CC to Tristan Gingold.

> Or, you can just set the new field to NULL.  All this means is that
> you won't get lazy loading of debug info.

That causes a crash when symfile.c:897 calls this function  
unconditionally:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x00000000 in ?? ()
(gdb) bt
#0  0x00000000 in ?? ()
#1  0x000b865b in syms_from_objfile (objfile=0x5f7000, addrs=0x63b810,  
offsets=0x0, num_offsets=0, mainline=1, verbo=0) at symfile.c:897
#2  0x000b897b in symbol_file_add_with_addrs_or_offsets (abfd=0x0,  
from_tty=0, addrs=0x0, offsets=0x0, num_offsets=0, mainline=1,  
flags=0) at symfile.c:1013
#3  0x000b9996 in symbol_file_add_main_1 (args=<value temporarily  
unavailable, due to optimizations>, from_tty=<value temporarily  
unavailable, due to optimizations>, flags=<value temporarily  
unavailable, due to optimizations>) at symfile.c:1156
#4  0x000d8be9 in catch_command_errors (command=0xb9af0  
<symbol_file_add_main>, arg=0xbffff3e9 "dynarr", from_tty=0,  
mask=2692864) at exceptions.c:531
#5  0x000da753 in captured_main (data=0xbffff2b0) at .././gdb/main.c:878
#6  0x000d8b42 in catch_errors (func=0xd9c00 <captured_main>,  
func_args=0xbffff2b0, errstring=0x246eec "", mask=2692864) at  
exceptions.c:516
#7  0x000db28f in gdb_main (args=0x8534a8) at .././gdb/main.c:998
#8  0x00001dc4 in main (argc=2692864, argv=0x291700) at gdb.c:33


Jonas

Re: [Archer] Re: Fixes for Mac OS X/x86 building/running

[Joel Brobecker]

> No, I don't have one. I never needed it because until now I've only  
> written fairly small patches to FSF source code. However, it indeed  
> might not be a bad idea to finally go through with it, because I may  
> start getting more involved in brushing up GDB's support for Pascal. So 
> yes, please.

Also, the guidelines recommend that if the number of changes start
adding up, it is best to have a copyright assignment.

> Does starting the process entail anything besides filling out 
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future 
>  and mailing it in?

This should be enough to get you started.

-- 
Joel

Re: Fixes for Mac OS X/x86 building/running

[Tom Tromey]

>>>>> "Jonas" == Jonas Maebe <jonas.maebe@elis.ugent.be> writes:

Tom> I've appended the corresponding change from elfread.c.
Tom> You probably want something a bit more like this.  The delayed symfile
Tom> stuff only works if you also arrange to create the quick addrmap.

Jonas> Thanks. I've now adjusted the machoread.c code in a similar way, but
Jonas> there's no change in the behaviour. I'll take it to the main gdb
Jonas> mailing list, with a CC to Tristan Gingold.

Just FYI, the machoread.c patch won't apply upstream, because we
haven't submitted the delayed partial symbol reading patch yet.

Tom> Or, you can just set the new field to NULL.  All this means is that
Tom> you won't get lazy loading of debug info.

Jonas> That causes a crash when symfile.c:897 calls this function
Jonas> unconditionally:

I suspect you must have added the NULL in the wrong spot, because that
line is:

  (*objfile->sf->sym_read) (objfile, mainline);

... but the new field is the next field in the struct,
sym_read_psymbols.

Tom

Re: Fixes for Mac OS X/x86 building/running

[Jonas Maebe]

On 12 Mar 2009, at 23:31, Tom Tromey wrote:

> Just FYI, the machoread.c patch won't apply upstream, because we
> haven't submitted the delayed partial symbol reading patch yet.

Ah, so that one is actually appropriate for this list :)

> I suspect you must have added the NULL in the wrong spot, because that
> line is:
>
>  (*objfile->sf->sym_read) (objfile, mainline);
>
> ... but the new field is the next field in the struct,
> sym_read_psymbols.

You're absolutely right, thanks! That's also the cause of the problems  
I experienced. Once I put the function pointer in the right place,  
everything started to work fine. I'll send a fixed patch tomorrow.


Jonas

PS: I've mailed the request for the copyright assignment form.

Re: Fixes for Mac OS X/x86 building/running

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> I suspect you must have added the NULL in the wrong spot
[...]

I checked in this patch on the archer-tromey-delayed-symfile branch.
It ought to fix this bug.  I also noticed another instance of this
problem, so I fixed it as well.

If you have the archer branch checked out, you can merge this over.
The commit itself is 1018589cd4646267c3dfc2225d1227f25a85f644, but
there's a merge after it on the branch since I forgot to pull
first... sigh.

Maybe it is time to submit the delayed symfile patch upstream.
I'm having trouble thinking of a reason why not.

Tom

2009-03-12  Jonas Maebe  <jonas.maebe@elis.ugent.be>
	    Tom Tromey  <tromey@redhat.com>

	* somread.c (som_sym_fns): Add sym_read_psymbols entry.
	* machoread.c (macho_sym_fns): Add sym_read_psymbols entry.

diff --git a/gdb/machoread.c b/gdb/machoread.c
index d8d3bd2..248fae2 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -681,6 +681,7 @@ static struct sym_fns macho_sym_fns = {
   macho_new_init,               /* sym_new_init: init anything gbl to entire symtab */
   macho_symfile_init,           /* sym_init: read initial info, setup for sym_read() */
   macho_symfile_read,           /* sym_read: read a symbol file into symtab */
+  NULL,				/* sym_read_psymbols */
   macho_symfile_finish,         /* sym_finish: finished with file, cleanup */
   macho_symfile_offsets,        /* sym_offsets:  xlate external to internal form */
   NULL                          /* next: pointer to next struct sym_fns */
diff --git a/gdb/somread.c b/gdb/somread.c
index 36a2b28..4d5bda9 100644
--- a/gdb/somread.c
+++ b/gdb/somread.c
@@ -435,6 +435,7 @@ static struct sym_fns som_sym_fns =
   som_new_init,			/* sym_new_init: init anything gbl to entire symtab */
   som_symfile_init,		/* sym_init: read initial info, setup for sym_read() */
   som_symfile_read,		/* sym_read: read a symbol file into symtab */
+  NULL,				/* sym_read_psymbols */
   som_symfile_finish,		/* sym_finish: finished with file, cleanup */
   som_symfile_offsets,		/* sym_offsets:  Translate ext. to int. relocation */
   default_symfile_segments,	/* sym_segments: Get segment information from

Re: Fixes for Mac OS X/x86 building/running

[Joel Brobecker]

Jonas,

> Archer (archer branch) is currently broken on Mac OS X/x86. The attached 
> (minor) patches fix this:
>
> * darwin-nat.c.patch fixes compilation errors due to some missing  
> "struct target_ops *" parameters. Probably caused by:

Just for the record, I applied this patch using the obvious rule
(there is no other way to fix the compilation failure):
http://www.sourceware.org/ml/gdb-patches/2009-03/msg00174.html

For the machoread.c change, I am not competent unless I do a little bit
of studying, which I'd like to avoid if I can.  I see that there might be
some dependency issues too, so let's get everything sent to gdb-patches@
as soon as you have something you're happy with. I'd like to have someone
such as Stan or Tristan have a look at it before we put it in.

-- 
Joel