This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: RFA: [symfile.c} Fix to symbol_file_add()


Fernando Nasser writes:
 > Thanks for reviewing my patch so promptly Elena.  Please see my comments
 > below.
 > 
 > Elena Zannoni wrote:
 > > 
 > > Fernando Nasser writes:
 > >  > Paul N. Hilfinger has pointed out to me that a few operations should be
 > >  > done every time a new symbol file is read.  This was an oversight in a
 > >  > patch I wrote in January.  The small patch attached fixes that.
 > >  >
 > >  > ChangeLog:
 > >  >
 > >  >      * symfile.c (symbol_file_command): Move cleanup operations
 > >  >      from here...
 > >  >      (symbol_file_add): ...to here, so they are run every time a new
 > >  >      symbol file is read.
 > >  >
 > > 
 > > This would change the behavior of all the calls to symbol_file_add().
 > > Which may not be what you want.
 > 
 > 
 > You are right, this bits:
 > >  > + #ifdef HPUXHPPA
 > >  > +   RESET_HP_UX_GLOBALS ();
 > >  > + #endif
 > >  > +
 > >  > +   set_initial_language ();
 > 
 > should only be done when a new "main" is loaded.  I suggested moving it
 > to symbol_file_add_main(), which is called exactly in those cases.
 > 

Yes, agreed.

 > However, the bit:
 > >  > +   /* Getting new symbols may change our opinion about
 > >  > +      what is frameless.  */
 > >  > +   reinit_frame_cache ();
 > 
 > should _always_ be done, whenever we load new symbols.
 > 
 > What happens today, and happened _before_ my patch (it hasn't changed
 > any call to symbol_file_add() nor symbol_file_add() itself except for
 > the clear part), is that _some_ of the callers do it and some do not.
 > 

Yes, true. But I wonder if that was intended. What happens if you
manually load a shared library? Do you need to clear the frame cache
at that point?

 > I guess this will be very difficult to control unless we move the
 > reinit_frame_cache () call to inside symbol_file_add().
 > 
 > So, I would, if you agree, post a patch (and ask Paul to help me test
 > it) that does:
 > 
 > 1) Move the HP and set_initial_language() bits to symbol_file_add_main()
 > 

OK.

 > 2) Move the reinit_frame_cache () bit to symbol_file_add(), as the
 > current patch does.
 > 

I need to understand better what happens with shared libraries.
SOLIB_ADD ends up calling this. I see it used in the attach command
and in this case a reinit_frame_cache is OK, so is for the case in
sol-thread.c but what about the other calls?

 > 3) Remove the duplicate call to reinit_frame_cache () from the targets.
 > 

After 2 is resolved.

 > 
 > How does it sound?

Mostly ok. 


Elena

 > 
 > 
 > Regards,
 > Fernando
 > 
 > 
 > 
 > > Later on in symfile.c there is another
 > > call to symbol_file_add which is followed by reinit_frame_cache(). In
 > > this case the same thing would be done twice, and in other calls it
 > > would be done, when it was not done before.  I am not sure why the
 > > other calls to symbol_file_add don't reinititalize things, but it
 > > seems a little too risky to do this, w/o making sure those platforms
 > > are not affected.
 > > 
 > > coff-solib.c:91:          objfile = symbol_file_add (filename, from_tty,
 > > cxux-nat.c:370:  objfile = symbol_file_add (LIBC_FILE, 0, NULL, 0, OBJF_READNOW);
 > > cxux-nat.c:390:       symbol_file_add (path_name, 1, &section_addrs, 0, 0);
 > > irix5-nat.c:850:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
 > > osfsolib.c:585:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
 > > pa64solib.c:273:  so->objfile = symbol_file_add (name, from_tty, &section_addrs, 0, OBJF_SHARED);
 > > remote-udi.c:1291:  symbol_file_add (strtok (args, " \t"), from_tty, NULL, 1, 0);
 > > remote-vx.c:670:  objfile = symbol_file_add (name, from_tty, NULL, 0, 0);
 > > solib.c:313:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
 > > somsolib.c:290:  so->objfile = symbol_file_add (name, from_tty, NULL, 0, OBJF_SHARED);
 > > symfile.c:905:  symbol_file_add (args, from_tty, NULL, 1, 0);
 > > symfile.c:982:            symbol_file_add (name, from_tty, NULL, 1, flags);
 > > symfile.c:1549:  symbol_file_add (filename, from_tty, &section_addrs, 0, flags);
 > > win32-nat.c:444:  p->ret = symbol_file_add (p->name, p->from_tty, p->addrs, p->mainline, p->flags);
 > > 
 > > Since the original code substitution was to replace calls to
 > > symbol_file_command() with symbol_file_add_main(), I would think that
 > > the lines below should be left in the symbol_file_command function and
 > > added to symbol_file_add_main() instead, therefore preserving the
 > > original semantics.
 > > 
 > > Elena
 > > 
 > >  > --
 > >  > Fernando Nasser
 > >  > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > >  > 2323 Yonge Street, Suite #300
 > >  > Toronto, Ontario   M4P 2C9Index: symfile.c
 > >  > ===================================================================
 > >  > RCS file: /cvs/src/src/gdb/symfile.c,v
 > >  > retrieving revision 1.31
 > >  > diff -c -p -r1.31 symfile.c
 > >  > *** symfile.c        2001/04/05 02:02:13     1.31
 > >  > --- symfile.c        2001/04/29 16:13:12
 > >  > *************** symbol_file_add (char *name, int from_tt
 > >  > *** 893,898 ****
 > >  > --- 893,907 ----
 > >  >     if (target_new_objfile_hook)
 > >  >       target_new_objfile_hook (objfile);
 > >  >
 > >  > + #ifdef HPUXHPPA
 > >  > +   RESET_HP_UX_GLOBALS ();
 > >  > + #endif
 > >  > +   /* Getting new symbols may change our opinion about
 > >  > +      what is frameless.  */
 > >  > +   reinit_frame_cache ();
 > >  > +
 > >  > +   set_initial_language ();
 > >  > +
 > >  >     return (objfile);
 > >  >   }
 > >  >
 > >  > *************** symbol_file_command (char *args, int fro
 > >  > *** 980,993 ****
 > >  >              {
 > >  >                     name = *argv;
 > >  >                symbol_file_add (name, from_tty, NULL, 1, flags);
 > >  > - #ifdef HPUXHPPA
 > >  > -              RESET_HP_UX_GLOBALS ();
 > >  > - #endif
 > >  > -              /* Getting new symbols may change our opinion about
 > >  > -                 what is frameless.  */
 > >  > -              reinit_frame_cache ();
 > >  > -
 > >  > -              set_initial_language ();
 > >  >              }
 > >  >        argv++;
 > >  >      }
 > >  > --- 989,994 ----
 > 
 > -- 
 > Fernando Nasser
 > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > 2323 Yonge Street, Suite #300
 > Toronto, Ontario   M4P 2C9
 > 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]