Re: gdbserver/ada testing broken

[Simon Marchi <simon.marchi@ericsson.com>]

On 16-06-30 08:10 AM, Pedro Alves wrote:
> BTW, while testing this, I noticed that all gdb.ada/ tests that build
> a binary with the same name as their test directory are broken.
> I suspect this is a regression caused by one of the gdb_remote_download,
> etc. patches.
> 
> For example:
> 
>  Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.ada/formatted_ref.exp ...
>  PASS: gdb.ada/formatted_ref.exp: compilation formatted_ref.adb
>  FAIL: gdb.ada/formatted_ref.exp: print/x s
>  FAIL: gdb.ada/formatted_ref.exp: print/x s'access
>  FAIL: gdb.ada/formatted_ref.exp: print s.x = 13
> 
> gdb.log shows:
> 
>  The program is not being run.
>  (gdb) file /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.ada/formatted_ref/formatted_ref/formatted_ref
>  Reading symbols from /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.ada/formatted_ref/formatted_ref/formatted_ref...done.
> ...
>  (gdb) spawn ../gdbserver/gdbserver --once :2346 /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.ada/formatted_ref/formatted_ref
>  Process /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.ada/formatted_ref/formatted_ref created; pid = 7553
>  Cannot exec /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.ada/formatted_ref/formatted_ref: Permission denied.
> 
>  Child exited with status 127
>  No program to debug
> 
> Note how gdb loads:
> 
>   .../gdb.ada/formatted_ref/formatted_ref/formatted_ref
> 
> while gdbserver tries to load:
> 
>   .../gdb.ada/formatted_ref/formatted_ref
> 
> ... which fails because it's the directory.
> 
> This is probably because gdb_remote_download only minds "tail" when
> deciding the destination filename:
> 
> proc gdb_remote_download {dest fromfile {tofile {}}} {
>     # If TOFILE is not given, default to the same filename as FROMFILE.
>     if {[string length $tofile] == 0} {
> 	set tofile [file tail $fromfile]
>     }
> 
>     if {[is_remote $dest]} {
> 	# When the DEST is remote, we simply send the file to DEST.
> 	global cleanfiles
> 
> 	set destname [remote_download $dest $fromfile $tofile]
> 	lappend cleanfiles $destname
> 
> 	return $destname
> 
> 
> and then gdbserver-base.exp does:
> 
> proc ${board}_download { board host dest } {
>     # We pass DEST in standard_output_file, regardless of whether it is absolute
>     # or relative, because we don't want the tests to be able to write outside
>     # their standard output directory.
>     set dest [standard_output_file $dest]
> 
>     file copy -force $host $dest
> 
>     return $dest
> }
> 
> Here $dest already exists when we get here -- it's the directory.
> 
> I haven't confirmed, just inspected the code.

I think you are right.

> Not sure exactly what the best fix is.  Maybe simply tweak
> the tests to never get into this situation, along with
> making standard_ada_testfile issue perror when it happens?
> 
> Most tests pass an explicit base_file name to standard_ada_testfile
> that avoids the conflict, like:
> 
>   gdb.ada/str_ref_cmp.exp:18:standard_ada_testfile foo
> 
> Maybe standard_ada_testfile's $base_file parameter should be
> defaulted, like:
> 
>  -proc standard_ada_testfile {base_file {dir ""}} {
>  +proc standard_ada_testfile {{base_file "test"} {dir ""}} {
> 
> and then most tests adjusted to not pass an explicit
> base_file at all.
> 
> Thanks,
> Pedro Alves
> 

Is there a reason have a double level of directories named after the test?

outputs
└── gdb.ada
    └── formatted_ref
        └── formatted_ref
            ├── b~formatted_ref.adb
            ├── b~formatted_ref.ads
            ├── b~formatted_ref.ali
            ├── b~formatted_ref.o
            ├── defs.ali
            ├── defs.o
            ├── formatted_ref
            ├── formatted_ref.ali
            └── formatted_ref.o

If we removed the extra "formatted_ref" level, we wouldn't have the problem at all.
The source and destination would become identical when running locally, just like
when testing C/C++.  Or maybe it's needed for some reason in Ada?

I made this patch quickly:
https://github.com/simark/binutils-gdb/commit/d451ff22f1980d4a89c5d6d126bc839734fa393f

A few tests that assume the current directory layout need to be adjusted, but other
than that it looks fine.  I'll clean it up, give it more testing and post it tomorrow,
unless you say it's a wrong approach in the mean time.