Re: [Lldb-commits] [PATCH] D18519: Allow building LLDB on Windows with MinGW 64/4.9.2 and later

2016-04-02 Thread Eran Ifrah via lldb-commits
eran.ifrah updated this revision to Diff 52463.
eran.ifrah marked 3 inline comments as done.
eran.ifrah added a comment.

- I have updated the patch with various indention fixes
- added the -mbig-obj for clang's CMakeLists.txt file (which makes the hack I 
did earlier in LLDB obsolete)
- answered some of your comments

Please see updated patch


http://reviews.llvm.org/D18519

Files:
  CMakeLists.txt
  cmake/modules/AddLLDB.cmake
  cmake/modules/LLDBConfig.cmake
  include/lldb/Host/windows/win32.h
  source/API/CMakeLists.txt
  source/API/SystemInitializerFull.cpp
  source/Host/common/OptionParser.cpp
  source/Host/windows/EditLineWin.cpp
  source/Host/windows/FileSystem.cpp
  source/Host/windows/ProcessRunLock.cpp
  source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
  source/Plugins/Process/Windows/Live/DebuggerThread.cpp
  source/Target/ProcessLaunchInfo.cpp
  source/Utility/PseudoTerminal.cpp
  tools/CMakeLists.txt
  tools/argdumper/CMakeLists.txt
  tools/driver/CMakeLists.txt
  tools/driver/Driver.cpp

Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -1299,7 +1299,7 @@
 }
 
 int
-#ifdef WIN32
+#if defined(_MSC_VER)
 wmain(int argc, wchar_t const *wargv[])
 #else
 main(int argc, char const *argv[])
@@ -1311,7 +1311,7 @@
 	setvbuf(stdin , NULL, _IONBF, 0);
 #endif
 
-#ifdef _WIN32
+#if defined(_MSC_VER)
 // Convert wide arguments to UTF-8
 std::vector argvStrings(argc);
 std::vector argvPointers(argc);
Index: tools/driver/CMakeLists.txt
===
--- tools/driver/CMakeLists.txt
+++ tools/driver/CMakeLists.txt
@@ -17,7 +17,14 @@
   add_dependencies(lldb debugserver)
 endif()
 
-target_link_libraries(lldb liblldb)
+if(MINGW)
+  # link directly against the dll file
+  add_dependencies(lldb liblldb)
+  target_link_libraries(lldb -L"${CMAKE_BINARY_DIR}/bin" -llldb)
+else()
+  target_link_libraries(lldb liblldb)
+endif()
+
 # TODO: why isn't this done by add_lldb_executable?
 #target_link_libraries(lldb ${LLDB_USED_LIBS})
 #llvm_config(lldb ${LLVM_LINK_COMPONENTS})
Index: tools/argdumper/CMakeLists.txt
===
--- tools/argdumper/CMakeLists.txt
+++ tools/argdumper/CMakeLists.txt
@@ -2,7 +2,13 @@
   argdumper.cpp
   )
 
-target_link_libraries(lldb-argdumper liblldb)
+if(MINGW)
+  # link directly against the dll file
+  add_dependencies(lldb-argdumper liblldb)
+  target_link_libraries(lldb-argdumper -L"${CMAKE_BINARY_DIR}/bin" -llldb)
+else()
+  target_link_libraries(lldb-argdumper liblldb)
+endif()
 
 install(TARGETS lldb-argdumper
   RUNTIME DESTINATION bin)
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -4,7 +4,8 @@
 endif()
   add_subdirectory(argdumper)
   add_subdirectory(driver)
-if (NOT __ANDROID_NDK__)
+if (NOT __ANDROID_NDK__ AND NOT MINGW)
+  # TODO: we should really make lldb-mi compile under MinGW
   add_subdirectory(lldb-mi)
 endif()
 if (LLDB_CAN_USE_LLDB_SERVER)
Index: source/Utility/PseudoTerminal.cpp
===
--- source/Utility/PseudoTerminal.cpp
+++ source/Utility/PseudoTerminal.cpp
@@ -20,7 +20,9 @@
 
 #ifdef _WIN32
 #include "lldb/Host/windows/win32.h"
+#if defined(_MSC_VER)
 typedef uint32_t pid_t;
+#endif
 // empty functions
 int posix_openpt(int flag) { return 0; }
 
Index: source/Target/ProcessLaunchInfo.cpp
===
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -366,7 +366,7 @@
  __FUNCTION__);
 
 int open_flags = O_RDWR | O_NOCTTY;
-#if !defined(_MSC_VER)
+#if !defined(LLVM_ON_WIN32)
 // We really shouldn't be specifying platform specific flags
 // that are intended for a system call in generic code.  But
 // this will have to do for now.
