[Lldb-commits] MinGW compilation support

[Virgile Bello]

Ok, here is an updated patch.

Separating Host made me include some more changes that I have been keeping
aside for future patches. More specifically, it includes switch from
pthread to Windows thread, as well as win32 ConditionVariable and Mutex
implementations. This makes it much closer to the visual studio branch I'm
currently working on.

Some additional notes:
- ProcessRunLock is now split into .h/.cpp
- I didn't reorder functions in common/Host.cpp (which would have merged
multiple #ifdef _WIN32 into one). This is done mostly so that merge/rebase
still goes smoothly until merged into trunk (reporting diffs in functions
that moved around need to be done manually). Of course no problem to do
that once the patch is accepted.
- For the CommandCompletions.cpp FileSpec::EnumerateDirectory switch, I
tested it with command completions of filenames under linux, it seemed to
work well. Let me know if you see any problem with it.
- If it gets merged, I would like to have some sort of continuous
integration/build bot for the mingw build. At least we would know when it
breaks and people maintaining it could fix it quickly (esp. since it might
not be compiled/tested actively by many people working on lldb). I noticed
http://lab.llvm.org:8011/console , is that what you're currently using?


On Sat, Aug 17, 2013 at 1:29 AM, Greg Clayton <gclayton at apple.com> wrote:

>
> On Aug 16, 2013, at 8:53 AM, Virgile Bello <virgile.bello at gmail.com>
> wrote:
>
> > OK, already did the dirent and FileSpec::EnumerateDirectory() things.
>
> Great!
> >
> > However, for source/Host/common/Host.cpp, it seems easy to move things
> around.
> > However there will still be a single big #ifndef _WIN32 around all those
> functions since they will still be shared between linux, freebsd and
> macosx. Is this OK?
>
> Yes, that is fine.
>
> >
> >
> > On Fri, Aug 16, 2013 at 10:38 AM, Greg Clayton <gclayton at apple.com>
> wrote:
> > Things I would like to see fixed:
> >
> > - Do we really need the “include/lldb/Host/windows/dirent.h”?
> "source/Commands/CommandCompletions.cpp” includes it but it shouldn’t. It
> CommandCompletions is using diropen and the other dirent calls, it should
> be switched over to use FileSpec::EnumerateDirectory().
> > - FileSpec::EnumerateDirectory() should use native Windows directory
> enumeration instead of the dirent.h compatibility layer if possible
> > - All code in "source/Host/common/Host.cpp” that us doing "#ifdef
> _WIN32” inside functions should be copied over into
> "source/Host/Win32/Host.cpp”. The copied functions should then have only
> the win32 code, and the code in "source/Host/common/Host.cpp” should remove
> all references to Win32.
> >
> > Let me know if we can get rid of “dirent.h” and “dirent.cpp”, and use a
> native windows directory enumeration to back
> FileSpec::EnumerateDirectory(). This will really clean up the code in LLDB
> to not use native unix code in the core of LLDB and always use the
> “lldb_private::Host” layer to do all host specific things.
> >
> > Greg
> >
> >
> >
> > On Aug 14, 2013, at 7:47 AM, João Matos <ripzonetriton at gmail.com> wrote:
> >
> > > Did Greg already approve this?
> > >
> > >
> > > On Wed, Aug 14, 2013 at 2:42 AM, Virgile Bello <
> virgile.bello at gmail.com> wrote:
> > > Thanks for the reviews!
> > > Please feel free to commit it to trunk (no SVN access here).
> > >
> > >
> > > On Wed, Aug 14, 2013 at 5:56 AM, Carlo Kok <ck at remobjects.com> wrote:
> > > Op 8-8-2013 17:03, Malea, Daniel schreef:
> > >
> > > Hi Virgile, I looked at the Cmake changes and they seem OK. There
> don't appear to be any regressions on Linux with your patch applied..
> > >
> > > Cool. Looks good to me too.
> > >
> > >
> > > Dan
> > >
> > > From: Virgile Bello <virgile.bello at gmail.com<mailto:
> virgile.bello at gmail.com>>
> > >
> > > Date: Wednesday, 7 August, 2013 1:01 PM
> > > To: Enrico Granata <egranata at apple.com<mailto:egranata at apple.com>>
> > > Cc: "lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>" <
> lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>>
> > >
> > > Subject: Re: [Lldb-commits] MinGW compilation support
> > >
> > > Rebased on latest master, with the regex fix, in case it helps.
> > > Any chance this could get reviewed/merged please?
> > >
> > >
> > >
> > > On Mon, Jul 29, 2013 at 11:47 PM, Virgile Bello <
> virgile.bello at gmail.com<mailto:virgile.bello at gmail.com>> wrote:
> > > Sure, I will put the #ifdef in NSDate formatters for now.
> > > Also, just noticed a small error, in DisassemblerLLVMC.cpp, I reversed
> a if:
> > >    if (!s_regex.Execute(out_string, &matches))
> > > should be
> > >    if (s_regex.Execute(out_string, &matches))
> > >
> > > Otherwise, anybody had time to take a look at the other parts?
> > >
> > >
> > > On Mon, Jul 22, 2013 at 10:37 AM, Enrico Granata <egranata at apple.com
> <mailto:egranata at apple.com>> wrote:
> > > All that GetOSXEpoch() enables is getting the right summary string for
> NSDate objects - whether that is or is not critical entirely depends on how
> much you care about that specific data type having a summary :) If I were
> to guess, I would say it's pretty minor and can wait for a follow-up patch.
> > >
> > > If you are going down the route of disabling it, you might also want
> to disable the NSDate formatter (in Cocoa.cpp) by making
> > > #ifdef WIN32
> > > return ""
> > > #else
> > > do your thing
> > > #endif
> > > That way you will just not get a summary for NSDate on Windows,
> instead of seeing the wrong date! It seems the safest option to me for now.
> > >
> > > Once things are stable, we might want to look into Host/TimeValue.cpp
> to see if it fits the bill, or it should be improved/changed and then port
> this code over to Host/TimeValue.cpp, but that's for later.
> > >
> > >
> > > On 07/21/13, Virgile Bello <virgile.bello at gmail.com<mailto:
> virgile.bello at gmail.com>> wrote:
> > > Yes sure that's what I was planning to do for GetOSXEpoch (using
> #ifdef for now, it doesn't need to move into Host right away IMHO).
> > > It should be easy enough to do with Win32 API, and I was planning to
> do it shortly after in another patch, this one focusing mainly on getting
> everything to at least compile
> > > (so I don't spend too much time rebasing this big one, and I can focus
> on such smaller tasks in individual patches right after, easier to
> review/integrate individually -- actually there are many things that I
> improved in otehr patches after actually trying to use LLDB on windows).
> > >
> > > Do you want me to do an implementation for this patch or you think it
> is OK for a later patch?
> > > I agree it might be dangerous to not do an implementation right now
> since it returns 0 instead of a proper error/assert, so maybe it would be
> better to implement something for the first version (it didn't seem to be
> used in important part of the code though)
> > >
> > >
> > > On Mon, Jul 22, 2013 at 5:47 AM, Enrico Granata <egranata at apple.com
> <mailto:egranata at apple.com>> wrote:
> > > Regarding GetOSXEpoch() specifically, while it is a fair point that
> POSIXy stuff should be moved to the Host layer, and I should spend some
> time refactoring my (albeit very small) need for time management functions
> behind something that is more portable, please bear in mind that all that
> the function does is compute the time_t value for midnight on January 1st
> 2001. This is because POSIX APIs depend on January 1st 1970 as the "epoch",
> but Cocoa instead chooses January 1st 2001.
> > >
> > > If Windows has time_t, you might get away with
> > > #ifdef _WIN32
> > > return <hardcoded>
> > > #else
> > > do all the POSIXy stuff
> > > #endif
> > >
> > > With the above said, refactoring my usage of POSIX into the Host layer
> is definitely a chunk of work that is on me. Once that is done, you should
> be able to just port the Host's time interface to Win32 vs. POSIX
> > >
> > > It has been a long time since I did any Windows development (and even
> then I was mostly using .net instead of Win32), so I am not sure what level
> of time management API Windows offers, and how close they are to the POSIX
> facilities. Those who have a better knowledge of that story, feel free to
> chime in.
> > >
> > >
> > > On 07/21/13, Virgile Bello <virgile.bello at gmail.com<mailto:
> virgile.bello at gmail.com>> wrote:
> > > 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
> <mailto: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
> <mailto: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<mailto: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<mailto:virgile.bello at gmail.com>> wrote:
> > >
> > > On Thu, Jul 11, 2013 at 4:18 AM, João Matos <ripzonetriton at gmail.com
> <mailto:ripzonetriton at gmail.com>> wrote:
> > > On Sun, Jul 7, 2013 at 10:44 AM, Virgile Bello <
> virgile.bello at gmail.com<mailto: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<mailto:lldb-commits at cs.uiuc.edu>
> > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> > >
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > lldb-commits mailing list
> > > lldb-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> > >
> > >
> > > _______________________________________________
> > > lldb-commits mailing list
> > > lldb-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> > >
> > >
> > > _______________________________________________
> > > lldb-commits mailing list
> > > lldb-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> > >
> > >
> > >
> > >
> > > --
> > > João Matos
> > > _______________________________________________
> > > 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/20130817/1a173350/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lldb-mingw32-v6.diff
Type: application/octet-stream
Size: 119613 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130817/1a173350/attachment.obj>