Fix makefiles to build shared libraries (DLLs) for tests on Windows

[amccarth]

Summary

Abstracted away some POSIX-isms that caused make to issue invalid commands on Windows. Added a new force-include for the test programs (C and C++) so that we can use platform-specific macros. I left uncaught_exception.h as is, since it's specific to C++.

TestSharedLib now builds and cleans up on Windows, though the test still fails some of the expectations. I want to deal with that later.

[zturner]

Looks like the right general idea. I have a few specific comments, so after you address those I think it's good to go.

test/make/Makefile.rules

152–153

See my comments in test_common.h about having a -D COMPILING_TEST_DLL. This seems like a good place to do it.

189–190

I feel like we should get rid of this and sink the logic into test_common.h. Mainly because force includes are gross, so it would be nice if there were just one force include for all platforms so it's more easy to audit what goes into them, and more straightforward for people modifying it so they don't accidentally introduce an order-dependency on what order the force includes happen.

The dummy definition of the function could be wrapped in something like:

#if defined(__cplusplus) && defined(_MSC_VER) && (_HAS_EXCEPTIONS==0)
// Declare the function here.
#endif

412–418

What is all this stuff for? Make isn't my thing, so I'm not sure why this is needed. Is it necessary for the shlib stuff?

test/make/test_common.h

4

I think this is another Microsoft specific thing, so it shouldn't (?) apply to people running tests on MinGW. So probably #if defined(_MSC_VER) is better.

5

Can we call the macro something like LLDB_TEST_EXPORT? Makes it clearer that this is something that's part of the test suite.

Also, I think we will need to be a little bit fancier. Only the DLL needs to see declspec(dllexport). The executable should see declspec(dllimport) or it won't be able to bind to the exports at runtime. So I think we need something else inside this conditional like

#if defined(COMPILING_TEST_DLL)
#define LLDB_TEST_EXPORT __declspec(dllexport)
#else
#define LLDB_TEST_EXPORT __declspec(dllimport)
#endif

The non windows codepath is fine, it can always just define it to be nothing. Then the makefile needs to do something to figure oout that for compiling the shared library, it needs to #define COMPILING_TEST_DLL. I commented earlier up about what looks like might be a good place.

[amccarth]

test/make/Makefile.rules

152–153

It's trickier than that, because if I add it to CFLAGS here, it'll apply to everything, not just the dynamic library objects. Instead, I've used Target-specific Variables down where it builds the dynamic library.

Target-specific Variable Values: http://www.gnu.org/software/make/manual/html_node/Target_002dspecific.html

189–190

Agreed and done.

412–418

These commands fire on almost every make command, and they were causing errors on Windows because the syntax wasn't right for CMD. In some cases, the errors prevented the shared library from being built.

test/make/test_common.h

4

Actually, it looks like MinGW does use dllimport/dllexport: http://www.mingw.org/wiki/sampledll

5

Can we call the macro something like LLDB_TEST_EXPORT?

Yes. I chose LLDB_TEST_API, since it's used for both import and export. I've seen other projects that use API as part of the macro name for this.

Only the DLL needs to see declspec(dllexport). The executable should see declspec(dllimport) or it won't be able to bind to the exports at runtime.

I was expecting to need to do that, but the need never arose in practice, so I defaulted to making the smallest possible change. It's a good idea nonetheless, so now I've gone ahead and implemented it.

[amccarth]

I'm still testing, and I've got to go through the other tests that build dynamic libraries in order to add the LLDB_TEST_API macro to the exported functions.

[zturner]

Looks good, after the remaining changes I'll commit

test/make/Makefile.rules

378

I think this should be called something like COMPILING_LLDB_TEST_DLL or COMPILING_TEST_DLL. Or anything as long as it has "DLL" in it. Because an EXE is technically an "LLDB Test" also, but it wouldn't have this definition.

413

