This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Per-inferior program arguments and io terminal
On Thursday 14 January 2010 15:09:17, Vladimir Prus wrote:
> On Thursday 14 January 2010 16:37:42 Pedro Alves wrote:
> > Better choices would be: set_inferior_tty_command/show_inferior_tty_command
> > or set_inferior_tty/show_inferior_tty.
>
> Done. Also done for args function.
Thanks.
> > > @@ -87,6 +88,10 @@ free_inferior (struct inferior *inf)
> > > {
> > > discard_all_inferior_continuations (inf);
> > > inferior_free_data (inf);
> > > + xfree (inf->args);
> >
> > > + xfree (inf->argv);
> >
> > Hmm, I was going to say that this usually leaks argv[0..argc], and
> > you should use freeargv, but, I now see that the only caller
> > of set_inferior_args_vector is captured_main, and the
> > argc,argv are presently a shallow copy of `main's arguments, so
> > definitely not ok to free this as is. This definitely needs a
> > comment in the description of inferior->argv, expanded from
> > the comment that was in inferior_argv.
>
> I've added a comment.
Thanks, but not correct yet...
> @@ -87,6 +88,10 @@ free_inferior (struct inferior *inf)
> {
> discard_all_inferior_continuations (inf);
> inferior_free_data (inf);
> + xfree (inf->args);
> + xfree (inf->argv);
> + xfree (inf->terminal);
> + free_environ (inf->environment);
> xfree (inf->private);
Maybe I confused you with saying "shallow" copy. This
argv comes from a direct `argv' pointer copy:
void
set_inferior_args_vector (int argc, char **argv)
{
current_inferior ()->argc = argc;
current_inferior ()->argv = argv;
}
This is only called from captured_main, like so:
set_inferior_args_vector (argc - optind, &argv[optind]);
So at best, you'd need to `xfree (&argv[-optind])', but
even that would be broken. Say, let's ignore that offset. argv
in that context is a pointer copy coming from:
static int
captured_main (void *data)
{
struct captured_main_args *context = data;
int argc = context->argc;
char **argv = context->argv;
and this context->argv itself comes from:
int
main (int argc, char **argv)
{
struct captured_main_args args;
memset (&args, 0, sizeof args);
args.argc = argc;
args.argv = argv;
^^^^^^^^^^^^^^^^
args.use_windows = 0;
args.interpreter_p = INTERP_CONSOLE;
return gdb_main (&args);
}
so, freeing that would be equivalent to:
int
main (int argc, char **argv)
{
free (argv);
}
which is not kosher (try it). You should _not_ free inferior->argv.
Actually, I'll try it myself:
> ./gdb -q --args /home/pedro/gdb/tests/threads --args foo bar
(gdb) clone-inferior
Added inferior 2.
(gdb) inferior 1
[Switching to inferior 1 [process 0] (/home/pedro/gdb/tests/threads)]
(gdb) inferior 2
[Switching to inferior 2 [process 0] (/home/pedro/gdb/tests/threads)]
(gdb) remove-inferior 1
*** glibc detected *** ./gdb: munmap_chunk(): invalid pointer: 0x00007fff9d5b4fe8 ***
======= Backtrace: =========
/lib/libc.so.6(cfree+0x1b6)[0x7f7f0e86cd46]
./gdb(xfree+0x1c)[0x462f1e]
:
Cancelled (core dumped)
And since you now made me try the patch... :-)
>qr
Warning: trailing whitespace in lines 230,238 of gdb/solib.c
Warning: trailing whitespace in lines 664,673,683 of gdb/main.c
Warning: trailing whitespace in line 266 of gdb/mi/mi-cmd-env.c
Warning: trailing whitespace in lines 129,186,200,213,2653,2654,2655 of gdb/infcmd.c
Refreshed patch per-inferior-args3.diff
--
Pedro Alves