[PATCH 0/3] GNAT test suite fixes for build sysroot

[PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej W. Rozycki]

Hi,

 In the course of setting up GCC regression testing for the RISC-V target 
I have discovered that the GNAT test suite does not correctly respond to 
the test environment settings passed from the test harness in my setup and 
consequently no test case works correctly.

 In my particular setup `--with-build-sysroot=' has been used to configure 
GCC and also additional linker flags are defined in the target board so 
that adjusted paths are used for the dynamic loader and shared libraries 
to be found in the build sysroot at the test suite run time.  That plays 
well with most of the test suites included with GCC, but not the GNAT one.

 I have tracked down the cause to three issues spread across `gnatmake', 
the GNAT test driver and the DejaGNU target driver.  These issues are 
independent from each other, however all have to be addressed for the test 
suite to run, so I have decided to group them into a series despite that 
each of them can be applied separately, and the DejaGNU one goes to a 
separate project even.

 With them all in place I get these results with the `riscv64-linux-gnu'
target:

		=== gnat Summary ===

# of expected passes		2926
# of unexpected failures	5
# of expected failures		23
# of unsupported tests		26

which I think are pretty good, especially compared to the original results 
without these changes:

		=== gnat Summary ===

# of expected passes		1580
# of unexpected failures	133
# of unexpected successes	23
# of unresolved testcases	875
# of unsupported tests		26

 See individual change descriptions for details.

  Maciej

[PATCH 1/3][GCC] gnatmake: Accept the `--sysroot=' GCC driver option

[Maciej W. Rozycki]

According to `gnatmake' documentation:

"Any uppercase or multi-character switch that is not a 'gnatmake' switch
is passed to 'gcc' (e.g., '-O', '-gnato,' etc.)"

however the `--sysroot=' switch is actually rejected:

gnatmake: invalid switch: --sysroot=...

likely because it is one of the very few GCC driver options that have a 
leading double dash and therefore we don't have a blanket fall-through 
for such switches that would satisfy what our documentation claims.

The option is actually shared between the compiler and the linker, so 
pass the switch to both build stages if requested, removing GNAT 
testsuite issues like:

gnatmake: invalid switch: --sysroot=.../sysroot
compiler exited with status 1
Executing on host: .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result   (timeout = 300)
spawn -ignore SIGHUP .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result
PASS: gnat.dg/abstract_with_anonymous_result.adb (test for excess errors)
UNRESOLVED: gnat.dg/abstract_with_anonymous_result.adb compilation failed to produce executable

in a test environment where `--with-build-sysroot=.../sysroot' has been 
used to build a cross-compiler.  Passing to the compilation stage only 
would lead to errors like:

.../bin/riscv64-linux-gnu-ld: cannot find crt1.o: No such file or directory
.../bin/riscv64-linux-gnu-ld: cannot find -lc
collect2: error: ld returned 1 exit status
gnatlink: error when calling .../gcc/xgcc
gnatmake: *** link failed.
compiler exited with status 1
Executing on host: .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result   (timeout = 300)
spawn -ignore SIGHUP .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result
./abstract_with_anonymous_result.ali
./abstract_with_anonymous_result.o
FAIL: gnat.dg/abstract_with_anonymous_result.adb (test for excess errors)
Excess errors:
.../bin/riscv64-linux-gnu-ld: cannot find crt1.o: No such file or directory
.../bin/riscv64-linux-gnu-ld: cannot find -lc
gnatlink: error when calling .../gcc/xgcc

UNRESOLVED: gnat.dg/abstract_with_anonymous_result.adb compilation failed to produce executable

instead.

	gcc/ada/
	* make.adb (Scan_Make_Arg): Also accept `--sysroot=' for the 
	compiler and the linker.
---
Hi,

 I think treating `--sysroot=' like this makes sense, as otherwise it'd 
have to be specified twice, once with `-largs' and the second time 
optionally with `-cargs'.  It's consistent with how `--param=' is handled.
I'm not sure if this peculiarity should be mentioned in the manual; it 
surely is not for the existing `--param=' switch.

 Unfortunately I have exhausted the limit of changes I can make to GCC 
without my WDC copyright paperwork sorted with FSF.  OK to apply once that 
has been completed?

  Maciej
---
 gcc/ada/make.adb |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

gcc-gnatmake-sysroot.diff
Index: gcc/gcc/ada/make.adb
===================================================================
--- gcc.orig/gcc/ada/make.adb
+++ gcc/gcc/ada/make.adb
@@ -4516,7 +4516,9 @@ package body Make is
                end;
             end if;
 
-         elsif Argv'Length >= 8 and then Argv (1 .. 8) = "--param=" then
+         elsif (Argv'Length >= 8 and then Argv (1 .. 8) = "--param=")
+           or (Argv'Length >= 10 and then Argv (1 .. 10) = "--sysroot=")
+         then
             Add_Switch (Argv, Compiler);
             Add_Switch (Argv, Linker);
 

[PATCH 2/3][GCC] GNAT/testsuite: Pass the `ada' option to target compilation

