public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp
@ 2017-04-03 23:10 Daniel Santos
  2017-04-03 23:28 ` [PATCH, testsuite] PR79867: " Daniel Santos
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Daniel Santos @ 2017-04-03 23:10 UTC (permalink / raw)
  To: gcc-patches, Mike Stump; +Cc: Anthony Green, Richard Henderson

We currently have two copies of target-libpath.exp in the tree under
gcc/testsuite/lib and libffi/testsuite/lib.  It was originally pulled
into the libffi project from downstream gcc in 2009
(https://github.com/libffi/libffi/commit/5cbe2058c128e848446ae79fe15ee54260a90559).
Then in 2012, Anthony Green (from libffi) modified it to correct this
Windows problem (thank you!
https://github.com/libffi/libffi/commit/bd78c9c3311244dd5f877c915b0dff91621dd253).
In 2015, this file got pulled from upstream libffi back into gcc, thus
beginning two separate development paths
(https://github.com/gcc-mirror/gcc/commit/89d8a412de548b218cf7c967e65ad98bceb1ed4e).

This patch merges the changes from libffi upstream which correctly solve
the Windows DLL load path problem and removes the duplicate from
libffi/testsuite/lib.  This fixes most of bug #79867, implementing
correct behaviour for set_ld_library_path_env_vars and
restore_ld_library_path_env_vars.  However, there is still incorrect
behaviour in DejaGNU's unix_load that should eventually be adddressed,
although I cannot yet point to a specific failure that it is causing.

gcc/ChangeLog:
2017-04-03  Daniel Santos <daniel.santos@pobox.com>

	PR testsuite/79867
	* testsuite/lib/target-libpath.exp (set_ld_library_path_env_vars,
	restore_ld_library_path_env_vars): Merge changes from libffi upstream,
	correcting DLL load path problems on Windows.

libffi/ChangeLog:
2017-04-03  Daniel Santos <daniel.santos@pobox.com>

	PR testsuite/79867
	* testsuite/lib/target-libpath.exp: Remove.
	* testsuite/Makefile.in: Remove target-libpath.exp.
	* testsuite/Makefile.am: Regenerated.

Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
---
 gcc/testsuite/lib/target-libpath.exp    |  21 +++
 libffi/testsuite/Makefile.am            |   2 +-
 libffi/testsuite/Makefile.in            |   2 +-
 libffi/testsuite/lib/target-libpath.exp | 283 --------------------------------
 4 files changed, 23 insertions(+), 285 deletions(-)
 delete mode 100644 libffi/testsuite/lib/target-libpath.exp

diff --git a/gcc/testsuite/lib/target-libpath.exp b/gcc/testsuite/lib/target-libpath.exp
index 9b3e201ed68..b6d01b31016 100644
--- a/gcc/testsuite/lib/target-libpath.exp
+++ b/gcc/testsuite/lib/target-libpath.exp
@@ -23,6 +23,7 @@ set orig_shlib_path_saved 0
 set orig_ld_library_path_32_saved 0
 set orig_ld_library_path_64_saved 0
 set orig_dyld_library_path_saved 0
+set orig_path_saved 0
 set orig_gcc_exec_prefix_saved 0
 set orig_gcc_exec_prefix_checked 0
 
@@ -55,6 +56,7 @@ proc set_ld_library_path_env_vars { } {
   global orig_ld_library_path_32_saved
   global orig_ld_library_path_64_saved
   global orig_dyld_library_path_saved
+  global orig_path_saved
   global orig_gcc_exec_prefix_saved
   global orig_gcc_exec_prefix_checked
   global orig_ld_library_path
@@ -63,6 +65,7 @@ proc set_ld_library_path_env_vars { } {
   global orig_ld_library_path_32
   global orig_ld_library_path_64
   global orig_dyld_library_path
+  global orig_path
   global orig_gcc_exec_prefix
   global env
 
@@ -110,6 +113,10 @@ proc set_ld_library_path_env_vars { } {
       set orig_dyld_library_path "$env(DYLD_LIBRARY_PATH)"
       set orig_dyld_library_path_saved 1
     }
+    if [info exists env(PATH)] {
+      set orig_path "$env(PATH)"
+      set orig_path_saved 1
+    }
   }
 
   # We need to set ld library path in the environment.  Currently,
@@ -164,6 +171,13 @@ proc set_ld_library_path_env_vars { } {
   } else {
     setenv DYLD_LIBRARY_PATH "$ld_library_path"
   }
+  if { [istarget *-*-cygwin*] || [istarget *-*-mingw*] } {
+    if { $orig_path_saved } {
+      setenv PATH "$ld_library_path:$orig_path"
+    } else {
+      setenv PATH "$ld_library_path"
+    }
+  }
 
   verbose -log "LD_LIBRARY_PATH=[getenv LD_LIBRARY_PATH]"
   verbose -log "LD_RUN_PATH=[getenv LD_RUN_PATH]"
@@ -201,12 +215,14 @@ proc restore_ld_library_path_env_vars { } {
   global orig_ld_library_path_32_saved
   global orig_ld_library_path_64_saved
   global orig_dyld_library_path_saved
+  global orig_path_saved
   global orig_ld_library_path
   global orig_ld_run_path
   global orig_shlib_path
   global orig_ld_library_path_32
   global orig_ld_library_path_64
   global orig_dyld_library_path
+  global orig_path
   global env
 
   restore_gcc_exec_prefix_env_var
@@ -245,6 +261,11 @@ proc restore_ld_library_path_env_vars { } {
   } elseif [info exists env(DYLD_LIBRARY_PATH)] {
     unsetenv DYLD_LIBRARY_PATH
   }
+  if { $orig_path_saved } {
+    setenv PATH "$orig_path"
+  } elseif [info exists env(PATH)] {
+    unsetenv PATH
+  }
 }
 
 #######################################
diff --git a/libffi/testsuite/Makefile.am b/libffi/testsuite/Makefile.am
index 209e8976635..b4eb7c2bce9 100644
--- a/libffi/testsuite/Makefile.am
+++ b/libffi/testsuite/Makefile.am
@@ -82,7 +82,7 @@ libffi.call/cls_align_uint64.c libffi.call/cls_4byte.c			\
 libffi.call/cls_6_1_byte.c			\
 libffi.call/cls_7_1_byte.c libffi.call/unwindtest.cc			\
 libffi.call/unwindtest_ffi_call.cc	\
-lib/wrapper.exp lib/target-libpath.exp	\
+lib/wrapper.exp	\
 lib/libffi.exp libffi.call/cls_struct_va1.c				\
 libffi.call/cls_uchar_va.c libffi.call/cls_uint_va.c			\
 libffi.call/cls_ulong_va.c libffi.call/cls_ushort_va.c			\
diff --git a/libffi/testsuite/Makefile.in b/libffi/testsuite/Makefile.in
index b7da4b0b3e7..aa79d05a272 100644
--- a/libffi/testsuite/Makefile.in
+++ b/libffi/testsuite/Makefile.in
@@ -295,7 +295,7 @@ libffi.call/cls_align_uint64.c libffi.call/cls_4byte.c			\
 libffi.call/cls_6_1_byte.c			\
 libffi.call/cls_7_1_byte.c libffi.call/unwindtest.cc			\
 libffi.call/unwindtest_ffi_call.cc	\
-lib/wrapper.exp lib/target-libpath.exp	\
+lib/wrapper.exp	\
 lib/libffi.exp libffi.call/cls_struct_va1.c				\
 libffi.call/cls_uchar_va.c libffi.call/cls_uint_va.c			\
 libffi.call/cls_ulong_va.c libffi.call/cls_ushort_va.c			\
diff --git a/libffi/testsuite/lib/target-libpath.exp b/libffi/testsuite/lib/target-libpath.exp
deleted file mode 100644
index 6b7beba9351..00000000000
--- a/libffi/testsuite/lib/target-libpath.exp
+++ /dev/null
@@ -1,283 +0,0 @@
-# Copyright (C) 2004, 2005, 2007 Free Software Foundation, Inc.
-
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with GCC; see the file COPYING3.  If not see
-# <http://www.gnu.org/licenses/>.
-
-# This file was contributed by John David Anglin (dave.anglin@nrc-cnrc.gc.ca)
-
-set orig_environment_saved 0
-set orig_ld_library_path_saved 0
-set orig_ld_run_path_saved 0
-set orig_shlib_path_saved 0
-set orig_ld_libraryn32_path_saved 0
-set orig_ld_library64_path_saved 0
-set orig_ld_library_path_32_saved 0
-set orig_ld_library_path_64_saved 0
-set orig_dyld_library_path_saved 0
-set orig_path_saved 0
-
-#######################################
-# proc set_ld_library_path_env_vars { }
-#######################################
-
-proc set_ld_library_path_env_vars { } {
-  global ld_library_path
-  global orig_environment_saved
-  global orig_ld_library_path_saved
-  global orig_ld_run_path_saved
-  global orig_shlib_path_saved
-  global orig_ld_libraryn32_path_saved
-  global orig_ld_library64_path_saved
-  global orig_ld_library_path_32_saved
-  global orig_ld_library_path_64_saved
-  global orig_dyld_library_path_saved
-  global orig_path_saved
-  global orig_ld_library_path
-  global orig_ld_run_path
-  global orig_shlib_path
-  global orig_ld_libraryn32_path
-  global orig_ld_library64_path
-  global orig_ld_library_path_32
-  global orig_ld_library_path_64
-  global orig_dyld_library_path
-  global orig_path
-  global GCC_EXEC_PREFIX
-
-  # Set the relocated compiler prefix, but only if the user hasn't specified one.
-  if { [info exists GCC_EXEC_PREFIX] && ![info exists env(GCC_EXEC_PREFIX)] } {
-    setenv GCC_EXEC_PREFIX "$GCC_EXEC_PREFIX"
-  }
-
-  # Setting the ld library path causes trouble when testing cross-compilers.
-  if { [is_remote target] } {
-    return
-  }
-
-  if { $orig_environment_saved == 0 } {
-    global env
-
-    set orig_environment_saved 1
-
-    # Save the original environment.
-    if [info exists env(LD_LIBRARY_PATH)] {
-      set orig_ld_library_path "$env(LD_LIBRARY_PATH)"
-      set orig_ld_library_path_saved 1
-    }
-    if [info exists env(LD_RUN_PATH)] {
-      set orig_ld_run_path "$env(LD_RUN_PATH)"
-      set orig_ld_run_path_saved 1
-    }
-    if [info exists env(SHLIB_PATH)] {
-      set orig_shlib_path "$env(SHLIB_PATH)"
-      set orig_shlib_path_saved 1
-    }
-    if [info exists env(LD_LIBRARYN32_PATH)] {
-      set orig_ld_libraryn32_path "$env(LD_LIBRARYN32_PATH)"
-      set orig_ld_libraryn32_path_saved 1
-    }
-    if [info exists env(LD_LIBRARY64_PATH)] {
-      set orig_ld_library64_path "$env(LD_LIBRARY64_PATH)"
-      set orig_ld_library64_path_saved 1
-    }
-    if [info exists env(LD_LIBRARY_PATH_32)] {
-      set orig_ld_library_path_32 "$env(LD_LIBRARY_PATH_32)"
-      set orig_ld_library_path_32_saved 1
-    }
-    if [info exists env(LD_LIBRARY_PATH_64)] {
-      set orig_ld_library_path_64 "$env(LD_LIBRARY_PATH_64)"
-      set orig_ld_library_path_64_saved 1
-    }
-    if [info exists env(DYLD_LIBRARY_PATH)] {
-      set orig_dyld_library_path "$env(DYLD_LIBRARY_PATH)"
-      set orig_dyld_library_path_saved 1
-    }
-    if [info exists env(PATH)] {
-      set orig_path "$env(PATH)"
-      set orig_path_saved 1
-    }
-  }
-
-  # We need to set ld library path in the environment.  Currently,
-  # unix.exp doesn't set the environment correctly for all systems.
-  # It only sets SHLIB_PATH and LD_LIBRARY_PATH when it executes a
-  # program.  We also need the environment set for compilations, etc.
-  #
-  # On IRIX 6, we have to set variables akin to LD_LIBRARY_PATH, but
-  # called LD_LIBRARYN32_PATH (for the N32 ABI) and LD_LIBRARY64_PATH
-  # (for the 64-bit ABI).  The same applies to Darwin (DYLD_LIBRARY_PATH),
-  # Solaris 32 bit (LD_LIBRARY_PATH_32), Solaris 64 bit (LD_LIBRARY_PATH_64),
-  # and HP-UX (SHLIB_PATH).  In some cases, the variables are independent
-  # of LD_LIBRARY_PATH, and in other cases LD_LIBRARY_PATH is used if the
-  # variable is not defined.
-  #
-  # Doing this is somewhat of a hack as ld_library_path gets repeated in
-  # SHLIB_PATH and LD_LIBRARY_PATH when unix_load sets these variables.
-  if { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARY_PATH "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARY_PATH "$ld_library_path"
-  }
-  if { $orig_ld_run_path_saved } {
-    setenv LD_RUN_PATH "$ld_library_path:$orig_ld_run_path"
-  } else {
-    setenv LD_RUN_PATH "$ld_library_path"
-  }
-  # The default shared library dynamic path search for 64-bit
-  # HP-UX executables searches LD_LIBRARY_PATH before SHLIB_PATH.
-  # LD_LIBRARY_PATH isn't used for 32-bit executables.  Thus, we
-  # set LD_LIBRARY_PATH and SHLIB_PATH as if they were independent.
-  if { $orig_shlib_path_saved } {
-    setenv SHLIB_PATH "$ld_library_path:$orig_shlib_path"
-  } else {
-    setenv SHLIB_PATH "$ld_library_path"
-  }
-  if { $orig_ld_libraryn32_path_saved } {
-    setenv LD_LIBRARYN32_PATH "$ld_library_path:$orig_ld_libraryn32_path"
-  } elseif { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARYN32_PATH "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARYN32_PATH "$ld_library_path"
-  }
-  if { $orig_ld_library64_path_saved } {
-    setenv LD_LIBRARY64_PATH "$ld_library_path:$orig_ld_library64_path"
-  } elseif { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARY64_PATH "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARY64_PATH "$ld_library_path"
-  }
-  if { $orig_ld_library_path_32_saved } {
-    setenv LD_LIBRARY_PATH_32 "$ld_library_path:$orig_ld_library_path_32"
-  } elseif { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARY_PATH_32 "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARY_PATH_32 "$ld_library_path"
-  }
-  if { $orig_ld_library_path_64_saved } {
-    setenv LD_LIBRARY_PATH_64 "$ld_library_path:$orig_ld_library_path_64"
-  } elseif { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARY_PATH_64 "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARY_PATH_64 "$ld_library_path"
-  }
-  if { $orig_dyld_library_path_saved } {
-    setenv DYLD_LIBRARY_PATH "$ld_library_path:$orig_dyld_library_path"
-  } else {
-    setenv DYLD_LIBRARY_PATH "$ld_library_path"
-  }
-  if { [istarget *-*-cygwin*] || [istarget *-*-mingw*] } {
-    if { $orig_path_saved } {
-      setenv PATH "$ld_library_path:$orig_path"
-    } else {
-      setenv PATH "$ld_library_path"
-    }
-  }
-
-  verbose -log "set_ld_library_path_env_vars: ld_library_path=$ld_library_path"
-}
-
-#######################################
-# proc restore_ld_library_path_env_vars { }
-#######################################
-
-proc restore_ld_library_path_env_vars { } {
-  global orig_environment_saved
-  global orig_ld_library_path_saved
-  global orig_ld_run_path_saved
-  global orig_shlib_path_saved
-  global orig_ld_libraryn32_path_saved
-  global orig_ld_library64_path_saved
-  global orig_ld_library_path_32_saved
-  global orig_ld_library_path_64_saved
-  global orig_dyld_library_path_saved
-  global orig_path_saved
-  global orig_ld_library_path
-  global orig_ld_run_path
-  global orig_shlib_path
-  global orig_ld_libraryn32_path
-  global orig_ld_library64_path
-  global orig_ld_library_path_32
-  global orig_ld_library_path_64
-  global orig_dyld_library_path
-  global orig_path
-
-  if { $orig_environment_saved == 0 } {
-    return
-  }
-
-  if { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARY_PATH "$orig_ld_library_path"
-  } elseif [info exists env(LD_LIBRARY_PATH)] {
-    unsetenv LD_LIBRARY_PATH
-  }
-  if { $orig_ld_run_path_saved } {
-    setenv LD_RUN_PATH "$orig_ld_run_path"
-  } elseif [info exists env(LD_RUN_PATH)] {
-    unsetenv LD_RUN_PATH
-  }
-  if { $orig_shlib_path_saved } {
-    setenv SHLIB_PATH "$orig_shlib_path"
-  } elseif [info exists env(SHLIB_PATH)] {
-    unsetenv SHLIB_PATH
-  }
-  if { $orig_ld_libraryn32_path_saved } {
-    setenv LD_LIBRARYN32_PATH "$orig_ld_libraryn32_path"
-  } elseif [info exists env(LD_LIBRARYN32_PATH)] {
-    unsetenv LD_LIBRARYN32_PATH
-  }
-  if { $orig_ld_library64_path_saved } {
-    setenv LD_LIBRARY64_PATH "$orig_ld_library64_path"
-  } elseif [info exists env(LD_LIBRARY64_PATH)] {
-    unsetenv LD_LIBRARY64_PATH
-  }
-  if { $orig_ld_library_path_32_saved } {
-    setenv LD_LIBRARY_PATH_32 "$orig_ld_library_path_32"
-  } elseif [info exists env(LD_LIBRARY_PATH_32)] {
-    unsetenv LD_LIBRARY_PATH_32
-  }
-  if { $orig_ld_library_path_64_saved } {
-    setenv LD_LIBRARY_PATH_64 "$orig_ld_library_path_64"
-  } elseif [info exists env(LD_LIBRARY_PATH_64)] {
-    unsetenv LD_LIBRARY_PATH_64
-  }
-  if { $orig_dyld_library_path_saved } {
-    setenv DYLD_LIBRARY_PATH "$orig_dyld_library_path"
-  } elseif [info exists env(DYLD_LIBRARY_PATH)] {
-    unsetenv DYLD_LIBRARY_PATH
-  }
-  if { $orig_path_saved } {
-    setenv PATH "$orig_path"
-  } elseif [info exists env(PATH)] {
-    unsetenv PATH
-  }
-}
-
-#######################################
-# proc get_shlib_extension { }
-#######################################
-
-proc get_shlib_extension { } {
-    global shlib_ext
-
-    if { [ istarget *-*-darwin* ] } {
-	set shlib_ext "dylib"
-    } elseif { [ istarget *-*-cygwin* ] || [ istarget *-*-mingw* ] } {
-	set shlib_ext "dll"
-    } elseif { [ istarget hppa*-*-hpux* ] } {
-	set shlib_ext "sl"
-    } else {
-	set shlib_ext "so"
-    }
-    return $shlib_ext
-}
-
-- 
2.11.0

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

* Re: [PATCH, testsuite] PR79867: Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp
  2017-04-03 23:10 [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp Daniel Santos
@ 2017-04-03 23:28 ` Daniel Santos
  2017-04-05 17:35 ` [testsuite] " Mike Stump
  2017-04-06 22:35 ` [PATCH v2,testsuite] PR79867: Merge fixes for windows DLL loading problem from libffi Daniel Santos
  2 siblings, 0 replies; 6+ messages in thread
