[Lldb-commits] MinGW compilation support

[Virgile Bello]

Improved all the things previously discussed:
- Moved windows specific headers in Host/windows (and including any
API/SB*.h header won't include <windows.h>). Probably better than inlining
it in lldb-*.h since it shouldn't be visible to external code (i.e. through
lldb-defines.h) and shouldn't automatically be added everywhere (through
lldb-private*.h). The .cpp files requiring the windows specific API should
include those directly since that's where it is used.
- Concerning LLDB_DISABLE_POSIX, it's done. However, on Windows, any
library including LLDB headers (such as API/SB*.h) would need to define it
as well. To improve the situation, I added a #define inside
Host/mingw/Config.h and added it at the top of every file checking for
LLDB_DISABLE_POSIX, so that both LLDB compilation and external project
including it don't need additional configuration.
(Maybe same thing should be done for LLDB_DISABLE_PYTHON on Win32?).
- Host::Kill added.
- GetOSXEpoch() still need to be fixed (timegm doesn't exist on Windows). I
was planning to fix that in a future patch (doing thing incrementally).

It should be a better base for discussion, but might still need some
additional improvements and testings.


On Thu, Jul 18, 2013 at 3:36 AM, Greg Clayton <gclayton at apple.com> wrote:

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130721/c887cd84/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lldb-mingw32-v4.diff
Type: application/octet-stream
Size: 83609 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130721/c887cd84/attachment.obj>