public inbox for libffi-discuss@sourceware.org
 help / color / mirror / Atom feed
* [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite
@ 2020-04-03 22:55 Maciej W. Rozycki
  2020-04-03 22:55 ` [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU Maciej W. Rozycki
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-03 22:55 UTC (permalink / raw)
  To: libffi-discuss; +Cc: Anthony Green, Mike Stump, gcc-patches

Hi,

 In the course of a discussion at one of the GCC mailing lists here: 
<https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542761.html> I have 
realised that some parts related to libffi testing have not been merged to 
GCC, and they are needed to choose the right compiler in a cross-compiler 
build environment where the build root option has been used.

 Additionally the right target run-time loader's library path has to be 
set via the LD_LIBRARY_PATH environment variable for running the test 
suite in a cross-compilation environment where the location of the host 
libraries is obviously irrelevant.

 We are keen to keep the GCC's copy of libffi as close as possible to the 
upstream version however while usable the current solution has some issues 
which we would rather avoid.  I have therefore decided to address some of 
them with the intent to have the result backported to GCC.  As it stands 
I'm told the current version of libffi cannot be fully merged to GCC, as 
there have been an ABI change that will require technical evaluation.  So 
the intent has been to backport the changes proposed here individually.

 See the individual change descriptions and any further discussion 
included for full details of each patch proposed.

 These changes have been regression-tested with the `x86_64-linux-gnu' 
native configuration, and also the `x86_64-linux-gnu' host and the 
`riscv64-linux-gnu' target using RISC-V QEMU in the Linux user emulation 
mode as the target board.  In the latter case I actually dropped libffi 
into GCC as a replacement of the version included there, with a minor 
update like below (+script regeneration) to add multilib support.

 Any questions, comments, or concerns?  Otherwise, please apply.

  Maciej

Index: gcc/libffi/Makefile.am
===================================================================
--- gcc.orig/libffi/Makefile.am
+++ gcc/libffi/Makefile.am
@@ -2,7 +2,7 @@
 
 AUTOMAKE_OPTIONS = foreign subdir-objects
 
-ACLOCAL_AMFLAGS = -I m4
+ACLOCAL_AMFLAGS = -I m4 -I ../config
 
 SUBDIRS = include testsuite man
 if BUILD_DOCS
@@ -158,3 +158,12 @@ AM_CCASFLAGS = $(AM_CPPFLAGS)
 	if [ -d $(top_srcdir)/.git ] ; then (cd $(top_srcdir); git log --no-decorate) ; else echo 'See git log for history.' ; fi > $(distdir)/ChangeLog
 	s=`awk '/was released on/{ print NR; exit}' $(top_srcdir)/README.md`; tail -n +$$(($$s-1)) $(top_srcdir)/README.md > $(distdir)/README.md
 
+# Multilib support.  Automake should provide these on its own.
+all-recursive: all-multi
+install-recursive: install-multi
+mostlyclean-recursive: mostlyclean-multi
+clean-recursive: clean-multi
+distclean-recursive: distclean-multi
+maintainer-clean-recursive: maintainer-clean-multi
+
+include $(top_srcdir)/../multilib.am
Index: gcc/libffi/configure.ac
===================================================================
--- gcc.orig/libffi/configure.ac
+++ gcc/libffi/configure.ac
@@ -5,6 +5,8 @@ AC_PREREQ(2.68)
 AC_INIT([libffi], [3.3], [http://github.com/libffi/libffi/issues])
 AC_CONFIG_HEADERS([fficonfig.h])
 
+AM_ENABLE_MULTILIB(, ..)
+
 AC_CANONICAL_SYSTEM
 target_alias=${target_alias-$host_alias}
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU
  2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
@ 2020-04-03 22:55 ` Maciej W. Rozycki
  2020-04-06 17:58   ` Jeff Law
  2020-04-03 22:55 ` [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor " Maciej W. Rozycki
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-03 22:55 UTC (permalink / raw)
  To: libffi-discuss; +Cc: Anthony Green, Mike Stump, gcc-patches

Use an Autoconf template rather an inline piece of scriptery to set 
DejaGNU's $CC_FOR_TARGET and $CXX_FOR_TARGET variables from $CC and $CXX 
respectively, making it easier to maintain and making it take advantage 
of Automake's dependency and rule generation.  Relocate the generated 
`local.exp' file to within testsuite/ so as to make its regeneration 
rule to actually work, i.e. (in testsuite/Makefile.in):

EXTRA_DEJAGNU_SITE_CONFIG = local.exp
site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
	[...]
local.exp: $(top_builddir)/config.status $(srcdir)/local.exp.in
	cd $(top_builddir) && $(SHELL) ./config.status $(subdir)/$@

It wouldn't work if the regeneration rule was placed in the top-level 
Makefile.in, which is what Automake would faithfully do if `local.exp' 
stayed in the top level directory.
---
Hi,

 I think having individual AC_CONFIG_FILES macro invocations for each 
output file or group of files would make this change (and code itself) 
more readable, however it hasn't been done before and I decided not to 
change the style on this occasion.  It may make sense as a follow-up 
clean-up.

  Maciej
---
 Makefile.am            |    3 ---
 configure.ac           |    7 +------
 testsuite/Makefile.am  |    2 +-
 testsuite/local.exp.in |    2 ++
 4 files changed, 4 insertions(+), 10 deletions(-)

libffi-test-cc-for-target-template.diff
Index: libffi/Makefile.am
===================================================================
--- libffi.orig/Makefile.am
+++ libffi/Makefile.am
@@ -23,9 +23,6 @@ EXTRA_DIST = LICENSE ChangeLog.old					\
 	libtool-ldflags libtool-version configure.host README.md        \
 	libffi.map.in LICENSE-BUILDTOOLS msvc_build make_sunver.pl	
 
-# local.exp is generated by configure
-DISTCLEANFILES = local.exp
-
 # Subdir rules rely on $(FLAGS_TO_PASS)
 FLAGS_TO_PASS = $(AM_MAKEFLAGS)
 
Index: libffi/configure.ac
===================================================================
--- libffi.orig/configure.ac
+++ libffi/configure.ac
@@ -56,11 +56,6 @@ if test "x$GCC" = "xyes"; then
   CFLAGS="$CFLAGS -fexceptions"
 fi
 
-cat > local.exp <<EOF
-set CC_FOR_TARGET "$CC"
-set CXX_FOR_TARGET "$CXX"
-EOF
-
 AM_MAINTAINER_MODE
 
 AC_CHECK_HEADERS(sys/mman.h)
@@ -401,7 +396,7 @@ test -d src || mkdir src
 test -d src/$TARGETDIR || mkdir src/$TARGETDIR
 ], [TARGETDIR="$TARGETDIR"])
 
-AC_CONFIG_FILES(include/Makefile include/ffi.h Makefile testsuite/Makefile man/Makefile doc/Makefile libffi.pc)
+AC_CONFIG_FILES(include/Makefile include/ffi.h Makefile testsuite/Makefile testsuite/local.exp man/Makefile doc/Makefile libffi.pc)
 
 AC_OUTPUT
 
Index: libffi/testsuite/Makefile.am
===================================================================
--- libffi.orig/testsuite/Makefile.am
+++ libffi/testsuite/Makefile.am
@@ -2,7 +2,7 @@
 
 AUTOMAKE_OPTIONS = foreign dejagnu
 
-EXTRA_DEJAGNU_SITE_CONFIG=../local.exp
+EXTRA_DEJAGNU_SITE_CONFIG = local.exp
 
 CLEANFILES = *.exe core* *.log *.sum
 
Index: libffi/testsuite/local.exp.in
===================================================================
--- /dev/null
+++ libffi/testsuite/local.exp.in
@@ -0,0 +1,2 @@
+set CC_FOR_TARGET "@CC@"
+set CXX_FOR_TARGET "@CXX@"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor to DejaGNU
  2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
  2020-04-03 22:55 ` [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU Maciej W. Rozycki
@ 2020-04-03 22:55 ` Maciej W. Rozycki
  2020-04-06 18:00   ` Jeff Law
  2020-04-03 22:56 ` [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET Maciej W. Rozycki
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-03 22:55 UTC (permalink / raw)
  To: libffi-discuss; +Cc: Anthony Green, Mike Stump, gcc-patches

Use Autoconf substitution in the template used for extra DejaGNU site 
configuration, which is a documented supported way to pass information 
from the `configure' script, rather than resorting to a hack with 
extracting an undocumented internal setting from `config.log' to pass 
the compiler vendor to DejaGNU.  No functional change (and no risk of 
breakage with some Autoconf version anymore).

Use AM_SUBST_NOTMAKE with the new substitution so as not to place it in 
Makefile.in files, where it is not needed, and set the Autmoake version 
requirement accordingly.
---
Hi,

 I chose to use AM_SUBST_NOTMAKE so as not to clutter Makefile.in files 
with the new variable as Automake does that by default.  That however 
requires the use of Automake 1.11 or newer.  By the look of our sources 
that shouldn't be an issue as far as I can tell, but the macro invocation 
can be dropped along with the requirement if it would.

  Maciej
---
 Makefile.am              |    3 ++-
 configure.ac             |    2 ++
 testsuite/lib/libffi.exp |    4 ----
 testsuite/local.exp.in   |    1 +
 4 files changed, 5 insertions(+), 5 deletions(-)

libffi-compiler-vendor.diff
Index: libffi/Makefile.am
===================================================================
--- libffi.orig/Makefile.am
+++ libffi/Makefile.am
@@ -1,6 +1,7 @@
 ## Process this with automake to create Makefile.in
 
-AUTOMAKE_OPTIONS = foreign subdir-objects
+# Automake 1.11 needed for AM_SUBST_NOTMAKE.
+AUTOMAKE_OPTIONS = 1.11 foreign subdir-objects
 
 ACLOCAL_AMFLAGS = -I m4
 
Index: libffi/configure.ac
===================================================================
--- libffi.orig/configure.ac
+++ libffi/configure.ac
@@ -45,6 +45,8 @@ AC_CONFIG_MACRO_DIR([m4])
 AC_CHECK_SIZEOF([size_t])
 
 AX_COMPILER_VENDOR
+AC_SUBST([compiler_vendor], [$ax_cv_c_compiler_vendor])
+AM_SUBST_NOTMAKE([compiler_vendor])
 AX_CC_MAXOPT
 # The AX_CFLAGS_WARN_ALL macro doesn't currently work for sunpro
 # compiler.
Index: libffi/testsuite/lib/libffi.exp
===================================================================
--- libffi.orig/testsuite/lib/libffi.exp
+++ libffi/testsuite/lib/libffi.exp
@@ -286,10 +286,6 @@ proc libffi-init { args } {
 
     verbose "libffi $blddirffi"
 
-    # Which compiler are we building with?
-    set tmp [grep "$blddirffi/config.log" "^ax_cv_c_compiler_vendor.*$"]
-    regexp -- {^[^=]*=(.*)$} $tmp nil compiler_vendor
-
     if { [string match $compiler_vendor "gnu"] } {
         set gccdir [lookfor_file $tool_root_dir gcc/libgcc.a]
         if {$gccdir != ""} {
Index: libffi/testsuite/local.exp.in
===================================================================
--- libffi.orig/testsuite/local.exp.in
+++ libffi/testsuite/local.exp.in
@@ -1,2 +1,3 @@
 set CC_FOR_TARGET "@CC@"
 set CXX_FOR_TARGET "@CXX@"
+set compiler_vendor "@compiler_vendor@"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET
  2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
  2020-04-03 22:55 ` [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU Maciej W. Rozycki
  2020-04-03 22:55 ` [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor " Maciej W. Rozycki
@ 2020-04-03 22:56 ` Maciej W. Rozycki
  2020-04-06 18:03   ` Jeff Law
  2020-04-03 22:56 ` [PATCH libffi 4/4] Correct indentation throughout `libffi-init' Maciej W. Rozycki
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-03 22:56 UTC (permalink / raw)
  To: libffi-discuss; +Cc: Anthony Green, Mike Stump, gcc-patches

Update code in `libffi-init' to use $CC_FOR_TARGET in determining the 
value of $ld_library_path, as using a different compiler location from 
one actually used in testing may have odd consequences.

As this obviously loses the setting of $gccdir provide a replacement way 
to determine the directory if feasible, however prefer the location of 
shared libgcc over static libgcc.  This helps in configurations where 
shared libgcc is, unlike libgcc, a location that is not specific to the 
compiler version, a common scenario.  It does not address the scenario 
however where there is a shared libgcc symlink installed pointing to the 
actual run-time library installed elsewhere; this would have to be dealt 
with in a board description file specific to the installation.

Use `remote_exec host' rather than `exec' to invoke the compiler so as 
to support remote configurations and also avoid the latter procedure's 
path length limitation that prevents execution in some actual scenarios.

As using `remote_exec host' precludes the use of our existing file name 
globbing to examine directory contents, reuse, with minor modifications 
needed to adjust to our structure, the piece I previously contributed to 
GCC with commit d42b84f427e4 ("testsuite: Fix run-time tracking down 
of `libgcc_s'").
---
Hi,

 This has its limitation in that it still doesn't default to the same 
compiler as `target_compile' (`default_target_compile') from target.exp in 
DejaGNU does, but I believe it is a step in the right direction.  That 
will only affect standalone (e.g. installed) testing iff $CC_FOR_TARGET 
hasn't been set.

 Also for C++ compilation our carefully crafted $ld_library_path is 
unfortunately overridden by `g++_link_flags' from libgloss.exp in DejaGNU 
called in `default_target_compile'.  This actually does cause test 
failures in my `riscv64-linux-gnu' cross-compilation setup:

FAIL: libffi.closures/unwindtest.cc -W -Wall -Wno-psabi -O0 execution test
FAIL: libffi.closures/unwindtest.cc -W -Wall -Wno-psabi -O2 execution test
FAIL: libffi.closures/unwindtest_ffi_call.cc -W -Wall -Wno-psabi -O0 execution test
FAIL: libffi.closures/unwindtest_ffi_call.cc -W -Wall -Wno-psabi -O2 execution test

and I am currently not sure how to best address it, i.e. whether to change
DejaGNU's `g++_link_flags' or to take advantage of a TCL's feature and 
provide our own copy of the procedure.  Something to consider sometime.

 NB I chose not to correct obvious indentation issues with lines not 
touched by this change so as not to obfuscate the patch unnecessarily.  A 
complementing formatting change follows.

  Maciej
---
 testsuite/lib/libffi.exp |   68 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 51 insertions(+), 17 deletions(-)

Index: libffi/testsuite/lib/libffi.exp
===================================================================
--- libffi.orig/testsuite/lib/libffi.exp
+++ libffi/testsuite/lib/libffi.exp
@@ -272,7 +272,7 @@ proc libffi-init { args } {
     global srcdir
     global blddirffi
     global objdir
-    global TOOL_OPTIONS
+    global CC_FOR_TARGET
     global tool
     global libffi_include
     global libffi_link_flags
@@ -287,29 +287,63 @@ proc libffi-init { args } {
     verbose "libffi $blddirffi"
 
     if { [string match $compiler_vendor "gnu"] } {
-        set gccdir [lookfor_file $tool_root_dir gcc/libgcc.a]
-        if {$gccdir != ""} {
-	    set gccdir [file dirname $gccdir]
-        }
+	if [info exists CC_FOR_TARGET] then {
+	    set compiler "$CC_FOR_TARGET"
+	    set libgcc_a_x [remote_exec host "$compiler" \
+			    "-print-file-name=libgcc.a"]
+	    if { [lindex $libgcc_a_x 0] == 0 } {
+		set gccdir [file dirname [lindex $libgcc_a_x 1]]
+	    } else {
+		set gccdir ""
+	    }
+	} else {
+	    set gccdir [lookfor_file $tool_root_dir gcc/libgcc.a]
+	    if {$gccdir != ""} {
+		set gccdir [file dirname $gccdir]
+	    }
+	    set compiler "${gccdir}/xgcc"
+	}
         verbose "gccdir $gccdir"
 
+	set shlib_ext [get_shlib_extension]
+	set libgcc_s_x [remote_exec host "$compiler" \
+			"-print-file-name=libgcc_s.${shlib_ext}"]
+	if { [lindex $libgcc_s_x 0] == 0 } {
+	    set libgcc_dir [file dirname [lindex $libgcc_s_x 1]]
+	} else {
+	    set libgcc_dir $gccdir
+	}
+	verbose "libgcc_dir $libgcc_dir"
+
         set ld_library_path "."
         append ld_library_path ":${gccdir}"
 
-        set compiler "${gccdir}/xgcc"
-        if { [is_remote host] == 0 && [which $compiler] != 0 } {
-	    foreach i "[exec $compiler --print-multi-lib]" {
-	        set mldir ""
-	        regexp -- "\[a-z0-9=_/\.-\]*;" $i mldir
-	        set mldir [string trimright $mldir "\;@"]
-	        if { "$mldir" == "." } {
+	set multi_dir_x [remote_exec host "$compiler" "-print-multi-directory"]
+	set multi_lib_x [remote_exec host "$compiler" "-print-multi-lib"]
+	if { [lindex $multi_dir_x 0] == 0 && [lindex $multi_lib_x 0] == 0 } {
+	    set multi_dir [string trim [lindex $multi_dir_x 1]]
+	    set multi_lib [string trim [lindex $multi_lib_x 1]]
+	    if { "$multi_dir" == "." } {
+		set multi_root "$libgcc_dir"
+	    } else {
+		set multi_match [string last "/$multi_dir" "$libgcc_dir"]
+		if { "$multi_match" >= 0 } {
+		    set multi_root [string range "$libgcc_dir" \
+				    0 [expr $multi_match - 1]]
+		} else {
+		    set multi_lib ""
+		}
+	    }
+	    foreach i "$multi_lib" {
+		set mldir ""
+		regexp -- "\[a-z0-9=_/\.-\]*;" $i mldir
+		set mldir [string trimright $mldir "\;@"]
+		if { "$mldir" == "$multi_dir" } {
 		    continue
-	        }
-	        if { [llength [glob -nocomplain ${gccdir}/${mldir}/libgcc_s*.so.*]] >= 1 } {
-		    append ld_library_path ":${gccdir}/${mldir}"
-	        }
+		}
+		append ld_library_path ":${multi_root}/${mldir}"
 	    }
-        }
+	}
     }
 
     # add the library path for libffi.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH libffi 4/4] Correct indentation throughout `libffi-init'
  2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
                   ` (2 preceding siblings ...)
  2020-04-03 22:56 ` [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET Maciej W. Rozycki
@ 2020-04-03 22:56 ` Maciej W. Rozycki
  2020-04-06 18:01   ` Jeff Law
  2020-04-14 13:59 ` [PING][PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
  2020-04-20 12:50 ` [PING^2][PATCH " Maciej W. Rozycki
  5 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-03 22:56 UTC (permalink / raw)
  To: libffi-discuss; +Cc: Anthony Green, Mike Stump, gcc-patches

---
 testsuite/lib/libffi.exp |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

libffi-init-whitespace.diff
Index: libffi/testsuite/lib/libffi.exp
===================================================================
--- libffi.orig/testsuite/lib/libffi.exp
+++ libffi/testsuite/lib/libffi.exp
@@ -303,7 +303,7 @@ proc libffi-init { args } {
 	    }
 	    set compiler "${gccdir}/xgcc"
 	}
-        verbose "gccdir $gccdir"
+	verbose "gccdir $gccdir"
 
 	set shlib_ext [get_shlib_extension]
 	set libgcc_s_x [remote_exec host "$compiler" \
@@ -315,8 +315,8 @@ proc libffi-init { args } {
 	}
 	verbose "libgcc_dir $libgcc_dir"
 
-        set ld_library_path "."
-        append ld_library_path ":${gccdir}"
+	set ld_library_path "."
+	append ld_library_path ":${gccdir}"
 
 	set multi_dir_x [remote_exec host "$compiler" "-print-multi-directory"]
 	set multi_lib_x [remote_exec host "$compiler" "-print-multi-lib"]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU
  2020-04-03 22:55 ` [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU Maciej W. Rozycki
@ 2020-04-06 17:58   ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2020-04-06 17:58 UTC (permalink / raw)
  To: Maciej W. Rozycki, libffi-discuss; +Cc: gcc-patches

On Fri, 2020-04-03 at 23:55 +0100, Maciej W. Rozycki via Gcc-patches wrote:
> Use an Autoconf template rather an inline piece of scriptery to set 
> DejaGNU's $CC_FOR_TARGET and $CXX_FOR_TARGET variables from $CC and $CXX 
> respectively, making it easier to maintain and making it take advantage 
> of Automake's dependency and rule generation.  Relocate the generated 
> `local.exp' file to within testsuite/ so as to make its regeneration 
> rule to actually work, i.e. (in testsuite/Makefile.in):
> 
> EXTRA_DEJAGNU_SITE_CONFIG = local.exp
> site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
> 	[...]
> local.exp: $(top_builddir)/config.status $(srcdir)/local.exp.in
> 	cd $(top_builddir) && $(SHELL) ./config.status $(subdir)/$@
> 
> It wouldn't work if the regeneration rule was placed in the top-level 
> Makefile.in, which is what Automake would faithfully do if `local.exp' 
> stayed in the top level directory.
> ---
> Hi,
> 
>  I think having individual AC_CONFIG_FILES macro invocations for each 
> output file or group of files would make this change (and code itself) 
> more readable, however it hasn't been done before and I decided not to 
> change the style on this occasion.  It may make sense as a follow-up 
> clean-up.
> 
>   Maciej
> ---
>  Makefile.am            |    3 ---
>  configure.ac           |    7 +------
>  testsuite/Makefile.am  |    2 +-
>  testsuite/local.exp.in |    2 ++
>  4 files changed, 4 insertions(+), 10 deletions(-)
So from the cover letter I got the impression this series was supposed to be
pulling down some bits from upstream libffi into gcc's copy.  But AFAICT that's
not the case.

I don't see anything inherently wrong here.  It was just a bit confusing.  It's
actually follow-on patches where you're cherry picking from the upstream libffi
repository.

OK for the trunk.
jeff
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor to DejaGNU
  2020-04-03 22:55 ` [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor " Maciej W. Rozycki
@ 2020-04-06 18:00   ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2020-04-06 18:00 UTC (permalink / raw)
  To: Maciej W. Rozycki, libffi-discuss; +Cc: gcc-patches

On Fri, 2020-04-03 at 23:55 +0100, Maciej W. Rozycki via Gcc-patches wrote:
> Use Autoconf substitution in the template used for extra DejaGNU site 
> configuration, which is a documented supported way to pass information 
> from the `configure' script, rather than resorting to a hack with 
> extracting an undocumented internal setting from `config.log' to pass 
> the compiler vendor to DejaGNU.  No functional change (and no risk of 
> breakage with some Autoconf version anymore).
> 
> Use AM_SUBST_NOTMAKE with the new substitution so as not to place it in 
> Makefile.in files, where it is not needed, and set the Autmoake version 
> requirement accordingly.
> ---
> Hi,
> 
>  I chose to use AM_SUBST_NOTMAKE so as not to clutter Makefile.in files 
> with the new variable as Automake does that by default.  That however 
> requires the use of Automake 1.11 or newer.  By the look of our sources 
> that shouldn't be an issue as far as I can tell, but the macro invocation 
> can be dropped along with the requirement if it would.
> 
>   Maciej
> ---
>  Makefile.am              |    3 ++-
>  configure.ac             |    2 ++
>  testsuite/lib/libffi.exp |    4 ----
>  testsuite/local.exp.in   |    1 +
>  4 files changed, 5 insertions(+), 5 deletions(-)
OK
jeff
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH libffi 4/4] Correct indentation throughout `libffi-init'
  2020-04-03 22:56 ` [PATCH libffi 4/4] Correct indentation throughout `libffi-init' Maciej W. Rozycki
@ 2020-04-06 18:01   ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2020-04-06 18:01 UTC (permalink / raw)
  To: Maciej W. Rozycki, libffi-discuss; +Cc: gcc-patches

On Fri, 2020-04-03 at 23:56 +0100, Maciej W. Rozycki via Gcc-patches wrote:
> ---
>  testsuite/lib/libffi.exp |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
OK when prereqs have been committed.

jeff
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET
  2020-04-03 22:56 ` [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET Maciej W. Rozycki
@ 2020-04-06 18:03   ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2020-04-06 18:03 UTC (permalink / raw)
  To: Maciej W. Rozycki, libffi-discuss; +Cc: gcc-patches

On Fri, 2020-04-03 at 23:56 +0100, Maciej W. Rozycki via Gcc-patches wrote:
> Update code in `libffi-init' to use $CC_FOR_TARGET in determining the 
> value of $ld_library_path, as using a different compiler location from 
> one actually used in testing may have odd consequences.
> 
> As this obviously loses the setting of $gccdir provide a replacement way 
> to determine the directory if feasible, however prefer the location of 
> shared libgcc over static libgcc.  This helps in configurations where 
> shared libgcc is, unlike libgcc, a location that is not specific to the 
> compiler version, a common scenario.  It does not address the scenario 
> however where there is a shared libgcc symlink installed pointing to the 
> actual run-time library installed elsewhere; this would have to be dealt 
> with in a board description file specific to the installation.
> 
> Use `remote_exec host' rather than `exec' to invoke the compiler so as 
> to support remote configurations and also avoid the latter procedure's 
> path length limitation that prevents execution in some actual scenarios.
> 
> As using `remote_exec host' precludes the use of our existing file name 
> globbing to examine directory contents, reuse, with minor modifications 
> needed to adjust to our structure, the piece I previously contributed to 
> GCC with commit d42b84f427e4 ("testsuite: Fix run-time tracking down 
> of `libgcc_s'").
> ---
> Hi,
> 
>  This has its limitation in that it still doesn't default to the same 
> compiler as `target_compile' (`default_target_compile') from target.exp in 
> DejaGNU does, but I believe it is a step in the right direction.  That 
> will only affect standalone (e.g. installed) testing iff $CC_FOR_TARGET 
> hasn't been set.
> 
>  Also for C++ compilation our carefully crafted $ld_library_path is 
> unfortunately overridden by `g++_link_flags' from libgloss.exp in DejaGNU 
> called in `default_target_compile'.  This actually does cause test 
> failures in my `riscv64-linux-gnu' cross-compilation setup:
> 
> FAIL: libffi.closures/unwindtest.cc -W -Wall -Wno-psabi -O0 execution test
> FAIL: libffi.closures/unwindtest.cc -W -Wall -Wno-psabi -O2 execution test
> FAIL: libffi.closures/unwindtest_ffi_call.cc -W -Wall -Wno-psabi -O0 execution
> test
> FAIL: libffi.closures/unwindtest_ffi_call.cc -W -Wall -Wno-psabi -O2 execution
> test
> 
> and I am currently not sure how to best address it, i.e. whether to change
> DejaGNU's `g++_link_flags' or to take advantage of a TCL's feature and 
> provide our own copy of the procedure.  Something to consider sometime.
> 
>  NB I chose not to correct obvious indentation issues with lines not 
> touched by this change so as not to obfuscate the patch unnecessarily.  A 
> complementing formatting change follows.
> 
>   Maciej
> ---
>  testsuite/lib/libffi.exp |   68 +++++++++++++++++++++++++++++++++++-----------
> -
>  1 file changed, 51 insertions(+), 17 deletions(-)
OK.  Note that all 4 patches in the series need ChangeLog entries.

jeff
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PING][PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite
  2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
                   ` (3 preceding siblings ...)
  2020-04-03 22:56 ` [PATCH libffi 4/4] Correct indentation throughout `libffi-init' Maciej W. Rozycki
@ 2020-04-14 13:59 ` Maciej W. Rozycki
  2020-04-20 12:50 ` [PING^2][PATCH " Maciej W. Rozycki
  5 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-14 13:59 UTC (permalink / raw)
  To: libffi-discuss

On Fri, 3 Apr 2020, Maciej W. Rozycki wrote:

>  We are keen to keep the GCC's copy of libffi as close as possible to the 
> upstream version however while usable the current solution has some issues 
> which we would rather avoid.  I have therefore decided to address some of 
> them with the intent to have the result backported to GCC.  As it stands 
> I'm told the current version of libffi cannot be fully merged to GCC, as 
> there have been an ABI change that will require technical evaluation.  So 
> the intent has been to backport the changes proposed here individually.
> 
>  See the individual change descriptions and any further discussion 
> included for full details of each patch proposed.

 Ping for:

<https://sourceware.org/pipermail/libffi-discuss/2020/002503.html>
<https://sourceware.org/pipermail/libffi-discuss/2020/002504.html>
<https://sourceware.org/pipermail/libffi-discuss/2020/002505.html>
<https://sourceware.org/pipermail/libffi-discuss/2020/002506.html>

  Maciej

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PING^2][PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite
  2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
                   ` (4 preceding siblings ...)
  2020-04-14 13:59 ` [PING][PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
@ 2020-04-20 12:50 ` Maciej W. Rozycki
  5 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2020-04-20 12:50 UTC (permalink / raw)
  To: libffi-discuss

On Fri, 3 Apr 2020, Maciej W. Rozycki wrote:

>  We are keen to keep the GCC's copy of libffi as close as possible to the 
> upstream version however while usable the current solution has some issues 
> which we would rather avoid.  I have therefore decided to address some of 
> them with the intent to have the result backported to GCC.  As it stands 
> I'm told the current version of libffi cannot be fully merged to GCC, as 
> there have been an ABI change that will require technical evaluation.  So 
> the intent has been to backport the changes proposed here individually.
> 
>  See the individual change descriptions and any further discussion 
> included for full details of each patch proposed.

 Ping for:

<https://sourceware.org/pipermail/libffi-discuss/2020/002503.html>
<https://sourceware.org/pipermail/libffi-discuss/2020/002504.html>
<https://sourceware.org/pipermail/libffi-discuss/2020/002505.html>
<https://sourceware.org/pipermail/libffi-discuss/2020/002506.html>

  Maciej

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-04-20 12:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 22:55 [PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
2020-04-03 22:55 ` [PATCH libffi 1/4] Use a template to pass $CC and $CXX to DejaGNU Maciej W. Rozycki
2020-04-06 17:58   ` Jeff Law
2020-04-03 22:55 ` [PATCH libffi 2/4] Use a documented way to pass $compiler_vendor " Maciej W. Rozycki
2020-04-06 18:00   ` Jeff Law
2020-04-03 22:56 ` [PATCH libffi 3/4] Make `libffi-init' use $CC_FOR_TARGET Maciej W. Rozycki
2020-04-06 18:03   ` Jeff Law
2020-04-03 22:56 ` [PATCH libffi 4/4] Correct indentation throughout `libffi-init' Maciej W. Rozycki
2020-04-06 18:01   ` Jeff Law
2020-04-14 13:59 ` [PING][PATCH libffi 0/4] Robustify compiler and library path selection in the testsuite Maciej W. Rozycki
2020-04-20 12:50 ` [PING^2][PATCH " Maciej W. Rozycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).