[PATCH] [cygwin] Refactor C++ exception handling for Win32 correctness

[Charles Wilson]

* tests/exception.at (common.h): New file, refactored from
(module.h): Move declaration of modexc to libcommon.
Include common.h. Use explicit import/export markings for
symbols when building on win32 or cygwin.
(common.cpp): New file.
(module.cpp): Ensure correct symbols markings when including
module.h.
(lib.h): Use explicit import/export markings for symbols
when building on win32 or cygwin.
(lib.cpp): Ensure correct symbol markings when including lib.h.
(main.cpp): Include common.h.
(commands): Ensure correct symbol markings when compiling
main.cpp, and lib.cpp, and module.cpp. Add command to compile
common.cpp, and to link libcommon.la.  Add libcommon.la when
linking module.la and main.  Add command to install libcommon.la.

Signed-off-by: Charles Wilson <...>
---
While this patch
  http://lists.gnu.org/archive/html/libtool-patches/2010-06/msg00111.html
ensure that the affected test passes on cygwin, the investigation
revealed two additional "errors" in this test, with regards to
the interaction of C++, Win32 DLLs, and exceptions.  However, these
errors aren't actually exposed by this test, but for correctness it
seems appropriate to address them as well.

For background, see:
  http://lists.gnu.org/archive/html/bug-libtool/2010-06/msg00069.html
  http://cygwin.com/ml/cygwin/2010-06/msg00392.html

There are ODR issues with the definition of the exception class(es).
While on linux, the code may technically be in violation, in practice
it is ok and works. But on win32 with DLLs...it is NOT ok, even if it
happens to work in this particular instance.  The problem is that
both main and module have their own definitions of modexc; both main
and lib have their own definitions of libexc.  To avoid this, module's
version of modexc should be dllexport, while main's version should be
dllimport. Ditto libexc.

However, to do this correctly, main needs to "statically" link to the
vtable for modexc (that is, resolve at linktime).  If this were done
directly to module.dll, then you'd have a direct dependence, and this
test wouldn't actually be able to "dlopen" the module; it'd be no
different than "dlopen-ing" the linktime-resolved dynamic library
liba.

So...to do this right, you have to also put all classes, which are not
simple POD types, and are exposed in the interface between the client
(main) and the module -- including exception types like modexc -- into
a separate DLL.  This way, both main and module can resolve elements
of those "interface classes" at linktime (such as vtables, typeinfo,
and typename data for modexc, in this case).

The attached patch does this by moving the definition of modexc
to a new libcommon.la library, and by carefully using dllimport
and dllexport attributes (when building on Win32 or cygwin).

The second problem discovered was a bug in the current version of
the cygwin linker where --export-all-symbols re-exports symbols
imported from another library. So, if (for instance) module.dll
were linked using -e-a-s, then it would re-export all of the symbols
it imports from the libcommon dll.  This is incorrect; however,
since we do not use --export-all-symbols when linking any of these
libraries, it is a non-issue.

Now, this same bug might manifest if one were to link module.la using
the libtool flags -export-symbols LIST or -export-symbols-regex REGEX,
if any symbols in the libcommon DLL appeared in LIST or matched the
REGEX.  Again, however, we do not use those flags in this test case
so it is a non-issue.

To prevent confusion, in case any of these flags are used by this test
in the future, explanatory comments have been added to exceptions.at.

Tested on cygwin, mingw, and linux.

NOTE: because of a long-standing packaging error with mingw-gcc-4.4.0,
you have to manually remove
     lib/gcc/mingw32/4.4.0/libstdc++.la
     lib/gcc/mingw32/4.4.0/libsupc++.la
or libtool is unable to link C++ images. But that's not related to
this C++ exceptions issue.

OK to push?

--
Chuck

 tests/exceptions.at |  133 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 121 insertions(+), 12 deletions(-)

diff --git a/tests/exceptions.at b/tests/exceptions.at
index 23442a3..61447c6 100644
--- a/tests/exceptions.at
+++ b/tests/exceptions.at
@@ -36,10 +36,59 @@ esac], [], [ignore])
 
 CPPFLAGS="$LTDLINCL $CPPFLAGS"
 
