Re: [RFA] Ensure deterministic result order in gdb.ada/info_auto_lang.exp

[Philippe Waroquiers <philippe.waroquiers@skynet.be>]

On Tue, 2018-12-18 at 23:30 -0500, Simon Marchi wrote:
> Adding Joel in CC so he has a chance to see this.
> 
> On 2018-12-01 8:38 a.m., Philippe Waroquiers wrote:
> > standard_ada_testfile, standard_test_file and the explicit
> > csrcfile assignment in info_auto_lang.exp all gives similar pathnames
> > prefix for a source, such as
> > /home/philippe/gdb/git/build_binutils-gdb/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.<something>.
> > 
> > However, the gnat compiler normalizes Ada sources path when compiling.
> > So, the 'Ada' .o object are referencing a pathname such as
> > /home/philippe/gdb/git/binutils-gdb/gdb/testsuite/gdb.ada/info_auto_lang/proc_in_ada.adb,
> > while the 'C' .o object still references the not normalized pathname.
> > 
> > As the results of 'info functions | ...' are sorted by pathname first,
> > the order of the results depends on the comparison between different directories,
> > leading to results that can change depending on these directories.
> > 
> > => Ensure the result order is always the same, by normalising the C source file.
> > 
> > Tested by running the testcase in 2 different builds, that without normalize
> > were giving different results.
> > 
> > Note: such 'set csrcfile' is used in 4 other tests mixing Ada and C.
> > If deemed better (Joel?), I can factorize building such a csrcfile
> > and normalizing it in an ada.exp standard_csrcfile_for_ada function.
> > 
> > gdb/testsuite/ChangeLog
> > 2018-12-01  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> > 	* gdb.ada/info_auto_lang.exp: Normalize some_c source file.
> > 	Update order of results accordingly.
> 
> I'd just like to clarify, because I don't think I see the same current behavior as you.  When I
> run the test without your patch applied, I see this output, for example:
> 
>  83 info functions proc_in_^M
>  84 All functions matching regular expression "proc_in_":^M
>  85 ^M
>  86 File /home/simark/src/binutils-gdb/gdb/testsuite/gdb.ada/info_auto_lang/proc_in_ada.adb:^M
>  87 17:     procedure proc_in_ada;^M
>  88 ^M
>  89 File /home/simark/src/binutils-gdb/gdb/testsuite/gdb.ada/info_auto_lang/some_c.c:^M
>  90 24:     void proc_in_c(void);^M
>  91 (gdb) FAIL: gdb.ada/info_auto_lang.exp: language_choice=auto: frame=0, frame_lang=c: info functions proc_in_
> 
> If I understand correctly, in your case, you see the C source file as non-normalized?  Note
> that I also build out-of-tree.
Effectively. Here is what I have without the patch :
  info functions proc_in_
  All functions matching regular expression "proc_in_":

  File /bd/home/philippe/gdb/git/binutils-gdb/gdb/testsuite/gdb.ada/info_auto_lang/proc_in_ada.adb:
  17:	procedure proc_in_ada;

  File /bd/home/philippe/gdb/git/build_binutils-gdb/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.ada/info_auto_lang/some_c.c:
  24:	void proc_in_c(void);
  (gdb) FAIL: gdb.ada/info_auto_lang.exp: language_choice=auto: frame=0, frame_lang=c: info functions proc_in_

And with the patch:
  info functions proc_in_
  All functions matching regular expression "proc_in_":

  File /bd/home/philippe/gdb/git/info_t/gdb/testsuite/gdb.ada/info_auto_lang/proc_in_ada.adb:
  17:	procedure proc_in_ada;

  File /bd/home/philippe/gdb/git/info_t/gdb/testsuite/gdb.ada/info_auto_lang/some_c.c:
  24:	void proc_in_c(void);
  (gdb) PASS: gdb.ada/info_auto_lang.exp: language_choice=auto: frame=0, frame_lang=c: info functions proc_in_

> 
> Since both paths appear already normalized in my case, the test does fails for me and your patch
> would fix it.  I am 99% sure I am fine with your fix, I just want to make sure we are on the same
> page.
A mystery to me why paths are already normalized in your build ...
What compilation command do you see in the gdb.log for some_c.c ?

> 
> About the other tests you mention, would it be important to normalize the paths there too, or
> you suggest to do it just for consistency?
As far as I know, for the other Ada tests, there is currently no need to normalize.
But we currently have the below:
grep 'set csrcfile' gdb/testsuite/gdb.ada/*exp
gdb/testsuite/gdb.ada/arraydim.exp:set csrcfile ${srcdir}/${subdir}/${testdir}/${cfile}.c
gdb/testsuite/gdb.ada/bp_c_mixed_case.exp:set csrcfile ${srcdir}/${subdir}/${testdir}/${cfile}.c
gdb/testsuite/gdb.ada/bp_c_mixed_case.exp:set csrcfile2 ${srcdir}/${subdir}/${testdir}/${cfile2}.c
gdb/testsuite/gdb.ada/cond_lang.exp:set csrcfile ${srcdir}/${subdir}/${testdir}/${cfile}.c
gdb/testsuite/gdb.ada/info_auto_lang.exp:set csrcfile [file normalize ${srcdir}/${subdir}/${testdir}/${cfile}.c]
gdb/testsuite/gdb.ada/lang_switch.exp:set csrcfile ${srcdir}/${subdir}/${testdir}/${cfile}.c

It looks a reasonable idea to factorize this in standard_csrcfile_for_ada,
and in this function, always normalize cannot harm, IMO.

But we can for sure work without that function, up to Joel to say if he believes it is
better to factorize (in this patch, or in another patch).

Thanks

Philippe