Prepare bmalloc to be cross platform
Created attachment 403134 [details] WIP Patch
Created attachment 403137 [details] WIP Patch
Created attachment 403140 [details] WIP Patch
Created attachment 403147 [details] xcode
Created attachment 403150 [details] Patch
Comment on attachment 403150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403150&action=review r=me, but I think we should get Geoff's review too. > Source/bmalloc/ChangeLog:13 > + Add stubs for an implementation of bmalloc on Windows. Split the CryptoRandom > + implementation into a Unix and Windows definition. Add a small abstraction around > + threads to allow a Windows definition. Add the Builtins.h file which will contain > + any GCC compatible __builtin implementations on Windows. > + > + The actual implementations will be coming in subsequent patches. It is not enabled in Windows yet, right? Then, it is OK. > Source/bmalloc/bmalloc/BAssert.h:70 > + *(int *)(uintptr_t)0xbbadbeef = 0; \ Let's remove this change. > Source/bmalloc/bmalloc/Builtins.h:41 > +{ > + return 0; > +} > + > +BINLINE int __builtin_usub_overflow(unsigned int a, unsigned int b, unsigned int *res) > +{ > + return 0; > +} Can you add FIXME? > Source/bmalloc/bmalloc/DebugHeap.cpp:112 > +#if BOS(WINDOWS) > + result = _aligned_malloc(size, alignment); > + RELEASE_BASSERT(action == FailureAction::ReturnNull || result); > +#else > if (posix_memalign(&result, alignment, size)) > RELEASE_BASSERT(action == FailureAction::ReturnNull || result); > +#endif We should have FIXME to call _aligned_free since Windows requires paired calls of this instead of using free. > Source/bmalloc/bmalloc/IsoTLS.cpp:38 > +#if BOS(WINDOWS) > +#define strcasecmp _stricmp > +#else > #include <stdio.h> > +#endif I think we can include `stdio.h` regardless of whether this is Windows or not. > Source/bmalloc/bmalloc/VMAllocate.h:65 > + // @TODO Can you put FIXME comment with bugzilla URL about cache size retrieval? > Source/bmalloc/bmalloc/VMAllocate.h:135 > + // @TODO Can you add FIXME comment with bugzilla URL about it? > Source/bmalloc/bmalloc/VMAllocate.h:156 > + // @TODO Ditto. > Source/bmalloc/bmalloc/VMAllocate.h:166 > + // @TODO Ditto. > Source/bmalloc/bmalloc/VMAllocate.h:176 > + // @TODO Ditto. > Source/bmalloc/bmalloc/VMAllocate.h:231 > + // @TODO Ditto. > Source/bmalloc/bmalloc/VMAllocate.h:246 > + // @TODO Ditto. > Source/bmalloc/bmalloc/win/CryptoRandomWin.cpp:31 > +{ Can you add a comment with FIXME?
Comment on attachment 403150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403150&action=review > Source/bmalloc/bmalloc/DebugHeap.cpp:108 > +#if BOS(WINDOWS) > + result = _aligned_malloc(size, alignment); > + RELEASE_BASSERT(action == FailureAction::ReturnNull || result); This is incorrect because it does not pair with _aligned_free. I think Windows just shouldn't use the debug heap.
Generally, I don't love the idea of landing non-functional pieces of a port without testing. Is there a way to do this development locally or on a branch until it's more complete? Alternatively, do you have a complete patch that you can post for reference?