From: Daniel Santos @ 2017-04-03 23:28 UTC (permalink / raw)
  To: gcc-patches, Mike Stump

I forgot to include PATCH and the PR in the subject line, sorry about 
that.  Also, I have run a full bootstrap and testsuite to verify that I 
haven't missed any references to the extraneous copy of 
target-libpath.exp in libffi.

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

* Re: [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp
  2017-04-03 23:10 [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp Daniel Santos
  2017-04-03 23:28 ` [PATCH, testsuite] PR79867: " Daniel Santos
@ 2017-04-05 17:35 ` Mike Stump
  2017-04-05 21:41   ` Daniel Santos
  2017-04-06 22:35 ` [PATCH v2,testsuite] PR79867: Merge fixes for windows DLL loading problem from libffi Daniel Santos
  2 siblings, 1 reply; 6+ messages in thread
From: Mike Stump @ 2017-04-05 17:35 UTC (permalink / raw)
  To: Daniel Santos
  Cc: gcc-patches, Anthony Green, Richard Henderson, Gerald Pfeifer

On Apr 3, 2017, at 4:14 PM, Daniel Santos <daniel.santos@pobox.com> wrote:
> 
> We currently have two copies of target-libpath.exp in the tree under
> gcc/testsuite/lib and libffi/testsuite/lib.  It was originally pulled
> into the libffi project from downstream gcc in 2009
> (https://github.com/libffi/libffi/commit/5cbe2058c128e848446ae79fe15ee54260a90559).
> Then in 2012, Anthony Green (from libffi) modified it to correct this
> Windows problem (thank you!
> https://github.com/libffi/libffi/commit/bd78c9c3311244dd5f877c915b0dff91621dd253).
> In 2015, this file got pulled from upstream libffi back into gcc, thus
> beginning two separate development paths
> (https://github.com/gcc-mirror/gcc/commit/89d8a412de548b218cf7c967e65ad98bceb1ed4e).
> 
> This patch merges the changes from libffi upstream which correctly solve
> the Windows DLL load path problem and removes the duplicate from
> libffi/testsuite/lib.  This fixes most of bug #79867, implementing
> correct behaviour for set_ld_library_path_env_vars and
> restore_ld_library_path_env_vars.  However, there is still incorrect
> behaviour in DejaGNU's unix_load that should eventually be adddressed,
> although I cannot yet point to a specific failure that it is causing.
> 
> gcc/ChangeLog:
> 2017-04-03  Daniel Santos <daniel.santos@pobox.com>
> 
> 	PR testsuite/79867
> 	* testsuite/lib/target-libpath.exp (set_ld_library_path_env_vars,
> 	restore_ld_library_path_env_vars): Merge changes from libffi upstream,
> 	correcting DLL load path problems on Windows.

These are Ok.

> libffi/ChangeLog:
> 2017-04-03  Daniel Santos <daniel.santos@pobox.com>
> 
> 	PR testsuite/79867
> 	* testsuite/lib/target-libpath.exp: Remove.
> 	* testsuite/Makefile.in: Remove target-libpath.exp.
> 	* testsuite/Makefile.am: Regenerated.

I don't think the libffi project wants to remove that file.  There is little point being different from them in this regard.  The dup should not hurt.

https://gcc.gnu.org/codingconventions.html seems like it should be updated to list libffi.  Not sure why it wasn't.  I just point it out if someone wants to contribute a patch for it.

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

* Re: [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp
  2017-04-05 17:35 ` [testsuite] " Mike Stump
@ 2017-04-05 21:41   ` Daniel Santos
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Santos @ 2017-04-05 21:41 UTC (permalink / raw)
  To: Mike Stump; +Cc: gcc-patches, Anthony Green, Richard Henderson, Gerald Pfeifer

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

On 04/05/2017 12:35 PM, Mike Stump wrote:
>> libffi/ChangeLog:
>> 2017-04-03  Daniel Santos <daniel.santos@pobox.com>
>>
>> 	PR testsuite/79867
>> 	* testsuite/lib/target-libpath.exp: Remove.
>> 	* testsuite/Makefile.in: Remove target-libpath.exp.
>> 	* testsuite/Makefile.am: Regenerated.
> I don't think the libffi project wants to remove that file.  There is little point being different from them in this regard.  The dup should not hurt.

Hmm.  There have been many changes to target-libpath.exp under 
gcc/testsuite/lib since libffi copied it.  I have attached a diff of 
them.  I'm not proposing removing target-libpath.exp from libffi 
upstream, but from the gcc tree.  I'm having trouble seeing how having 
two different copies evolving independently can be a good thing.

Daniel

[-- Attachment #2: libffi-gcc-target-libpath.exp.diff --]
[-- Type: text/x-patch, Size: 8196 bytes --]

--- target-libpath.exp	2017-04-05 16:39:38.939768810 -0500
+++ gcc/testsuite/lib/target-libpath.exp	2017-04-05 16:39:49.350768260 -0500
@@ -1,4 +1,4 @@
-# Copyright (C) 2004, 2005, 2007 Free Software Foundation, Inc.
+# Copyright (C) 2004-2017 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -20,12 +20,28 @@
 set orig_ld_library_path_saved 0
 set orig_ld_run_path_saved 0
 set orig_shlib_path_saved 0
-set orig_ld_libraryn32_path_saved 0
-set orig_ld_library64_path_saved 0
 set orig_ld_library_path_32_saved 0
 set orig_ld_library_path_64_saved 0
 set orig_dyld_library_path_saved 0
 set orig_path_saved 0
+set orig_gcc_exec_prefix_saved 0
+set orig_gcc_exec_prefix_checked 0
+
+
+#######################################
+# proc set_gcc_exec_prefix_env_var { }
+#######################################
+
+proc set_gcc_exec_prefix_env_var { } {
+  global TEST_GCC_EXEC_PREFIX
+  global env
+
+  # Set GCC_EXEC_PREFIX for the compiler under test to pick up files not in
+  # the build tree from a specified location (normally the install tree).
+  if [info exists TEST_GCC_EXEC_PREFIX] {
+    setenv GCC_EXEC_PREFIX "$TEST_GCC_EXEC_PREFIX"
+  }
+}
 
 #######################################
 # proc set_ld_library_path_env_vars { }
@@ -37,36 +53,39 @@
   global orig_ld_library_path_saved
   global orig_ld_run_path_saved
   global orig_shlib_path_saved
-  global orig_ld_libraryn32_path_saved
-  global orig_ld_library64_path_saved
   global orig_ld_library_path_32_saved
   global orig_ld_library_path_64_saved
   global orig_dyld_library_path_saved
   global orig_path_saved
+  global orig_gcc_exec_prefix_saved
+  global orig_gcc_exec_prefix_checked
   global orig_ld_library_path
   global orig_ld_run_path
   global orig_shlib_path
-  global orig_ld_libraryn32_path
-  global orig_ld_library64_path
   global orig_ld_library_path_32
   global orig_ld_library_path_64
   global orig_dyld_library_path
   global orig_path
-  global GCC_EXEC_PREFIX
+  global orig_gcc_exec_prefix
+  global env
 
-  # Set the relocated compiler prefix, but only if the user hasn't specified one.
-  if { [info exists GCC_EXEC_PREFIX] && ![info exists env(GCC_EXEC_PREFIX)] } {
-    setenv GCC_EXEC_PREFIX "$GCC_EXEC_PREFIX"
+  # Save the original GCC_EXEC_PREFIX.
+  if { $orig_gcc_exec_prefix_checked == 0 } {
+    if [info exists env(GCC_EXEC_PREFIX)] {
+      set orig_gcc_exec_prefix "$env(GCC_EXEC_PREFIX)"
+      set orig_gcc_exec_prefix_saved 1
+    }
+    set orig_gcc_exec_prefix_checked 1
   }
 
+  set_gcc_exec_prefix_env_var
+
   # Setting the ld library path causes trouble when testing cross-compilers.
   if { [is_remote target] } {
     return
   }
 
   if { $orig_environment_saved == 0 } {
-    global env
-
     set orig_environment_saved 1
 
     # Save the original environment.
@@ -82,14 +101,6 @@
       set orig_shlib_path "$env(SHLIB_PATH)"
       set orig_shlib_path_saved 1
     }
-    if [info exists env(LD_LIBRARYN32_PATH)] {
-      set orig_ld_libraryn32_path "$env(LD_LIBRARYN32_PATH)"
-      set orig_ld_libraryn32_path_saved 1
-    }
-    if [info exists env(LD_LIBRARY64_PATH)] {
-      set orig_ld_library64_path "$env(LD_LIBRARY64_PATH)"
-      set orig_ld_library64_path_saved 1
-    }
     if [info exists env(LD_LIBRARY_PATH_32)] {
       set orig_ld_library_path_32 "$env(LD_LIBRARY_PATH_32)"
       set orig_ld_library_path_32_saved 1
@@ -113,12 +124,11 @@
   # It only sets SHLIB_PATH and LD_LIBRARY_PATH when it executes a
   # program.  We also need the environment set for compilations, etc.
   #
-  # On IRIX 6, we have to set variables akin to LD_LIBRARY_PATH, but
-  # called LD_LIBRARYN32_PATH (for the N32 ABI) and LD_LIBRARY64_PATH
-  # (for the 64-bit ABI).  The same applies to Darwin (DYLD_LIBRARY_PATH),
-  # Solaris 32 bit (LD_LIBRARY_PATH_32), Solaris 64 bit (LD_LIBRARY_PATH_64),
-  # and HP-UX (SHLIB_PATH).  In some cases, the variables are independent
-  # of LD_LIBRARY_PATH, and in other cases LD_LIBRARY_PATH is used if the
+  # On Darwin, we have to set variables akin to LD_LIBRARY_PATH, but called
+  # DYLD_LIBRARY_PATH.  The same applies to Solaris 32 bit
+  # (LD_LIBRARY_PATH_32), Solaris 64 bit (LD_LIBRARY_PATH_64), and HP-UX
+  # (SHLIB_PATH).  In some cases, the variables are independent of
+  # LD_LIBRARY_PATH, and in other cases LD_LIBRARY_PATH is used if the
   # variable is not defined.
   #
   # Doing this is somewhat of a hack as ld_library_path gets repeated in
@@ -142,20 +152,6 @@
   } else {
     setenv SHLIB_PATH "$ld_library_path"
   }
-  if { $orig_ld_libraryn32_path_saved } {
-    setenv LD_LIBRARYN32_PATH "$ld_library_path:$orig_ld_libraryn32_path"
-  } elseif { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARYN32_PATH "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARYN32_PATH "$ld_library_path"
-  }
-  if { $orig_ld_library64_path_saved } {
-    setenv LD_LIBRARY64_PATH "$ld_library_path:$orig_ld_library64_path"
-  } elseif { $orig_ld_library_path_saved } {
-    setenv LD_LIBRARY64_PATH "$ld_library_path:$orig_ld_library_path"
-  } else {
-    setenv LD_LIBRARY64_PATH "$ld_library_path"
-  }
   if { $orig_ld_library_path_32_saved } {
     setenv LD_LIBRARY_PATH_32 "$ld_library_path:$orig_ld_library_path_32"
   } elseif { $orig_ld_library_path_saved } {
@@ -183,7 +179,28 @@
     }
   }
 
-  verbose -log "set_ld_library_path_env_vars: ld_library_path=$ld_library_path"
+  verbose -log "LD_LIBRARY_PATH=[getenv LD_LIBRARY_PATH]"
+  verbose -log "LD_RUN_PATH=[getenv LD_RUN_PATH]"
+  verbose -log "SHLIB_PATH=[getenv SHLIB_PATH]"
+  verbose -log "LD_LIBRARY_PATH_32=[getenv LD_LIBRARY_PATH_32]"
+  verbose -log "LD_LIBRARY_PATH_64=[getenv LD_LIBRARY_PATH_64]"
+  verbose -log "DYLD_LIBRARY_PATH=[getenv DYLD_LIBRARY_PATH]"
+}
+
+#######################################
+# proc restore_gcc_exec_prefix_env_var { }
+#######################################
+
+proc restore_gcc_exec_prefix_env_var { } {
+  global orig_gcc_exec_prefix_saved
+  global orig_gcc_exec_prefix
+  global env
+
+  if { $orig_gcc_exec_prefix_saved } {
+    setenv GCC_EXEC_PREFIX "$orig_gcc_exec_prefix"
+  } elseif [info exists env(GCC_EXEC_PREFIX)] {
+    unsetenv GCC_EXEC_PREFIX
+  }
 }
 
 #######################################
@@ -195,8 +212,6 @@
   global orig_ld_library_path_saved
   global orig_ld_run_path_saved
   global orig_shlib_path_saved
-  global orig_ld_libraryn32_path_saved
-  global orig_ld_library64_path_saved
   global orig_ld_library_path_32_saved
   global orig_ld_library_path_64_saved
   global orig_dyld_library_path_saved
@@ -204,12 +219,13 @@
   global orig_ld_library_path
   global orig_ld_run_path
   global orig_shlib_path
-  global orig_ld_libraryn32_path
-  global orig_ld_library64_path
   global orig_ld_library_path_32
   global orig_ld_library_path_64
   global orig_dyld_library_path
   global orig_path
+  global env
+
+  restore_gcc_exec_prefix_env_var
 
   if { $orig_environment_saved == 0 } {
     return
@@ -230,16 +246,6 @@
   } elseif [info exists env(SHLIB_PATH)] {
     unsetenv SHLIB_PATH
   }
-  if { $orig_ld_libraryn32_path_saved } {
-    setenv LD_LIBRARYN32_PATH "$orig_ld_libraryn32_path"
-  } elseif [info exists env(LD_LIBRARYN32_PATH)] {
-    unsetenv LD_LIBRARYN32_PATH
-  }
-  if { $orig_ld_library64_path_saved } {
-    setenv LD_LIBRARY64_PATH "$orig_ld_library64_path"
-  } elseif [info exists env(LD_LIBRARY64_PATH)] {
-    unsetenv LD_LIBRARY64_PATH
-  }
   if { $orig_ld_library_path_32_saved } {
     setenv LD_LIBRARY_PATH_32 "$orig_ld_library_path_32"
   } elseif [info exists env(LD_LIBRARY_PATH_32)] {
@@ -269,11 +275,11 @@
 proc get_shlib_extension { } {
     global shlib_ext
 
-    if { [ istarget *-*-darwin* ] } {
+    if { [istarget *-*-darwin*] } {
 	set shlib_ext "dylib"
-    } elseif { [ istarget *-*-cygwin* ] || [ istarget *-*-mingw* ] } {
+    } elseif { [istarget *-*-cygwin*] || [istarget *-*-mingw*] } {
 	set shlib_ext "dll"
-    } elseif { [ istarget hppa*-*-hpux* ] } {
+    } elseif { [istarget hppa*-*-hpux*] } {
 	set shlib_ext "sl"
     } else {
 	set shlib_ext "so"

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

* [PATCH v2,testsuite] PR79867: Merge fixes for windows DLL loading problem from libffi
  2017-04-03 23:10 [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp Daniel Santos
  2017-04-03 23:28 ` [PATCH, testsuite] PR79867: " Daniel Santos
  2017-04-05 17:35 ` [testsuite] " Mike Stump
@ 2017-04-06 22:35 ` Daniel Santos
  2017-04-10 17:46   ` Mike Stump
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Santos @ 2017-04-06 22:35 UTC (permalink / raw)
  To: gcc-patches, Mike Stump; +Cc: Anthony Green, Richard Henderson, Gerald Pfeifer

We currently have two copies of target-libpath.exp in the tree under
gcc/testsuite/lib and libffi/testsuite/lib.  It was originally pulled
into the libffi project (from downstream gcc) in 2009
(https://github.com/libffi/libffi/commit/5cbe2058c128e848446ae79fe15ee54260a90559).
Then in 2012, Anthony Green (from libffi) modified it to correct this
Windows problem (and thank you:
https://github.com/libffi/libffi/commit/bd78c9c3311244dd5f877c915b0dff91621dd253).
In 2015, this file got pulled from upstream libffi back into gcc, thus
beginning two separate development paths
(https://github.com/gcc-mirror/gcc/commit/89d8a412de548b218cf7c967e65ad98bceb1ed4e).

This patch merges the changes from libffi upstream which correctly solve
the Windows DLL load path problem in set_ld_library_path_env_vars and
restore_ld_library_path_env_vars, thus fixing most PR79867.  However,
there is still incorrect behaviour in DejaGNU's unix_load that should
eventually be adddressed, although I cannot yet point to a specific
failure that it is causing.

Ultimately, I think that this functionality should be moved upstream to
DejaGNU where it can be managed more cleanly in board config files, but
we'll have to keep this code in gcc for when DejaGNU doesn't have
set/restore or push/pop libpath functionality.

Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
---
 gcc/testsuite/lib/target-libpath.exp | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gcc/testsuite/lib/target-libpath.exp b/gcc/testsuite/lib/target-libpath.exp
index 9b3e201ed68..b6d01b31016 100644
--- a/gcc/testsuite/lib/target-libpath.exp
+++ b/gcc/testsuite/lib/target-libpath.exp
@@ -23,6 +23,7 @@ set orig_shlib_path_saved 0
 set orig_ld_library_path_32_saved 0
 set orig_ld_library_path_64_saved 0
 set orig_dyld_library_path_saved 0
+set orig_path_saved 0
 set orig_gcc_exec_prefix_saved 0
 set orig_gcc_exec_prefix_checked 0
 
@@ -55,6 +56,7 @@ proc set_ld_library_path_env_vars { } {
   global orig_ld_library_path_32_saved
   global orig_ld_library_path_64_saved
   global orig_dyld_library_path_saved
+  global orig_path_saved
   global orig_gcc_exec_prefix_saved
   global orig_gcc_exec_prefix_checked
   global orig_ld_library_path
@@ -63,6 +65,7 @@ proc set_ld_library_path_env_vars { } {
   global orig_ld_library_path_32
   global orig_ld_library_path_64
   global orig_dyld_library_path
+  global orig_path
   global orig_gcc_exec_prefix
   global env
 
@@ -110,6 +113,10 @@ proc set_ld_library_path_env_vars { } {
       set orig_dyld_library_path "$env(DYLD_LIBRARY_PATH)"
       set orig_dyld_library_path_saved 1
     }
+    if [info exists env(PATH)] {
+      set orig_path "$env(PATH)"
+      set orig_path_saved 1
+    }
   }
 
   # We need to set ld library path in the environment.  Currently,
@@ -164,6 +171,13 @@ proc set_ld_library_path_env_vars { } {
   } else {
     setenv DYLD_LIBRARY_PATH "$ld_library_path"
   }
+  if { [istarget *-*-cygwin*] || [istarget *-*-mingw*] } {
+    if { $orig_path_saved } {
+      setenv PATH "$ld_library_path:$orig_path"
+    } else {
+      setenv PATH "$ld_library_path"
+    }
+  }
 
   verbose -log "LD_LIBRARY_PATH=[getenv LD_LIBRARY_PATH]"
   verbose -log "LD_RUN_PATH=[getenv LD_RUN_PATH]"
@@ -201,12 +215,14 @@ proc restore_ld_library_path_env_vars { } {
   global orig_ld_library_path_32_saved
   global orig_ld_library_path_64_saved
   global orig_dyld_library_path_saved
+  global orig_path_saved
   global orig_ld_library_path
   global orig_ld_run_path
   global orig_shlib_path
   global orig_ld_library_path_32
   global orig_ld_library_path_64
   global orig_dyld_library_path
+  global orig_path
   global env
 
   restore_gcc_exec_prefix_env_var
@@ -245,6 +261,11 @@ proc restore_ld_library_path_env_vars { } {
   } elseif [info exists env(DYLD_LIBRARY_PATH)] {
     unsetenv DYLD_LIBRARY_PATH
   }
+  if { $orig_path_saved } {
+    setenv PATH "$orig_path"
+  } elseif [info exists env(PATH)] {
+    unsetenv PATH
+  }
 }
 
 #######################################
-- 
2.11.0

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

* Re: [PATCH v2,testsuite] PR79867: Merge fixes for windows DLL loading problem from libffi
  2017-04-06 22:35 ` [PATCH v2,testsuite] PR79867: Merge fixes for windows DLL loading problem from libffi Daniel Santos
@ 2017-04-10 17:46   ` Mike Stump
  0 siblings, 0 replies; 6+ messages in thread
From: Mike Stump @ 2017-04-10 17:46 UTC (permalink / raw)
  To: Daniel Santos
  Cc: gcc-patches, Anthony Green, Richard Henderson, Gerald Pfeifer

On Apr 6, 2017, at 3:40 PM, Daniel Santos <daniel.santos@pobox.com> wrote:
> This patch merges the changes from libffi upstream which correctly solve
> the Windows DLL load path problem in set_ld_library_path_env_vars and
> restore_ld_library_path_env_vars, thus fixing most PR79867.

Ok.

I checked it in for you, after stubbing in a ChangeLog entry for it.

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

end of thread, other threads:[~2017-04-10 17:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-03 23:10 [testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp Daniel Santos
2017-04-03 23:28 ` [PATCH, testsuite] PR79867: " Daniel Santos
2017-04-05 17:35 ` [testsuite] " Mike Stump
2017-04-05 21:41   ` Daniel Santos
2017-04-06 22:35 ` [PATCH v2,testsuite] PR79867: Merge fixes for windows DLL loading problem from libffi Daniel Santos
2017-04-10 17:46   ` Mike Stump

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).