public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] PE direct linking to dlls, accept any filename.
@ 2006-12-15 15:44 Pedro Alves
  2006-12-15 15:48 ` Christopher Faylor
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Pedro Alves @ 2006-12-15 15:44 UTC (permalink / raw)
  To: binutils

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

Hi all,

The gdb testsuite in some shared lib tests, builds shared libs with .so 
or .sl extension uncondicionally,
and then tries to do a direct link, even on PE platforms (eg:cygwin).  
That fails to link with current ld.
I sent a patch there to fix a few cases, by introducing a $SOEXT var in 
the testsuite.
But thinking again, I can't see a reason ld doesn't let us do a direct 
link to a dll with an extension
other than ".dll" or ".DLL".  The Windows loader is happy to load the 
dlls, and in fact there are many
examples of dlls without a ".dll" extension in production.

The attached patch makes ld be able to direct link any dll without 
looking at the filename extension.

Built and regtested on i686-pc-cygwin, and confirmed manually that we 
identify dlls as dlls no matter
what the filename is, that we don't mistake exe images with dlls.  Also 
confirmed linking with msvcrt.dll
to be sure we can identify a dll built with MSVC as such.

Please review and commit.

Cheers,
Pedro Alves

----
ld/

2006-12-15  Pedro Alves  <pedro_alves@portugalmail.pt>

	* emultempl/pe.em (gld_${EMULATION_NAME}_recognized_file):
	Detect dlls using bfd not the filename extension.

ld/testsuite/

2006-12-15  Pedro Alves  <pedro_alves@portugalmail.pt>

	* ld-pe/direct.exp: New file.
	* ld-pe/direct_client.c: Likewise.
	* ld-pe/direct_dll.c: Likewise.



[-- Attachment #2: direct.diff --]
[-- Type: text/plain, Size: 4398 bytes --]

Index: emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.121
diff -u -p -r1.121 pe.em
--- emultempl/pe.em	3 Oct 2006 10:06:26 -0000	1.121
+++ emultempl/pe.em	15 Dec 2006 15:17:21 -0000
@@ -1418,15 +1418,13 @@ gld_${EMULATION_NAME}_recognized_file (l
   if (bfd_get_format (entry->the_bfd) == bfd_object)
     {
       char fbuf[LD_PATHMAX + 1];
-      const char *ext;
 
       if (REALPATH (entry->filename, fbuf) == NULL)
 	strncpy (fbuf, entry->filename, sizeof (fbuf));
 
-      ext = fbuf + strlen (fbuf) - 4;
-
-      if (strcmp (ext, ".dll") == 0 || strcmp (ext, ".DLL") == 0)
-	return pe_implied_import_dll (fbuf);
+      if (obj_pe (entry->the_bfd)
+          && pe_data (entry->the_bfd)->dll)
+        return pe_implied_import_dll (fbuf);
     }
 #endif
   return FALSE;
--- /dev/null	2006-12-15 15:20:26.499625000 +0000
+++ testsuite/ld-pe/direct.exp	2006-12-15 15:12:19.062125000 +0000
@@ -0,0 +1,91 @@
+# Expect script for direct linking from dll tests
+#   Copyright 2006
+#   Free Software Foundation, Inc.
+#
+# This file 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 2 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 this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
+#
+# Written by Pedro Alves <pedro_alves@portugalmail.pt>
+#
+ 
+# Note: 
+# 
+# This test checks the "direct linking to a dll" functionality.
+# 
+# The test has 4 stages: 
+# 
+# 1. compile and link a test dll with ".dll" extension.
+#
+# 2. compile and link a test dll with ".sl" (i.e. != ".dll") extension.
+#
+# 3. compile and link a client application linking directly to the ".dll" dll built in 1. 
+#    This should produce no errors. 
+#
+# 4. compile and link a client application linking directly to the ".sl" dll built in 2. 
+#    This should produce no errors. 
+
+# This test can only be run on PE/COFF platforms.
+if {    ![istarget *-*-cygwin*]
+     && ![istarget *-*-mingw*]
+     && ![istarget *-*-pe] } {
+    return
+}
+
+# No compiler, no test.
+if { [which $CC] == 0 } {
+    untested "Direct linking to dll test"
+    return
+}
+
+set tmpdir tmpdir
+
+proc test_direct_link_dll {} {
+    global CC
+    global CFLAGS
+    global srcdir
+    global subdir
+    global tmpdir
+    
+    # Compile the dll.
+    if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_dll.c $tmpdir/direct_dll.o ] {
+	fail "compiling shared lib"
+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.dll "$tmpdir/direct_dll.o" ] {
+	fail "linking shared lib (.dll)"
+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.sl "$tmpdir/direct_dll.o" ] {
+	fail "linking shared lib (.sl)"
+    } else {
+	# Compile and link the client program.
+	if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_client.c $tmpdir/direct_client.o ] {
+	    fail "compiling client"
+	} else {
+	    # Check linking directly to direct_dll.dll.
+	    set msg "linking client (.dll)"
+	    if [ld_simple_link $CC $tmpdir/direct_client.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.dll" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+
+	    # Check linking directly to direct_dll.sl.
+	    set msg "linking client (.sl)"
+	    if [ld_simple_link $CC $tmpdir/direct_client.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.sl" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+	}
+    }
+}
+
+test_direct_link_dll
--- /dev/null	2006-12-15 15:20:35.921500000 +0000
+++ testsuite/ld-pe/direct_dll.c	2006-12-15 15:10:30.609000000 +0000
@@ -0,0 +1,5 @@
+__declspec(dllexport) int
+dll_func (void)
+{
+  return 10;
+}
--- /dev/null	2006-12-15 15:20:40.265250000 +0000
+++ testsuite/ld-pe/direct_client.c	2006-12-15 15:10:50.640250000 +0000
@@ -0,0 +1,8 @@
+__declspec(dllimport) int dll_func (void);
+
+int
+main()
+{
+  dll_func ();
+  return 0;
+}

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-15 15:44 [PATCH] PE direct linking to dlls, accept any filename Pedro Alves
@ 2006-12-15 15:48 ` Christopher Faylor
  2006-12-15 15:56 ` Daniel Jacobowitz
  2006-12-16  2:59 ` Danny Smith
  2 siblings, 0 replies; 15+ messages in thread
From: Christopher Faylor @ 2006-12-15 15:48 UTC (permalink / raw)
  To: Pedro Alves, binutils

On Fri, Dec 15, 2006 at 03:45:03PM +0000, Pedro Alves wrote:
>Hi all,
>
>The gdb testsuite in some shared lib tests, builds shared libs with .so 
>or .sl extension uncondicionally,
>and then tries to do a direct link, even on PE platforms (eg:cygwin).  
>That fails to link with current ld.
>I sent a patch there to fix a few cases, by introducing a $SOEXT var in 
>the testsuite.
>But thinking again, I can't see a reason ld doesn't let us do a direct 
>link to a dll with an extension
>other than ".dll" or ".DLL".  The Windows loader is happy to load the 
>dlls, and in fact there are many
>examples of dlls without a ".dll" extension in production.
>
>The attached patch makes ld be able to direct link any dll without 
>looking at the filename extension.
>
>Built and regtested on i686-pc-cygwin, and confirmed manually that we 
>identify dlls as dlls no matter
>what the filename is, that we don't mistake exe images with dlls.  Also 
>confirmed linking with msvcrt.dll
>to be sure we can identify a dll built with MSVC as such.
>
>Please review and commit.

