[Lldb-commits] MinGW compilation support

Tue Aug 13 18:42:09 PDT 2013

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:egr**anata at apple.com<egranata at apple.com>
>> >>
>> Cc: "lldb-commits at cs.uiuc.edu<**mailto:lldb-commits at cs.uiuc.**edu<lldb-commits at cs.uiuc.edu>>"
>> <lldb-commits at cs.uiuc.edu<**mailto:lldb-commits at cs.uiuc.**edu<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:egr**anata at apple.com <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:egr**anata at apple.com <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:
>> gcl**ayton at apple.com <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 PluginObjectContainerBSDArchiv**e 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 lldbPluginObjectContainerBSDAr**chive and
>>> lldbPluginSymbolVendorELF from the MinGW build?
>>> No ar.h for lldbPluginObjectContainerBSDAr**chive  (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<lldb-commits at cs.uiuc.edu>
>>> >
>>> http://lists.cs.uiuc.edu/**mailman/listinfo/lldb-commits<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<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<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/20130814/496cc526/attachment.html>