I'd probably change the name from SEMICOLON to something like JOIN_COMMAND. It's a little confusing to see SEMICOLON = &.

test/make/test_common.h

20

I know you were just copying over my code, but my code was actually wrong here :) __uncaught_exception() should return bool. Can you change this to an inline bool which returns true? https://msdn.microsoft.com/en-us/library/ff770584.aspx?f=255&MSPPError=-2147217396 This is probably only of theoretical value, but we might as well.

[amccarth]

test/make/Makefile.rules

378

Done.

413

Agreed and done.

test/make/test_common.h

20

I've changed this to #include <eh.h> and rewritten the comment to describe the better understand we arrived at last night.

[amccarth]

I had to fix a little Python as well. There may be more to do, but we now build shared libs on Windows, the cleanup is a little more reliable, and some tests that were failing in the build are now actually building and failing in the expectations.

[zturner]

looks good, i'll commit later. Thanks!

[amccarth]

r232220

test/lang/c/shared_lib/foo.h

 struct foo;
 
 struct sub_foo
 {
   int sub_1;
   char *sub_2;
 };
 
-struct foo *GetMeAFoo();
-struct sub_foo *GetMeASubFoo (struct foo *in_foo);
+LLDB_TEST_API struct foo *GetMeAFoo();
+LLDB_TEST_API struct sub_foo *GetMeASubFoo (struct foo *in_foo);
 
 

test/lldbtest.py

def buildLibrary(self, sources, lib_name):
         if sys.platform.startswith("darwin"):
             dsym = os.path.join(self.lib_dir, 'LLDB.framework', 'LLDB')
             d = {'DYLIB_CXX_SOURCES' : sources,
                  'DYLIB_NAME' : lib_name,
                  'CFLAGS_EXTRAS' : "%s -stdlib=libc++" % stdflag,
                  'FRAMEWORK_INCLUDES' : "-F%s" % self.lib_dir,
                  'LD_EXTRAS' : "%s -Wl,-rpath,%s -dynamiclib" % (dsym, self.lib_dir),
                 }
-        elif sys.platform.startswith('freebsd') or sys.platform.startswith("linux") or os.environ.get('LLDB_BUILD_TYPE') == 'Makefile':
+        elif sys.platform.startswith('freebsd') or sys.platform.startswith("linux") or sys.platform.startswith("win") or os.environ.get('LLDB_BUILD_TYPE') == 'Makefile':
             d = {'DYLIB_CXX_SOURCES' : sources,
                  'DYLIB_NAME' : lib_name,
                  'CFLAGS_EXTRAS' : "%s -I%s -fPIC" % (stdflag, os.path.join(os.environ["LLDB_SRC"], "include")),
                  'LD_EXTRAS' : "-shared -L%s -llldb" % self.lib_dir}
         if self.TraceOn():
             print "Building LLDB Library (%s) from sources %s" % (lib_name, sources)
 
         self.buildDefault(dictionary=d)

test/make/Makefile.rules

else
 
   ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
     DSYM = $(EXE).debug
   endif
 endif
 
 CFLAGS ?= -g -O0
 CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include
+CFLAGS += -include $(THIS_FILE_DIR)test_common.h
 
 # Use this one if you want to build one part of the result without debug information:
 CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS)
 
 CXXFLAGS += -std=c++11
 CXXFLAGS += $(CFLAGS)
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
ifeq (,$(filter $(OS), Windows_NT Android))
     endif
 endif
 OBJECTS =
 EXE ?= a.out
 
 ifneq "$(DYLIB_NAME)" ""
   ifeq "$(OS)" "Darwin"
     DYLIB_FILENAME = lib$(DYLIB_NAME).dylib
