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.

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

411–417

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

3

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.

4

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

190

Agreed and done.

411–417

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

3

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

4

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

379

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.

412

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

379

Done.

412

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);
+DLLEXPORT struct foo *GetMeAFoo();
+DLLEXPORT struct sub_foo *GetMeASubFoo (struct foo *in_foo);
 
 

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)), \

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

 else
 $(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
 endif
 endif
 
 #----------------------------------------------------------------------
 # Make the dylib
 #----------------------------------------------------------------------
 $(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"
+  SEMICOLON = &
+  QUOTE = "
+else
+  SEMICOLON = ;
+  QUOTE = '
+endif
+
 %.d: %.c
-  @rm -f $@; \
+  @rm -f $@ $(SEMICOLON) \
   $(CC) -M $(CFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(SEMICOLON) \
   rm -f $@.tmp
 
 %.d: %.cpp
-  @rm -f $@; \
+  @rm -f $@ $(SEMICOLON) \
   $(CXX) -M $(CXXFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(SEMICOLON) \
   rm -f $@.tmp
 
 %.d: %.m
-  @rm -f $@; \
+  @rm -f $@ $(SEMICOLON) \
   $(CC) -M $(CFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(SEMICOLON) \
   rm -f $@.tmp
 
 %.d: %.mm
-  @rm -f $@; \
+  @rm -f $@ $(SEMICOLON) \
   $(CXX) -M $(CXXFLAGS) $< > $@.tmp && \
-  sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.tmp > $@; \
+  sed $(QUOTE)s,\($*\)\.o[ :]*,\1.o $@ : ,g$(QUOTE) < $@.tmp > $@ $(SEMICOLON) \
   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)
+    #define DLLEXPORT __declspec(dllexport)
+#else
+    #define DLLEXPORT
+#endif