[PATCH] Fix LD test FAIL: export table in executable on Cygwin

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

    Hi all,

  The testcase ld-cygwin/exe-export.exp currently fails for two reasons.

#1.  The first is that the .DEF file omits the .dll suffix from the filename
given to the LIBRARY command:

----------------------------------<snip>----------------------------------
$  cat ld/testsuite/ld-cygwin/testdll.def
LIBRARY testdll

EXPORTS
dllwrite

$
----------------------------------<snip>----------------------------------

  The 'ld' manual says this about LIBRARY command syntax:

----------------------------------<snip>----------------------------------
          The optional `LIBRARY <name>' command indicates the _internal_
          name of the output DLL. If `<name>' does not include a suffix,
          the default library suffix, `.DLL' is appended.

          When the .DEF file is used to build an application, rather
          than a library, the `NAME <name>' command should be used
          instead of `LIBRARY'. If `<name>' does not include a suffix,
          the default executable suffix, `.EXE' is appended.
----------------------------------<snip>----------------------------------

  However, in this test dlltool is used to parse the .DEF file and generate an
import library, and dlltool does not have the same behaviour.  Note that the
manual for dlltool suggests that it in fact does:

----------------------------------<snip>----------------------------------
14.1 The format of the `dlltool' `.def' file
============================================

A `.def' file contains any number of the following commands:

`NAME' NAME `[ ,' BASE `]'
     The result is going to be named NAME`.exe'.

