diff mbox

[testsuite] Fix loading wrong DLLs on Windows, merge duplicate target-libpath.exp

Message ID 20170403231430.17602-1-daniel.santos@pobox.com
State New
Headers show

Commit Message

Daniel Santos April 3, 2017, 11:14 p.m. UTC 
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

Comments

Daniel Santos April 3, 2017, 11:32 p.m. UTC | #1 
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.
Mike Stump April 5, 2017, 5:35 p.m. UTC | #2 
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.
diff mbox

Patch

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
-}
-