Re: [RFC/RFA] (testsuite/Ada) Add gdb_compile_ada

[Joel Brobecker <brobecker at gnat dot com>]

Thank you very much for your very helpful comments, Daniel!

> OK; any interest in breaking out the language-independent bits of this
> feature for review and discussion?  While it's pretty uninteresting for
> C/C++, it should work there.  For Java and Fortran this could be
> useful, since they also have the concept of a main program.  At least
> GCJ does, I'm not sure about Java in general.

Sure. I'll make a note of starting a new thread about this. I think it
will be useful even in C. Using "begin" is shorter than doing "tbreak
main; run" (although one can use a user-defined function, true).

> By the way, there's a problem with gdb_begin: it can only be used once
> because it counts as a PASS/FAIL with a specific hardcoded name.  It
> should either not be a test, or more likely, should have a unique name.

True (but one could argue that runto_main has the same problem, right?  :-)
We can add an extra optional parameter (to be used only when this
function is used more than once in a given testcase), but...

> At that point, why have a procedure for it?  You're just passing its
> arguments to gdb_test.

... that would not be necessary if we decide that it's not worth
creating this new function. Indeed, it is just as simple to call
gdb_test directly. Except that I just remembered that there are cases
where we will need to pass an argument to the begin command [1].

I know of only one system (VxWorks) where this is extra parameter
is necessary. On this system, and with GNAT, the extra parameter is
identical to the name of the executable (ie we've decided that the entry
point name would be identical to the name of main procedure).  So I
think we eventually want gdb_begin to be intelligent enough to compute
the name of the entry-point, and then fill in the extra parameter
automatically. I haven't thought this through yet, but I see one way of
doing it:

proc gdb_begin {entry_point expected_output opt_id} {
   set begin_command "begin"

   if [is_remote host] {
       if [istarget "*-*-vxworks*"] {
           # The VxWorks debugger requires that we provide the name of
           # the entry-point in the module we want to debug.
           set begin_command "$begin_command $entry_point"
       }
   }

   gdb_test "begin $entry_point" "$expected_output" "begin command $opt_id"
}

As I don't see any supported platform where this would be necessary yet,
I simply suggest that we keep the original gdb_begin procedure,
but with an extra comment inside saying that we expect that this
function might eventually be upgraded to support certain more tricky
cases such as remote-debugging on VxWorks targets for instance (and with
an extra optional parameter to address the issue you raised above).
In the meantime, this function allows us to track all calls to the
"begin" command.

Sounds fair?

> > So far, I don't anticipate anything else in ada.exp, but my little
> > finger tells me that we will likely need something else one day which
> > will fit perfectly into that file. Perhaps even our new gdb_compile_ada
> > procedure should be placed into this file?  I am perfectly happy to put
> > this proc in gdb.exp, however, and drop the idea of a new ada.exp file.
> 
> I'm perfectly happy with it in ada.exp.  If we do that, let's put
> gdb_compile_ada there also.

OK, will move the function there if we agree on keeping gdb_begin.
Which will eventually have to be moved to gdb.exp if we later agree
on implementing the begin command for all languages :-).