Index: source/Plugins/Process/Windows/Live/DebuggerThread.cpp
===
--- source/Plugins/Process/Windows/Live/DebuggerThread.cpp
+++ source/Plugins/Process/Windows/Live/DebuggerThread.cpp
@@ -377,9 +377,10 @@
 // we use simply to wake up the DebuggerThread so that we can close out the debug loop.
 if (m_pid_to_detach != 0 && info.ExceptionRecord.ExceptionCode == EXCEPTION_BREAKPOINT)
 {
+DWORD dwPidToDetach = m_pid_to_detach;
 WINLOG_IFANY(WINDOWS_LOG_EVENT | WINDOWS_LOG_EXCEPTION | WINDOWS_LOG_PROCESS,
 "Breakpoint exception is cue to detach from process 0x%x",
-m_pid_to_detach);
+dwPidToDetach);
 ::DebugActiveProcessStop(m_pid_to_detach);
 m_de

Re: [Lldb-commits] [PATCH] D18519: Allow building LLDB on Windows with MinGW 64/4.9.2 and later

2016-04-02 Thread Eran Ifrah via lldb-commits
eran.ifrah added a comment.

Spoke too soon, the hack into `CMakeLists.txt` is still needed
Passing ` -mbig-obj` crashes `ld.exe`  (although it crashes in a much later 
stage, at about 77% in the build)



Comment at: CMakeLists.txt:3-5
@@ -2,1 +2,5 @@
 
+if(MINGW_DEBUG)
+# force debugging info into lldb sources
+message("-- Building LLDB in Debug mode")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O0")

zturner wrote:
> Can you try the /bigobj solution proposed by Pavel?  which specific object 
> file fails to link, is it an LLDB obj or a clang obj?  Because as Pavel 
> mentions clang has had this problem before, so maybe you just need to specify 
> /bigobj for MinGW in the clang library and not in LLDB?
I fixed the top level `CMakeLists.txt` (LLVM one)


Comment at: cmake/modules/LLDBConfig.cmake:225-236
@@ -224,3 +224,4 @@
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
 check_cxx_compiler_flag("-Wno-deprecated-register"
 CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
 if (CXX_SUPPORTS_NO_DEPRECATED_REGISTER)

@@ -232,4 +233,5 @@
 if (CXX_SUPPORTS_NO_VLA_EXTENSION)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
 endif ()
+endif()
 

zturner wrote:
> Still indentation problems here.
Are you calling the missing space before the opening paren an indentation 
problem? otherwise, I think I am missing something here
its 2 spaces indented


Comment at: source/API/CMakeLists.txt:74-85
@@ -73,6 +73,14 @@
 # want every single library taking a dependency on the script interpreters.
+if(MINGW)
 target_link_libraries(liblldb PRIVATE
   lldbPluginScriptInterpreterNone
   lldbPluginScriptInterpreterPython
+Dbghelp # Needed for MiniDumpWriteDump
   )
+else()
+  target_link_libraries(liblldb PRIVATE
+lldbPluginScriptInterpreterNone
+lldbPluginScriptInterpreterPython
+)
+endif()
 

zturner wrote:
> How about this instead:
> 
> target_link_libraries(liblldb PRIVATE
>   lldbPluginScriptInterpreterNone
>   lldbPluginScriptInterpreterPython)
> 
> if(MINGW)
>   target_link_libraries(liblldb PRIVATE Dbghelp)
> endif()
> 
This was my first try, however, this yields a warning in cmake 3.0 and later: 
target_link_libraries can only be used once per target


Comment at: source/Plugins/Process/Windows/Live/DebuggerThread.cpp:380-383
@@ -379,5 +379,6 @@
 {
+DWORD dwPidToDetach = m_pid_to_detach;
 WINLOG_IFANY(WINDOWS_LOG_EVENT | WINDOWS_LOG_EXCEPTION | 
WINDOWS_LOG_PROCESS,
 "Breakpoint exception is cue to detach from 
process 0x%x",
-m_pid_to_detach);
+dwPidToDetach);
 ::DebugActiveProcessStop(m_pid_to_detach);

zturner wrote:
> Can this line be reverted?  I'm not sure what the point of saving it into a 
> temp variable is.
No, this change was done on purpose. Removing the fix I added, will trigger 
this error from the compiler:
```
use of deleted function
```



Comment at: tools/argdumper/CMakeLists.txt:6-8
@@ +5,5 @@
+if(MINGW)
+# link directly against the dll file
+add_dependencies(lldb-argdumper liblldb)
+target_link_libraries(lldb-argdumper -L"${CMAKE_BINARY_DIR}/bin" -llldb)
+else()

zturner wrote:
> 2 space indent for CMake, not 4.  Also the `target_link_libraries in the 
> `else()` branch should be indented.  As Pavel says, I'm not sure why this is 
> needed, but I can't really argue against it because I don't know much about 
> MinGW
This is needed because otherwise, you will get multiple definitions for many 
symbols - this is the side effect when linking with a `.dll.a` in MinGW systems 
when you don't really need to do this, since you can link directly to `.dll` 
file


Comment at: tools/driver/CMakeLists.txt:20
@@ -19,1 +19,3 @@
 
+if(MINGW)
+# link directly against the dll file

Same answer


http://reviews.llvm.org/D18519



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits