Re: [RFA/RFC] New command: ``start''

Re: [RFA/RFC] New command: ``start''

[Michael Elizabeth Chastain]

jb> Some programs contain an elaboration phase where some startup code is
jb> executed before the main program is called.  This depends on the
jb> languages used to write your program. In C@t{++} for instance,
jb> constructors for static and global objects are executed before
jb> @code{main} is called.  It is therefore possible that the debugger stops
jb> before reaching the main procedure.  However, the temporary breakpoint
jb> will remain to halt execution.

It works for me!

Michael C

Re: [RFA/RFC] New command: ``start''

[Michael Elizabeth Chastain]

Uh, sorry I didn't throw in my two cents earlier, but I am
confused by one clause in this paragraph:

  Some programs contain an elaboration phase where some startup code is
  executed before the main program is called.  This depends on the
  languages used to write your program. In C@t{++} for instance, class
  constructors are executed before @code{main} is called.  It is therefore
  possible that the debugger stops before reaching the main procedure.
  However, the temporary breakpoint will remain to halt execution.

I don't think 'class constructors' is right here.  It's more like
'constructors for static and global objects'.

Other than that, the new doco looks great to me.

Michael C

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> I don't think 'class constructors' is right here.  It's more like
> 'constructors for static and global objects'.

Yeah, my knowledge of C++ is not so good...

I have changed the latest version to use this instead:

+Some programs contain an elaboration phase where some startup code is
+executed before the main program is called.  This depends on the
+languages used to write your program. In C@t{++} for instance,
+constructors for static and global objects are executed before
+@code{main} is called.  It is therefore possible that the debugger stops
+before reaching the main procedure.  However, the temporary breakpoint
+will remain to halt execution.

-- 
Joel

[RFA/RFC] New command: ``start''

[Joel Brobecker]

Hello,

As briefly discussed on gdb@ and gdb-patches@, here is a first proposal
for the addition of a new command: ``start''.

I handled the language-dependent issue by adding a new method to the
language vector. I returns a newly allocated string that tells GDB
where to insert the breakpoint. I named it "main_program_name" because
I would expect it to always return the name of a procedure. However,
it can actually return any location expression. So a language could
use a different approach and return something like "*0xdeadbeef" or
"source.c:10" for instance.

The other decision I made was to allow this new method to be undefined
(NULL). In that case, we use "main" as the location where to insert the
temporary breakpoint. I am not too sure about this approach, but I
selected it because it avoids a xstrdup ("main")/xfree sequence.
On the other hand, forcing the method to always be set would make the
implemention clearer, I think, just at the expense of an unnecessary
xstrdup/xfree sequence. I can add a procedure default_main_program_name
that would return a xstrdup of "main". The other option I thought about
was to still allow a NULL method, and then provide a procedure in
language.[hc] get_main_program_name (void) (or should maybe take a
language as argument) that would return the name of the main procedure
name based on the value of the new method (NULL => "main", otherwise
return string from method call).

I am also adding the documentation for this new command.

2004-05-17  Joel Brobecker  <brobecker@gnat.com>

        * language.h (language_defn): New field, la_xmain_procedure_name.
        * language.c (unknown_language_defn): Set new field to NULL.
        (auto_language_defn): Likewise.
        (local_language_defn): Likewise.
        * ada-lang.c (ada_language_defn): Likewise.
        * c-lang.c (c_language_defn): Likewise.
        (cplus_language_defn): Likewise.
        (asm_language_defn): Likewise.
        (minimal_language): Likewise.
        * f-lang.c (f_language_defn): Likewise.
        * jv-lang.c (java_language_defn): Likewise.
        * m2-lang.c (m2_language_defn): Likewise.
        * objc_language (objc_language_defn): Likewise.
        * p-lang.c (pascal_language_defn): Likewise.
        * scm-lang.c (scm_language_defn): Likewise.
        * infcmd.c (kill_if_already_running): New function, extracted
        from run_command().
        (run_command): Replace extracted code by call to
        kill_if_already_running().
        (start_command): New function.
        (_initialize_infcmd): Add "start" command.

2004-05-17  Joel Brobecker  <brobecker@gnat.com>

        * gdb.texinfo (Starting): Document new start command.

Tested on x86-linux.
Comments? OK?

-- 
Joel
Attachment:
start_cmd.diff
Description: Text document
Attachment:
start_doc.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Eli Zaretskii]

> Date: Mon, 17 May 2004 19:47:00 -0700
> From: Joel Brobecker <brobecker@gnat.com>
> 
> As briefly discussed on gdb@ and gdb-patches@, here is a first proposal
> for the addition of a new command: ``start''.

Thanks.

> The other decision I made was to allow this new method to be undefined
> (NULL). In that case, we use "main" as the location where to insert the
> temporary breakpoint. I am not too sure about this approach, but I
> selected it because it avoids a xstrdup ("main")/xfree sequence.
> On the other hand, forcing the method to always be set would make the
> implemention clearer, I think, just at the expense of an unnecessary
> xstrdup/xfree sequence.

I think it's better to have a non-NULL string there.  The benfit is
that whoever reads the code doesn't need to look elsewhere to
understand what effect does NULL have there.

> +  if (current_language->la_xmain_procedure_name == NULL)
> +    {
> +      main_program_name = "main";
> +    }
> +  else
> +    {
> +      main_program_name =
> +        current_language->la_xmain_procedure_name ();
> +
> +      if (main_program_name == NULL)
> +        error ("Unable to get the main program name.");

If we use "main" when the method is NULL, we should also use "main"
if the method returns NULL, I think.

> +  c = add_com ("start", class_run, start_command,
> +               "\
> +Start the debugged program until the beginning of the main procedure.\n\

I think this should say "Run the debugged program until ...".  "Start
until" sounds like incorrect English.

> +Depending on the language, the name of the main procedure can vary.
> +With languages such as C or C++, the main procedure name is always

Please use "C@t{++}" instead of "C++", the former looks prettier in
print.

> +@code{main()}, but other languages such as Ada do not require a specific

Please say "@code{main}", without the parens.  "main()" looks like a
call to `main' with no arguments, which is not what you mean here.

> +The @code{start} command does the equivalent of setting a temporary
> +breakpoint at the beginning of the main procedure and then performing
> +a @code{run}.

`run' should be in @samp here, and I think "invoking the @samp{run}
command" is better than "performing a @samp{run}".

> Some programs contain an elaboration phase that will be
> +performed before the main procedure is reached, and it is possible that
> +the debugger will stop before reaching the main procedure. However,
> +the temporary breakpoint will remain to halt execution.

Sorry, I don't understand this part (what is ``elaboration''?).
Could you perhaps make this text more explanatory?

> +The arguments used when using this command are directly passed to the
> +@code{run} command.

You mean, if you type "run" thereafter, it will reuse the arguments
you type for the "start" command?  If so, we should reword this text
to make that more clear.  ``Directly passed'' is confusing, since
nothing is really passed anywhere.

Finally, our standard is to have 2 spaces after the period that ends a
sentence.  Please make sure you do that in your patch.

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

Eli,

Thanks for your useful comments. Attached are a couple of revised
patches based on your input.

> > The other decision I made was to allow this new method to be undefined
> > (NULL). In that case, we use "main" as the location where to insert the
> > temporary breakpoint. I am not too sure about this approach, but I
> > selected it because it avoids a xstrdup ("main")/xfree sequence.
> > On the other hand, forcing the method to always be set would make the
> > implemention clearer, I think, just at the expense of an unnecessary
> > xstrdup/xfree sequence.
> 
> I think it's better to have a non-NULL string there.  The benfit is
> that whoever reads the code doesn't need to look elsewhere to
> understand what effect does NULL have there.

OK. So I defined a new procedure in language.h, that returns a strdup
of "main". That way, all languages use the same approach, rather than
having only a few language having a non-null main_procedure_name
language method. Nice and consistent.

Just one additional comment, that may not be obvious: In Ada, the name
of the main procedure is not specified by Ada, it's up to the user to
choose it. That means that the name of that main procedure depends on
the program. When Ada-support is activated, we'll implement our own
main_procedure_name method that will dig out that name from the
executable. JIC.

> If we use "main" when the method is NULL, we should also use "main"
> if the method returns NULL, I think.

I changed the implementation to force the method to be defined.
I prefer that we report an error when the method failed to find the
name of the main procedure, rather than silently stop in start.

> > +  c = add_com ("start", class_run, start_command,
> > +               "\
> > +Start the debugged program until the beginning of the main procedure.\n\
> 
> I think this should say "Run the debugged program until ...".  "Start
> until" sounds like incorrect English.

Fixed. I also modified the description in an attempt to clarify how
arguments to this function are used.

> Please use "C@t{++}" instead of "C++", the former looks prettier in
> print.

Fixed.

> > +@code{main()}, but other languages such as Ada do not require a specific
> 
> Please say "@code{main}", without the parens.  "main()" looks like a
> call to `main' with no arguments, which is not what you mean here.

Fixed.

> > +The @code{start} command does the equivalent of setting a temporary
> > +breakpoint at the beginning of the main procedure and then performing
> > +a @code{run}.
> 
> `run' should be in @samp here, and I think "invoking the @samp{run}
> command" is better than "performing a @samp{run}".

Fixed. I also put the `start' in @samp, for consistency. I hope it was
the right thing to do.

> > Some programs contain an elaboration phase that will be
> > +performed before the main procedure is reached, and it is possible that
> > +the debugger will stop before reaching the main procedure. However,
> > +the temporary breakpoint will remain to halt execution.
> 
> Sorry, I don't understand this part (what is ``elaboration''?).
> Could you perhaps make this text more explanatory?

I tried to make it more explanatory by explaining what happens during
the elaboration (some code is run before the main procedure is called),
and also by providing an example using C++ constructors.

> > +The arguments used when using this command are directly passed to the
> > +@code{run} command.
> 
> You mean, if you type "run" thereafter, it will reuse the arguments
> you type for the "start" command?  If so, we should reword this text
> to make that more clear.  ``Directly passed'' is confusing, since
> nothing is really passed anywhere.

I have reworked the documentation. Is it better?
I was also wondering whether it would be better to switch para 1 and 2.
That way, the user immediately knows what this command does. He then
gets the explanation of what the "main procedure" is.

> Finally, our standard is to have 2 spaces after the period that ends a
> sentence.  Please make sure you do that in your patch.

Ouch! Sorry about that. I should know this by now, but I don't have
the reflex to remember it for the docs yet.

2004-05-17  Joel Brobecker  <brobecker@gnat.com>

        * language.h (language_defn): New field, la_xmain_procedure_name.
        (xdefault_main_program_name): Add declaration.
        * language.c (xdefault_main_program_name): New function.
        (unknown_language_defn): Set new field to xdefault_main_program_name.
        (auto_language_defn): Likewise.
        (local_language_defn): Likewise.
        * ada-lang.c (ada_language_defn): Likewise.
        * c-lang.c (c_language_defn): Likewise.
        (cplus_language_defn): Likewise.
        (asm_language_defn): Likewise.
        (minimal_language): Likewise.
        * f-lang.c (f_language_defn): Likewise.
        * jv-lang.c (java_language_defn): Likewise.
        * m2-lang.c (m2_language_defn): Likewise.
        * objc_language (objc_language_defn): Likewise.
        * p-lang.c (pascal_language_defn): Likewise.
        * scm-lang.c (scm_language_defn): Likewise.
        * infcmd.c (kill_if_already_running): New function, extracted
        from run_command().
        (run_command): Replace extracted code by call to
        kill_if_already_running().
        (start_command): New function.
        (_initialize_infcmd): Add "start" command.

2004-05-17  Joel Brobecker  <brobecker@gnat.com>

        * gdb.texinfo (Starting): Document new start command.

-- 
Joel
Attachment:
start_cmd.diff
Description: Text document
Attachment:
start_doc.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Eli Zaretskii]

> Date: Tue, 18 May 2004 10:05:17 -0700
> From: Joel Brobecker <brobecker@gnat.com>
> 
> Just one additional comment, that may not be obvious: In Ada, the name
> of the main procedure is not specified by Ada, it's up to the user to
> choose it. That means that the name of that main procedure depends on
> the program. When Ada-support is activated, we'll implement our own
> main_procedure_name method that will dig out that name from the
> executable. JIC.

Understood.

> > Please use "C@t{++}" instead of "C++", the former looks prettier in
> > print.
> 
> Fixed.

Thanks, but there's still one place where "C++" is used ;-)

> I tried to make it more explanatory by explaining what happens during
> the elaboration (some code is run before the main procedure is called),
> and also by providing an example using C++ constructors.

Okay, this is much more clear now.  Perhaps it would be useful to
mention the term ``startup code'' there?

> > You mean, if you type "run" thereafter, it will reuse the arguments
> > you type for the "start" command?  If so, we should reword this text
> > to make that more clear.  ``Directly passed'' is confusing, since
> > nothing is really passed anywhere.
> 
> I have reworked the documentation. Is it better?

It's much better, thanks.  However, I understand that the same
arguments will be reused later if the user invokes "run", is that
true?  If so, I think we should mention that.

> +@table @code
> +@kindex start

I think an additional index entry here saying

  @cindex run to main procedure

would help.

Otherwise, the documentation patch is approved; thanks.

For the code patch, please wait for the responsible person(s) to give
their final blessing.  (Hmm... who is that in this case? MAINTAINERS
says "Blanket Write Privs Maintainers".  Andrew?)

Re: [RFA/RFC] New command: ``start''

[Andrew Cagney]

Otherwise, the documentation patch is approved; thanks.

For the code patch, please wait for the responsible person(s) to give
their final blessing.  (Hmm... who is that in this case? MAINTAINERS
says "Blanket Write Privs Maintainers".  Andrew?)
That's you :-)  The language maintainers might have an opinion - their 
vector is being changed but the actual changes are mechanical :-)

Andrew

Re: [RFA/RFC] New command: ``start''

[Eli Zaretskii]

> Date: Tue, 18 May 2004 15:03:18 -0400
> From: Andrew Cagney <cagney@gnu.org>
> > 
> > For the code patch, please wait for the responsible person(s) to give
> > their final blessing.  (Hmm... who is that in this case? MAINTAINERS
> > says "Blanket Write Privs Maintainers".  Andrew?)
> 
> That's you :-)

Okay ;-)

But since other developers have spoken, I will refrain from okaying
the code patches until the raised issues are resolved.

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> Okay, this is much more clear now.  Perhaps it would be useful to
> mention the term ``startup code'' there?

Sure.

> It's much better, thanks.  However, I understand that the same
> arguments will be reused later if the user invokes "run", is that
> true?  If so, I think we should mention that.

That's correct. I added a small sentence explaining this. I agree,
I was trying to imply this by saying that we call "run" underneath,
but that's much clearer that way:

    Note that the same arguments will be reused if no argument
    is provided during subsequent calls to @samp{start} or @samp{run}.

> > +@table @code
> > +@kindex start
> 
> I think an additional index entry here saying
> 
>   @cindex run to main procedure

Added.

> Otherwise, the documentation patch is approved; thanks.

Thanks! Attached is the latest version of the patch.

-- 
Joel
Attachment:
start_doc.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Mon, May 17, 2004 at 07:47:00PM -0700, Joel Brobecker wrote:
> Hello,
> 
> As briefly discussed on gdb@ and gdb-patches@, here is a first proposal
> for the addition of a new command: ``start''.
> 
> I handled the language-dependent issue by adding a new method to the
> language vector. I returns a newly allocated string that tells GDB
> where to insert the breakpoint. I named it "main_program_name" because
> I would expect it to always return the name of a procedure. However,
> it can actually return any location expression. So a language could
> use a different approach and return something like "*0xdeadbeef" or
> "source.c:10" for instance.

We already have a function for this: main_name.  Is it adequate for
Ada?


-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> > As briefly discussed on gdb@ and gdb-patches@, here is a first proposal
> > for the addition of a new command: ``start''.
> > 
> > I handled the language-dependent issue by adding a new method to the
> > language vector. I returns a newly allocated string that tells GDB
> > where to insert the breakpoint. I named it "main_program_name" because
> > I would expect it to always return the name of a procedure. However,
> > it can actually return any location expression. So a language could
> > use a different approach and return something like "*0xdeadbeef" or
> > "source.c:10" for instance.
> 
> We already have a function for this: main_name.  Is it adequate for
> Ada?

I forgot about this function. But I don't think it's adequate for the
start command. If I understand everything correctly, this function
relies on some information provided in stabs via N_MAIN symbols.
Otherwise, it defaults to "main". This wouldn't necessarily work
with any debug format.

Also, the purpose of this function is slightly different from what
I am trying to achieve with the language method: Despite the fact that
most users see Ada programs starting at the begining of their main
procedure, a closer approximation is that it starts inside procedure
main() too. 

To give you a better understanding how who it works in Ada (as defined
in the Reference Manual [RM for short]), the execution of an Ada program
has 3 important phases: 1. Elaboration
                        2. Program execution
                        3. Finalization
The RM says that the elaboration should be performed inside a procedure
called "adainit", and that the finalization should be performed by a
procedure called "adafinal". The elaboration and finalization order
is computed by a tool called the "binder" that checks all the
dependencies, and generate a small source file that looks like this:

        void
        adainit (void)
        {
          /* Do the program elaboration here.  */
        }
        
        void
        adafinal (void)
        {
          /* Finalize the program.  */
        }
        
        void
        int
        main (...)
        {
          adainit ();
          the_user_main_program ();
          adafinal ();
        }

The last step when building a program is then to call the gnat linker
that will do 2 things: compile the file generated by the binder,
and then do the link. (this is all automated by our gnatmake tool :).

So, even though most Ada users will usually only care about their
own main procedure, I occasionally need to go up the stack up
to procedure "main" to inspect it.

Adapting main_name() to fit the purpose of the start command would
cause the backtrace (amount other things) to be a bit shorter, and
also depend on the current language.

One thing that might be worthwhile, though, is call it from
xdefault_main_program_name, instead of hard-coding "main" a second time.
But that might introduce an unwanted dependency.

-- 
Joel

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Tue, May 18, 2004 at 03:27:40PM -0700, Joel Brobecker wrote:
> > > As briefly discussed on gdb@ and gdb-patches@, here is a first proposal
> > > for the addition of a new command: ``start''.
> > > 
> > > I handled the language-dependent issue by adding a new method to the
> > > language vector. I returns a newly allocated string that tells GDB
> > > where to insert the breakpoint. I named it "main_program_name" because
> > > I would expect it to always return the name of a procedure. However,
> > > it can actually return any location expression. So a language could
> > > use a different approach and return something like "*0xdeadbeef" or
> > > "source.c:10" for instance.
> > 
> > We already have a function for this: main_name.  Is it adequate for
> > Ada?
> 
> I forgot about this function. But I don't think it's adequate for the
> start command. If I understand everything correctly, this function
> relies on some information provided in stabs via N_MAIN symbols.
> Otherwise, it defaults to "main". This wouldn't necessarily work
> with any debug format.

You're right, GDB doesn't currently set this from dwarf2 data.  Fortran
has a dwarf tag for something similar, but it doesn't look like it's
quite the same: DW_AT_calling_convention DW_CC_program.

> Also, the purpose of this function is slightly different from what
> I am trying to achieve with the language method: Despite the fact that
> most users see Ada programs starting at the begining of their main
> procedure, a closer approximation is that it starts inside procedure
> main() too. 

I would say that this was the meaning of main_name, rather than the
entry point.  Global constructors for C++ traditionally happen before
the "main program" and don't appear on the backtrace; same seems
reasonable for Ada.  Without "set backtrace past-main on" you can't see
the caller of the user's code; that seems reasonable for Ada also.  Do
you need backtraces to continue to "main"?

I don't want to proliferate mechanisms.  It would be nice if the debug
readers could fill this in, and (if necessary, to support existing
tools) a language specific hook could be called as a fallback.

> So, even though most Ada users will usually only care about their
> own main procedure, I occasionally need to go up the stack up
> to procedure "main" to inspect it.
> 
> Adapting main_name() to fit the purpose of the start command would
> cause the backtrace (amount other things) to be a bit shorter, and
> also depend on the current language.

The same points apply to Java; gcj even uses pretty much the same
mechanism as gnatbind.

-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

Hello Daniel,

> > Also, the purpose of this function is slightly different from what
> > I am trying to achieve with the language method: Despite the fact that
> > most users see Ada programs starting at the begining of their main
> > procedure, a closer approximation is that it starts inside procedure
> > main() too. 
> 
> I would say that this was the meaning of main_name, rather than the
> entry point.  Global constructors for C++ traditionally happen before
> the "main program" and don't appear on the backtrace; same seems
> reasonable for Ada.  Without "set backtrace past-main on" you can't see
> the caller of the user's code; that seems reasonable for Ada also.  Do
> you need backtraces to continue to "main"?

Your suggestion makes sense.

> I don't want to proliferate mechanisms.  It would be nice if the debug
> readers could fill this in, and (if necessary, to support existing
> tools) a language specific hook could be called as a fallback.

So let's go for that route. How about:
  1. I add the language-specific hook the way I designed it
     in my last patch.
  2. Use that hook in main_name() when the debug info didn't provide
     the name of the main procedure.
  3. Modify start_command to use main_name() instead of the language
     hook.

Would that work for you?

-- 
Joel

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Wed, May 19, 2004 at 08:36:15AM -0700, Joel Brobecker wrote:
> So let's go for that route. How about:
>   1. I add the language-specific hook the way I designed it
>      in my last patch.
>   2. Use that hook in main_name() when the debug info didn't provide
>      the name of the main procedure.
>   3. Modify start_command to use main_name() instead of the language
>      hook.
> 
> Would that work for you?

That sounds good to me.  Note that now you can avoid playing with
xmalloc/xfree and just return the constant "main".

-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> That sounds good to me.  Note that now you can avoid playing with
> xmalloc/xfree and just return the constant "main".

Before starting on this, there are a couple of things we might want
to discuss:

   . It think it would be better if we cached the main name, so as
     to avoid recomputing it each time we do a backtrace. But that
     causes a problem when debugging 2 different executables one
     after the other during the same GDB session. During the second
     debugging run, we'll end up using the wrong main procedure name.
     We need to insert an observer I think at the begining of
     exec_file_attach() to signal us that the cached main_name
     is obsolete.

   . Could you clarify a bit more what you mean about the xmalloc/xfree
     juggle? I see that set_main_name() takes care of this for me, so
     do you mean that the language method should be returning a string
     located in a temporary buffer? Something like: result is good until
     next call? (I am not too keen on that).

Thanks,
-- 
Joel

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

Daniel,

On Wed, May 19, 2004 at 11:41:55AM -0400, Daniel Jacobowitz wrote:
> On Wed, May 19, 2004 at 08:36:15AM -0700, Joel Brobecker wrote:
> > So let's go for that route. How about:
> >   1. I add the language-specific hook the way I designed it
> >      in my last patch.
> >   2. Use that hook in main_name() when the debug info didn't provide
> >      the name of the main procedure.
> >   3. Modify start_command to use main_name() instead of the language
> >      hook.
> > 
> > Would that work for you?
> 
> That sounds good to me.

I was implementing this approach, with symtab.c caching the name_of_main
so that we would compute it only once per executable loaded, but then
just realized something that made me very uneasy.

Basically, because of the caching, we've transformed name_of_main into
a random value, because it depends on the value of the current language
at a certain random point in time. And that value will not be changed
again until the user reloads his executable again (via exec-file).

That could lead to situations where the wrong value for name_of_main
is computed. Consider the following example:

   The user has an Ada program. Like every program, it has some bits
   and pieces that are not Ada. It could be a multi-language application
   but also an Ada program linked to the libc. Anyway, he inserts a
   breakpoint inside some non-Ada routine, does a "run", then a
   backtrace => we set "name_of_main" in symtab.c using the non-ada
   language vector => name_of_main will likely be wrong.

   At this point, GDB will unwind up to the wrong frame (one frame
   too high in case of Ada programs), but even more problematic is
   the fact that the "start" command will stop at the wrong place.
   And forcing the language to Ada won't help.
   
We could get rid of the caching mechanism, and recompute name_of_main
everytime (unless found in the debug info), but I think that would be
very costly, especially with graphical front-ends that have a tendency
of asking for the callstack at every stop...

We could try to make it a little smarter, by recomputing the
name_of_main if the value we previously computed was for a different
language. I think that we will still end up recomputing it fairly often
as callstacks involving more than one language can be fairly common.

Or another better approach, is to store an array of name_of_main, one
per language, and then get main_name() to return the correct one based
on the language given to it. That way, we only compute it once per
language (will usually be once or twice per session, three times at
most, I would anticipate). Unfortunately, this raises the same sort
of issues, because there will be times when we call main_name() with
the idea of getting the main program name from the user's point of
view, and we will simply not know which language to select for that.

Or the last approach would be to leave main_name() alone, and make
the "start" command independent of that mechanism (my initial approach).

Computing the main name lazily and caching sounds like a perfect receipe
for incorrect behavior. So I think we have two possible alternatives at
this stage:
  
  . Recompute name_of_main every time (big performance penalty I think)

  . Dissociate the "start" command infrastructure from the
    name_of_main() infrastructure, and keep my initial approach.

I know you would like to avoid having several mechanisms computing the
name of main, but I think this is our best alternative in this case.

What do you think? I am attaching a patch to symtab.c just to illustrate
what I was trying to do.

-- 
Joel
Attachment:
symtab.c.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Eli Zaretskii]

> Date: Wed, 19 May 2004 18:01:45 -0700
> From: Joel Brobecker <brobecker@gnat.com>
>    
> We could get rid of the caching mechanism, and recompute name_of_main
> everytime (unless found in the debug info), but I think that would be
> very costly, especially with graphical front-ends that have a tendency
> of asking for the callstack at every stop...

How about confirming this?  Perhaps there's no real problem after all.

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Wed, May 19, 2004 at 06:01:45PM -0700, Joel Brobecker wrote:
> Daniel,
> 
> On Wed, May 19, 2004 at 11:41:55AM -0400, Daniel Jacobowitz wrote:
> > On Wed, May 19, 2004 at 08:36:15AM -0700, Joel Brobecker wrote:
> > > So let's go for that route. How about:
> > >   1. I add the language-specific hook the way I designed it
> > >      in my last patch.
> > >   2. Use that hook in main_name() when the debug info didn't provide
> > >      the name of the main procedure.
> > >   3. Modify start_command to use main_name() instead of the language
> > >      hook.
> > > 
> > > Would that work for you?
> > 
> > That sounds good to me.
> 
> I was implementing this approach, with symtab.c caching the name_of_main
> so that we would compute it only once per executable loaded, but then
> just realized something that made me very uneasy.
> 
> Basically, because of the caching, we've transformed name_of_main into
> a random value, because it depends on the value of the current language
> at a certain random point in time. And that value will not be changed
> again until the user reloads his executable again (via exec-file).

You're right.  This is a problem.

> We could get rid of the caching mechanism, and recompute name_of_main
> everytime (unless found in the debug info), but I think that would be
> very costly, especially with graphical front-ends that have a tendency
> of asking for the callstack at every stop...

I don't think it would be costly.  However, I think it would be very
confusing!  Consider: we're in an Ada routine.  We backtrace.  This is
the main program, so the backtrace stops at this procedure.  We
single-step into a C subroutine and backtrace; now the backtrace goes
back to the Ada procedure and then back again further!

I'm having the same problem reviewing this that I did with
SYMBOL_SEARCH_NAME: namely, you're introducing interfaces that don't
exist in our Ada support files, without the Ada implementation of the
interface.  Could you show me what the Ada function for guessing the
name of main does?  I think the best approach may be to iterate over
the languages included in the object file, asking each of them whether
this language appears to contain a main procedure.  Or even to simply
skip the langhook complexity and just call an Ada find-main function!
For gcj I suspect we will just change the debug information.

-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> > We could get rid of the caching mechanism, and recompute name_of_main
> > everytime (unless found in the debug info), but I think that would be
> > very costly, especially with graphical front-ends that have a tendency
> > of asking for the callstack at every stop...
> 
> I don't think it would be costly.  However, I think it would be very
> confusing!  Consider: we're in an Ada routine.  We backtrace.  This is
> the main program, so the backtrace stops at this procedure.  We
> single-step into a C subroutine and backtrace; now the backtrace goes
> back to the Ada procedure and then back again further!

Yes, very confusing indeed.

> I'm having the same problem reviewing this that I did with
> SYMBOL_SEARCH_NAME: namely, you're introducing interfaces that don't
> exist in our Ada support files, without the Ada implementation of the
> interface.  Could you show me what the Ada function for guessing the
> name of main does?

That's because the command name we've been using for that was "begin".
But people prefered "start" (which I like better too), so I changed
the name.

Here is how we find the name of the main procedure: We lookup a certain
symbol which is created by our binder, __gnat_ada_main_program_name.
This symbol points to a string holding the name of the main procedure.
You'll find the code in ada-lang.c:begin_command().

BTW: I think Paul was working on submitting a more recent version of
the ada-* files. I'll double-check with him.

> I think the best approach may be to iterate over the languages
> included in the object file, asking each of them whether this language
> appears to contain a main procedure.  Or even to simply skip the
> langhook complexity and just call an Ada find-main function!  For gcj
> I suspect we will just change the debug information.

I thought about several approaches along the same idea. One of them
was to implement a sniffing mechanism, with certain sniffers having
certain priorities. But you know, I really like the idea of dropping
the langhook, and just call the ada function. Something like this:

        char *
        main_name ()
        {
          /* If we found the name of main from the debug info, or
             already looked it up, then return the name of main.  */
          if (name_of_main != NULL)
            return name_of_main;

          /* Is the main in Ada?  */
          tmp_main_name = ada_find_main_name ();
          if (tmp_main_name != NULL)
            {
              set_main_name (tmp_main_name);
              return name_of_main;
            }

          /* Is the main in Java?  */
          tmp_main_name = java_find_main_name ();
          [etc...]

          /* Fallback: main_name must be the usual "main".  */
          set_main_name ("main);
          return name_of_main;
        }

The only drawback I see from this is that I will need to include
ada-lang.h. It would have been nice to avoid this, which is possible
with the langhooks. I could also add the ada_find_main_name()
declaration in another more common .h file, but that would be a dirty
trick, IMO.

-- 
Joel

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Thu, May 20, 2004 at 09:02:59AM -0700, Joel Brobecker wrote:
> Here is how we find the name of the main procedure: We lookup a certain
> symbol which is created by our binder, __gnat_ada_main_program_name.
> This symbol points to a string holding the name of the main procedure.
> You'll find the code in ada-lang.c:begin_command().

OK.  So this has the property that I was hoping for; it won't return a
false positive for a non-Ada main program.

> > I think the best approach may be to iterate over the languages
> > included in the object file, asking each of them whether this language
> > appears to contain a main procedure.  Or even to simply skip the
> > langhook complexity and just call an Ada find-main function!  For gcj
> > I suspect we will just change the debug information.
> 
> I thought about several approaches along the same idea. One of them
> was to implement a sniffing mechanism, with certain sniffers having
> certain priorities. But you know, I really like the idea of dropping
> the langhook, and just call the ada function. Something like this:
> 
>         char *
>         main_name ()
>         {
>           /* If we found the name of main from the debug info, or
>              already looked it up, then return the name of main.  */
>           if (name_of_main != NULL)
>             return name_of_main;
> 
>           /* Is the main in Ada?  */
>           tmp_main_name = ada_find_main_name ();
>           if (tmp_main_name != NULL)
>             {
>               set_main_name (tmp_main_name);
>               return name_of_main;
>             }
> 
>           /* Is the main in Java?  */
>           tmp_main_name = java_find_main_name ();
>           [etc...]
> 
>           /* Fallback: main_name must be the usual "main".  */
>           set_main_name ("main);
>           return name_of_main;
>         }
> 
> The only drawback I see from this is that I will need to include
> ada-lang.h. It would have been nice to avoid this, which is possible
> with the langhooks. I could also add the ada_find_main_name()
> declaration in another more common .h file, but that would be a dirty
> trick, IMO.

This is fine with me.  Let's see if anyone else objects to it.

Since I don't know of any language other than Ada that will have a
fallback for this, I don't think it's worth inventing a lot of
machinery for it.  I'm not especially interested in allowing a language
to be compiled out, either.

-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Paul Gilliam]

On Thursday 20 May 2004 10:14, Daniel Jacobowitz wrote:
> On Thu, May 20, 2004 at 09:02:59AM -0700, Joel Brobecker wrote:
> > Here is how we find the name of the main procedure: We lookup a certain
> > symbol which is created by our binder, __gnat_ada_main_program_name.
> > This symbol points to a string holding the name of the main procedure.
> > You'll find the code in ada-lang.c:begin_command().
>
> OK.  So this has the property that I was hoping for; it won't return a
> false positive for a non-Ada main program.
>
> > > I think the best approach may be to iterate over the languages
> > > included in the object file, asking each of them whether this language
> > > appears to contain a main procedure.  Or even to simply skip the
> > > langhook complexity and just call an Ada find-main function!  For gcj
> > > I suspect we will just change the debug information.
> >
> > I thought about several approaches along the same idea. One of them
> > was to implement a sniffing mechanism, with certain sniffers having
> > certain priorities. But you know, I really like the idea of dropping
> > the langhook, and just call the ada function. Something like this:
> >
> >         char *
> >         main_name ()
> >         {
> >           /* If we found the name of main from the debug info, or
> >              already looked it up, then return the name of main.  */
> >           if (name_of_main != NULL)
> >             return name_of_main;
> >
> >           /* Is the main in Ada?  */
> >           tmp_main_name = ada_find_main_name ();
> >           if (tmp_main_name != NULL)
> >             {
> >               set_main_name (tmp_main_name);
> >               return name_of_main;
> >             }
> >
> >           /* Is the main in Java?  */
> >           tmp_main_name = java_find_main_name ();
> >           [etc...]
> >
> >           /* Fallback: main_name must be the usual "main".  */
> >           set_main_name ("main);
s/"main);/"main");/
> >           return name_of_main;
> >         }
> >
> > The only drawback I see from this is that I will need to include
> > ada-lang.h. It would have been nice to avoid this, which is possible
> > with the langhooks. I could also add the ada_find_main_name()
> > declaration in another more common .h file, but that would be a dirty
> > trick, IMO.
>
> This is fine with me.  Let's see if anyone else objects to it.
>
> Since I don't know of any language other than Ada that will have a
> fallback for this, I don't think it's worth inventing a lot of
> machinery for it.  I'm not especially interested in allowing a language
> to be compiled out, either.

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> OK.  So this has the property that I was hoping for; it won't return a
> false positive for a non-Ada main program.

That's correct.

> This is fine with me.  Let's see if anyone else objects to it.
> 
> Since I don't know of any language other than Ada that will have a
> fallback for this, I don't think it's worth inventing a lot of
> machinery for it.  I'm not especially interested in allowing a language
> to be compiled out, either.

OK, here is a new patch for the "start" command. I don't need to modify
main_name() right now, so I left it out of this patch.

When Ada support is activated, then the next step will be to add the
new-executable notification, and add a call to the ada_main_name
routine in main_name(). I am attaching a patch to symtab.c for
illustration.

2004-05-20  Joel Brobecker  <brobecker@gnat.com>

        * infcmd.c (kill_if_already_running): New function, extracted
        from run_command().
        (run_command): Replace extracted code by call to
        kill_if_already_running().
        (start_command): New function.
        (_initialize_infcmd): Add "start" command.

The documentation for this new command (which has been already approved
by Eli) is also attached, for easier reference.

2004-05-20  Joel Brobecker  <brobecker@gnat.com>

        * gdb.texinfo (Starting): Document new start command.

Tested on x86-linux.

Thanks,
-- 
Joel
Attachment:
infcmd.c.diff
Description: Text document
Attachment:
start-doc.diff
Description: Text document
Attachment:
symtab.c.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Thu, May 20, 2004 at 03:12:14PM -0700, Joel Brobecker wrote:
> OK, here is a new patch for the "start" command. I don't need to modify
> main_name() right now, so I left it out of this patch.
> 
> When Ada support is activated, then the next step will be to add the
> new-executable notification, and add a call to the ada_main_name
> routine in main_name(). I am attaching a patch to symtab.c for
> illustration.

This looks pretty good.  I have one question:

> +  /* Check that there is a program to debug.  Some languages such as Ada
> +     need to search inside the program symbols for the location where to
> +     put the temporary breakpoint before starting.  */
> +  if (!have_full_symbols () && !have_partial_symbols ())
> +    error ("No symbol table loaded.  Use the \"file\" command.");

Shouldn't you accept have_minimal_symbols here?


-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> This looks pretty good.  I have one question:
> 
> > +  /* Check that there is a program to debug.  Some languages such as Ada
> > +     need to search inside the program symbols for the location where to
> > +     put the temporary breakpoint before starting.  */
> > +  if (!have_full_symbols () && !have_partial_symbols ())
> > +    error ("No symbol table loaded.  Use the \"file\" command.");
> 
> Shouldn't you accept have_minimal_symbols here?

Hmmmm, that's a very sharp remark. 

Looking at our current implementation in begin_command, we indeed only
rely on minimal symbols, which makes sense (we only need the address,
we know how to read it afterwards). So a check against
have_minimal_symbols is indeed more appropriate.

Thanks for catching this.
Here is an updated version.

2004-05-20  Joel Brobecker  <brobecker@gnat.com>

        * infcmd.c (kill_if_already_running): New function, extracted
        from run_command().
        (run_command): Replace extracted code by call to
        kill_if_already_running().
        (start_command): New function.
        (_initialize_infcmd): Add "start" command.

-- 
Joel
Attachment:
infcmd.c.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Daniel Jacobowitz]

On Thu, May 20, 2004 at 06:31:33PM -0700, Joel Brobecker wrote:
> > This looks pretty good.  I have one question:
> > 
> > > +  /* Check that there is a program to debug.  Some languages such as Ada
> > > +     need to search inside the program symbols for the location where to
> > > +     put the temporary breakpoint before starting.  */
> > > +  if (!have_full_symbols () && !have_partial_symbols ())
> > > +    error ("No symbol table loaded.  Use the \"file\" command.");
> > 
> > Shouldn't you accept have_minimal_symbols here?
> 
> Hmmmm, that's a very sharp remark. 
> 
> Looking at our current implementation in begin_command, we indeed only
> rely on minimal symbols, which makes sense (we only need the address,
> we know how to read it afterwards). So a check against
> have_minimal_symbols is indeed more appropriate.
> 
> Thanks for catching this.
> Here is an updated version.
> 
> 2004-05-20  Joel Brobecker  <brobecker@gnat.com>
> 
>         * infcmd.c (kill_if_already_running): New function, extracted
>         from run_command().
>         (run_command): Replace extracted code by call to
>         kill_if_already_running().
>         (start_command): New function.
>         (_initialize_infcmd): Add "start" command.

I believe everyone's comments have been satisfied now, so this is OK. 
Please make some minor typographical fixes and check it in:
> +static void
> +run_command (char *args, int from_tty)
> +{
> +  char *exec_file;
> +
> +  dont_repeat ();
> +
> +

Extra blank line.

> +  /* Insert the temporary breakpoint, and run...  */
> +  tbreak_command (main_name(), 0);

Missing space.

> +Run the debugged program until the beginning of the main procedure.\n\
> +This command is a combination of a tbreak command followed by run.\n\
> +You may specify arguments to give to your program, they will be given\n\
> +to the underlying run command.");

How would you feel about this instead?

+Run the debugged program until the beginning of the main procedure.\n\
+You may specify arguments to give to your program, just as with the\n\
+\"run\" command.");


-- 
Daniel Jacobowitz

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> > 2004-05-20  Joel Brobecker  <brobecker@gnat.com>
> > 
> >         * infcmd.c (kill_if_already_running): New function, extracted
> >         from run_command().
> >         (run_command): Replace extracted code by call to
> >         kill_if_already_running().
> >         (start_command): New function.
> >         (_initialize_infcmd): Add "start" command.
> 
> I believe everyone's comments have been satisfied now, so this is OK. 
> Please make some minor typographical fixes and check it in:

Thanks a lot. I made the changes you suggested and committed the patch.
I am attaching the version of the patch that got checked in.

I also checked the documentation in. A change for the NEWS file is
coming up.

> How would you feel about this instead?
> 
> +Run the debugged program until the beginning of the main procedure.\n\
> +You may specify arguments to give to your program, just as with the\n\
> +\"run\" command.");

This looks fine to me. So I used your suggestion.

Thank you,
-- 
Joel
Attachment:
start.diff
Description: Text document

Re: [RFA/RFC] New command: ``start''

[Andrew Cagney]

Hmm,

I just went to describe this new command and found I was struggling with 
``start'' (I described it as ``run-to'').  Given this, and more 
importantly given that ``runto'' is already a part of our vocabulary 
((with "runto_main" in the testsuite) would ``runto'' be a better name?

GDB's CLI can be set up so that all abbreviations such as:

	(gdb) r
	(gdb) ru
	(gdb) run
all map to "run".

Andrew

Re: [RFA/RFC] New command: ``start''

[Joel Brobecker]

> I just went to describe this new command and found I was struggling with 
> ``start'' (I described it as ``run-to'').  Given this, and more 
> importantly given that ``runto'' is already a part of our vocabulary 
> ((with "runto_main" in the testsuite) would ``runto'' be a better name?

``runto'' strikes me as an imcomplete command name (runto what?).

Personally, I much prefer ``start''. I've been using ``begin'' for the
past 3 years so I'm so used to it that I thought it would be a problem
switching to the new command name, but within the two hours or
implementing and testing it, I grew to love it more.

-- 
Joel

Re: [RFA/RFC] New command: ``start''

[Eli Zaretskii]

> Date: Wed, 19 May 2004 08:39:16 -0700
> From: Joel Brobecker <brobecker@gnat.com>
> 
> ``runto'' strikes me as an imcomplete command name (runto what?).

I second that.

> Personally, I much prefer ``start''.

Likewise.

Andrew, can you tell why did you find yourself ``struggling'' with
"start"?  AFAICS, typing "sta TAB" would be all you need, no?

Re: [RFA/RFC] New command: ``start''

[Andrew Cagney]

Date: Wed, 19 May 2004 08:39:16 -0700
From: Joel Brobecker <brobecker@gnat.com>
``runto'' strikes me as an imcomplete command name (runto what?).


I second that.


Personally, I much prefer ``start''.


Likewise.

Andrew, can you tell why did you find yourself ``struggling'' with
"start"?  AFAICS, typing "sta TAB" would be all you need, no?
It depends on how we expect to use it:

(gdb) runto main

flows well but, yes:

(gdb) runto

doesn't.  so I guess which ever.

Andrew