`LIBRARY' NAME `[ ,' BASE `]'
     The result is going to be named NAME`.dll'.
----------------------------------<snip>----------------------------------

but no such code to implement this behaviour exists.  The lack of a suffix on
the actual filename in the import section of the DLL fatally confuses the
runtime loader.  Trivially fixed by copying the logic used in ld into dlltool.

#2.  Another problem is that this uniquely minimal application, unlike all
other windows executables, doesn't link kernel32.dll, because the app doesn't
import from it (or from any DLL that imports from it).  However the windows
process startup code does expect it to be there (it's usually the one source
of functions you can safely call during DllMain, for example) and this is
normally guaranteed by linking with the C runtime, which is bound to import
from it.

  As we're bypassing all the standard CRT stuff in this test, we end up with
the loader initialising the DLL, then calling NtContinue with a CONTEXT
structure that points $eip to KERNEL32!BaseProcessStartThunk - but it isn't
there!  (Don't ask me exactly how ntdll.dll has managed to get the proc
address of that function without loading it; probably something to do with the
fact that the same image sections for KnownDlls are mapped into all processes.)

  It's easy to fix this; just add an import to the testcase executable to
ensure that kernel32 does, after all, get loaded and mapped at runtime.

  Ok?

binutils/ChangeLog

	* dlltool.c (set_dll_name_from_def):  Accept new second arg that
	indicates if we are building DLL or EXE, and use it to add a
	default suffix to the output filename when none is already present.
	(def_name):  Indicate we are building an EXE when calling it.
	(def_library):  Indicate we are building a DLL when calling it.

ld/testsuite/ChangeLog

	* ld-cygwin/exe-export.exp:  Add "-lkernel32" when linking test exe.
	* ld-cygwin/testexe.c (testexe_main):  Indicate whether global_a
	was set to correct final value using error return status.
	(testexe_dummy):  Dummy function calls an import from kernel32.dll
	to ensure it is mapped into the process space at runtime.


    cheers,
      DaveK

Index: binutils/dlltool.c
===================================================================
RCS file: /cvs/src/src/binutils/dlltool.c,v
retrieving revision 1.91
diff -p -u -r1.91 dlltool.c
--- binutils/dlltool.c	26 Jan 2009 15:52:55 -0000	1.91
+++ binutils/dlltool.c	15 Mar 2009 20:31:07 -0000
@@ -782,7 +782,7 @@ static void fill_ordinals (export_type *
 static void mangle_defs (void);
 static void usage (FILE *, int);
 static void inform (const char *, ...) ATTRIBUTE_PRINTF_1;
-static void set_dll_name_from_def (const char *);
+static void set_dll_name_from_def (const char *name, char is_dll);
 
 static char *
 prefix_encode (char *start, unsigned code)
@@ -1001,13 +1001,22 @@ def_exports (const char *name, const cha
 }
 
 static void
-set_dll_name_from_def (const char * name)
+set_dll_name_from_def (const char *name, char is_dll)
 {
-  const char* image_basename = lbasename (name);
+  const char *image_basename = lbasename (name);
   if (image_basename != name)
     non_fatal (_("%s: Path components stripped from image name, '%s'."),
 	      def_file, name);
-  dll_name = xstrdup (image_basename);
+  /* Append the default suffix, if none specified.  */ 
+  if (strchr (image_basename, '.') == 0)
+    {
+      const char * suffix = is_dll ? ".dll" : ".exe";
+
+      dll_name = xmalloc (strlen (image_basename) + strlen (suffix) + 1);
+      sprintf (dll_name, "%s%s", image_basename, suffix);
+    }
+  else
+    dll_name = xstrdup (image_basename);
 }
 
 void
@@ -1021,8 +1030,8 @@ def_name (const char *name, int base)
 
   /* If --dllname not provided, use the one in the DEF file.
      FIXME: Is this appropriate for executables?  */
-  if (! dll_name)
-    set_dll_name_from_def (name);
+  if (!dll_name)
+    set_dll_name_from_def (name, 0);
   d_is_exe = 1;
 }
 
@@ -1036,8 +1045,8 @@ def_library (const char *name, int base)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
   /* If --dllname not provided, use the one in the DEF file.  */
-  if (! dll_name)
-    set_dll_name_from_def (name);
+  if (!dll_name)
+    set_dll_name_from_def (name, 1);
   d_is_dll = 1;
 }
 
Index: ld/testsuite/ld-cygwin/exe-export.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-cygwin/exe-export.exp,v
retrieving revision 1.3
diff -p -u -r1.3 exe-export.exp
--- ld/testsuite/ld-cygwin/exe-export.exp	6 Jul 2007 14:09:42 -0000	1.3
+++ ld/testsuite/ld-cygwin/exe-export.exp	15 Mar 2009 20:31:08 -0000
@@ -119,7 +119,7 @@ if ![ld_compile "$CC $CFLAGS" $srcdir/$s
     return
 }
 
-if ![ld_special_link "$CC $LDFLAGS $MYLDFLAGS -e _testexe_main@16" $tmpdir/testexe.exe "$tmpdir/testexe.o $srcdir/$subdir/testexe.def $tmpdir/testdll.lib"] {
+if ![ld_special_link "$CC $LDFLAGS $MYLDFLAGS -e _testexe_main@16" $tmpdir/testexe.exe "$tmpdir/testexe.o $srcdir/$subdir/testexe.def $tmpdir/testdll.lib -lkernel32"] {
     fail "linking executable"
     return
 } 
Index: ld/testsuite/ld-cygwin/testexe.c
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-cygwin/testexe.c,v
retrieving revision 1.1
diff -p -u -r1.1 testexe.c
--- ld/testsuite/ld-cygwin/testexe.c	2 Jun 2003 09:11:01 -0000	1.1
+++ ld/testsuite/ld-cygwin/testexe.c	15 Mar 2009 20:31:08 -0000
@@ -11,6 +11,21 @@ extern void dllwrite (void);
 int _stdcall
 testexe_main (void* p1, void *p2, char* p3, int p4)
 {
-  dllwrite ();
-  return 0;
+  dllwrite ();
+  /* We can't print or assert in a minimal app like this,
+     so use the return status to indicate if global_a
+     ended up with the correct expected value.  */
+  return 1 - global_a;
 }
+
+/* We have to import something, anything at all, from 
+   kernel32, in order to have the thread and process
+   base thunk routines loaded when we start running!.  */
+extern __attribute((dllimport)) void _stdcall Sleep (unsigned int duration);
+
+int _stdcall
+testexe_dummy (unsigned int foobar)
+{
+  Sleep (foobar);
+}
+