[Maciej W. Rozycki]

Pass the `ada' option to DejaGNU's `target_compile' procedure, which by 
default calls `default_target_compile', so that it arranges for an Ada 
compilation rather the default of C.  We set the compiler to `gnatmake' 
manually here, so that part of the logic in `default_target_compile' is 
not used, but it affects other settings, such as the use of `adaflags'.

	gcc/testsuite/
	* lib/gnat.exp (gnat_target_compile): Pass the `ada' option to 
	`target_compile'.
---
Hi,

 Unfortunately I have exhausted the limit of changes I can make to GCC
without my WDC copyright paperwork sorted with FSF.  OK to apply once that
has been completed?

  Maciej
---
 gcc/testsuite/lib/gnat.exp |    2 ++
 1 file changed, 2 insertions(+)

gcc-test-gnat-options-ada.diff
Index: gcc/gcc/testsuite/lib/gnat.exp
===================================================================
--- gcc.orig/gcc/testsuite/lib/gnat.exp
+++ gcc/gcc/testsuite/lib/gnat.exp
@@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t
 	set options [concat "additional_flags=$TOOL_OPTIONS" $options]
     }
 
+    set options [concat "{ada}" $options]
+
     return [target_compile $source $dest $type $options]
 }
 

[PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Maciej W. Rozycki]

Unrecognized `gnatmake' switches are not implicitly passed on to the 
linker, so just pasting board `ldflags' and any other linker flags 
verbatim into `add_flags' to use for the invocation line of `gnatmake' 
will make them ignored at best.

For example in a GCC test environment that has:

set_board_info ldflags "-Wl,-dynamic-linker,.../sysroot/lib/ld-linux-riscv64-lp64d.so.1 -Wl,-rpath,.../sysroot/lib64/lp64d -Wl,-rpath,.../sysroot/usr/lib64/lp64d"

so that sysroot paths are correctly embedded with the binaries linked 
for use with the dynamic loader and shared library dependencies, the 
setting will be ignored for the GNAT test suite making all the execution 
tests fail, e.g.:

PASS: gnat.dg/abstract_with_anonymous_result.adb (test for excess errors)
spawn qemu-riscv64 ./abstract_with_anonymous_result.exe
/lib/ld-linux-riscv64-lp64d.so.1: No such file or directory
FAIL: gnat.dg/abstract_with_anonymous_result.adb execution test

For `gnatmake' to pass switches on to the linker the `-largs' switch has 
to be used, which affects all the switches that follow until a switch is
seen that changes the selection, like `-margs', which resets to the 
initial state of the switch interpretation machine.

Wrap linker flags into `-largs'/`-margs' for Ada then, carefully 
preserving the place these flags are placed within `add_flags', as 
surely someone will have depended on that, correcting test failures like 
above:

PASS: gnat.dg/abstract_with_anonymous_result.adb (test for excess errors)
spawn qemu-riscv64 ./abstract_with_anonymous_result.exe
PASS: gnat.dg/abstract_with_anonymous_result.adb execution test

	* lib/target.exp (default_target_compile): Wrap linker flags into 
	`-largs'/`-margs' for Ada.

Signed-off-by: Maciej W. Rozycki <macro@wdc.com>
---
Hi,

 My WDC copyright paperwork has not been sorted with FSF yet, however I 
have not contributed to DejaGNU on behalf of WDC so far and I believe this 
change falls within the limit of roughly 15 lines to be considered legally 
insignificant given that:

"A regular series of repeated changes, such as renaming a symbol, is not
legally significant even if the symbol has to be renamed in many places."

 Please apply then; this does not rely on 2/3 in any way, as we ought to
handle Ada compilations correctly regardless of whether our caller does 
the right thing there.

  Maciej 
---
 lib/target.exp |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