Wow.  I'm giddy.  I like patches which generalize things like this and
you even included a test case!

Unless someone (Danny?) objects in the next few days, I'll check this
in.

Thank you, Pedro.

cgf

>ld/
>
>2006-12-15  Pedro Alves  <pedro_alves@portugalmail.pt>
>
>	* emultempl/pe.em (gld_${EMULATION_NAME}_recognized_file):
>	Detect dlls using bfd not the filename extension.
>
>ld/testsuite/
>
>2006-12-15  Pedro Alves  <pedro_alves@portugalmail.pt>
>
>	* ld-pe/direct.exp: New file.
>	* ld-pe/direct_client.c: Likewise.
>	* ld-pe/direct_dll.c: Likewise.
>
>

>Index: emultempl/pe.em
>===================================================================
>RCS file: /cvs/src/src/ld/emultempl/pe.em,v
>retrieving revision 1.121
>diff -u -p -r1.121 pe.em
>--- emultempl/pe.em	3 Oct 2006 10:06:26 -0000	1.121
>+++ emultempl/pe.em	15 Dec 2006 15:17:21 -0000
>@@ -1418,15 +1418,13 @@ gld_${EMULATION_NAME}_recognized_file (l
>   if (bfd_get_format (entry->the_bfd) == bfd_object)
>     {
>       char fbuf[LD_PATHMAX + 1];
>-      const char *ext;
> 
>       if (REALPATH (entry->filename, fbuf) == NULL)
> 	strncpy (fbuf, entry->filename, sizeof (fbuf));
> 
>-      ext = fbuf + strlen (fbuf) - 4;
>-
>-      if (strcmp (ext, ".dll") == 0 || strcmp (ext, ".DLL") == 0)
>-	return pe_implied_import_dll (fbuf);
>+      if (obj_pe (entry->the_bfd)
>+          && pe_data (entry->the_bfd)->dll)
>+        return pe_implied_import_dll (fbuf);
>     }
> #endif
>   return FALSE;
>--- /dev/null	2006-12-15 15:20:26.499625000 +0000
>+++ testsuite/ld-pe/direct.exp	2006-12-15 15:12:19.062125000 +0000
>@@ -0,0 +1,91 @@
>+# Expect script for direct linking from dll tests
>+#   Copyright 2006
>+#   Free Software Foundation, Inc.
>+#
>+# This file 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 2 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 this program; if not, write to the Free Software
>+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
>+#
>+# Written by Pedro Alves <pedro_alves@portugalmail.pt>
>+#
>+ 
>+# Note: 
>+# 
>+# This test checks the "direct linking to a dll" functionality.
>+# 
>+# The test has 4 stages: 
>+# 
>+# 1. compile and link a test dll with ".dll" extension.
>+#
>+# 2. compile and link a test dll with ".sl" (i.e. != ".dll") extension.
>+#
>+# 3. compile and link a client application linking directly to the ".dll" dll built in 1. 
>+#    This should produce no errors. 
>+#
>+# 4. compile and link a client application linking directly to the ".sl" dll built in 2. 
>+#    This should produce no errors. 
>+
>+# This test can only be run on PE/COFF platforms.
>+if {    ![istarget *-*-cygwin*]
>+     && ![istarget *-*-mingw*]
>+     && ![istarget *-*-pe] } {
>+    return
>+}
>+
>+# No compiler, no test.
>+if { [which $CC] == 0 } {
>+    untested "Direct linking to dll test"
>+    return
>+}
>+
>+set tmpdir tmpdir
>+
>+proc test_direct_link_dll {} {
>+    global CC
>+    global CFLAGS
>+    global srcdir
>+    global subdir
>+    global tmpdir
>+    
>+    # Compile the dll.
>+    if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_dll.c $tmpdir/direct_dll.o ] {
>+	fail "compiling shared lib"
>+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.dll "$tmpdir/direct_dll.o" ] {
>+	fail "linking shared lib (.dll)"
>+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.sl "$tmpdir/direct_dll.o" ] {
>+	fail "linking shared lib (.sl)"
>+    } else {
>+	# Compile and link the client program.
>+	if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_client.c $tmpdir/direct_client.o ] {
>+	    fail "compiling client"
>+	} else {
>+	    # Check linking directly to direct_dll.dll.
>+	    set msg "linking client (.dll)"
>+	    if [ld_simple_link $CC $tmpdir/direct_client.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.dll" ] {
>+		pass $msg
>+	    } else {
>+		fail $msg 
>+	    }
>+
>+	    # Check linking directly to direct_dll.sl.
>+	    set msg "linking client (.sl)"
>+	    if [ld_simple_link $CC $tmpdir/direct_client.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.sl" ] {
>+		pass $msg
>+	    } else {
>+		fail $msg 
>+	    }
>+	}
>+    }
>+}
>+
>+test_direct_link_dll
>--- /dev/null	2006-12-15 15:20:35.921500000 +0000
>+++ testsuite/ld-pe/direct_dll.c	2006-12-15 15:10:30.609000000 +0000
>@@ -0,0 +1,5 @@
>+__declspec(dllexport) int
>+dll_func (void)
>+{
>+  return 10;
>+}
>--- /dev/null	2006-12-15 15:20:40.265250000 +0000
>+++ testsuite/ld-pe/direct_client.c	2006-12-15 15:10:50.640250000 +0000
>@@ -0,0 +1,8 @@
>+__declspec(dllimport) int dll_func (void);
>+
>+int
>+main()
>+{
>+  dll_func ();
>+  return 0;
>+}

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-15 15:44 [PATCH] PE direct linking to dlls, accept any filename Pedro Alves
  2006-12-15 15:48 ` Christopher Faylor
@ 2006-12-15 15:56 ` Daniel Jacobowitz
  2006-12-16  2:59 ` Danny Smith
  2 siblings, 0 replies; 15+ messages in thread
From: Daniel Jacobowitz @ 2006-12-15 15:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: binutils

On Fri, Dec 15, 2006 at 03:45:03PM +0000, Pedro Alves wrote:
> The gdb testsuite in some shared lib tests, builds shared libs with .so 
> or .sl extension uncondicionally,
> and then tries to do a direct link, even on PE platforms (eg:cygwin).  
> That fails to link with current ld.
> I sent a patch there to fix a few cases, by introducing a $SOEXT var in 
> the testsuite.
> But thinking again, I can't see a reason ld doesn't let us do a direct 
> link to a dll with an extension
> other than ".dll" or ".DLL".  The Windows loader is happy to load the 
> dlls, and in fact there are many
> examples of dlls without a ".dll" extension in production.
> 
> The attached patch makes ld be able to direct link any dll without 
> looking at the filename extension.

Sorry I hadn't gotten around to looking at the GDB patches yet.  But,
this is exactly what I wondered when I saw them.  I had foolishly
assumed it was a Windows limitation.  But if you can do this instead,
I like it much better!

-- 
Daniel Jacobowitz
CodeSourcery

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

* RE: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-15 15:44 [PATCH] PE direct linking to dlls, accept any filename Pedro Alves
  2006-12-15 15:48 ` Christopher Faylor
  2006-12-15 15:56 ` Daniel Jacobowitz
@ 2006-12-16  2:59 ` Danny Smith
  2006-12-16 15:47   ` Pedro Alves
  2 siblings, 1 reply; 15+ messages in thread
From: Danny Smith @ 2006-12-16  2:59 UTC (permalink / raw)
  To: 'Pedro Alves', Binutils

Pedro Alves
Saturday, 16 December 2006 4:45 a.m.
> Hi all,
> 
> The gdb testsuite in some shared lib tests, builds shared 
> libs with .so 
> or .sl extension uncondicionally,
> and then tries to do a direct link, even on PE platforms 
> (eg:cygwin).  
> That fails to link with current ld.
> I sent a patch there to fix a few cases, by introducing a 
> $SOEXT var in 
> the testsuite.
> But thinking again, I can't see a reason ld doesn't let us do 
> a direct 
> link to a dll with an extension
> other than ".dll" or ".DLL".  The Windows loader is happy to load the 
> dlls, and in fact there are many
> examples of dlls without a ".dll" extension in production.
> 
> The attached patch makes ld be able to direct link any dll without 
> looking at the filename extension.
> 
> Built and regtested on i686-pc-cygwin, and confirmed manually that we 
> identify dlls as dlls no matter
> what the filename is, that we don't mistake exe images with 
> dlls.  Also 
> confirmed linking with msvcrt.dll
> to be sure we can identify a dll built with MSVC as such.
> 
> Please review and commit.
> 

The new direct_dll.sl test fails on mingw32. 

gcc -B/develop/cvs/binutils/build/ld/tmpdir/ld/ -L/mingw/mingw32/lib
-L/mingw/lib -L/usr/local/lib -L/lib -L/usr/lib  -o
tmpdir/direct_client.exe tmpdir/direct_client.o tmpdir/direct_dll.sl
tmpdir/direct_dll.sl: In function `atexit':
C:/develop/cvs/winsup/src/winsup/mingw/dllcrt1.c:161: multiple
definition of `atexit'
c:/mingw/bin/../lib/gcc/i686-pc-mingw32dw2/4.2.0/../../../crt2.o:C:\deve
lop\cvs\winsup\src\winsup\mingw/crt1.c:272: first defined here
tmpdir/direct_dll.sl: In function `onexit':
C:/develop/cvs/winsup/src/winsup/mingw/dllcrt1.c:177: multiple
definition of `_onexit'
c:/mingw/bin/../lib/gcc/i686-pc-mingw32dw2/4.2.0/../../../crt2.o:C:\deve
lop\cvs\winsup\src\winsup\mingw/crt1.c:280: first defined here
tmpdir/direct_dll.sl: In function `onexit':
tmpdir/direct_client.o:direct_client.c:(.text+0x18): undefined reference
to `_imp__dll_func'
collect2: ld returned 1 exit status
FAIL: linking client (.sl)>

Danny


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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-16  2:59 ` Danny Smith
@ 2006-12-16 15:47   ` Pedro Alves
  2006-12-17  1:41     ` Danny Smith
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2006-12-16 15:47 UTC (permalink / raw)
  To: Danny Smith; +Cc: Binutils

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