+  else ifeq "$(OS)" "Windows_NT"
+    DYLIB_FILENAME = $(DYLIB_NAME).dll
   else
     DYLIB_FILENAME = lib$(DYLIB_NAME).so
   endif
 endif
 
 # Function that returns the counterpart C++ compiler, given $(CC) as arg.
 cxx_compiler_notdir = $(if $(findstring clang,$(1)), \
                            $(subst clang,clang++,$(1)), \

 cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1)))),$(call cxx_linker_notdir,$(1)))
 
 #----------------------------------------------------------------------
 # Windows specific options
 #----------------------------------------------------------------------
 ifeq "$(OS)" "Windows_NT"
   ifneq (,$(findstring clang,$(CC)))
     # Clang for Windows doesn't support C++ Exceptions
     CXXFLAGS += -fno-exceptions
-    CXXFLAGS += -include $(THIS_FILE_DIR)uncaught_exception.h
     CXXFLAGS += -D_HAS_EXCEPTIONS=0
     # The MSVC linker doesn't understand long section names
     # generated by the clang compiler.
     LDFLAGS += -fuse-ld=lld
   endif
 endif
 
 #----------------------------------------------------------------------
 # Android specific options

 else
 $(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
 endif
 endif
 
 #----------------------------------------------------------------------
 # Make the dylib
 #----------------------------------------------------------------------
+$(DYLIB_OBJECTS) : CFLAGS += -DCOMPILING_LLDB_TEST_DLL
+
 $(DYLIB_FILENAME) : $(DYLIB_OBJECTS)
 ifeq "$(OS)" "Darwin"
   $(LD) $(LDFLAGS) $(DYLIB_OBJECTS) -install_name "@executable_path/$(DYLIB_FILENAME)" -dynamiclib -o "$(DYLIB_FILENAME)"
 ifneq "$(MAKE_DSYM)" "NO"
 ifneq "$(DS)" ""
   "$(DS)" $(DSFLAGS) "$(DYLIB_FILENAME)"
 endif
 endif
 else
   $(LD) $(LDFLAGS) $(DYLIB_OBJECTS) -shared -o "$(DYLIB_FILENAME)"
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
   objcopy --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug"
   objcopy --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)"
 endif
 endif
 
 #----------------------------------------------------------------------
 # Automatic variables based on items already entered. Below we create
-# an objects lists from the list of sources by replacing all entries
+# an object's lists from the list of sources by replacing all entries
 # that end with .c with .o, and we also create a list of prerequisite
 # files by replacing all .c files with .d.
 #----------------------------------------------------------------------
 PREREQS := $(OBJECTS:.o=.d)
 ifneq "$(DYLIB_NAME)" ""
   DYLIB_PREREQS := $(DYLIB_OBJECTS:.o=.d)
 endif
 
 #----------------------------------------------------------------------
 # Rule for Generating Prerequisites Automatically using .d files and
 # the compiler -MM option. The -M option will list all system headers,
 # and the -MM option will list all non-system dependencies.
 #----------------------------------------------------------------------
+ifeq "$(OS)" "Windows_NT"
+  JOIN_CMD = &
+  QUOTE = "
+else
+  JOIN_CMD = ;
+  QUOTE = '
+endif
+
 %.d: %.c