-AT_DATA([module.h],
-[[#include <exception>
+# Win32 (and cygwin) notes
+# ------------------------
+# When using C++ and Win32 DLLs, data types used in the DLL's interface
+# which are other-than-POD, must have vtables, typeinfo, and other
+# elements resolved when the client is linked.  This includes exception
+# classes.  Therefore, the exception class "modexc" thrown by the
+# dynamically-loaded module must be defined in a separate DLL, to which
+# both that module and main must be directly linked; hence, the 'common'
+# library. This treatment is not necessary for liba (e.g. the libexc
+# exception class), because that library is not dynamically loaded.
+# However, for simplicity, the refactoring of modexc into a separate
+# library is done for all platforms.
+#
+# Also, when linking a C++ DLL with another C++ DLL, some versions of
+# the GNU toolchain on Win32 (or cygwin) mistakenly re-export symbols
+# that were imported from the other DLL, when the client DLL is linked
+# using -export-all-symbols.  Similar issues MAY also arise with those
+# versions of the GNU toolchain if using the libtool link flags
+# -export-symbols LIST or -export-symbols-regex REGEX, if any symbols
+# from the dependency, rather than client, library are listed (or match
+# the regex).  However, in this test, none of these situations apply,
+# so we don't directly address it.  Otherwise, the correct mechanism
+# would be to avoid all of those flags, and instead explicitly decorate
+# all symbols with appropriate __attribute__ ((dllexport)) or
+# __attribute__ ((dllimport)) flags when building the DLLs and the
+# clients.
+#
+# For more information, see these two threads:
+#   http://lists.gnu.org/archive/html/bug-libtool/2010-06/msg00069.html
+#   http://cygwin.com/ml/cygwin/2010-06/msg00392.html
+# To sum up: C++ is complicated.
+AT_DATA([common.h],
+[[#ifndef LIBTOOL_TEST_COMMON_HEADER
+#define LIBTOOL_TEST_COMMON_HEADER
+
+#include <exception>
 #include <string>
-class modexc : public std::exception {
+
+#if defined(__CYGWIN__) || defined(_WIN32)
+# if defined(DLL_EXPORT) || defined(USING_COMMON_DLL)
+#  if defined(LIBTOOL_TEST_IN_COMMON)
+#   define COMMON_IMPEXP __attribute__ ((dllexport))
+#  else
+#   define COMMON_IMPEXP __attribute__ ((dllimport))
+#  endif
+# else
+#  define COMMON_IMPEXP
+# endif
+#else
+# define COMMON_IMPEXP
+#endif
+
+class COMMON_IMPEXP modexc : public std::exception {
 public:
   modexc (std::string str) : message (str) { }
   ~modexc () throw () { }
@@ -50,11 +99,45 @@ public:
 private:
   std::string message;
 };
-extern "C" int modfoo () throw (modexc);
+
+extern "C" int COMMON_IMPEXP common_dummy (void);
+#endif
+]])
+
+AT_DATA([common.cpp],
+[[#define LIBTOOL_TEST_IN_COMMON
+#include "common.h"
+
+extern "C"
+int common_dummy (void)
+{
+  return 0;
+}
+]])
+
+AT_DATA([module.h],
+[[#include "common.h"
+
+#if defined(__CYGWIN__) || defined(_WIN32)
+# if defined(DLL_EXPORT) || defined(USING_MODULE_DLL)
+#  if defined(LIBTOOL_TEST_IN_MODULE)
+#   define MODULE_IMPEXP __attribute__ ((dllexport))
+#  else
+#   define MODULE_IMPEXP __attribute__ ((dllimport))
+#  endif
+# else
+#  define MODULE_IMPEXP
+# endif
+#else
+# define MODULE_IMPEXP
+#endif
+
+extern "C" int MODULE_IMPEXP modfoo () throw (modexc);
 ]])
 
 AT_DATA([module.cpp],
 [[#include <iostream>
+#define LIBTOOL_TEST_IN_MODULE
 #include "module.h"
 
 int modbar (void) throw (modexc)
@@ -79,7 +162,23 @@ int modfoo (void) throw (modexc)
 AT_DATA([lib.h],
 [[#include <exception>
 #include <string>
-class libexc : public std::exception {
+
+
+#if defined(__CYGWIN__) || defined(_WIN32)
+# if defined(DLL_EXPORT) || defined(USING_LIB_DLL)
+#  if defined(LIBTOOL_TEST_IN_LIB)
+#   define LIB_IMPEXP __attribute__ ((dllexport))
+#  else
+#   define LIB_IMPEXP __attribute__ ((dllimport))
+#  endif
+# else
+#  define LIB_IMPEXP
+# endif
+#else
+# define LIB_IMPEXP
+#endif
+
+class LIB_IMPEXP libexc : public std::exception {
 public:
   libexc (std::string str) : message (str) { }
   ~libexc () throw () { }
@@ -90,11 +189,12 @@ public:
 private:
   std::string message;
 };
-int libfoo () throw (libexc);
+int LIB_IMPEXP libfoo () throw (libexc);
 ]])
 
 AT_DATA([lib.cpp],
 [[#include <iostream>
+#define LIBTOOL_TEST_IN_LIB
 #include "lib.h"
 
 int libbar (void) throw (libexc)
@@ -121,6 +221,7 @@ AT_DATA([main.cpp],
 #include <iostream>
 #include <exception>
 #include <string>
+#include "common.h"
 #include "lib.h"
 #include "module.h"
 
@@ -250,26 +351,34 @@ moddir=$inst/mod
 mkdir l m $inst $libdir $bindir $moddir
 
 # If the C++ compiler isn't capable, don't bother.
-AT_CHECK([$CXX $CPPFLAGS $CXXFLAGS -c main.cpp || exit 77], [], [ignore], 
[ignore])
+AT_CHECK([$CXX $CPPFLAGS $CXXFLAGS -DUSING_COMMON_DLL -DUSING_MODULE_DLL 
-DUSING_LIB_DLL -c main.cpp || exit 77], [], [ignore], [ignore])
 
-for file in lib.cpp module.cpp; do
-  AT_CHECK([$LIBTOOL --mode=compile --tag=CXX $CXX $CPPFLAGS $CXXFLAGS -c 
$file],
+AT_CHECK([$LIBTOOL --mode=compile --tag=CXX $CXX $CPPFLAGS $CXXFLAGS -c 
common.cpp],
+          [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=compile --tag=CXX $CXX $CPPFLAGS $CXXFLAGS -c 
lib.cpp],
+          [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=compile --tag=CXX $CXX $CPPFLAGS $CXXFLAGS 
-DUSING_COMMON_DLL -c module.cpp],
           [], [ignore], [ignore])
-done
+
+AT_CHECK([$LIBTOOL --mode=link --tag=CXX $CXX $CXXFLAGS $LDFLAGS -o 
l/libcommon.la ]dnl
+        [common.lo -no-undefined -version-info 1:0:0 -rpath $libdir],
+        [], [ignore], [ignore])
 AT_CHECK([$LIBTOOL --mode=link --tag=CXX $CXX $CXXFLAGS $LDFLAGS -o l/liba.la 
]dnl
         [lib.lo -no-undefined -version-info 1:0:0 -rpath $libdir],
         [], [ignore], [ignore])
 AT_CHECK([$LIBTOOL --mode=link --tag=CXX $CXX $CXXFLAGS $LDFLAGS -o 
m/module.la ]dnl
-        [module.lo -module -avoid-version -no-undefined -rpath $moddir],
+        [module.lo l/libcommon.la -module -avoid-version -no-undefined -rpath 
$moddir],
         [], [ignore], [ignore])
 
 # We need -export-dynamic for the exception handling in modules to work.
 AT_CHECK([$LIBTOOL --mode=link --tag=CXX $CXX $CXXFLAGS $LDFLAGS -o 
main$EXEEXT ]dnl
-        [main.$OBJEXT l/liba.la -dlopen m/module.la $LIBLTDL -export-dynamic],
+        [main.$OBJEXT l/liba.la l/libcommon.la -dlopen m/module.la $LIBLTDL 
-export-dynamic],
         [], [ignore], [ignore])
 
 LT_AT_NOINST_EXEC_CHECK([./main], [-dlopen m/module.la], [], [ignore], 
[ignore])
 
+AT_CHECK([$LIBTOOL --mode=install cp l/libcommon.la $libdir],
+        [], [ignore], [ignore])
 AT_CHECK([$LIBTOOL --mode=install cp l/liba.la $libdir],
         [], [ignore], [ignore])
 AT_CHECK([$LIBTOOL --mode=install cp m/module.la $moddir],
-- 
1.7.0.4