Hi Danny,

First of all, thanks for taking the trouble of testing this.

I installed stock MinGW + msys + 'a few GnuWin32 (bison/texutils/flex/libiconv/libintl)
apps' to be able to build binutils.  It built fine, but I don't know yet how
to run the testsuite (no runtest/dejagnu).  So I ran the test manually.  More comments
below.

Danny Smith escreveu:

> 
> The new direct_dll.sl test fails on mingw32. 
> 
> gcc -B/develop/cvs/binutils/build/ld/tmpdir/ld/ -L/mingw/mingw32/lib
> -L/mingw/lib -L/usr/local/lib -L/lib -L/usr/lib  -o
> tmpdir/direct_client.exe tmpdir/direct_client.o tmpdir/direct_dll.sl
> tmpdir/direct_dll.sl: In function `atexit':
> C:/develop/cvs/winsup/src/winsup/mingw/dllcrt1.c:161: multiple
> definition of `atexit'
> c:/mingw/bin/../lib/gcc/i686-pc-mingw32dw2/4.2.0/../../../crt2.o:C:\deve
> lop\cvs\winsup\src\winsup\mingw/crt1.c:272: first defined here
> tmpdir/direct_dll.sl: In function `onexit':
> C:/develop/cvs/winsup/src/winsup/mingw/dllcrt1.c:177: multiple
> definition of `_onexit'
> c:/mingw/bin/../lib/gcc/i686-pc-mingw32dw2/4.2.0/../../../crt2.o:C:\deve
> lop\cvs\winsup\src\winsup\mingw/crt1.c:280: first defined here
> tmpdir/direct_dll.sl: In function `onexit':
> tmpdir/direct_client.o:direct_client.c:(.text+0x18): undefined reference
> to `_imp__dll_func'
> collect2: ld returned 1 exit status
> FAIL: linking client (.sl)>
> 

This is exactly what happens when linking with the unpatched ld.
Don't you have an ld.exe link/copy to ../../ld-new.exe in /develop/cvs/binutils/build/ld/tmpdir/ld/ ?
If there isn't one, then the one in the system will be picked up.
Maybe I missed something in direct.exp?
This is surelly a test bug, since my manual testing was successful
once I copied ld-new.exe to tmpdir/ld/ld.exe.

What does adding -Wl,-v show?
gcc -v -B/d/cegccsf/cegcc/cegcc/src/build-binutils_cvs_mingw/ld/tmpdir/ld/ \
    -o tmpdir/direct_client_dll.exe tmpdir/direct_client.o \
    tmpdir/direct_dll.sl -Wl,-v

It should show the something like:
GNU ld version 2.17.50 20061215

I will google a bit to see how to install/run dejagnu on msys/MinGW.  If you have any
hints, they would be appreciated.

Attached is an updated/cleanup patch.  Same functionality, just moved the dll detection
into pe-dll.c, and updated direct.exp to link both exes with different names.

Cheers,
Pedro Alves

----
ld/

2006-12-16  Pedro Alves  <pedro_alves@portugalmail.pt>

      * pe-dll.c (pe_bfd_is_dll): New function.
      * pe-dll.h (pe_bfd_is_dll): Declare.
      * emultempl/pe.em (gld_${EMULATION_NAME}_recognized_file):
      Recognize dlls using pe_bfd_is_dll instead of the filename extension.

ld/testsuite/

2006-12-16  Pedro Alves  <pedro_alves@portugalmail.pt>

      * ld-pe/direct.exp: New file.
      * ld-pe/direct_client.c: Likewise.
      * ld-pe/direct_dll.c: Likewise.



[-- Attachment #2: direct2.diff --]
[-- Type: text/plain, Size: 5560 bytes --]

Index: pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.92
diff -u -p -r1.92 pe-dll.c
--- pe-dll.c	1 Nov 2006 00:48:25 -0000	1.92
+++ pe-dll.c	16 Dec 2006 12:59:30 -0000
@@ -2830,3 +2830,11 @@ pe_exe_fill_sections (bfd *abfd, struct 
     }
   reloc_s->contents = reloc_d;
 }
+
+bfd_boolean
+pe_bfd_is_dll (bfd *abfd)
+{
+  return (bfd_get_format (abfd) == bfd_object
+          && obj_pe (abfd)
+          && pe_data (abfd)->dll);
+}
Index: pe-dll.h
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.h,v
retrieving revision 1.13
diff -u -p -r1.13 pe-dll.h
--- pe-dll.h	12 May 2005 07:32:03 -0000	1.13
+++ pe-dll.h	16 Dec 2006 12:59:31 -0000
@@ -59,4 +59,7 @@ extern void pe_walk_relocs_of_symbol
   (struct bfd_link_info *, const char *, int (*) (arelent *, asection *));
 extern void pe_create_import_fixup
   (arelent * rel, asection *, int);
+extern bfd_boolean pe_bfd_is_dll
+  (bfd *);
+
 #endif /* PE_DLL_H */
Index: emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.121
diff -u -p -r1.121 pe.em
--- emultempl/pe.em	3 Oct 2006 10:06:26 -0000	1.121
+++ emultempl/pe.em	16 Dec 2006 12:59:35 -0000
@@ -1415,18 +1415,12 @@ gld_${EMULATION_NAME}_recognized_file (l
 #ifdef TARGET_IS_arm_wince_pe
   pe_dll_id_target ("pei-arm-wince-little");
 #endif
-  if (bfd_get_format (entry->the_bfd) == bfd_object)
+  if (pe_bfd_is_dll (entry->the_bfd))
     {
       char fbuf[LD_PATHMAX + 1];
-      const char *ext;
-
       if (REALPATH (entry->filename, fbuf) == NULL)
-	strncpy (fbuf, entry->filename, sizeof (fbuf));
-
-      ext = fbuf + strlen (fbuf) - 4;
-
-      if (strcmp (ext, ".dll") == 0 || strcmp (ext, ".DLL") == 0)
-	return pe_implied_import_dll (fbuf);
+        strncpy (fbuf, entry->filename, sizeof (fbuf));
+      return pe_implied_import_dll (fbuf);
     }
 #endif
   return FALSE;
--- /dev/null	2006-12-16 15:37:30.154362700 +0000
+++ testsuite/ld-pe/direct.exp	2006-12-16 13:07:56.000000000 +0000
@@ -0,0 +1,91 @@
+# Expect script for direct linking from dll tests
+#   Copyright 2006
+#   Free Software Foundation, Inc.
+#
+# This file 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 2 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 this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
+#
+# Written by Pedro Alves <pedro_alves@portugalmail.pt>
+#
+ 
+# Note: 
+# 
+# This test checks the "direct linking to a dll" functionality.
+# 
+# The test has 4 stages: 
+# 
+# 1. compile and link a test dll with ".dll" extension.
+#
+# 2. compile and link a test dll with ".sl" (i.e. != ".dll") extension.
+#
+# 3. compile and link a client application linking directly to the ".dll" dll built in 1. 
+#    This should produce no errors. 
+#
+# 4. compile and link a client application linking directly to the ".sl" dll built in 2. 
+#    This should produce no errors. 
+
+# This test can only be run on PE/COFF platforms.
+if {    ![istarget *-*-cygwin*]
+     && ![istarget *-*-mingw*]
+     && ![istarget *-*-pe] } {
+    return
+}
+
+# No compiler, no test.
+if { [which $CC] == 0 } {
+    untested "Direct linking to dll test"
+    return
+}
+
+set tmpdir tmpdir
+
+proc test_direct_link_dll {} {
+    global CC
+    global CFLAGS
+    global srcdir
+    global subdir
+    global tmpdir
+    
+    # Compile the dll.
+    if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_dll.c $tmpdir/direct_dll.o ] {
+	fail "compiling shared lib"
+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.dll "$tmpdir/direct_dll.o" ] {
+	fail "linking shared lib (.dll)"
+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.sl "$tmpdir/direct_dll.o" ] {
+	fail "linking shared lib (.sl)"
+    } else {
+	# Compile and link the client program.
+	if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_client.c $tmpdir/direct_client.o ] {
+	    fail "compiling client"
+	} else {
+	    # Check linking directly to direct_dll.dll.
+	    set msg "linking client (.dll)"
+	    if [ld_simple_link $CC $tmpdir/direct_client_dll.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.dll" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+
+	    # Check linking directly to direct_dll.sl.
+	    set msg "linking client (.sl)"
+	    if [ld_simple_link $CC $tmpdir/direct_client_sl.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.sl" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+	}
+    }
+}
+
+test_direct_link_dll
--- /dev/null	2006-12-16 15:37:41.761052300 +0000
+++ testsuite/ld-pe/direct_dll.c	2006-12-15 20:47:14.000000000 +0000
@@ -0,0 +1,5 @@
+__declspec(dllexport) int
+dll_func (void)
+{
+  return 10;
+}
--- /dev/null	2006-12-16 15:37:46.728194700 +0000
+++ testsuite/ld-pe/direct_client.c	2006-12-15 20:47:14.000000000 +0000
@@ -0,0 +1,8 @@
+__declspec(dllimport) int dll_func (void);
+
+int
+main()
+{
+  dll_func ();
+  return 0;
+}


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

* RE: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-16 15:47   ` Pedro Alves
@ 2006-12-17  1:41     ` Danny Smith
  2006-12-17  6:05       ` Christopher Faylor
  2006-12-18 11:47       ` Pedro Alves
  0 siblings, 2 replies; 15+ messages in thread
From: Danny Smith @ 2006-12-17  1:41 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: 'Binutils'

> 
> This is exactly what happens when linking with the unpatched ld.
> Don't you have an ld.exe link/copy to ../../ld-new.exe in 
> /develop/cvs/binutils/build/ld/tmpdir/ld/ ?
> If there isn't one, then the one in the system will be picked up.
> Maybe I missed something in direct.exp?
> This is surelly a test bug, since my manual testing was successful
> once I copied ld-new.exe to tmpdir/ld/ld.exe.

Most be a bug, at least in my instantiation, of deja-gnu.
If I 'make install' newly built ld.exe, the test passes.   

So I withdraw my objection to the patch.

Could you add a test for symlinks (on cygwin), eg where libfoo.a is a
symlink to $PATH/foo.dll.
This is what 
       if (REALPATH (entry->filename, fbuf) == NULL)
		strncpy (fbuf, entry->filename, sizeof (fbuf));
is about I think.


Danny

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-17  1:41     ` Danny Smith
@ 2006-12-17  6:05       ` Christopher Faylor
  2006-12-18  8:07         ` Danny Smith
  2006-12-18 11:47       ` Pedro Alves
  1 sibling, 1 reply; 15+ messages in thread
From: Christopher Faylor @ 2006-12-17  6:05 UTC (permalink / raw)
  To: 'Pedro Alves', 'Binutils'

On Sun, Dec 17, 2006 at 02:41:55PM +1300, Danny Smith wrote:
>> 
>> This is exactly what happens when linking with the unpatched ld.
>> Don't you have an ld.exe link/copy to ../../ld-new.exe in 
>> /develop/cvs/binutils/build/ld/tmpdir/ld/ ?
>> If there isn't one, then the one in the system will be picked up.
>> Maybe I missed something in direct.exp?
>> This is surelly a test bug, since my manual testing was successful
>> once I copied ld-new.exe to tmpdir/ld/ld.exe.
>
>Most be a bug, at least in my instantiation, of deja-gnu.
>If I 'make install' newly built ld.exe, the test passes.   
>
>So I withdraw my objection to the patch.
>
>Could you add a test for symlinks (on cygwin), eg where libfoo.a is a
>symlink to $PATH/foo.dll.
>This is what 
>       if (REALPATH (entry->filename, fbuf) == NULL)
>		strncpy (fbuf, entry->filename, sizeof (fbuf));
>is about I think.

Does it not work with cygwin symlinks currently?

cgf

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

* RE: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-17  6:05       ` Christopher Faylor
@ 2006-12-18  8:07         ` Danny Smith
  0 siblings, 0 replies; 15+ messages in thread
From: Danny Smith @ 2006-12-18  8:07 UTC (permalink / raw)
  To: 'Pedro Alves', 'Binutils'


 Christopher Faylor
 Sunday, 17 December 2006 7:05 p.m.
> 
> On Sun, Dec 17, 2006 at 02:41:55PM +1300, Danny Smith wrote:
> >> 
> >> This is exactly what happens when linking with the unpatched ld.
> >> Don't you have an ld.exe link/copy to ../../ld-new.exe in 
> >> /develop/cvs/binutils/build/ld/tmpdir/ld/ ?
> >> If there isn't one, then the one in the system will be picked up.
> >> Maybe I missed something in direct.exp?
> >> This is surelly a test bug, since my manual testing was successful
> >> once I copied ld-new.exe to tmpdir/ld/ld.exe.
> >
> >Most be a bug, at least in my instantiation, of deja-gnu.
> >If I 'make install' newly built ld.exe, the test passes.   
> >
> >So I withdraw my objection to the patch.
> >
> >Could you add a test for symlinks (on cygwin), eg where libfoo.a is a
> >symlink to $PATH/foo.dll.
> >This is what 
> >       if (REALPATH (entry->filename, fbuf) == NULL)
> >		strncpy (fbuf, entry->filename, sizeof (fbuf));
> >is about I think.
> 
> Does it not work with cygwin symlinks currently?

It does work with cygwin symlinks currently and with Pedro's patch, with
a manual test.
Danny
 

> 
> cgf
> 

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-17  1:41     ` Danny Smith
  2006-12-17  6:05       ` Christopher Faylor
@ 2006-12-18 11:47       ` Pedro Alves
  2006-12-18 12:03         ` Pedro Alves
                           ` (2 more replies)
  1 sibling, 3 replies; 15+ messages in thread
From: Pedro Alves @ 2006-12-18 11:47 UTC (permalink / raw)
  To: Danny Smith; +Cc: 'Binutils'

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

Danny Smith wrote:

> Could you add a test for symlinks (on cygwin), eg where libfoo.a is a
> symlink to $PATH/foo.dll.
> This is what 
>        if (REALPATH (entry->filename, fbuf) == NULL)
> 		strncpy (fbuf, entry->filename, sizeof (fbuf));
> is about I think.
> 

You're right.  Then, the REALPATH call isn't needed anymore.
It was there so we could get the filename extension of the dll
in the case you mention.

I've added tests for symlinks.  Should I guard them for MinGW?  How?
The symlink functionality comes from the host not from the target.
Using msys or 'gcc -mno-cygwin' should work, no?

Attached is the new patch, tested on i686-pc-cygwin. I'll try
testing on MinGW later.

Cheers,
Pedro Alves

[-- Attachment #2: direct3.diff --]
[-- Type: text/plain, Size: 7413 bytes --]

Index: pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.92
diff -u -p -r1.92 pe-dll.c
--- pe-dll.c	1 Nov 2006 00:48:25 -0000	1.92
+++ pe-dll.c	18 Dec 2006 10:32:46 -0000
@@ -2830,3 +2830,11 @@ pe_exe_fill_sections (bfd *abfd, struct 
     }
   reloc_s->contents = reloc_d;
 }
+
+bfd_boolean
+pe_bfd_is_dll (bfd *abfd)
+{
+  return (bfd_get_format (abfd) == bfd_object
+          && obj_pe (abfd)
+          && pe_data (abfd)->dll);
+}
Index: pe-dll.h
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.h,v
retrieving revision 1.13
diff -u -p -r1.13 pe-dll.h
--- pe-dll.h	12 May 2005 07:32:03 -0000	1.13
+++ pe-dll.h	18 Dec 2006 10:32:46 -0000
@@ -59,4 +59,7 @@ extern void pe_walk_relocs_of_symbol
   (struct bfd_link_info *, const char *, int (*) (arelent *, asection *));
 extern void pe_create_import_fixup
   (arelent * rel, asection *, int);
+extern bfd_boolean pe_bfd_is_dll
+  (bfd *);
+
 #endif /* PE_DLL_H */
Index: emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.121
diff -u -p -r1.121 pe.em
--- emultempl/pe.em	3 Oct 2006 10:06:26 -0000	1.121
+++ emultempl/pe.em	18 Dec 2006 10:33:28 -0000
@@ -1415,19 +1415,8 @@ gld_${EMULATION_NAME}_recognized_file (l
 #ifdef TARGET_IS_arm_wince_pe
   pe_dll_id_target ("pei-arm-wince-little");
 #endif
-  if (bfd_get_format (entry->the_bfd) == bfd_object)
-    {
-      char fbuf[LD_PATHMAX + 1];
-      const char *ext;
-
-      if (REALPATH (entry->filename, fbuf) == NULL)
-	strncpy (fbuf, entry->filename, sizeof (fbuf));
-
-      ext = fbuf + strlen (fbuf) - 4;
-
-      if (strcmp (ext, ".dll") == 0 || strcmp (ext, ".DLL") == 0)
-	return pe_implied_import_dll (fbuf);
-    }
+  if (pe_bfd_is_dll (entry->the_bfd))
+    return pe_implied_import_dll (entry->filename);
 #endif
   return FALSE;
 }
--- /dev/null	2006-12-18 11:44:12.140625000 +0000
+++ testsuite/ld-pe/direct.exp	2006-12-18 11:43:44.500000000 +0000
@@ -0,0 +1,143 @@
+# Expect script for direct linking from dll tests
+#   Copyright 2006
+#   Free Software Foundation, Inc.
+#
+# This file 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 2 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 this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
+#
+# Written by Pedro Alves <pedro_alves@portugalmail.pt>
+#
+ 
+# Note:
+# 
+# This test checks the "direct linking to a dll" functionality.
+# 
+# The test has 7 stages:
+# 
+# 1. compile and link a test dll with ".dll" extension.
+#
+# 2. compile and link a test dll with ".sl" (i.e. != ".dll") extension.
+#
+# 3. compile and link a client application linking directly to the ".dll" dll built in 1.
+#    This should produce no errors.
+#
+# 4. compile and link a client application linking directly to the ".sl" dll built in 2.
+#    This should produce no errors.
+#
+# 5. compile and link a client application linking directly to a symlink into 
+#    the ".dll" dll built in 1.
+#    This should produce no errors.
+#
+# 6. compile and link a client application linking directly to a symlink into 
+#    the ".sl" dll built in 1.
+#    This should produce no errors.
+#
+# 7. run the produced executables
+
+# This test can only be run on PE/COFF platforms.
+if {    ![istarget *-*-cygwin*]
+     && ![istarget *-*-mingw*]
+     && ![istarget *-*-pe] } {
+    return
+}
+
+# No compiler, no test.
+if { [which $CC] == 0 } {
+    untested "Direct linking to dll test"
+    return
+}
+
+set tmpdir tmpdir
+
+proc test_direct_link_dll {} {
+    global CC
+    global CFLAGS
+    global srcdir
+    global subdir
+    global tmpdir
+    
+    # Compile the dll.
+    if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_dll.c $tmpdir/direct_dll.o ] {
+	fail "compiling shared lib"
+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.dll "$tmpdir/direct_dll.o" ] {
+	fail "linking shared lib (.dll)"
+    } elseif ![ld_simple_link "$CC -shared" $tmpdir/direct_dll.sl "$tmpdir/direct_dll.o" ] {
+	fail "linking shared lib (.sl)"
+    } else {
+	# Compile and link the client program.
+	if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/direct_client.c $tmpdir/direct_client.o ] {
+	    fail "compiling client"
+	} else {
+	    # Check linking directly to direct_dll.dll.
+	    set msg "linking client (.dll)"
+	    if [ld_simple_link $CC $tmpdir/direct_client_dll.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.dll" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+
+	    # Check linking directly to direct_dll.sl.
+	    set msg "linking client (.sl)"
+	    if [ld_simple_link $CC $tmpdir/direct_client_sl.exe "$tmpdir/direct_client.o $tmpdir/direct_dll.sl" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+
+	    # Check dll direct linking through symlink to .dll.
+	    # Create symbolic link.
+	    catch "exec ln -fs direct_dll.dll $tmpdir/libdirect_dll.dll.a" ln_catch
+	    set msg "linking client (symlink -> .dll)"
+	    if [ld_simple_link $CC $tmpdir/direct_client_symlink_dll.exe "$tmpdir/direct_client.o $tmpdir/libdirect_dll.dll.a" ] {
+	        pass $msg
+	    } else {
+		fail $msg
+	    }
+		
+	    # Check dll direct linking through symlink to .sl.
+	    # Create symbolic link.
+	    catch "exec ln -fs direct_dll.sl $tmpdir/libdirect_sl.dll.a" ln_catch
+	    set msg "linking client (symlink -> .sl)"
+	    if [ld_simple_link $CC $tmpdir/direct_client_symlink_sl.exe "$tmpdir/direct_client.o $tmpdir/libdirect_sl.dll.a" ] {
+		pass $msg
+	    } else {
+		fail $msg 
+	    }
+	}
+    }
+}
+
+proc directdll_execute {exe msg} {
+    set expected ""
+    catch "exec $exe" prog_output
+    if [string match $expected $prog_output] then {
+        pass $msg
+    } else {
+        verbose $prog_output
+        fail $msg
+    }
+}
+
+test_direct_link_dll
+
+# This is as far as we can go with a cross-compiler
+if ![isnative] then {
+    verbose "Not running natively, so cannot execute binaries"
+    return
+}
+
+directdll_execute "$tmpdir/direct_client_dll.exe" "running direct linked dll (.dll)"
+directdll_execute "$tmpdir/direct_client_sl.exe" "running direct linked dll (.sl)"
+directdll_execute "$tmpdir/direct_client_symlink_sl.exe" "running direct linked dll (symlink -> .sl)"
+directdll_execute "$tmpdir/direct_client_symlink_dll.exe" "running direct linked dll (symlink -> .dll)"
--- /dev/null	2006-12-18 11:44:19.578125000 +0000
+++ testsuite/ld-pe/direct_dll.c	2006-12-18 10:32:46.671875000 +0000
@@ -0,0 +1,5 @@
+__declspec(dllexport) int
+dll_func (void)
+{
+  return 10;
+}
--- /dev/null	2006-12-18 11:44:24.765625000 +0000
+++ testsuite/ld-pe/direct_client.c	2006-12-18 10:32:46.671875000 +0000
@@ -0,0 +1,8 @@
+__declspec(dllimport) int dll_func (void);
+
+int
+main()
+{
+  dll_func ();
+  return 0;
+}

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-18 11:47       ` Pedro Alves
@ 2006-12-18 12:03         ` Pedro Alves
  2006-12-18 20:46         ` Pedro Alves
  2006-12-19  8:49         ` Danny Smith
  2 siblings, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2006-12-18 12:03 UTC (permalink / raw)
  To: Danny Smith; +Cc: 'Binutils'

Pedro Alves wrote:

> Attached is the new patch, tested on i686-pc-cygwin. I'll try
> testing on MinGW later.
> 

Sorry, forgot the ChangeLog.  It is the same as before.

Cheers,
Pedro Alves

----
ld/

2006-12-18  Pedro Alves  <pedro_alves@portugalmail.pt>

      * pe-dll.c (pe_bfd_is_dll): New function.
      * pe-dll.h (pe_bfd_is_dll): Declare.
      * emultempl/pe.em (gld_${EMULATION_NAME}_recognized_file):
      Recognize dlls using pe_bfd_is_dll instead of using the filename
      extension.

ld/testsuite/

2006-12-18  Pedro Alves  <pedro_alves@portugalmail.pt>

      * ld-pe/direct.exp: New file.
      * ld-pe/direct_client.c: Likewise.
      * ld-pe/direct_dll.c: Likewise.

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-18 11:47       ` Pedro Alves
  2006-12-18 12:03         ` Pedro Alves
@ 2006-12-18 20:46         ` Pedro Alves
  2006-12-18 22:40           ` Christopher Faylor
  2006-12-19  8:49         ` Danny Smith
  2 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2006-12-18 20:46 UTC (permalink / raw)
  To: Danny Smith; +Cc: 'Binutils'

Pedro Alves escreveu:
> Danny Smith wrote:
> 
> I've added tests for symlinks.  Should I guard them for MinGW?  How?
> The symlink functionality comes from the host not from the target.
> Using msys or 'gcc -mno-cygwin' should work, no?
> 
> Attached is the new patch, tested on i686-pc-cygwin. I'll try
> testing on MinGW later.
> 

Just tested manually on MinGW+msys and all tests passed OK.
I've been learning that setting up dejagnu on MinGW is a bit
convoluted, so I'll postpone that to the next weekend the soonest.
Would be great if the patch could be applied nonetheless.

(On cygwin, I've also tested that deleting the dlls before
the run time tests by injecting in the proper place in direct.exp a:
catch "exec rm -f $tmpdir/direct_dll.sl" ln_catch
really FAILs with "child process exited abnormally" output.
)

Cheers,
Pedro Alves

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-18 20:46         ` Pedro Alves
@ 2006-12-18 22:40           ` Christopher Faylor
  2007-01-02  7:42             ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: Christopher Faylor @ 2006-12-18 22:40 UTC (permalink / raw)
  To: Pedro Alves, 'Binutils'

On Mon, Dec 18, 2006 at 08:45:53PM +0000, Pedro Alves wrote:
>Pedro Alves escreveu:
>>Danny Smith wrote:
>>I've added tests for symlinks.  Should I guard them for MinGW?  How?
>>The symlink functionality comes from the host not from the target.
>>Using msys or 'gcc -mno-cygwin' should work, no?
>>
>>Attached is the new patch, tested on i686-pc-cygwin.  I'll try testing
>>on MinGW later.
>
>Just tested manually on MinGW+msys and all tests passed OK.  I've been
>learning that setting up dejagnu on MinGW is a bit convoluted, so I'll
>postpone that to the next weekend the soonest.  Would be great if the
>patch could be applied nonetheless.

It was applied, nonetheless.

Thanks again.

cgf

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

* RE: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-18 11:47       ` Pedro Alves
  2006-12-18 12:03         ` Pedro Alves
  2006-12-18 20:46         ` Pedro Alves
@ 2006-12-19  8:49         ` Danny Smith
  2006-12-21 17:12           ` Pedro Alves
  2 siblings, 1 reply; 15+ messages in thread
From: Danny Smith @ 2006-12-19  8:49 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: Binutils



Pedro Alves
Tuesday, 19 December 2006 12:47 a.m.
> To: Danny Smith
> Cc: 'Binutils'
> Subject: Re: [PATCH] PE direct linking to dlls, accept any filename.
> 
> 
> Danny Smith wrote:
> 
> > Could you add a test for symlinks (on cygwin), eg where 
> libfoo.a is a
> > symlink to $PATH/foo.dll.
> > This is what 
> >        if (REALPATH (entry->filename, fbuf) == NULL)
> > 		strncpy (fbuf, entry->filename, sizeof (fbuf));
> > is about I think.
> > 
> 
> You're right.  Then, the REALPATH call isn't needed anymore.
> It was there so we could get the filename extension of the dll
> in the case you mention.
> 
> I've added tests for symlinks.  Should I guard them for MinGW?  How?
> The symlink functionality comes from the host not from the target.
> Using msys or 'gcc -mno-cygwin' should work, no?
> 

Mingw-hosted ld.exe (whether in  msys env  or built with -mno-cygwin,
depends on MS msvcrt.dll,
which has not a clue about cygwin symlinks. Cygwin-hosted ld.exe depends
on cygwin1.dll so does recognize symlinks.

> Attached is the new patch, tested on i686-pc-cygwin. I'll try
> testing on MinGW later.
> 
That will be difficult.   MSVCRT.dll, hence mingw ld.exe,  has not a
clue about symlinks.  The deja-gnu tests expects ln -s to work.

In past, I have just accepted that if it doesn't break cygwin testsuite
results, its OK for mingw. (Many of the testcases--the bootstrap ones in
particular,  assume a POSIXish file system).

That is my long-winded way of saying  'nonetheless', thanks for the
patch  and thank Christopher for applying.
 
Danny

> Cheers,
> Pedro Alves
> 

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-19  8:49         ` Danny Smith
@ 2006-12-21 17:12           ` Pedro Alves
  0 siblings, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2006-12-21 17:12 UTC (permalink / raw)
  To: Danny Smith; +Cc: Binutils

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

Danny Smith wrote:
 >
 > Pedro Alves
 > Tuesday, 19 December 2006 12:47 a.m.
 >>
 >>Danny Smith wrote:
 >>
 >>
 >>>Could you add a test for symlinks (on cygwin), eg where
 >>
 >>
 >>
 >>I've added tests for symlinks.  Should I guard them for MinGW?  How?
 >>The symlink functionality comes from the host not from the target.
 >>Using msys or 'gcc -mno-cygwin' should work, no?
 >>
 >
 >
 > Mingw-hosted ld.exe (whether in  msys env  or built with -mno-cygwin,
 > depends on MS msvcrt.dll,
 > which has not a clue about cygwin symlinks. Cygwin-hosted ld.exe depends
 > on cygwin1.dll so does recognize symlinks.
 >
 >

I understand that.  What I was missing, was the fact that you probably
are using cygwin to drive dejagnu, but building with
--host=i686-pc-mingw32.
Well, I tried doing the same and this is what I came up with.

First, I tried puting c:/MinGW first on the PATH, and then building with
configure --host=i686-pc-mingw32 --target=i686-pc-mingw32.  That has
the problem that absolute paths won't match between cygwin and MinGW, so
I came up with the scripts in the attached cygming_wrappers.tar.gz.
They are wrappers around gcc, g++, gas and ld, which will do the path
translation using cygpath.  Works pretty well, but to run the testsuite,
we would need the mingw.exp --host_board file attached.  It was only after
doing this, that I realized that MinGW understands the /mingw path, so
if I do a /mingw mount on cygwin, the paths will match in both environments.
Next, I put the binutils sources under /mingw/src, and I don't need the
scripts anymore. :) Talk about overengineering. :)
Well, the scripts are still useful, me thinks, because they allow me to
build MinGW stuff even when the sources are not under /mingw.  It is
just for the testsuite that they become a nuisance.  So, I attached
them anyway since someone may find them useful.  (Of course, they are
a proof of concept, so don't expect them to be complete, or even fully correct.
But, they did build binutils.  )

Now back to the /mingw mount mode.

I now see the problem you where seing.  Cygwin's ln -s puts a symlink
under tmpdir/ld, but the mingw/msvcrt ld doesn't understand it.  To
me this sounds clearly as a "if I do this, then it breaks. Then don't"
do it!" case.  I solved it in two ways.  One without any changes to
binutils whatsoever, and another one with a testsuite patch.

-  The first solution was to devise an "ln" wrapper script, that filters
out "-s | --symlink" and converts a symlink request into a hardlink,
which resolves to a copy on Windows.  This makes cygwin's ln look like
msys ln.  This works pretty well, and doesn't need any change in
binutils, but it is awkward to have to remember to put that script on
the PATH.

- The second solution is to change a bit the testsuite to detect
when we are building a mingw target under cygwin.  That is implemented
in the diff attached.  I only implemented it in ld, but if this is
wanted I can provide patches for the rest of the testsuites.

What do you guys think?  Both versions of the "ln -s" fix look ugly, but
I would rather go the with the testsuite fix.

Cheers,
Pedro Alves


[-- Attachment #2: cygming_wrappers.tar.gz --]
[-- Type: application/gzip, Size: 1435 bytes --]

[-- Attachment #3: ln --]
[-- Type: text/plain, Size: 1008 bytes --]

#!/bin/bash
#
# A wrapper for calling stripping the -s and --symlink options from ln invocations.
# Author: Pedro Alves <pedro_alves@portugalmail.pt>
# Version: 0.01
#

ME="`basename $0`"
EXEC_CMD="/usr/bin/ln"

#exec -a "$ME" "$EXEC_CMD" "$@"
echo "------------------------------------------------------------------------------"

TARGET=""
LINK_NAME=""

ARGS=""
while [ -n "$1" ]; do
   arg="$1"
   shift
   case "$arg" in
      -s | --sy*)
         continue
         ;;
      --*s*)
         ;;
      -*s*)
         arg=`echo ${arg} | sed s/s//g`
         ;;
      -*)
         ;;
      *)
         if [ "x$TARGET" = "x" ]; then
           TARGET=$arg
         elif [ "x$LINK_NAME" = "x" ]; then
           LINK_NAME=$arg
         fi
         continue
         ;;
   esac
   ARGS="$ARGS '$arg'"
done

if [ "x$TARGET" != "x" ] && [ "x$LINK_NAME" != "x" ]; then
  DIR=`dirname $LINK_NAME`
  TARGET=$DIR/$TARGET
fi

eval "set -- $ARGS $TARGET $LINK_NAME"

echo "$EXEC_CMD" "$@"
exec -a "$ME" "$EXEC_CMD" "$@"

[-- Attachment #4: binutils_mingw_ln.diff --]
[-- Type: text/plain, Size: 3655 bytes --]

Index: ld/testsuite/config/default.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/config/default.exp,v
retrieving revision 1.11
diff -u -p -r1.11 default.exp
--- ld/testsuite/config/default.exp	27 May 2005 17:27:03 -0000	1.11
+++ ld/testsuite/config/default.exp	21 Dec 2006 12:43:33 -0000
@@ -51,12 +51,37 @@ if {![file isdirectory tmpdir]} then {
     catch "exec mkdir tmpdir" status
 }
 
+#
+# ld_symlink
+#	ln -s wrapper
+#
+proc ld_symlink { target link_name up_result } {
+    upvar 1 $up_result result
+    set uname "unknown"
+    catch "exec uname -o " uname
+  
+    if { [string match $uname "Cygwin"] && [istarget *-pc-mingw*] } {
+        # If a relative path was passed, we must adjust
+        # target, since in symlinks they are relative to link_name,
+        # while in cp, they are relative to $pwd    
+        if { [string first "/" $target] != 0 } {
+            set dirname [file dirname $link_name]
+            if { [string compare $dirname ""] != 0 } {      
+                set target "$dirname/$target"
+            }
+    }
+    catch "exec cp $target $link_name" result
+  } else {
+    catch "exec ln -s $target $link_name " result
+  }
+}
+
 # Make a symlink from tmpdir/as to the assembler in the build tree, so
 # that we can use a -B option to gcc to force it to use the newly
 # built assembler.
 if {![file isdirectory tmpdir/gas]} then {
     catch "exec mkdir tmpdir/gas" status
-    catch "exec ln -s ../../../gas/as-new tmpdir/gas/as" status
+    ld_symlink "../../../gas/as-new" "tmpdir/gas/as" status
 }
 set gcc_gas_flag "-B[pwd]/tmpdir/gas/"
 
@@ -65,7 +90,9 @@ set gcc_gas_flag "-B[pwd]/tmpdir/gas/"
 # built linker. 
 if {![file isdirectory tmpdir/ld]} then {
     catch "exec mkdir tmpdir/ld" status
-    catch "exec ln -s ../../ld-new tmpdir/ld/ld" status
+    verbose "calling ld_symlink "../../ld-new" "tmpdir/ld/ld""
+    ld_symlink "../../ld-new" "tmpdir/ld/ld" status
+    verbose "status=$status"
 }
 set gcc_ld_flag "-B[pwd]/tmpdir/ld/"
 
@@ -209,7 +236,7 @@ proc ld_nm { nm nmflags object } {
 
 #
 # ld_exec
-#	execute ithe target
+#	execute the target
 #
 proc ld_exec { target output } {
 	default_ld_exec $target $output
Index: ld/testsuite/ld-pe/direct.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-pe/direct.exp,v
retrieving revision 1.1
diff -u -p -r1.1 direct.exp
--- ld/testsuite/ld-pe/direct.exp	19 Dec 2006 01:51:02 -0000	1.1
+++ ld/testsuite/ld-pe/direct.exp	21 Dec 2006 11:58:19 -0000
@@ -97,7 +97,8 @@ proc test_direct_link_dll {} {
 
 	    # Check dll direct linking through symlink to .dll.
 	    # Create symbolic link.
-	    catch "exec ln -fs direct_dll.dll $tmpdir/libdirect_dll.dll.a" ln_catch
+	    catch "exec rm -f $tmpdir/libdirect_dll.dll.a" ln_catch
+	    ld_symlink direct_dll.dll $tmpdir/libdirect_dll.dll.a ln_catch
 	    set msg "linking client (symlink -> .dll)"
 	    if [ld_simple_link $CC $tmpdir/direct_client_symlink_dll.exe "$tmpdir/direct_client.o $tmpdir/libdirect_dll.dll.a" ] {
 	        pass $msg
@@ -107,7 +108,8 @@ proc test_direct_link_dll {} {
 		
 	    # Check dll direct linking through symlink to .sl.
 	    # Create symbolic link.
-	    catch "exec ln -fs direct_dll.sl $tmpdir/libdirect_sl.dll.a" ln_catch
+	    catch "exec rm -f $tmpdir/libdirect_sl.dll.a" ln_catch
+	    ld_symlink direct_dll.sl $tmpdir/libdirect_sl.dll.a ln_catch
 	    set msg "linking client (symlink -> .sl)"
 	    if [ld_simple_link $CC $tmpdir/direct_client_symlink_sl.exe "$tmpdir/direct_client.o $tmpdir/libdirect_sl.dll.a" ] {
 		pass $msg

[-- Attachment #5: mingw.exp --]
[-- Type: text/plain, Size: 894 bytes --]

# The canonical unix board description.
load_generic_config "unix";

process_multilib_options "";

set_board_info compiler  "[find_gcc]";

set_board_info bmk,use_alarm 1;

set_board_info gdb,noinferiorio 1;

set_board_info use_cygpath 1;

#Need a way to auto-detect which testsuite is running
#so the set AS* below always work...

#These are needed for the gas testsuite
set AS_NEW [findfile $base_dir/../as-new "../as-new" [transform as]]
set AS "as-new $AS_NEW";

#These are needed for the ld testsuite
#set as_new [findfile $base_dir/../gas/as-new $base_dir/../gas/as-new [transform as]]
set as_new $base_dir/../gas/as-new
set as "as-new $as_new";
set AS "as-new $as_new";
set ld_new [findfile $base_dir/ld-new $base_dir/ld-new [transform ld]]
set ld "ld-new $ld_new";
set LD "ld-new $ld_new";

set_board_info ldflags "-Wl,--enable-auto-import"

send_user "configuring for mingw testing\n";

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

* Re: [PATCH] PE direct linking to dlls, accept any filename.
  2006-12-18 22:40           ` Christopher Faylor
@ 2007-01-02  7:42             ` Alan Modra
  0 siblings, 0 replies; 15+ messages in thread
From: Alan Modra @ 2007-01-02  7:42 UTC (permalink / raw)
  To: Pedro Alves, binutils

Fixes compile and link errors introduced with the 2006-12-18 change
to these files.

	* pe-dll.c: Include pe-dll.h.
	* pep-dll.c (pe_bfd_is_dll): Define.

Index: ld/pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.93
diff -u -p -r1.93 pe-dll.c
--- ld/pe-dll.c	18 Dec 2006 22:38:53 -0000	1.93
+++ ld/pe-dll.c	2 Jan 2007 07:30:25 -0000
@@ -40,6 +40,7 @@
 #include "coff/internal.h"
 #include "../bfd/libcoff.h"
 #include "deffile.h"
+#include "pe-dll.h"
 
 #ifdef pe_use_x86_64
 
Index: ld/pep-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pep-dll.c,v
retrieving revision 1.1
diff -u -p -r1.1 pep-dll.c
--- ld/pep-dll.c	20 Sep 2006 11:35:09 -0000	1.1
+++ ld/pep-dll.c	2 Jan 2007 07:30:25 -0000
@@ -50,6 +50,7 @@
 #define pe_dll_generate_implib      pep_dll_generate_implib
 #define pe_dll_add_excludes         pep_dll_add_excludes
 #define pe_walk_relocs_of_symbol    pep_walk_relocs_of_symbol
+#define pe_bfd_is_dll		    pep_bfd_is_dll
 
 /* Uses x86_64 PE+.  */
 #define pe_use_x86_64

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

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

end of thread, other threads:[~2007-01-02  7:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-15 15:44 [PATCH] PE direct linking to dlls, accept any filename Pedro Alves
2006-12-15 15:48 ` Christopher Faylor
2006-12-15 15:56 ` Daniel Jacobowitz
2006-12-16  2:59 ` Danny Smith
2006-12-16 15:47   ` Pedro Alves
2006-12-17  1:41     ` Danny Smith
2006-12-17  6:05       ` Christopher Faylor
2006-12-18  8:07         ` Danny Smith
2006-12-18 11:47       ` Pedro Alves
2006-12-18 12:03         ` Pedro Alves
2006-12-18 20:46         ` Pedro Alves
2006-12-18 22:40           ` Christopher Faylor
2007-01-02  7:42             ` Alan Modra
2006-12-19  8:49         ` Danny Smith
2006-12-21 17:12           ` Pedro Alves

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