> >   - In libgloss.exp, I need to add a new function "find_gnatmake"
> > 
> > <<
> > +proc find_gnatmake {} {
> > +    global tool_root_dir
> > +
> > +    set root "$tool_root_dir/gcc"
> > +    set GM ""
> > +
> > +    if ![is_remote host] {
> > +        set file [lookfor_file $root gnatmake]
> > +        if { $file != "" } {
> > +            set GM "$file -I$root/ada/rts --GCC=$root/xgcc --GNATBIND=$root/gnatbind --GNATLINK=$root/gnatlink -cargs -B$root -largs --GCC=$root/xgcc -margs";
> > +        }
> > +    }
> > +
> > +    if {$GM == ""} {
> > +        set GM [transform gnatmake]
> > +    }
> > +
> > +    return $GM
> > +}
> > +
> > >>
> 
> Looks plausible to me.  Note, we could also put this in ada.exp.  TCL
> doesn't much care.  But if you'll need changes to dejagnu anyway then
> libgloss.exp is probably the right place; I hadn't noticed this next
> bit:
> 
> >   - In target.exp (default_target_compile): I need to add handling of
> >     Ada sources via the "ada" option keyword:
> > 
> > <<
> > @@ -333,6 +333,19 @@ proc default_target_compile {source dest
> >      }
> >  
> >      foreach i $options {
> > +       if { $i == "ada" } {
> > +           set compiler_type "ada"
> > +           if [board_info $dest exists gnatmakeflags] {
> > +               append add_flags " [target_info gnatmakeflags]"
> > +           }
> > +           # append add_flags " [gnatmake_include_flags]";
> > +           if [board_info $dest exists gnatmakecompiler] {
> > +               set compiler [target_info gnatmakecompiler];
> > +           } else {
> > +               set compiler [find_gnatmake];
> > +           }
> > +       }
> > +
> >         if { $i == "c++" } {
> >             set compiler_type "c++"
> >             if [board_info $dest exists cxxflags] {
> 
> How would you feel about adaflags and adacompiler instead, for parallel
> with the other languages?  Maybe adaflags and gnatmake, since gnatmake
> isn't strictly speaking the "compiler".

I am happy with "adaflags", but as you have guessed, I'm a bit
relunctant with "adacompiler". I like "gnatmake" better than
"gnatmakecompiler" (yeeewww!). Maybe "adamake"?

> > @@ -412,6 +425,7 @@ proc default_target_compile {source dest
> >      global CC_FOR_TARGET
> >      global CXX_FOR_TARGET
> >      global F77_FOR_TARGET
> > +    global GNATMAKE_FOR_TARGET
> >      
> >      if [info exists CC_FOR_TARGET] {
> >         if { $compiler == "" } {
> > @@ -428,6 +442,12 @@ proc default_target_compile {source dest
> >      if [info exists F77_FOR_TARGET] {
> >         if { $compiler_type == "f77" } {
> >             set compiler $F77_FOR_TARGET
> > +       }
> > +    }
> > +
> > +    if [info exists GNATMAKE_FOR_TARGET] {
> > +       if { $compiler_type == "ada" } {
> > +           set compiler $GNATMAKE_FOR_TARGET
> >         }
> >      }
> > >> 
> 
> Looks good... these bits will of course have to go to dejagnu@gnu.org
> once you have something working.  I'd prefer to find a way to bail out
> gracefully if the host DejaGNU doesn't have them, too.  We can use
> [info proc find_gnatmake] for that, in gdb_compile_ada.

It sounded like a good idea, and I added the following check.

    if ![info proc find_gnatmake] {
        # If the version of dejagnu being used does not provide
        # procedure find_gnatmake, then we know the build will fail.
        # Better to bail out gracefully now rather than having
        # dejagnu abort the entire run later when it finds it needs
        # this procedure but can't find it.
        verbose "find_gnatmake procedure not found. Update dejagnu?"
        return "Ada compilation failed: find_gnatmake procedure not found."
    }

However, since find_gnatmake will only be called iff target_compile
(which is in dejagnu:target.exp) is upgraded to recognize the "ada"
option keyword. Otherwise, I've tried it, it defaults to using gcc.
And the gdb_compile_ada then fails without aborting the entire
testsuite run. So it looks like we can do without the extra check.

> > Then my new gdb_compile_ada function in gdb.exp simply becomes:
> > 
> > <<
> > proc gdb_compile_ada {source dest objdir type options} {
> > 
> >     append options " ada"
> >     append options " additional_flags=-P${objdir}/gnat_ada"
> > 
> >     set result [target_compile $source $dest $type $options]
> > 
> >     # Make sure that the dest file has been created.  Otherwise,
> >     # the build has failed.
> 
> Please mention explicitly that gnatmake produces output even if
> successful.

Good idea. I just addded:

    # The Ada build always produces some output, even when the build
    # succeeds. Thus, we can not use the output the same way we do in
    # gdb_compile to determine whether the build has succeeded or not.
    # We therefore simply check whether the dest file has been created
    # or not. Unless not present, the build has succeeded.


> The test itself looks sane.

Cool! :-)

Thanks again, Daniel.

-- 
Joel

[1] At least on VxWorks, the user needs to provide the name of the
    entry-point into the "executable" (aka "module") he wants to run.
    This allow GDB to tell the target where to start the execution of
    the inferior from. The extra argument is also necessary for the
    "run" command too.