Bug 19775

This problem may have to do with this FIXME: in Vector.h: "Nothing guarantees this buffer is appropriately aligned to hold objects of type T". You could use non-standard gcc extension __align__ to make vector inline buffer aligned. Alternatively, you could make the buffer an array of elements that have suitable alignment requirements for a cross-compiler solution (patches are welcome!) What are the cases where Sparc requires 64-bit alignment? Would an array of size32_t elements without padding be problematic, for example? Apple's sources for iPhone have some alignment fixes for ARM, which you can look up at Apple's open source site, if you are OK with agreeing to the Apple Public Source License, but I don't know if Sparc requirements are the same.

(In reply to comment #1) > This problem may have to do with this FIXME: in Vector.h: "Nothing guarantees > this buffer is appropriately aligned to hold objects of type T". You could use > non-standard gcc extension __align__ to make vector inline buffer aligned. > Alternatively, you could make the buffer an array of elements that have > suitable alignment requirements for a cross-compiler solution (patches are > welcome!) I'll try to try these, but will have to do some research first... Or maybe do you have good pointers already? > What are the cases where Sparc requires 64-bit alignment? Would an array of > size32_t elements without padding be problematic, for example? Basically, any 32-bit access must be 32-bit aligned, as with ARM, and any 64-bit access must be 64-bit aligned. Some assignments end up doing 64-bit accesses. > Apple's sources > for iPhone have some alignment fixes for ARM, which you can look up at Apple's > open source site, if you are OK with agreeing to the Apple Public Source > License, but I don't know if Sparc requirements are the same. AFAIK, ARM only requires 32-bit alignment, not 64-bit alignment. ARM alignment requirements are already fulfilled by webkit code, apparently.

(In reply to comment #2) > I'll try to try these, but will have to do some research first... Or maybe do > you have good pointers already? You can look up __align__ usage in iPhone sources. For a compiler-neutral technique, I was thinking about something like -char m_inlineBuffer[m_inlineBufferSize]; +uint32_t m_inlineBuffer[(m_inlineBufferSize + 3) / 4]; > AFAIK, ARM only requires 32-bit alignment, not 64-bit alignment. ARM alignment > requirements are already fulfilled by webkit code, apparently. Not for Vector inline buffer though, which is not aligned at all. I don't know if that's the only case.

(In reply to comment #3) > You can look up __align__ usage in iPhone sources. For a compiler-neutral > technique, I was thinking about something like > -char m_inlineBuffer[m_inlineBufferSize]; > +uint32_t m_inlineBuffer[(m_inlineBufferSize + 3) / 4]; This looks just like what we already have for FastMalloc's pageheap_memory. I'll try that.

Created attachment 21963 [details] beginnings of a patch This patch allows GtkLauncher to start and display the google homepage. I still have to parse the build logs to look for "increases required alignment of target type" warnings, that should all be problematic on sparc. Note I also added, in this patch, a generic thing for "unknown" platforms, to avoid to assume little endianness.

Comment on attachment 21963 [details] beginnings of a patch >- // FIXME: Nothing guarantees this buffer is appropriately aligned to hold objects of type T. >- char m_inlineBuffer[m_inlineBufferSize]; >+ uuint64_t m_inlineBuffer[(m_inlineBufferSize + sizeof(uint64_t) - 1) / sizeof(uint64_t)]; This should read uint64_t, not uuint64_t...

Created attachment 22105 [details] Patch I'm applying to the debian package This is the patch I came up with for debian. It is not fully tested yet: I know it doesn't break the build on amd64, and it seems to be okay on sparc. I'll obviously know better when the upload will have been done and our buildds will have tried it. There should still be alignment warnings during the build on sparc, but most should just be fine, and I don't know for some other few. I invite you to take a look at the buildlogs on http://buildd.debian.org/webkit. Note I added an aligment hack for arm, too, because I see no reason why this wouldn't be required.

See also: bug 16925 (the suggested change for Vector uses GCC and MSVC extensions instead of the uint64_t trick).

On XScale/ARM, this I faced another alignment issue. #19946 might be relevant for alignment on Sparc, too.

Sorry, here's the link: https://bugs.webkit.org/show_bug.cgi?id=19946

I don't want to hijack this report, but i think i have the same problem on OpenBSD/sparc64 -current with webkit-1.0.1. It works fine on amd64 and i386 archs though. Will try a rebuild (12 hours..) will all the patches provided (the debian one, and #19946 #16925) and will let you know how it goes on my Ultra60. My trace seems to match the one on http://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg535386.html Program received signal SIGBUS, Bus error. 0x0000000051b00614 in WTF::HashSet<WebCore::StringImpl*, WebCore::StringHash,WTF::HashTraits<WebCore::StringImpl*> >::add<WebCore::UCharBuffer,WebCore::UCharBufferTranslator> () from /usr/local/lib/libwebkit-1.0.so.0.0 (gdb) bt #0 0x0000000051b00614 in WTF::HashSet<WebCore::StringImpl*, WebCore::StringHash,WTF::HashTraits<WebCore::StringImpl*> >::add<WebCore::UCharBuffer,WebCore::UCharBufferTranslator> () from /usr/local/lib/libwebkit-1.0.so.0.0 #1 0x0000000051aff4a4 in WebCore::AtomicString::add () from /usr/local/lib/libwebkit-1.0.so.0.0 #2 0x0000000051da7734 in cssyyparse () from /usr/local/lib/libwebkit-1.0.so.0.0 #3 0x00000000518b2a20 in WebCore::CSSParser::parseSheet () from /usr/local/lib/libwebkit-1.0.so.0.0 #4 0x00000000518f8ecc in WebCore::CSSStyleSheet::parseString () from /usr/local/lib/libwebkit-1.0.so.0.0 #5 0x00000000518d3368 in WebCore::parseUASheet () from /usr/local/lib/libwebkit-1.0.so.0.0 #6 0x00000000518d348c in WebCore::loadDefaultStyle () from /usr/local/lib/libwebkit-1.0.so.0.0 #7 0x00000000518f5688 in WebCore::CSSStyleSelector::CSSStyleSelector () from /usr/local/lib/libwebkit-1.0.so.0.0 #8 0x0000000051910d44 in WebCore::Document::attach () from /usr/local/lib/libwebkit-1.0.so.0.0 #9 0x0000000051ad3e7c in WebCore::Frame::setDocument () from /usr/local/lib/libwebkit-1.0.so.0.0 #10 0x0000000051a67858 in WebCore::FrameLoader::begin () from /usr/local/lib/libwebkit-1.0.so.0.0 #11 0x0000000051a75344 in WebCore::FrameLoader::receivedFirstData () from /usr/local/lib/libwebkit-1.0.so.0.0 #12 0x0000000051a757a4 in WebCore::FrameLoader::setEncoding () from /usr/local/lib/libwebkit-1.0.so.0.0 #13 0x00000000518546e0 in WebKit::FrameLoaderClient::committedLoad () from /usr/local/lib/libwebkit-1.0.so.0.0 #14 0x0000000051853280 in WebKit::FrameLoaderClient::finishedLoading () from /usr/local/lib/libwebkit-1.0.so.0.0 #15 0x0000000051a78934 in WebCore::FrameLoader::finishedLoadingDocument () from /usr/local/lib/libwebkit-1.0.so.0.0 #16 0x0000000051a59850 in WebCore::DocumentLoader::finishedLoading () from /usr/local/lib/libwebkit-1.0.so.0.0 #17 0x0000000051a7929c in WebCore::FrameLoader::init () from /usr/local/lib/libwebkit-1.0.so.0.0 #18 0x0000000051844be4 in webkit_web_frame_new () from /usr/local/lib/libwebkit-1.0.so.0.0 #19 0x000000005184d33c in webkit_web_view_init () from /usr/local/lib/libwebkit-1.0.so.0.0 #20 0x0000000056ae1548 in g_type_create_instance () from /usr/local/lib/libgobject-2.0.so.1600.0 #21 0x0000000056aca178 in g_object_new_valist () from /usr/local/lib/libgobject-2.0.so.1600.0 #22 0x0000000056ac9924 in g_object_newv () from /usr/local/lib/libgobject-2.0.so.1600.0 #23 0x0000000056aca11c in g_object_new_valist () from /usr/local/lib/libgobject-2.0.so.1600.0 #24 0x0000000056ac9658 in g_object_new () from /usr/local/lib/libgobject-2.0.so.1600.0 #25 0x0000000051848594 in webkit_web_view_new () from /usr/local/lib/libwebkit-1.0.so.0.0 #26 0x00000000001021e8 in main () Landry, OpenBSD porter.

Created attachment 22185 [details] patches applied on openbsd Patches currently applied against webkit 1.0.1 on OpenBSD : - signbit and isfinite are being added to our libm, in the meantime they're implemented in source/#defined - JavaScriptCore/kjs/collector.cpp is patched to work with our pthread - The other patches are backported from the debian patch (but in Platform.h we have __sparc64__ instead of __sparc__ which is for sparc32 architecture (sun4, sun4c and sun4m processors, cf http://openbsd.org/sparc.html#hardware) - JavaScriptCore/wtf/Threading.h : atomicIncrement/atomicDecrement are patched to use the fallback code instead of libestdc++ 4.2. - WebKitTools/DumpRenderTree/DumpRenderTree.h : we don't have support for wchars.

With the attached patches against webkit 1.0.1, GtkLauncher and midori builds & works fine on OpenBSD/i386/amd64/macppc. On sparc64 (Ultra60), i get a 'new' SIGBUS when launching GtkLauncher (window shows though, with midori it doesn't even show up), trace follows : (i know it's pretty useless without line numbers, will try to build a debug version. Hope this helps anyway, i'm open to all suggestion/patches to test) Program received signal SIGBUS, Bus error. 0x0000000049c61864 in KJS::PropertyMap::insert () from /usr/local/lib/libwebkit-1.0.so.0.0 (gdb) bt #0 0x0000000049c61864 in KJS::PropertyMap::insert () from /usr/local/lib/libwebkit-1.0.so.0.0 #1 0x0000000049c6e884 in KJS::PropertyMap::createTable () from /usr/local/lib/libwebkit-1.0.so.0.0 #2 0x0000000049c6e914 in KJS::PropertyMap::put () from /usr/local/lib/libwebkit-1.0.so.0.0 #3 0x0000000049c82fd8 in KJS::FunctionPrototype::FunctionPrototype () from /usr/local/lib/libwebkit-1.0.so.0.0 #4 0x0000000049c3d228 in KJS::JSGlobalObject::reset () from /usr/local/lib/libwebkit-1.0.so.0.0 #5 0x00000000495cd318 in WebCore::JSDOMWindowBase::JSDOMWindowBase () from /usr/local/lib/libwebkit-1.0.so.0.0 #6 0x0000000049b2e84c in WebCore::JSDOMWindow::JSDOMWindow () from /usr/local/lib/libwebkit-1.0.so.0.0 #7 0x00000000495d71c0 in WebCore::JSDOMWindowShell::JSDOMWindowShell () from /usr/local/lib/libwebkit-1.0.so.0.0 #8 0x00000000495edb08 in WebCore::ScriptController::initScript () from /usr/local/lib/libwebkit-1.0.so.0.0 #9 0x00000000495edda8 in WebCore::ScriptController::evaluate () from /usr/local/lib/libwebkit-1.0.so.0.0 #10 0x00000000497e0578 in WebCore::FrameLoader::executeScript () from /usr/local/lib/libwebkit-1.0.so.0.0 #11 0x00000000497a2234 in WebCore::HTMLTokenizer::scriptExecution () from /usr/local/lib/libwebkit-1.0.so.0.0 #12 0x00000000497a3158 in WebCore::HTMLTokenizer::scriptHandler () from /usr/local/lib/libwebkit-1.0.so.0.0 #13 0x00000000497a3bf4 in WebCore::HTMLTokenizer::parseSpecial () from /usr/local/lib/libwebkit-1.0.so.0.0 #14 0x00000000497a62b8 in WebCore::HTMLTokenizer::parseTag () from /usr/local/lib/libwebkit-1.0.so.0.0 #15 0x00000000497a70d0 in WebCore::HTMLTokenizer::write () from /usr/local/lib/libwebkit-1.0.so.0.0 #16 0x00000000497cb518 in WebCore::FrameLoader::write () from /usr/local/lib/libwebkit-1.0.so.0.0 #17 0x00000000495c14b4 in WebKit::FrameLoaderClient::committedLoad () from /usr/local/lib/libwebkit-1.0.so.0.0 #18 0x00000000497c7398 in WebCore::FrameLoader::committedLoad () from /usr/local/lib/libwebkit-1.0.so.0.0 #19 0x00000000497beafc in WebCore::DocumentLoader::commitLoad () from /usr/local/lib/libwebkit-1.0.so.0.0 #20 0x00000000497f8010 in WebCore::ResourceLoader::didReceiveData () from /usr/local/lib/libwebkit-1.0.so.0.0 #21 0x00000000497f1ccc in WebCore::MainResourceLoader::didReceiveData () from /usr/local/lib/libwebkit-1.0.so.0.0 #22 0x00000000497f7944 in WebCore::ResourceLoader::didReceiveData () from /usr/local/lib/libwebkit-1.0.so.0.0 #23 0x0000000049967568 in WebCore::writeCallback () from /usr/local/lib/libwebkit-1.0.so.0.0 #24 0x0000000055888e68 in Curl_client_write () from /usr/local/lib/libcurl.so.10.0 #25 0x00000000558a3734 in inflate_stream () from /usr/local/lib/libcurl.so.10.0 #26 0x00000000558a3b34 in Curl_unencode_gzip_write () from /usr/local/lib/libcurl.so.10.0 #27 0x000000005589c2b8 in Curl_readwrite () from /usr/local/lib/libcurl.so.10.0 #28 0x00000000558a21e4 in multi_runsingle () from /usr/local/lib/libcurl.so.10.0 #29 0x00000000558a24e4 in curl_multi_perform () from /usr/local/lib/libcurl.so.10.0 #30 0x0000000049968250 in WebCore::ResourceHandleManager::downloadTimerCallback () from /usr/local/lib/libwebkit-1.0.so.0.0 #31 0x0000000049969228 in WebCore::Timer<WebCore::ResourceHandleManager>::fired () from /usr/local/lib/libwebkit-1.0.so.0.0 #32 0x0000000049894ee0 in WebCore::TimerBase::fireTimers () from /usr/local/lib/libwebkit-1.0.so.0.0 #33 0x0000000049894f7c in WebCore::TimerBase::sharedTimerFired () from /usr/local/lib/libwebkit-1.0.so.0.0 #34 0x0000000049aeb830 in WebCore::timeout_cb () from /usr/local/lib/libwebkit-1.0.so.0.0 #35 0x0000000056b4c098 in g_main_context_is_owner () from /usr/local/lib/libglib-2.0.so.1600.0 #36 0x0000000056b48798 in g_source_is_destroyed () from /usr/local/lib/libglib-2.0.so.1600.0 #37 0x0000000056b48798 in g_source_is_destroyed () from /usr/local/lib/libglib-2.0.so.1600.0 Previous frame identical to this frame (corrupt stack?)

Comment on attachment 22185 [details] patches applied on openbsd Thanks for submitting this patch for review, Landry! In order for this patch to be integrated upstream into WebKit, you will need to define a new PLATFORM(OPENBSD) macro in JavaScriptCore/wtf/Platform.h and use that instead of removing/changing existing code. I would also suggest filing a separate bug for the OpenBSD fixes that aren't covered by this bug, although that's up to you. You may also want to consider setting up an OpenBSD buildbot that could be added to <http://build.webkit.org/> to make sure the build doesn't break on OpenBSD. Please chat with "bdash" on the #webkit IRC channel on irc.freenode.net if you'd like more information about this. >--- JavaScriptCore/kjs/JSImmediate.h.orig Sat Jun 28 15:28:13 2008 >+++ JavaScriptCore/kjs/JSImmediate.h Sat Jun 28 15:38:46 2008 >@@ -31,6 +31,22 @@ > #include <stdint.h> > #include <stdlib.h> > >+typedef union { >+ double value; >+ int64_t bits; >+} ieee_double_union; >+ >+static __inline int signbit(double x) >+{ >+ ieee_double_union mask; >+ mask.value = -0.0; >+ int64_t sign_bit = mask.bits; >+ >+ ieee_double_union number; >+ number.value = x; >+ return (number.bits & sign_bit) != 0; >+} >+ > namespace KJS { > > class ExecState; Probably needs #if PLATFORM(OPENBSD)/#endif around this addition. >--- JavaScriptCore/kjs/collector.cpp.orig Mon Jun 16 01:40:06 2008 >+++ JavaScriptCore/kjs/collector.cpp Sun Jul 6 22:54:31 2008 >@@ -33,9 +33,7 @@ > #include <wtf/HashCountedSet.h> > #include <wtf/UnusedParam.h> > >-#if USE(MULTIPLE_THREADS) > #include <pthread.h> >-#endif > > #if PLATFORM(DARWIN) > >@@ -61,11 +59,7 @@ > #include <thread.h> > #endif > >-#if HAVE(PTHREAD_NP_H) > #include <pthread_np.h> >-#else >-#include <pthread.h> >-#endif > > #endif > Instead of removing code here, you probably should define USE(MULTIPLE_THREADS) and HAVE(PTHREAD_NP_H) for PLATFORM(OPENBSD) in Platform.h. >@@ -332,27 +326,10 @@ static inline void* currentThreadStackBase() > thr_stksegment(&s); > return s.ss_sp; > #elif PLATFORM(UNIX) >- static void* stackBase = 0; >- static size_t stackSize = 0; >- static pthread_t stackThread; > pthread_t thread = pthread_self(); >- if (stackBase == 0 || thread != stackThread) { >- pthread_attr_t sattr; >- pthread_attr_init(&sattr); >-#if HAVE(PTHREAD_NP_H) >- // e.g. on FreeBSD 5.4, neundorf@kde.org >- pthread_attr_get_np(thread, &sattr); >-#else >- // FIXME: this function is non-portable; other POSIX systems may have different np alternatives >- pthread_getattr_np(thread, &sattr); >-#endif >- int rc = pthread_attr_getstack(&sattr, &stackBase, &stackSize); >- (void)rc; // FIXME: Deal with error code somehow? Seems fatal. >- ASSERT(stackBase); >- pthread_attr_destroy(&sattr); >- stackThread = thread; >- } >- return static_cast<char*>(stackBase) + stackSize; >+ stack_t stack; >+ pthread_stackseg_np(thread, &stack); >+ return stack.ss_sp; > #else > #error Need a way to get the stack base on this platform > #endif The OpenBSD code should be in its own #elif PLATFORM(OPENBSD) section before #elif PLATFORM(UNIX) rather than removing this code. >--- JavaScriptCore/kjs/date_object.cpp.orig Mon Jun 16 01:40:06 2008 >+++ JavaScriptCore/kjs/date_object.cpp Sun Jul 6 19:00:35 2008 >@@ -63,6 +63,8 @@ > #include <CoreFoundation/CoreFoundation.h> > #endif > >+#define isfinite(x) finite(x) >+ > using namespace WTF; > > namespace KJS { Needs #if PLATFORM(OPENBSD)/#endif wrapper. (Or you may want to consider add this to a header file; perhaps config.h.) >--- JavaScriptCore/wtf/FastMalloc.cpp.orig Tue Jul 8 23:21:56 2008 >+++ JavaScriptCore/wtf/FastMalloc.cpp Tue Jul 8 23:22:46 2008 >@@ -1824,13 +1824,13 @@ static TCMalloc_Central_FreeListPadded central_cache[k > > // Page-level allocator > static SpinLock pageheap_lock = SPINLOCK_INITIALIZER; >-static void* pageheap_memory[(sizeof(TCMalloc_PageHeap) + sizeof(void*) - 1) / sizeof(void*)]; >+static uint64_t pageheap_memory[(sizeof(TCMalloc_PageHeap) + sizeof(uint64_t) - 1) / sizeof(uint64_t)]; > static bool phinited = false; > > // Avoid extra level of indirection by making "pageheap" be just an alias > // of pageheap_memory. > typedef union { >- void* m_memory; >+ uint64_t* m_memory; > TCMalloc_PageHeap* m_pageHeap; > } PageHeapUnion; > >$OpenBSD$ >--- JavaScriptCore/wtf/ListHashSet.h.orig Tue Jul 8 23:23:01 2008 >+++ JavaScriptCore/wtf/ListHashSet.h Tue Jul 8 23:24:03 2008 >@@ -122,7 +122,7 @@ namespace WTF { > : m_freeList(pool()) > , m_isDoneWithInitialFreeList(false) > { >- memset(m_pool.pool, 0, sizeof(m_pool.pool)); >+ memset(m_pool, 0, sizeof(m_pool)); > } > > Node* allocate() >@@ -166,7 +166,7 @@ namespace WTF { > } > > private: >- Node* pool() { return reinterpret_cast<Node*>(m_pool.pool); } >+ Node* pool() { return reinterpret_cast<Node*>(m_pool); } > Node* pastPool() { return pool() + m_poolSize; } > > bool inPool(Node* node) >@@ -177,10 +177,7 @@ namespace WTF { > Node* m_freeList; > bool m_isDoneWithInitialFreeList; > static const size_t m_poolSize = 256; >- union { >- char pool[sizeof(Node) * m_poolSize]; >- double forAlignment; >- } m_pool; >+ uint32_t m_pool[(sizeof(Node) * m_poolSize + sizeof(uint32_t) - 1) / sizeof(uint32_t)]; > }; > > template<typename ValueArg> struct ListHashSetNode { I think these changes should be reviewed in Mike's patch separately. >--- JavaScriptCore/wtf/Platform.h.orig Tue Jul 8 23:24:28 2008 >+++ JavaScriptCore/wtf/Platform.h Tue Jul 8 23:25:54 2008 >@@ -172,6 +172,12 @@ > #define WTF_PLATFORM_X86_64 1 > #endif > >+/* PLATFORM(SPARC64) */ >+#if defined(__sparc64__) >+#define WTF_PLATFORM_SPARC64 1 >+#define WTF_PLATFORM_BIG_ENDIAN 1 >+#endif >+ > /* Compiler */ > > /* COMPILER(MSVC) */ This looks good! >--- JavaScriptCore/wtf/Threading.h.orig Sun Jul 6 19:10:20 2008 >+++ JavaScriptCore/wtf/Threading.h Sun Jul 6 19:10:49 2008 >@@ -168,26 +168,6 @@ class ThreadCondition : Noncopyable { (private) > PlatformCondition m_condition; > }; > >-#if PLATFORM(WIN_OS) && !COMPILER(MSVC7) >-#define WTF_USE_LOCKFREE_THREADSAFESHARED 1 >- >-inline void atomicIncrement(int volatile* addend) { InterlockedIncrement(reinterpret_cast<long volatile*>(addend)); } >-inline int atomicDecrement(int volatile* addend) { return InterlockedDecrement(reinterpret_cast<long volatile*>(addend)); } >- >-#elif PLATFORM(DARWIN) >-#define WTF_USE_LOCKFREE_THREADSAFESHARED 1 >- >-inline void atomicIncrement(int volatile* addend) { OSAtomicIncrement32Barrier(const_cast<int*>(addend)); } >-inline int atomicDecrement(int volatile* addend) { return OSAtomicDecrement32Barrier(const_cast<int*>(addend)); } >- >-#elif COMPILER(GCC) >-#define WTF_USE_LOCKFREE_THREADSAFESHARED 1 >- >-inline void atomicIncrement(int volatile* addend) { __gnu_cxx::__atomic_add(addend, 1); } >-inline int atomicDecrement(int volatile* addend) { return __gnu_cxx::__exchange_and_add(addend, -1) - 1; } >- >-#endif >- > template<class T> class ThreadSafeShared : Noncopyable { > public: > ThreadSafeShared(int initialRefCount = 1) Should probably change #elif COMPILER(GCC) to: #elif COMPILER(GCC) && !PLATFORM(OPENBSD) >@@ -197,25 +177,17 @@ template<class T> class ThreadSafeShared : Noncopyable > > void ref() > { >-#if USE(LOCKFREE_THREADSAFESHARED) >- atomicIncrement(&m_refCount); >-#else > MutexLocker locker(m_mutex); > ++m_refCount; >-#endif > } > > void deref() > { >-#if USE(LOCKFREE_THREADSAFESHARED) >- if (atomicDecrement(&m_refCount) <= 0) >-#else > { > MutexLocker locker(m_mutex); > --m_refCount; > } > if (m_refCount <= 0) >-#endif > delete static_cast<T*>(this); > } > >@@ -226,17 +198,13 @@ template<class T> class ThreadSafeShared : Noncopyable > > int refCount() const > { >-#if !USE(LOCKFREE_THREADSAFESHARED) > MutexLocker locker(m_mutex); >-#endif > return static_cast<int const volatile &>(m_refCount); > } > > private: > int m_refCount; >-#if !USE(LOCKFREE_THREADSAFESHARED) > mutable Mutex m_mutex; >-#endif > }; > > // This function must be called from the main thread. It is safe to call it repeatedly. These changes aren't needed if you change the #eilf COMPILER(GCC) above. >--- JavaScriptCore/wtf/Vector.h.orig Tue Jul 8 23:26:12 2008 >+++ JavaScriptCore/wtf/Vector.h Tue Jul 8 23:26:41 2008 >@@ -386,8 +386,7 @@ namespace WTF { > static const size_t m_inlineBufferSize = inlineCapacity * sizeof(T); > T* inlineBuffer() { return reinterpret_cast<T*>(&m_inlineBuffer); } > >- // FIXME: Nothing guarantees this buffer is appropriately aligned to hold objects of type T. >- char m_inlineBuffer[m_inlineBufferSize]; >+ uint64_t m_inlineBuffer[(m_inlineBufferSize + sizeof(uint64_t) - 1) / sizeof(uint64_t)]; > }; > > template<typename T, size_t inlineCapacity = 0> I think these changes should be reviewed in Mike's patch separately. >--- WebCore/html/CanvasRenderingContext2D.cpp.orig Sat Jun 28 15:31:53 2008 >+++ WebCore/html/CanvasRenderingContext2D.cpp Sat Jun 28 15:33:07 2008 >@@ -64,6 +64,8 @@ > #include <cairo.h> > #endif > >+#define isfinite(x) finite(x) >+ > namespace WebCore { > > using namespace HTMLNames; >$OpenBSD$ >--- WebCore/platform/graphics/cairo/PathCairo.cpp.orig Sat Jun 28 15:32:43 2008 >+++ WebCore/platform/graphics/cairo/PathCairo.cpp Sat Jun 28 15:32:55 2008 >@@ -34,6 +34,8 @@ > #include <math.h> > #include <wtf/MathExtras.h> > >+#define isfinite(x) finite(x) >+ > namespace WebCore { > > Path::Path() >$OpenBSD$ Again, defining this in config.h means you don't have to define it everywhere. >--- WebCore/platform/text/AtomicString.cpp.orig Tue Jul 8 23:27:07 2008 >+++ WebCore/platform/text/AtomicString.cpp Tue Jul 8 23:27:35 2008 >@@ -94,7 +94,7 @@ static inline bool equal(StringImpl* string, const UCh > if (string->length() != length) > return false; > >-#if PLATFORM(ARM) >+#if PLATFORM(ARM) || PLATFORM(SPARC64) > const UChar* stringCharacters = string->characters(); > for (unsigned i = 0; i != length; ++i) { > if (*stringCharacters++ != *characters++) Looks good. >--- WebCore/platform/text/StringHash.h.orig Tue Jul 8 23:27:50 2008 >+++ WebCore/platform/text/StringHash.h Tue Jul 8 23:29:14 2008 >@@ -46,6 +46,15 @@ namespace WebCore { > if (aLength != bLength) > return false; > >+#if PLATFORM(ARM) || PLATFORM(SPARC64) >+ const UChar* aChars = a->characters(); >+ const UChar* bChars = b->characters(); >+ for (unsigned i = 0; i != aLength; ++i) >+ if (*aChars++ != *bChars++) >+ return false; >+ >+ return true; >+#else > const uint32_t* aChars = reinterpret_cast<const uint32_t*>(a->characters()); > const uint32_t* bChars = reinterpret_cast<const uint32_t*>(b->characters()); > >@@ -58,6 +67,7 @@ namespace WebCore { > return false; > > return true; >+#endif > } > > static unsigned hash(const RefPtr<StringImpl>& key) { return key->hash(); } It would be nice if this change were relative to Mike's patch. If Mike doesn't mind, though, maybe these changes could be combined (if you're willing to resubmit the patch). >--- WebKitTools/DumpRenderTree/DumpRenderTree.h.orig Sat Jun 28 15:34:07 2008 >+++ WebKitTools/DumpRenderTree/DumpRenderTree.h Sat Jun 28 15:34:15 2008 >@@ -46,8 +46,6 @@ extern CFRunLoopTimerRef waitToDumpWatchdog; > > #include <string> > >-std::wstring urlSuitableForTestResult(const std::wstring& url); >- > class LayoutTestController; > > extern volatile bool done; Needs #if !PLATFORM(OPENBSD)/#endif around the code (unless there is another macro that's used to ignore wide strings). >--- autogen.sh.orig Tue Jul 1 18:02:03 2008 >+++ autogen.sh Tue Jul 1 18:02:25 2008 >@@ -42,8 +42,6 @@ if test "$DIE" -eq 1; then > exit 1 > fi > >-rm -rf $top_srcdir/autom4te.cache >- > touch README INSTALL > > aclocal || exit $? >@@ -51,7 +49,3 @@ $LIBTOOLIZE --force || exit $? > autoheader || exit $? > automake --foreign --add-missing || exit $? > autoconf || exit $? >- >-cd $ORIGDIR || exit 1 >- >-$srcdir/configure $AUTOGEN_CONFIGURE_ARGS "$@" || exit $? You should probably confer with the Gtk porters to make sure these changes are okay (to the point where it may be beneficial to have a separate bug just for these autogen.sh changes). Thanks again for the patch, Landry!

(In reply to comment #14) > (From update of attachment 22185 [details] [edit]) > Thanks for submitting this patch for review, Landry! > > In order for this patch to be integrated upstream into WebKit, you will need to > define a new PLATFORM(OPENBSD) macro in JavaScriptCore/wtf/Platform.h and use > that instead of removing/changing existing code. > > I would also suggest filing a separate bug for the OpenBSD fixes that aren't > covered by this bug, although that's up to you. I forget: r- since we need a PLATFORM(OPENBSD) macro that will let you ignore code when compiling without removing it. Thanks!

> >--- WebCore/platform/text/StringHash.h.orig Tue Jul 8 23:27:50 2008 > >+++ WebCore/platform/text/StringHash.h Tue Jul 8 23:29:14 2008 > >@@ -46,6 +46,15 @@ namespace WebCore { > > if (aLength != bLength) > > return false; > > > >+#if PLATFORM(ARM) || PLATFORM(SPARC64) > >+ const UChar* aChars = a->characters(); > >+ const UChar* bChars = b->characters(); > >+ for (unsigned i = 0; i != aLength; ++i) > >+ if (*aChars++ != *bChars++) > >+ return false; > >+ > >+ return true; > >+#else > > const uint32_t* aChars = reinterpret_cast<const uint32_t*>(a->characters()); > > const uint32_t* bChars = reinterpret_cast<const uint32_t*>(b->characters()); > > > >@@ -58,6 +67,7 @@ namespace WebCore { > > return false; > > > > return true; > >+#endif > > } > > > > static unsigned hash(const RefPtr<StringImpl>& key) { return key->hash(); } > > It would be nice if this change were relative to Mike's patch. If Mike doesn't > mind, though, maybe these changes could be combined (if you're willing to > resubmit the patch). This one is in my patch, already ;)

It might be better to split the patch into logical parts for review - some parts seem to be better understood than others, and there is no reason to keep sending it all back and forth for substantial discussion about its parts or coding style nitpicks (which we care about a lot).

(In reply to comment #16) > > >--- WebCore/platform/text/StringHash.h.orig Tue Jul 8 23:27:50 2008 > > >+++ WebCore/platform/text/StringHash.h Tue Jul 8 23:29:14 2008 > > >@@ -46,6 +46,15 @@ namespace WebCore { > > > if (aLength != bLength) > > > return false; > > > > > >+#if PLATFORM(ARM) || PLATFORM(SPARC64) > > >+ const UChar* aChars = a->characters(); > > >+ const UChar* bChars = b->characters(); > > >+ for (unsigned i = 0; i != aLength; ++i) > > >+ if (*aChars++ != *bChars++) > > >+ return false; > > >+ > > >+ return true; > > >+#else > > > const uint32_t* aChars = reinterpret_cast<const uint32_t*>(a->characters()); > > > const uint32_t* bChars = reinterpret_cast<const uint32_t*>(b->characters()); > > > > > >@@ -58,6 +67,7 @@ namespace WebCore { > > > return false; > > > > > > return true; > > >+#endif > > > } > > > > > > static unsigned hash(const RefPtr<StringImpl>& key) { return key->hash(); } > > > > It would be nice if this change were relative to Mike's patch. If Mike doesn't > > mind, though, maybe these changes could be combined (if you're willing to > > resubmit the patch). > > This one is in my patch, already ;) > Well, yours has SPARC not SPARC64. Maybe we want to differentiate those cases, dunno what's defined on debian/linux, but on OpenBSD __sparc__ and __sparc64__ are different architectures and don't share the same obligations. sparc64 is stricter (64-bits alignment, __sparc__ is for 32-bits cpus). Btw, thanks david for your comments, my patches were a first shot to get webkit working fine, in then end i was clearly planning to have PLATFORM(OPENBSD) to get all the patches integrated. Beign able to compile webkit without patches would be awesome. The autogen.sh patch is here for two things: - $top_srcdir is not defined, hence systrace (used in the ports infrastructure) complains about it trying to erase a file in / - generally, autogen.sh scripts only runs auto*hell (i even personally prefer if possible using autoreconf and avoid hand-rolled scripts), and configure is called afterwards. This way it fits better in the ports-tree infrastructure. (autogen is called in post-patch, the configure is called separately) I'll open a separate bug for all OpenBSD-specific changes. But btw my 'new' SIGBUS remains, i'll get more info with my --enable-debug next build.

> Well, yours has SPARC not SPARC64. Maybe we want to differentiate those cases, > dunno what's defined on debian/linux, but on OpenBSD __sparc__ and __sparc64__ > are different architectures and don't share the same obligations. sparc64 is > stricter (64-bits alignment, __sparc__ is for 32-bits cpus). For this specific case, that changes nothing. adding || PLATFORM(SPARC64) to my patch would be enough

(In reply to comment #14) > >--- JavaScriptCore/kjs/JSImmediate.h.orig Sat Jun 28 15:28:13 2008 > >+++ JavaScriptCore/kjs/JSImmediate.h Sat Jun 28 15:38:46 2008 > >@@ -31,6 +31,22 @@ > > #include <stdint.h> > > #include <stdlib.h> > > > >+typedef union { > >+ double value; > >+ int64_t bits; > >+} ieee_double_union; > >+ > >+static __inline int signbit(double x) > >+{ > >+ ieee_double_union mask; > >+ mask.value = -0.0; > >+ int64_t sign_bit = mask.bits; > >+ > >+ ieee_double_union number; > >+ number.value = x; > >+ return (number.bits & sign_bit) != 0; > >+} > >+ > > namespace KJS { > > > > class ExecState; > > Probably needs #if PLATFORM(OPENBSD)/#endif around this addition. Last note on this patch (and #define isfinite) : a diff is floating around to add soon signbit and isfinite to libm in -CURRENT, so i don't think it's worth adding those things in webkit svn. More to follow in the other bug report to come, sorry for hijacking with non-fully-related patches :)

Done, https://bugs.webkit.org/show_bug.cgi?id=19977 deals with the autogen.sh, and https://bugs.webkit.org/show_bug.cgi?id=19975 has openbsd-specific patches.

Comment on attachment 22105 [details] Patch I'm applying to the debian package #if PLATFORM(ARM) || PLATFORM(SPARC) 50 const UChar* aChars = a->characters(); 51 const UChar* bChars = b->characters(); 52 for (unsigned i = 0; i != aLength; ++i) 53 if (*aChars++ != *bChars++) 54 return false; 55 56 return true; 57 #else These really should be broken out into separate functions which are called one or the other depending on the platform. Especially since the same code is already copy/paste in AtomicString.

Comment on attachment 22105 [details] Patch I'm applying to the debian package This patch a) breaks the build on Mac OS X, and b) will almost certainly lead to incorrect code generation due to the strict aliasing violations it introduces. Here are some of the errors that are produced: ./wtf/Platform.h:198:5: error: "__BYTE_ORDER" is not defined ./wtf/Platform.h:198:21: error: "__BIG_ENDIAN" is not defined ./wtf/Vector.h: In member function 'T* WTF::VectorBuffer<T, inlineCapacity>::inlineBuffer() [with T = KJS::Register, long unsigned int inlineCapacity = 8ul]': ./wtf/Vector.h:345: instantiated from 'WTF::VectorBuffer<T, inlineCapacity>::VectorBuffer() [with T = KJS::Register, long unsigned int inlineCapacity = 8ul]' ./wtf/Vector.h:407: instantiated from 'WTF::Vector<T, inlineCapacity>::Vector() [with T = KJS::Register, long unsigned int inlineCapacity = 8ul]' /Volumes/Data/Users/mrowe/Documents/Work/WebKit-git/OpenSource/JavaScriptCore/kjs/ArgList.h:49: instantiated from here ./wtf/Vector.h:389: warning: dereferencing type-punned pointer will break strict-aliasing rules

For the record, webkit 1.1.18 still gets SIGBUS at startup on OpenBSD/sparc64, Even though the original debian bug is closed.. The only relevant patch i apply to the source is the following : $OpenBSD: patch-JavaScriptCore_wtf_Platform_h,v 1.1 2009/10/14 11:17:58 landry Exp $ --- JavaScriptCore/wtf/Platform.h.orig Tue Jan 5 17:07:27 2010 +++ JavaScriptCore/wtf/Platform.h Tue Jan 5 23:03:33 2010 @@ -804,7 +804,7 @@ #endif #if !defined(WTF_USE_JSVALUE64) && !defined(WTF_USE_JSVALUE32) && !defined(WTF_USE_JSVALUE32_64) -#if (CPU(X86_64) && (OS(UNIX) || OS(WINDOWS))) || CPU(IA64) || CPU(ALPHA) +#if (CPU(X86_64) && (OS(UNIX) || OS(WINDOWS))) || CPU(IA64) || CPU(ALPHA) || CPU(SPARC64) #define WTF_USE_JSVALUE64 1 #elif CPU(ARM) || CPU(PPC64) #define WTF_USE_JSVALUE32 1 backtrace is still the same : Program received signal SIGBUS, Bus error. 0x000000004befd180 in WTF::HashSet<WebCore::StringImpl*, WebCore::StringHash, WTF::HashTraits<WebCore::StringImpl*> >::add<WebCore::UCharBuffer, WebCore::UCharBufferTranslator> () from /usr/local/lib/libwebkit-1.0.so.1.0 (gdb) bt #0 0x00000000423ed180 in WTF::HashSet<WebCore::StringImpl*, WebCore::StringHash, WTF::HashTraits<WebCore::StringImpl*> >::add<WebCore::UCharBuffer, WebCore::UCharBufferTranslator> () from /usr/local/lib/libwebkit-1.0.so.1.0 #1 0x00000000423ebff0 in WebCore::AtomicString::add () from /usr/local/lib/libwebkit-1.0.so.1.0 #2 0x000000004272c548 in cssyyparse () from /usr/local/lib/libwebkit-1.0.so.1.0 #3 0x00000000420a77a0 in WebCore::CSSParser::parseSheet () from /usr/local/lib/libwebkit-1.0.so.1.0 #4 0x00000000420fbcc8 in WebCore::CSSStyleSheet::parseString () from /usr/local/lib/libwebkit-1.0.so.1.0 #5 0x00000000420d1a84 in WebCore::parseUASheet () from /usr/local/lib/libwebkit-1.0.so.1.0 #6 0x00000000420d1b00 in WebCore::parseUASheet () from /usr/local/lib/libwebkit-1.0.so.1.0 #7 0x00000000420f1438 in WebCore::loadSimpleDefaultStyle () from /usr/local/lib/libwebkit-1.0.so.1.0 #8 0x00000000420f5490 in WebCore::CSSStyleSelector::CSSStyleSelector () from /usr/local/lib/libwebkit-1.0.so.1.0 #9 0x000000004212c4f0 in WebCore::Document::attach () from /usr/local/lib/libwebkit-1.0.so.1.0 #10 0x000000004235933c in WebCore::Frame::setDocument () from /usr/local/lib/libwebkit-1.0.so.1.0 #11 0x00000000422fcfb8 in WebCore::FrameLoader::begin () from /usr/local/lib/libwebkit-1.0.so.1.0 #12 0x00000000422fd6a4 in WebCore::FrameLoader::receivedFirstData () from /usr/local/lib/libwebkit-1.0.so.1.0 #13 0x00000000422fde04 in WebCore::FrameLoader::setEncoding () from /usr/local/lib/libwebkit-1.0.so.1.0 #14 0x0000000041eb532c in WebKit::FrameLoaderClient::committedLoad () from /usr/local/lib/libwebkit-1.0.so.1.0 #15 0x0000000041eb3248 in WebKit::FrameLoaderClient::finishedLoading () from /usr/local/lib/libwebkit-1.0.so.1.0 #16 0x00000000422fde74 in WebCore::FrameLoader::finishedLoadingDocument () from /usr/local/lib/libwebkit-1.0.so.1.0 #17 0x00000000422df930 in WebCore::DocumentLoader::finishedLoading () from /usr/local/lib/libwebkit-1.0.so.1.0 #18 0x00000000422fe97c in WebCore::FrameLoader::init () from /usr/local/lib/libwebkit-1.0.so.1.0 #19 0x0000000041ec2e78 in webkit_web_frame_new () from /usr/local/lib/libwebkit-1.0.so.1.0 #20 0x0000000041ed80e8 in webkit_web_view_init () from /usr/local/lib/libwebkit-1.0.so.1.0 #21 0x000000005291aca4 in g_type_create_instance () from /usr/local/lib/libgobject-2.0.so.1802.0 #22 0x0000000052902a58 in g_object_new_valist () from /usr/local/lib/libgobject-2.0.so.1802.0 #23 0x00000000529026b4 in g_object_newv () from /usr/local/lib/libgobject-2.0.so.1802.0 #24 0x00000000529029fc in g_object_new_valist () from /usr/local/lib/libgobject-2.0.so.1802.0 #25 0x0000000052901f18 in g_object_new () from /usr/local/lib/libgobject-2.0.so.1802.0 #26 0x0000000041ed04b4 in webkit_web_view_new () from /usr/local/lib/libwebkit-1.0.so.1.0 I'd be happy to test anything to fix that.. and it takes 20h to build. Otherwise, webkit still builds & works fine on OpenBSD/i386/amd64/powerpc.

(In reply to comment #24) > For the record, webkit 1.1.18 still gets SIGBUS at startup on OpenBSD/sparc64, > I'd be happy to test anything to fix that.. and it takes 20h to build. > Otherwise, webkit still builds & works fine on OpenBSD/i386/amd64/powerpc. You probably have to change two places in the code: In WebCore/platform/text/StringHash.h, change "#if CPU(ARM) || CPU(SH4)" to "#if CPU(ARM) || CPU(SH4) || CPU(SPARC64)" In WebCore/platform/text/AtomicString.cpp, do the same thing. As far as a patch goes, it should probably follow the advice of one of the comments: // FIXME: perhaps we should have a more abstract macro that indicates when // going 4 bytes at a time is unsafe I can explain more if you want to submit a patch.

Created attachment 46136 [details] patches to run on OpenBSD/sparc64 I can confirm that with the attached patch, webkit 1.1.18 builds and renders pages in midori and surf on OpenBSD/sparc64 -current.(well, at least google and some other basic sites i tested on a ssh forwarded X) If needed, i can compile a list of "warning: cast from 'XX' to 'YY' increases required alignment of target type" from the build logs..

Patches no longer apply on 1.2.3, but I cannot test if they are still needed since I don't own any of affected systems (I only have x86 and amd64 just now)

(In reply to comment #27) > Patches no longer apply on 1.2.3, but I cannot test if they are still needed since I don't own any of affected systems (I only have x86 and amd64 just now) Two of them are still needed, and can be found in our tree. http://www.openbsd.org/cgi-bin/cvsweb/ports/www/webkit/patches/patch-WebCore_platform_text_AtomicString_cpp?rev=1.1 http://www.openbsd.org/cgi-bin/cvsweb/ports/www/webkit/patches/patch-WebCore_platform_text_StringHash_h?rev=1.1

Created attachment 62571 [details] webkit-gtk-1.2.3-fix-pool-sparc.patch We will finally use debian patch for this :-), thanks for taking care

I think the last patch is wrong in SPARC32 case. SPARC != SPARC64. Isn't adding style fixes (missing braces) in the last hunk of a patch fixing a particular bug against webkit coding style/rules ? I'm pretty sure i tested those part of the debian patch (m_pool.pool + unrolling union) in the past, and they didn't fix the SIGBUS i was seeing. Will need to retest.

Everyone is still welcome to submit a patch for WebKit trunk, there is no need to maintain patch sets for various platforms. Information on contributing patches is available at <http://webkit.org/coding/contributing.html>.

As reported downstream at: https://bugs.gentoo.org/show_bug.cgi?id=389963 webkitgtk still doesn't build on sparc and we haven't find any updated patch to fix this issue: https://389963.bugs.gentoo.org/attachment.cgi?id=292115 Thanks for your help

*** Bug 20662 has been marked as a duplicate of this bug. ***

Pacho, the problem you see is being fixed by the patch I attached in bug 79045. The alignment problems, I think they're long gone.

For reference, this is the problem Pacho is talking about: In file included from ./Source/JavaScriptCore/wtf/dtoa/double-conversion.h:31:0, from Source/JavaScriptCore/wtf/dtoa.h:24, from Source/JavaScriptCore/wtf/ThreadingPthreads.cpp:37: ./Source/JavaScriptCore/wtf/dtoa/utils.h:62:2: error: #error Target architecture was not detected as supported by Double-Conversion.

I'm not sure it should be marked as FIXED, since it still SIGBUS'es at runtime (this is with midori/webkitgtk 1.6.1 on OpenBSD/sparc64): Program received signal SIGBUS, Bus error. 0x0000000208ff9334 in JSC::Lexer::lex () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 (gdb) bt #0 0x0000000208ff9334 in JSC::Lexer::lex () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #1 0x0000000208fc0380 in JSC::JSParser::JSParser () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #2 0x0000000208fc2aa0 in JSC::jsParse () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #3 0x00000002090008c4 in JSC::Parser::parse () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #4 0x000000020903c390 in JSC::Parser::parse<JSC::ProgramNode> () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #5 0x00000002090398b0 in JSC::ProgramExecutable::compileInternal () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #6 0x0000000208fb99c0 in JSC::Interpreter::execute () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #7 0x0000000209025338 in JSC::evaluate () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 #8 0x0000000208f1e5a0 in JSEvaluateScript () from /usr/local/lib/libjavascriptcoregtk-1.0.so.0.0 So should i reopen, or file a new bug ?

Since the original report was about CSSParser, and this crash is in JSParser, a new bug is more appropriate.