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

Fernando Nasser
2001-04-30T16:36:00Z

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.

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.

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()

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

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

How does it sound?

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 ----