dejagnu-target-ada-ldflags.diff
Index: dejagnu/lib/target.exp
===================================================================
--- dejagnu.orig/lib/target.exp
+++ dejagnu/lib/target.exp
@@ -518,11 +518,12 @@ proc default_target_compile {source dest
     }
 
     if { $type eq "executable" } {
+	set extra_ldflags ""
 	if {[board_info $dest exists ldflags]} {
-	    append add_flags " [board_info $dest ldflags]"
+	    append extra_ldflags " [board_info $dest ldflags]"
 	}
 	if { $compiler_type eq "c++" } {
-	    append add_flags " [g++_link_flags]"
+	    append extra_ldflags " [g++_link_flags]"
 	}
 	if {[isnative]} {
 	    # This is a lose.
@@ -530,16 +531,23 @@ proc default_target_compile {source dest
 	    if { $tmp ne "" } {
 		if {[regexp ".*solaris2.*" $target_triplet]} {
 		    # Solaris 2
-		    append add_flags " -R$tool_root_dir/libstdc++"
+		    append extra_ldflags " -R$tool_root_dir/libstdc++"
 		} elseif {[regexp ".*(osf|irix5|linux).*" $target_triplet]} {
 		    # OSF/1 or IRIX 5
-		    append add_flags " -Wl,-rpath,$tool_root_dir/libstdc++"
+		    append extra_ldflags " -Wl,-rpath,$tool_root_dir/libstdc++"
 		} elseif {[regexp ".*hppa.*" $target_triplet]} {
 		    # HP-UX
-		    append add_flags " -Wl,-a,shared_archive"
+		    append extra_ldflags " -Wl,-a,shared_archive"
 		}
 	    }
 	}
+	if { $extra_ldflags ne "" } {
+	    if { $compiler_type eq "ada" } {
+		append add_flags " -largs $extra_ldflags -margs"
+	    } else {
+		append add_flags " $extra_ldflags"
+	    }
+	}
     }
 
     if {![info exists ldscript]} {
@@ -561,7 +569,11 @@ proc default_target_compile {source dest
     }
 
     if { $type eq "executable" } {
-	append add_flags " $ldflags"
+	if { $compiler_type eq "ada" } {
+	    append add_flags " -largs $ldflags -margs"
+	} else {
+	    append add_flags " $ldflags"
+	}
 	foreach x $libs {
 	    if {[file exists $x]} {
 		append source " $x"

Re: [PATCH 2/3][GCC] GNAT/testsuite: Pass the `ada' option to target compilation

[Jacob Bachmeyer]

Maciej W. Rozycki wrote:
> [...]
> ---
>  gcc/testsuite/lib/gnat.exp |    2 ++
>  1 file changed, 2 insertions(+)
>
> gcc-test-gnat-options-ada.diff
> Index: gcc/gcc/testsuite/lib/gnat.exp
> ===================================================================
> --- gcc.orig/gcc/testsuite/lib/gnat.exp
> +++ gcc/gcc/testsuite/lib/gnat.exp
> @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t
>  	set options [concat "additional_flags=$TOOL_OPTIONS" $options]
>      }
>  
> +    set options [concat "{ada}" $options]
> +
>      return [target_compile $source $dest $type $options]
>  }
>   
Your Tcl syntax looks suspicious to me.  Is there a reason for "ada" to 
be in both double quotes and braces?

Perhaps {lappend options ada} might be simpler?  Is placing ada at the 
beginning of the list important?

-- Jacob

Re: [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Jacob Bachmeyer]

Maciej W. Rozycki wrote:
> Unrecognized `gnatmake' switches are not implicitly passed on to the 
> linker, so just pasting board `ldflags' and any other linker flags 
> verbatim into `add_flags' to use for the invocation line of `gnatmake' 
> will make them ignored at best.
>
> [...]
>
> For `gnatmake' to pass switches on to the linker the `-largs' switch has 
> to be used, which affects all the switches that follow until a switch is
> seen that changes the selection, like `-margs', which resets to the 
> initial state of the switch interpretation machine.
>
> Wrap linker flags into `-largs'/`-margs' for Ada then, carefully 
> preserving the place these flags are placed within `add_flags', as 
> surely someone will have depended on that, [...]

Fortunately, `add_flags' is a procedure local variable in 
default_target_compile, so it is not visible outside of that procedure.

This patch really exposes a significant deficiency in our current 
implementation of default_target_compile:  the order of various flags 
can be significant, but we only have that order implicitly expressed in 
the code, which goes all the way back to (of course) the "Initial 
revision" that is probably from a time before Tcl had the features that 
will allow significant cleanup in here.

Rather than introducing more variables, I propose changing add_flags to 
an array and collecting each category of flags into its own element, 
then emitting those elements in a fixed order to build the `opts' list. 
This would also allow us to easily support other cases, for example, 
prefixing "special" linker flags with "-Wl," or similar handling for 
other frontends. I think we only need to support GCC command syntax, 
which simplifies the issue a bit, but even GCC frontends are not 100% 
consistent, as this issue with gnatmake shows.


What categories do the flags currently accumulated in `add_flags' 
cover?  I see at least:
    - compiler flags (adaflags/cxxflags/dflags/f77flags/f90flags)
    - explicit additional flags ("additional_flags=" option)
    - explicit ldflags ("ldflags=" option)
    - libraries ("libs=" option)
    - preprocessor search paths ("incdir=" option)
    - linker search paths ("libdir=" option and [board_info $dest libs])
    - linker script ("ldscript=" option or [board_info $dest ldscript])
    - optimization flags ("optimize=" option)
    - target compiler flags from host ([board_info $host cflags_for_target])
    - type selection flag ("-c"/"-E"/"-S")
    - target compiler flags ([board_info $dest cflags] *regardless* of 
the compiler selected)
    - target linker flags ([board_info $dest ldflags])
    - special flags for C++ ([g++_link_flags])
    - an attempt to locate libstdc++, also regardless of compiler
    - debug flags, if the "debug" option is given
    - the math library
    - "-Wl,-r" if board_info knows a "remote_link" key
    - "-Wl,-oformat,[board_info $dest output_format]" if that is defined
    - multilib flags, currently *prepended* if defined
    - a destination file

Some of these could probably be combined and I may have combined 
categories that should be separate in the above list.  The GNU toolchain 
has always been a kind of "magic box that just works" (until it doesn't 
and the manual explains the problem) for me, so I am uncertain what the 
ordering rules for combining these categories should be.  Anyone know 
the traditional rules and, perhaps more importantly, what systems need 
which rules?

-- Jacob

Re: [PATCH 2/3][GCC] GNAT/testsuite: Pass the `ada' option to target compilation

[Maciej W. Rozycki]

On Wed, 15 May 2019, Jacob Bachmeyer wrote:

> > Index: gcc/gcc/testsuite/lib/gnat.exp
> > ===================================================================
> > --- gcc.orig/gcc/testsuite/lib/gnat.exp
> > +++ gcc/gcc/testsuite/lib/gnat.exp
> > @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t
> >  	set options [concat "additional_flags=$TOOL_OPTIONS" $options]
> >      }
> >  
> > +    set options [concat "{ada}" $options]
> > +
> >      return [target_compile $source $dest $type $options]
> >  }
> >   
> Your Tcl syntax looks suspicious to me.  Is there a reason for "ada" to 
> be in both double quotes and braces?

 Most existing `options' elements are lists, as shown by:

clone_output "options: $options\n"

placed at the top of `default_target_compile' (leading paths stripped 
here):

options: {ada} {additional_flags=-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never --sysroot=.../sysroot} {additional_flags=-gnatJ -c -u} {compiler=.../gcc/gnatmake --GCC=.../gcc/xgcc --GNATBIND=.../gcc/gnatbind --GNATLINK=.../gcc/gnatlink -cargs -B.../gcc -largs --GCC=.../gcc/xgcc\ -B.../gcc\ -march=rv64imafdc\ -mabi=lp64d -margs --RTS=.../riscv64-linux-gnu/lib64/lp64d/libada -q -f} timeout=300

so I did this for consistency, although in reality it doesn't matter, not 
at least for `default_target_compile', and either approach would work.

 We are not consistent here in `gnat_target_compile' anyway, as you can 
see from the two existing `concat' invocations, and also the `timeout=300' 
element.

> Perhaps {lappend options ada} might be simpler?  Is placing ada at the 
> beginning of the list important?

 It can't be last because we override the default compiler otherwise
selected by this option in `default_target_compile', and then options 
passed in may override it further.  Overall I felt it to be safer if we 
placed the compiler type selection first rather than somewhere in the 
middle.

 I hope it clears your concerns.

  Maciej

Re: [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Maciej W. Rozycki]

On Wed, 15 May 2019, Jacob Bachmeyer wrote:

> This patch really exposes a significant deficiency in our current 
> implementation of default_target_compile:  the order of various flags 
> can be significant, but we only have that order implicitly expressed in 
> the code, which goes all the way back to (of course) the "Initial 
> revision" that is probably from a time before Tcl had the features that 
> will allow significant cleanup in here.

 I suspect the origins may be different, however as valuable as your 
observation is functional problems have precedence over issues with code 
structuring, so we need to fix the problem at hand first.  I'm sure 
DejaGNU maintainers will be happy to review your implementation of code 
restructuring afterwards.

> Some of these could probably be combined and I may have combined 
> categories that should be separate in the above list.  The GNU toolchain 
> has always been a kind of "magic box that just works" (until it doesn't 
> and the manual explains the problem) for me, so I am uncertain what the 
> ordering rules for combining these categories should be.  Anyone know 
> the traditional rules and, perhaps more importantly, what systems need 
> which rules?

 The ordering rules are system-specific I'm afraid and we have to be 
careful not to break working systems out there.  People may be forced to a 
DejaGNU upgrate, due to a newer version of a program being tested having 
such a requirement, and can legitimately expect their system to continue 
working.

 NB I have been repeatedly observing cases where a forced upgrade of a 
system component I neither care nor I am competent about, triggered by an 
upgrade of a component I do care about, caused the system to malfunction 
in a way that I find both unacceptable and extremely hard to debug.  It 
seems to have become more frequent in the recent years, and I find this 
both very frustrating and have wasted lots of time trying to fix the 
damage caused.  I would therefore suggest to take all the measures 
possible to save people from going through such an experience.

 FWIW,

  Maciej

Re: [PATCH 2/3][GCC] GNAT/testsuite: Pass the `ada' option to target compilation

[Jacob Bachmeyer]

Maciej W. Rozycki wrote:
> On Wed, 15 May 2019, Jacob Bachmeyer wrote:
> [...]
>  We are not consistent here in `gnat_target_compile' anyway, as you can 
> see from the two existing `concat' invocations, and also the `timeout=300' 
> element.
>   

That is the GCC testsuite rather than DejaGnu itself, so it is less of a 
concern to me.

>> Perhaps {lappend options ada} might be simpler?  Is placing ada at the 
>> beginning of the list important?
>>     
>
>  It can't be last because we override the default compiler otherwise
> selected by this option in `default_target_compile', and then options 
> passed in may override it further.  Overall I felt it to be safer if we 
> placed the compiler type selection first rather than somewhere in the 
> middle.
>   

This is probably a bug in DejaGnu, (those options should set defaults 
rather than override whatever else has been given) but you will still 
need to work around it for the installed base.

>  I hope it clears your concerns.
>   

As far as the patch to GCC goes, I am not worried.

-- Jacob

Re: [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Jacob Bachmeyer]

Maciej W. Rozycki wrote:
> On Wed, 15 May 2019, Jacob Bachmeyer wrote:
>   
>> This patch really exposes a significant deficiency in our current 
>> implementation of default_target_compile:  the order of various flags 
>> can be significant, but we only have that order implicitly expressed in 
>> the code, which goes all the way back to (of course) the "Initial 
>> revision" that is probably from a time before Tcl had the features that 
>> will allow significant cleanup in here.
>>     
>
>  I suspect the origins may be different, however as valuable as your 
> observation is functional problems have precedence over issues with code 
> structuring, so we need to fix the problem at hand first.  I'm sure 
> DejaGNU maintainers will be happy to review your implementation of code 
> restructuring afterwards.
>   

My concern is that your patch may only solve a small part of the problem 
-- enough to make your specific case work, yes, but then someone else 
will hit other parts of the problem later and we spiral towards "tissue 
of hacks" unmaintainability.

The biggest hint to me that your patch is incomplete is that it only 
adds -largs/-margs to wrap LDFLAGS.  I suspect that there are other 
-?args options that should be used also with other flag sets, but those 
do not appear in this patch.  Do we know what the GNU Ada frontend 
actually expects?

>> Some of these could probably be combined and I may have combined 
>> categories that should be separate in the above list.  The GNU toolchain 
>> has always been a kind of "magic box that just works" (until it doesn't 
>> and the manual explains the problem) for me, so I am uncertain what the 
>> ordering rules for combining these categories should be.  Anyone know 
>> the traditional rules and, perhaps more importantly, what systems need 
>> which rules?
>>     
>
>  The ordering rules are system-specific I'm afraid and we have to be 
> careful not to break working systems out there.  People may be forced to a 
> DejaGNU upgrate, due to a newer version of a program being tested having 
> such a requirement, and can legitimately expect their system to continue 
> working.
>   

We can start by simply preserving the existing ordering until we know 
something should change, but the main goal of my previous message was to 
collect the requirements for a specification for default_target_compile 
so I can write regression tests (some of which will fail due to known 
bugs like broken Ada support in our current implementation) before 
embarking on extensive changes to that procedure.  Improving 
"target.test" was already on my local TODO list.

>  NB I have been repeatedly observing cases where a forced upgrade of a 
> system component I neither care nor I am competent about, triggered by an 
> upgrade of a component I do care about, caused the system to malfunction 
> in a way that I find both unacceptable and extremely hard to debug.  It 
> seems to have become more frequent in the recent years, and I find this 
> both very frustrating and have wasted lots of time trying to fix the 
> damage caused.  I would therefore suggest to take all the measures 
> possible to save people from going through such an experience.
>   

Yes, I have also noticed an attitude that can be summed up as "Who cares 
about backwards compatibility?  New!  Shiny!" usually from people who 
have no clue and no business being anywhere near a source editor.  
(Surprise!  Their code has lots of bugs, usually severe, too.)  The 
problem is not new -- jwz called it out as the "Cascade of 
Attention-Deficit Teenagers" model, noting that it seemed to 
particularly plague GNOME, long ago.
Unfortunately, people with that particular attitude seem to have 
acquired outsize influence over the last few years.  I would suspect an 
organized attack if I were more conspiracy-oriented, but Hanlon's razor 
strongly suggests that this is simply a consequence of lowering barriers 
to entry.

-- Jacob

Re: [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Maciej Rozycki]

On Thu, 16 May 2019, Jacob Bachmeyer wrote:

> >  I suspect the origins may be different, however as valuable as your 
> > observation is functional problems have precedence over issues with code 
> > structuring, so we need to fix the problem at hand first.  I'm sure 
> > DejaGNU maintainers will be happy to review your implementation of code 
> > restructuring afterwards.
> 
> My concern is that your patch may only solve a small part of the problem 
> -- enough to make your specific case work, yes, but then someone else 
> will hit other parts of the problem later and we spiral towards "tissue 
> of hacks" unmaintainability.

 I think however that fixing problems in small steps as they are 
discovered is also a reasonable approach and a way to move forward: 
perfect is the enemy of good.

 So I don't think the prospect of making a comprehensive solution should 
prevent a simple fix for the problem at hand that has been already 
developed from being applied.

 IOW I don't discourage you from developing a comprehensive solution, 
however applying my proposal right away will help at least some people and 
will not block you in any way.

> The biggest hint to me that your patch is incomplete is that it only 
> adds -largs/-margs to wrap LDFLAGS.  I suspect that there are other 
> -?args options that should be used also with other flag sets, but those 
> do not appear in this patch.  Do we know what the GNU Ada frontend 
> actually expects?

 At first glance it looks to me we should be safe overall as compiler 
flags are supposed to be passed through by `gnatmake' (barring switch 
processing bugs, as observed with 1/3), and IIUC assembler flags are 
considered compiler flags for the purpose of this consideration as 
`gnatmake' does not make assembly a separate build stage.  So while we 
could wrap compiler flags into `-cargs'/`-margs', it would only serve to 
avoid possible `gnatmake' switch processing bugs.

 There's also `-bargs' for binder switches, but I can't see any use for it 
here.

 Finally boards are offered the choice of `adaflags', `cflags', 
`cxxflags', etc. for the individual languages, where the correct syntax 
can be used if anything unusual is needed beyond what I have noted above.

 I'll defer any further consideration to the Ada maintainers cc-ed; I do 
hope I haven't missed anything here, but then Ada is far from being my 
primary area of experience.

> >  The ordering rules are system-specific I'm afraid and we have to be 
> > careful not to break working systems out there.  People may be forced to a 
> > DejaGNU upgrate, due to a newer version of a program being tested having 
> > such a requirement, and can legitimately expect their system to continue 
> > working.
> 
> We can start by simply preserving the existing ordering until we know 
> something should change, but the main goal of my previous message was to 
> collect the requirements for a specification for default_target_compile 
> so I can write regression tests (some of which will fail due to known 
> bugs like broken Ada support in our current implementation) before 
> embarking on extensive changes to that procedure.  Improving 
> "target.test" was already on my local TODO list.

 You are welcome to go ahead with your effort as far as I am concerned.

> Unfortunately, people with that particular attitude seem to have 
> acquired outsize influence over the last few years.  I would suspect an 
> organized attack if I were more conspiracy-oriented, but Hanlon's razor 
> strongly suggests that this is simply a consequence of lowering barriers 
> to entry.

 Nod.

  Maciej

Re: [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Jacob Bachmeyer]

Maciej Rozycki wrote:
> On Thu, 16 May 2019, Jacob Bachmeyer wrote:
>
>   
>>>  I suspect the origins may be different, however as valuable as your 
>>> observation is functional problems have precedence over issues with code 
>>> structuring, so we need to fix the problem at hand first.  I'm sure 
>>> DejaGNU maintainers will be happy to review your implementation of code 
>>> restructuring afterwards.
>>>       
>> My concern is that your patch may only solve a small part of the problem 
>> -- enough to make your specific case work, yes, but then someone else 
>> will hit other parts of the problem later and we spiral towards "tissue 
>> of hacks" unmaintainability.
>>     
>
>  I think however that fixing problems in small steps as they are 
> discovered is also a reasonable approach and a way to move forward: 
> perfect is the enemy of good.
>   

Fair enough; observe the small patches I have recently submitted to DejaGnu.

>  So I don't think the prospect of making a comprehensive solution should 
> prevent a simple fix for the problem at hand that has been already 
> developed from being applied.
>   

I recognize a difference between "simple but complete" (an ideal 
sometimes achieved in practice) and "simple because incomplete" (which 
does not actually fix the problem).  My concerns are that your patch may 
be the latter.

>  IOW I don't discourage you from developing a comprehensive solution, 
> however applying my proposal right away will help at least some people and 
> will not block you in any way.
>   

Correct, although, considering how long my FSF paperwork took, I might 
be able to finish a comprehensive patch before your paperwork is 
completed.  :-)

>> The biggest hint to me that your patch is incomplete is that it only 
>> adds -largs/-margs to wrap LDFLAGS.  I suspect that there are other 
>> -?args options that should be used also with other flag sets, but those 
>> do not appear in this patch.  Do we know what the GNU Ada frontend 
>> actually expects?
>>     
>
>  At first glance it looks to me we should be safe overall as compiler 
> flags are supposed to be passed through by `gnatmake' (barring switch 
> processing bugs, as observed with 1/3), and IIUC assembler flags are 
> considered compiler flags for the purpose of this consideration as 
> `gnatmake' does not make assembly a separate build stage.  So while we 
> could wrap compiler flags into `-cargs'/`-margs', it would only serve to 
> avoid possible `gnatmake' switch processing bugs.
>   

I am not sure if those are actually bugs in `gnatmake' or the result of 
an incomplete specification for `gnatmake' -- I suspect that --sysroot= 
may have been added to the rest of GCC after `gnatmake' was written and 
whoever added it did not update the Ada frontend.

>  There's also `-bargs' for binder switches, but I can't see any use for it 
> here.
>
>  Finally boards are offered the choice of `adaflags', `cflags', 
> `cxxflags', etc. for the individual languages, where the correct syntax 
> can be used if anything unusual is needed beyond what I have noted above.
>   

Which also raises the issue of "cflags_for_target" (used regardless of 
language and currently always taken from the "unix" board configuration) 
and how that is supposed to make sense and whether it should be 
similarly split into language-specific values or simply removed.  I have 
already submitted a patch to draw that value from the actual host board 
configuration.

>  I'll defer any further consideration to the Ada maintainers cc-ed; I do 
> hope I haven't missed anything here, but then Ada is far from being my 
> primary area of experience.
>   

Likewise, hopefully some of the Ada maintainers will be able to shed 
light on this issue.  And I hope Ben (the DejaGnu maintainer) is okay -- 
I would have expected him to comment by now.

>>>  The ordering rules are system-specific I'm afraid and we have to be 
>>> careful not to break working systems out there.  People may be forced to a 
>>> DejaGNU upgrate, due to a newer version of a program being tested having 
>>> such a requirement, and can legitimately expect their system to continue 
>>> working.
>>>       
>> We can start by simply preserving the existing ordering until we know 
>> something should change, but the main goal of my previous message was to 
>> collect the requirements for a specification for default_target_compile 
>> so I can write regression tests (some of which will fail due to known 
>> bugs like broken Ada support in our current implementation) before 
>> embarking on extensive changes to that procedure.  Improving 
>> "target.test" was already on my local TODO list.
>>     
>
>  You are welcome to go ahead with your effort as far as I am concerned.
>   

I am working on it.  :-)

-- Jacob

[PING][PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej Rozycki]

On Tue, 14 May 2019, Maciej W. Rozycki wrote:

>  In the course of setting up GCC regression testing for the RISC-V target 
> I have discovered that the GNAT test suite does not correctly respond to 
> the test environment settings passed from the test harness in my setup and 
> consequently no test case works correctly.

 Ping for:

<https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00753.html>
<https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00754.html>
<https://lists.gnu.org/archive/html/dejagnu/2019-05/msg00000.html>

  Maciej

Re: [PING][PATCH 0/3] GNAT test suite fixes for build sysroot

[Arnaud Charlet]

> >  In the course of setting up GCC regression testing for the RISC-V target 
> > I have discovered that the GNAT test suite does not correctly respond to 
> > the test environment settings passed from the test harness in my setup and 
> > consequently no test case works correctly.
> 
>  Ping for:
> 
> <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00753.html>

Have you resolved your copyright assignment issues since then?

The above patch needs to use "or else" instead of "or". OK with this change
on the above patch.

Arno

Re: [PING][PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej Rozycki]

On Wed, 19 Jun 2019, Arnaud Charlet wrote:

> >  Ping for:
> > 
> > <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00753.html>
> 
> Have you resolved your copyright assignment issues since then?

 The ball is now in FSF's court I'm told.

> The above patch needs to use "or else" instead of "or". OK with this change
> on the above patch.

 OK, I have updated that in my patch.

 Technically both variants of the expression achieve the same effect here 
as there is no problem with evaluating both sides of the OR operation in 
all cases, but your suggestion might help the readers avoid scratching 
their heads.

 Thank you for your review.  I will apply the change in due course.

  Maciej

Re: [PING][PATCH 0/3] GNAT test suite fixes for build sysroot

[Arnaud Charlet]

> > Have you resolved your copyright assignment issues since then?
> 
>  The ball is now in FSF's court I'm told.

OK

> > The above patch needs to use "or else" instead of "or". OK with this change
> > on the above patch.
> 
>  OK, I have updated that in my patch.
> 
>  Technically both variants of the expression achieve the same effect here 
> as there is no problem with evaluating both sides of the OR operation in 
> all cases, but your suggestion might help the readers avoid scratching 
> their heads.

The performance isn't the same, and more importantly, this is the documented
Ada coding style for GNAT: https://gcc.gnu.org/onlinedocs/gnat-style/Statements.html#Statements

Arno

Re: [PING][PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej Rozycki]

On Thu, 20 Jun 2019, Arnaud Charlet wrote:

> >  Technically both variants of the expression achieve the same effect here 
> > as there is no problem with evaluating both sides of the OR operation in 
> > all cases, but your suggestion might help the readers avoid scratching 
> > their heads.
> 
> The performance isn't the same, and more importantly, this is the documented
> Ada coding style for GNAT: https://gcc.gnu.org/onlinedocs/gnat-style/Statements.html#Statements

 Thanks for the pointer, good to know!

  Maciej

[PING^2][PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej W. Rozycki]

On Tue, 14 May 2019, Maciej W. Rozycki wrote:

>  In the course of setting up GCC regression testing for the RISC-V target 
> I have discovered that the GNAT test suite does not correctly respond to 
> the test environment settings passed from the test harness in my setup and 
> consequently no test case works correctly.

 Ping for:

<https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00754.html>
<https://lists.gnu.org/archive/html/dejagnu/2019-05/msg00000.html>

  Maciej

Re: [PING^2][PATCH 0/3] GNAT test suite fixes for build sysroot

[Arnaud Charlet]

> > In the course of setting up GCC regression testing for the RISC-V target 
> > I have discovered that the GNAT test suite does not correctly respond to 
> > the test environment settings passed from the test harness in my setup and 
> > consequently no test case works correctly.
> 
>  Ping for:
> 
> <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00754.html>
> <https://lists.gnu.org/archive/html/dejagnu/2019-05/msg00000.html>

Assuming good Ada test results with these changes for both
RISC-V and native linux x86_64, OK for me.

Arno

Re: [PING^2][PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej W. Rozycki]

On Mon, 16 Sep 2019, Arnaud Charlet wrote:

> > > In the course of setting up GCC regression testing for the RISC-V target 
> > > I have discovered that the GNAT test suite does not correctly respond to 
> > > the test environment settings passed from the test harness in my setup and 
> > > consequently no test case works correctly.
> > 
> >  Ping for:
> > 
> > <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00754.html>
> > <https://lists.gnu.org/archive/html/dejagnu/2019-05/msg00000.html>
> 
> Assuming good Ada test results with these changes for both
> RISC-V and native linux x86_64, OK for me.

 I have completed regression-testing now and there were no changes at all 
for native x86_64/Linux and only progressions for crossed RISC-V/Linux:

$ diff -u old/gnat.sum new/gnat.sum | sed -n '/^\(+FAIL\|-PASS\)/p;/Summary/,${s,^.*/,,;p}'
 		=== gnat Summary ===
 
-# of expected passes		2137
-# of unexpected failures	710
+# of expected passes		3184
+# of unexpected failures	6
 # of expected failures		23
 # of unsupported tests		28
gnatmake version 10.0.0 20190917 (experimental)

I have therefore committed 2/3 now, however 3/3 requires a DejaGNU 
maintainer's attention.

 Thanks for your review.

  Maciej

[PING^3][PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Maciej W. Rozycki]

On Tue, 21 May 2019, Jacob Bachmeyer wrote:

> >  IOW I don't discourage you from developing a comprehensive solution, 
> > however applying my proposal right away will help at least some people and 
> > will not block you in any way.
> >   
> 
> Correct, although, considering how long my FSF paperwork took, I might 
> be able to finish a comprehensive patch before your paperwork is 
> completed.  :-)

 So by now the FSF paperwork has been long completed actually.

> >  You are welcome to go ahead with your effort as far as I am concerned.
> >   
> 
> I am working on it.  :-)

 Hopefully you have made good progress now.  Otherwise this is a ping for:

<https://lists.gnu.org/archive/html/dejagnu/2019-05/msg00000.html>

Once complete your change can go on top of mine and meanwhile we'll have 
a working GCC test suite.

  Maciej

Re: [PING^3][PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada

[Jacob Bachmeyer]

Maciej W. Rozycki wrote:
> On Tue, 21 May 2019, Jacob Bachmeyer wrote:
>   
>>>  IOW I don't discourage you from developing a comprehensive solution, 
>>> however applying my proposal right away will help at least some people and 
>>> will not block you in any way.
>>>   
>>>       
>> Correct, although, considering how long my FSF paperwork took, I might 
>> be able to finish a comprehensive patch before your paperwork is 
>> completed.  :-)
>>     
>
>  So by now the FSF paperwork has been long completed actually.
>   

Yes, that turned out to be optimistic, for a few reasons.

>>>  You are welcome to go ahead with your effort as far as I am concerned.
>>>   
>>>       
>> I am working on it.  :-)
>>     
>
>  Hopefully you have made good progress now.

Progress is stalled because the DejaGNU maintainer has not been seen on 
this list since March and I am unsure about working too far ahead of the 
"accepted" line.

> Otherwise this is a ping for:
>
> <https://lists.gnu.org/archive/html/dejagnu/2019-05/msg00000.html>
>
> Once complete your change can go on top of mine and meanwhile we'll have 
> a working GCC test suite.
>   

There might be a merge conflict, but that will be easy to resolve by 
overwriting your patch with mine.  I will make sure to include the 
functionality in the rewrite.

I have just sent a patch to the list that has been waiting in my local 
repository since June.  It adds unit tests for the 
default_target_compile procedure, but currently verifies the broken Ada 
handling.  Would you be willing to supply a patch to update those tests 
to the correct behavior?  If so, I will also merge your code on my local 
branch and we might even avoid the merge conflict down the line.

While you are doing that, could you also explain what the various -?args 
GNU Ada driver options do and if any others are needed or could be 
needed?  I will ensure that the rewrite handles all cases if I can get a 
solid description of what those cases actually are.  The rewrite will 
group complier/linker/etc. options in separate lists internally, so 
using those options will be easy without adding more hacks to a 
procedure that has already become a tissue of hacks.

-- Jacob