[Lldb-commits] MinGW compilation support

[Virgile Bello]

Renamed to ThreadLocalStorage. Also noticed I didn't switch
to ThreadLocalStorageGet/Set in Timer.cpp (there was still a #ifdef for
pthread) => fixed.

Sending a patch again because I don't have SVN write permissions (possible
to have it?).

Also, when running test suite on linux in x64, it keeps freezing at some
specific test (usually test_int_type_with_dwarf), anybody is having similar
issues? (x86 is ok)



On Tue, Aug 20, 2013 at 2:21 AM, Greg Clayton <gclayton at apple.com> wrote:

> Great patch! The only things that need to be fixed are in
> "include/lldb/Host/Host.h", please spell out "Tls" into
> "ThreadLocalStorage" in all function calls and defines.
>
> No need for another patch, just rename "Tls" and check it in!
>
> Greg
>
>
>
> On Aug 17, 2013, at 7:30 AM, Virgile Bello <virgile.bello at gmail.com>
> wrote:
>
> > 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
> > >
> > >
> >
> >
> > <lldb-mingw32-v6.diff>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130820/56c5f9e5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lldb-mingw32-v7.diff
Type: application/octet-stream
Size: 111629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130820/56c5f9e5/attachment.obj>