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

Elena Zannoni
2001-04-30T20:29:00Z

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
>