[Lldb-commits] MinGW compilation support

[Greg Clayton]

A few issues:

Where is _POSIX_SOURCE supposed to come from? I don't see anything that defines this in any makefiles, and it doesn't get defined on darwin which causes compilation problems. 

We currently assume posix support in LLDB, so I would rather turn these defines into something that needs to be defined to opt out of posix support like:

#ifndef DISABLE_POSIX_SUPPORT

#endif

This way, most unix builds will continue to work without any intervention, and the Windows and MinGW builds will need to make sure DISABLE_POSIX_SUPPORT is defined so all POSIX stuff gets disabled.

This patch is also missing the point of the "include/Host" and "source/Host" code. There are many places where a new "lldb/lldb-*.h" file was added, that is essentially doing the job of what the code in "include/Host" and "source/Host" should be doing.

Anything that works differently on different platforms should be in the host layer, not in header files that are in "lldb/*.h". A few examples that need to be fixed:

RegularExpression.h currently has:

#include "lldb/lldb-regex.h"

This all of code from that header file:

#ifdef _WIN32
#include "../lib/Support/regex_impl.h"

typedef llvm_regmatch_t regmatch_t;
typedef llvm_regex_t regex_t;

inline int regcomp(llvm_regex_t * a, const char *b, int c)
{
    return llvm_regcomp(a, b, c);
}

inline size_t regerror(int a, const llvm_regex_t *b, char *c, size_t d)
{
    return llvm_regerror(a, b, c, d);
}

inline int	regexec(const llvm_regex_t * a, const char * b, size_t c,
                    llvm_regmatch_t d[], int e)
{
    return llvm_regexec(a,b,c,d,e);
}

inline void regfree(llvm_regex_t * a)
{
    llvm_regfree(a);
}

#else
#include <regex.h>
#endif



Should just be done in "RegularExpression.h". We then need to fix DisassemblerLLVMC.cpp to use the RegularExpression class and not try to use regex directly.

We shouldn't need "lldb-windows.h" or "lldb-win32.h", it should be just inlined into "lldb-private.h", or each one should be split up to fit into one of:

#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"

"lldb-socket.h" doesn't really need to exist and can be inlined into the two source files that currently include it. There are so many socket header files and each source file only requires a few of these things, so making one generic header file that includes all socket related headers doesn't make sense.

"lldb-dirent.h" is not needed, this should be inlined into FileSpec.cpp and all LLDB code that enumerates directories should use FileSpec::EnumerateDirectory(...).

lldb_private::formatters::GetOSXEpoch () disables the function using #ifndef _WIN32, shouldn't it be using DISABLE_POSIX_SUPPORT?

Seems like LLDB current uses int kill(pid_t pid, int signo)" in code like ProcessGDBRemote::KillDebugserverProcess() and other places. This should be moved into the Host layer so we do something like:

Host::Kill (lldb::pid_t pid, int signo);

Then each platform should be able to translate a unix signal into an appropriate kill (TerminateProcess is how windows does this).


So if you can fix these issues and resubmit a patch, I will take a look. I did verify that changing over to using "#ifndef DISABLE_POSIX_SUPPORT" that the project does build on MacOSX.


Greg
On Jul 13, 2013, at 8:30 AM, Virgile Bello <virgile.bello at gmail.com> wrote:

> Sorry, quick patch update to fix DataBufferMemoryMap::MemoryMapFromFileDescriptor.
> 
> 
> On Sat, Jul 13, 2013 at 12:24 PM, Virgile Bello <virgile.bello at gmail.com> wrote:
> Ok, attached the updated patch.
> It improves quite a few things:
> - readded some libraries such as PluginObjectContainerBSDArchive and SymbolVendorELF
> - added missing functions in Windows.cpp
> - full support for cmake under MingW.
> 
> Note that for automake build, you need to add std=c++11 and -DLLDB_DISABLE_PYTHON (automatic for cmake build).
> 
> 
> On Thu, Jul 11, 2013 at 10:02 AM, Virgile Bello <virgile.bello at gmail.com> wrote:
> On Thu, Jul 11, 2013 at 4:18 AM, João Matos <ripzonetriton at gmail.com> wrote:
> On Sun, Jul 7, 2013 at 10:44 AM, Virgile Bello <virgile.bello at gmail.com> wrote:
> Here is a patch allowing compilation of LLDB with MinGW.
> (it still compiles on Linux after the patch)
> Can someone please review (and merge in trunk if everything seems good)?
> 
> Soon some other patches should follow:
> - 1 for MSVC compilation
> - 1 to add a lldbProcessWindows using Win32 debugging API, so that gdb/clang compiled programs can be debugged in Windows.
> 
> Hopefully I can get all this into trunk quickly so that I don't need to rebase/merge too much.
> Thanks!
> 
> Just looked at the patch and it mostly LGTM. I think long-term it would be nice to have the POSIX-specific stuff a little better abstracted, instead of trying to hack around it in the Windows port, though for now it seems the way forward and it can be fixed incrementally in the future.
> I agree. I tried to keep the changes as small as possible (my priority was that it would be easy to integrate into trunk) but long-term it might require some more abstraction.
>  
> 
> Some questions:
> 
> 1. Why do we need to define these in "lldb-socket.h"?
> 
> +#define NTDDI_VERSION NTDDI_VISTA
> +#define _WIN32_WINNT _WIN32_WINNT_VISTA
> 
> Can't you just include "lldb-windows.h" which already includes Windows.h?
> You're totally right.
> Actually I thought that was in the MinGW patch but I did right after in the MSVC branch.
> I will backport it and improve this patch a little bit then, in order to avoid further changes later.
> 
> 
> 2. Why are you excluding lldbPluginObjectContainerBSDArchive and lldbPluginSymbolVendorELF from the MinGW build?
> No ar.h for lldbPluginObjectContainerBSDArchive  (same as before, I patched it in MSVC branch so I should probably backport that as well).
> For lldbPluginSymbolVendorELF I will double-check.
> 
> I will soon get back to you with an updated patch covering 1 and 2.
> 
> 
> 3. Why add support for two build systems (CMake and automake)?
> 
> IIRC LLVM and Clang are looking to kill all non-CMake build scripts to ease maintenance and reduce duplication, so IMHO LLDB being an umbrella project should do the same.
> I see, so I don't need to support Automake makefile anymore? Good to know. Actually I was still using it for the MingW build (and CMake for MSVC build), but never tested CMake + MingW. I have to give it a try and check everything works fine, esp. if it is the future-proof way.
>  
> 
> Apart from that, looking forward to see this getting in and for further Windows debugging patches.
> 
> -- 
> João Matos
> 
> 
> 
> <lldb-mingw32-v3.diff>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits