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.<div><br /></div><div>If you are going down the route of disabling it, you might also want to disable the NSDate formatter (in Cocoa.cpp) by making</div><div>#ifdef WIN32</div><div>return ""</div><div>#else</div><div>do your thing</div><div>#endif</div><div>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.</div><div><br /></div><div>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.</div><div><br /><br /><span>On 07/21/13, <b class="name">Virgile Bello </b> <virgile.bello@gmail.com> wrote:</span><blockquote cite="mid:CAEigO1pc1Cy3AsbvEMvAfG=D7paVR3cVoL1nVM9V7o-=zC8VEQ@mail.gmail.com" class="iwcQuote" style="border-left: 1px solid #00F; padding-left: 13px; margin-left: 0;" type="cite"><div class="mimepart text html"><div dir="ltr">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).<div>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</div>
<div>(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).<div>
<br /><div>Do you want me to do an implementation for this patch or you think it is OK for a later patch?<br /></div></div><div>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)</div>
</div></div><div class="gmail_extra"><br /><br /><div class="gmail_quote">On Mon, Jul 22, 2013 at 5:47 AM, Enrico Granata <span dir="ltr"><<a href="mailto:egranata@apple.com" target="1">egranata@apple.com</a>></span> wrote:<br />
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.<div>
<br /></div><div>If Windows has time_t, you might get away with</div><div>#ifdef _WIN32</div><div>return <hardcoded></div><div>#else</div><div>do all the POSIXy stuff</div><div>#endif</div><div><br /></div><div>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</div>
<div><br /></div><div>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.<div>
<div class="h5"><br /><br /><span>On 07/21/13, <b>Virgile Bello </b> <<a href="mailto:virgile.bello@gmail.com" target="1">virgile.bello@gmail.com</a>> wrote:</span><blockquote style="border-left:1px solid #00f;padding-left:13px;margin-left:0" type="cite">
<div><div dir="ltr">Improved all the things previously discussed:<div>- 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.</div>
<div><div><span style="font-size:13px;font-family:arial,sans-serif">- Concerning LLDB_DISABLE_POSIX, it's done. However, on Windows, any library including LLDB headers (such as </span>API/<span style="font-size:13px;font-family:arial,sans-serif">SB*.h) would need to define it as well. To improve the situation, I</span><span style="font-size:13px;font-family:arial,sans-serif"> 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.</span><br />
</div><div><span style="font-size:13px;font-family:arial,sans-serif">(Maybe same thing should be done for LLDB_DISABLE_PYTHON on Win32?).</span></div>
<div>- Host::Kill added.</div><div>- <span style="font-size:13px;font-family:arial,sans-serif">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).</span></div>
<div><font face="arial, sans-serif"><br /></font><div>It should be a better base for discussion, but might still need some additional improvements and testings.</div></div>
</div></div><div class="gmail_extra"><br /><br /><div class="gmail_quote">On Thu, Jul 18, 2013 at 3:36 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="1">gclayton@apple.com</a>></span> wrote:<br />
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">A few issues:<br />
<br />
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.<br />
<br />
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:<br />
<br />
#ifndef DISABLE_POSIX_SUPPORT<br />
<br />
#endif<br />
<br />
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.<br />
<br />
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.<br />
<br />
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:<br />
<br />
RegularExpression.h currently has:<br />
<br />
#include "lldb/lldb-regex.h"<br />
<br />
This all of code from that header file:<br />
<br />
#ifdef _WIN32<br />
#include "../lib/Support/regex_impl.h"<br />
<br />
typedef llvm_regmatch_t regmatch_t;<br />
typedef llvm_regex_t regex_t;<br />
<br />
inline int regcomp(llvm_regex_t * a, const char *b, int c)<br />
{<br />
return llvm_regcomp(a, b, c);<br />
}<br />
<br />
inline size_t regerror(int a, const llvm_regex_t *b, char *c, size_t d)<br />
{<br />
return llvm_regerror(a, b, c, d);<br />
}<br />
<br />
inline int regexec(const llvm_regex_t * a, const char * b, size_t c,<br />
llvm_regmatch_t d[], int e)<br />
{<br />
return llvm_regexec(a,b,c,d,e);<br />
}<br />
<br />
inline void regfree(llvm_regex_t * a)<br />
{<br />
llvm_regfree(a);<br />
}<br />
<br />
#else<br />
#include <regex.h><br />
#endif<br />
<br />
<br />
<br />
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.<br />
<br />
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:<br />
<br />
#include "lldb/lldb-defines.h"<br />
#include "lldb/lldb-enumerations.h"<br />
#include "lldb/lldb-forward.h"<br />
#include "lldb/lldb-types.h"<br />
<br />
"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.<br />
<br />
"lldb-dirent.h" is not needed, this should be inlined into FileSpec.cpp and all LLDB code that enumerates directories should use FileSpec::EnumerateDirectory(...).<br />
<br />
lldb_private::formatters::GetOSXEpoch () disables the function using #ifndef _WIN32, shouldn't it be using DISABLE_POSIX_SUPPORT?<br />
<br />
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:<br />
<br />
Host::Kill (lldb::pid_t pid, int signo);<br />
<br />
Then each platform should be able to translate a unix signal into an appropriate kill (TerminateProcess is how windows does this).<br />
<br />
<br />
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.<br />
<br />
<br />
Greg<br />
<div><div>On Jul 13, 2013, at 8:30 AM, Virgile Bello <<a href="mailto:virgile.bello@gmail.com" target="1">virgile.bello@gmail.com</a>> wrote:<br />
<br />
> Sorry, quick patch update to fix DataBufferMemoryMap::MemoryMapFromFileDescriptor.<br />
><br />
><br />
> On Sat, Jul 13, 2013 at 12:24 PM, Virgile Bello <<a href="mailto:virgile.bello@gmail.com" target="1">virgile.bello@gmail.com</a>> wrote:<br />
> Ok, attached the updated patch.<br />
> It improves quite a few things:<br />
> - readded some libraries such as PluginObjectContainerBSDArchive and SymbolVendorELF<br />
> - added missing functions in Windows.cpp<br />
> - full support for cmake under MingW.<br />
><br />
> Note that for automake build, you need to add std=c++11 and -DLLDB_DISABLE_PYTHON (automatic for cmake build).<br />
><br />
><br />
> On Thu, Jul 11, 2013 at 10:02 AM, Virgile Bello <<a href="mailto:virgile.bello@gmail.com" target="1">virgile.bello@gmail.com</a>> wrote:<br />
> On Thu, Jul 11, 2013 at 4:18 AM, Joćo Matos <<a href="mailto:ripzonetriton@gmail.com" target="1">ripzonetriton@gmail.com</a>> wrote:<br />
> On Sun, Jul 7, 2013 at 10:44 AM, Virgile Bello <<a href="mailto:virgile.bello@gmail.com" target="1">virgile.bello@gmail.com</a>> wrote:<br />
> Here is a patch allowing compilation of LLDB with MinGW.<br />
> (it still compiles on Linux after the patch)<br />
> Can someone please review (and merge in trunk if everything seems good)?<br />
><br />
> Soon some other patches should follow:<br />
> - 1 for MSVC compilation<br />
> - 1 to add a lldbProcessWindows using Win32 debugging API, so that gdb/clang compiled programs can be debugged in Windows.<br />
><br />
> Hopefully I can get all this into trunk quickly so that I don't need to rebase/merge too much.<br />
> Thanks!<br />
><br />
> 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.<br />
> 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.<br />
><br />
><br />
> Some questions:<br />
><br />
> 1. Why do we need to define these in "lldb-socket.h"?<br />
><br />
> +#define NTDDI_VERSION NTDDI_VISTA<br />
> +#define _WIN32_WINNT _WIN32_WINNT_VISTA<br />
><br />
> Can't you just include "lldb-windows.h" which already includes Windows.h?<br />
> You're totally right.<br />
> Actually I thought that was in the MinGW patch but I did right after in the MSVC branch.<br />
> I will backport it and improve this patch a little bit then, in order to avoid further changes later.<br />
><br />
><br />
> 2. Why are you excluding lldbPluginObjectContainerBSDArchive and lldbPluginSymbolVendorELF from the MinGW build?<br />
> No ar.h for lldbPluginObjectContainerBSDArchive (same as before, I patched it in MSVC branch so I should probably backport that as well).<br />
> For lldbPluginSymbolVendorELF I will double-check.<br />
><br />
> I will soon get back to you with an updated patch covering 1 and 2.<br />
><br />
><br />
> 3. Why add support for two build systems (CMake and automake)?<br />
><br />
> 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.<br />
> 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.<br />
><br />
><br />
> Apart from that, looking forward to see this getting in and for further Windows debugging patches.<br />
><br />
> --<br />
> Joćo Matos<br />
><br />
><br />
><br />
</div></div>> <lldb-mingw32-v3.diff>_______________________________________________<br />
> lldb-commits mailing list<br />
> <a href="mailto:lldb-commits@cs.uiuc.edu" target="1">lldb-commits@cs.uiuc.edu</a><br />
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="1">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br />
<br />
</blockquote></div><br /></div>
</div></blockquote></div></div></div>
</blockquote></div><br /></div>
</div></blockquote></div>