-  @rm -f $@; \
+  @rm -f $@ $(JOIN_CMD) \
   $(CC) -M $(CFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(JOIN_CMD) \
   rm -f $@.tmp
 
 %.d: %.cpp
-  @rm -f $@; \
+  @rm -f $@ $(JOIN_CMD) \
   $(CXX) -M $(CXXFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(JOIN_CMD) \
   rm -f $@.tmp
 
 %.d: %.m
-  @rm -f $@; \
+  @rm -f $@ $(JOIN_CMD) \
   $(CC) -M $(CFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(JOIN_CMD) \
   rm -f $@.tmp
 
 %.d: %.mm
-  @rm -f $@; \
+  @rm -f $@ $(JOIN_CMD) \
   $(CXX) -M $(CXXFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(JOIN_CMD) \
   rm -f $@.tmp
 
 #----------------------------------------------------------------------
 # Include all of the makefiles for each source file so we don't have
 # to manually track all of the prerequisites for each source file.
 #----------------------------------------------------------------------
 sinclude $(PREREQS)
 ifneq "$(DYLIB_NAME)" ""
ifneq "$(DYLIB_NAME)" ""
   $(RM) -r $(DYLIB_FILENAME).dSYM
   $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug
 endif
 ifneq "$(DSYM)" ""
   $(RM) -r "$(DSYM)"
 endif
 ifeq "$(OS)" "Windows_NT"
   $(RM) "$(EXE).manifest" $(wildcard *.pdb *.ilk)
+  ifneq "$(DYLIB_NAME)" ""
+    $(RM) $(DYLIB_FILENAME).manifest
+    $(RM) $(DYLIB_NAME).lib $(DYLIB_NAME).exp
+  endif
 endif
 
 #----------------------------------------------------------------------
 # From http://blog.melski.net/tag/debugging-makefiles/
 # 
 # Usage: make print-CC print-CXX print-LD
 #----------------------------------------------------------------------
 print-%:

test/make/test_common.h

+// This header is included in all the test programs (C and C++) and provides a
+// hook for dealing with platform-specifics.
+#if defined(_WIN32) || defined(_WIN64)
+    #ifdef COMPILING_LLDB_TEST_DLL
+      #define LLDB_TEST_API __declspec(dllexport)
+    #else
+      #define LLDB_TEST_API __declspec(dllimport)
+    #endif
+#else
+    #define LLDB_TEST_API
+#endif
+
+
+#if defined(__cplusplus) && defined(_MSC_VER) && (_HAS_EXCEPTIONS==0)
+// Compiling MSVC libraries with _HAS_EXCEPTIONS=0, eliminates most but not all
+// calls to __uncaught_exception.  Unfortunately, it does seem to eliminate
+// the delcaration of __uncaught_excpeiton.  Including <eh.h> ensures that it is
+// declared.  This may not be necessary after MSVC 12.
+#include <eh.h>
+#endif

test/make/uncaught_exception.h

-// MSVC header files have compilation issues when compiling with exceptions disabled.  Notably,
-// this function is compiled out when _HAS_EXCEPTIONS=0, but this function is called from another
-// place even when _HAS_EXCEPTIONS=0.  So we define a dummy implementation as a workaround and
-// force include this header file.
-static void *__uncaught_exception() { return nullptr; }

test/plugins/builder_base.py

 """
 If the build* function is passed the compiler argument, for example, 'llvm-gcc',
 it is passed as a make variable to the make command.  Otherwise, we check the
 LLDB_CC environment variable; if it is defined, it is passed as a make variable
 to the make command.
 
 If neither the compiler keyword argument nor the LLDB_CC environment variable is
 specified, no CC make variable is passed to the make command.  The Makefile gets
 to define the default CC being used.
 
 Same idea holds for LLDB_ARCH environment variable, which maps to the ARCH make
 variable.
 """
 
-import os
+import os, sys
 import platform
 import lldbtest
 
 def getArchitecture():
     """Returns the architecture in effect the test suite is running with."""
     return os.environ["ARCH"] if "ARCH" in os.environ else ""
 
 def getCompiler():
def getCmdLine(d):
     """
     Helper function to return a properly formatted command line argument(s)
     string used for the make system.
     """
 
     # If d is None or an empty mapping, just return an empty string.
     if not d:
         return ""
-
-    cmdline = " ".join(["%s='%s'" % (k, v) for k, v in d.items()])
+    pattern = '%s="%s"' if "win32" in sys.platform else "%s='%s'"
+    cmdline = " ".join([pattern % (k, v) for k, v in d.items()])
 
     return cmdline
 
 
 def buildDefault(sender=None, architecture=None, compiler=None, dictionary=None, clean=True):
     """Build the binaries the default way."""
     commands = []
     if clean: