When testing JSC on Haiku I got segfaults in certain JavaScript tests. It turned out that the p and e pointers used in markStackObjectsConservatively were not aligned perfectly, so when the GC ran the != check failed and the loop continued into invalid memory. Using < is safer and works just the same as !=, stopping the loop when appropriate. See attached patch.
Created attachment 17328 [details] Use < not != in markStackObjectsConservatively
There is a small typo in my ChangeLog entry in the patch. At the end this: "same was as !=" should be "same way as !=" I figure the submitter can fix that rather than me attaching another patch.
Also I am not opposed to the idea of this being a Haiku bug, since I assume p and e are never misaligned on other platforms (otherwise this would already have been fixed.) I'd rather not hide Haiku bugs if that is the case. But I'll let you guys decide that, as you know more about the GC than I do.
(In reply to comment #3) > Also I am not opposed to the idea of this being a Haiku bug, since I assume p > and e are never misaligned on other platforms (otherwise this would already > have been fixed.) I'd rather not hide Haiku bugs if that is the case. But I'll > let you guys decide that, as you know more about the GC than I do. Ryan, do you have the ASSERT() macro implemented for your Haiku port? If so, do you hit the asserts in markStackObjectsConservatively() when you run? Those should be the early-warning signs (on debug builds) if p and e weren't aligned. http://trac.webkit.org/projects/webkit/browser/trunk/JavaScriptCore/kjs/collector.cpp#L491 Furthermore, if they're not aligned, won't this leak small chunks of memory as well?
(In reply to comment #4) > > Ryan, do you have the ASSERT() macro implemented for your Haiku port? If so, > do you hit the asserts in markStackObjectsConservatively() when you run? Those > should be the early-warning signs (on debug builds) if p and e weren't aligned. That is a good point. I think for some reason I turned off the debug build option. I will trying turning it on, change the code back to use != and see what happens. > Furthermore, if they're not aligned, won't this leak small chunks of memory as > well? Yes I was thinking the same thing. Let me test things a bit more and maybe email some other people in the Haiku community before you guys consider this patch.
Comment on attachment 17328 [details] Use < not != in markStackObjectsConservatively If memory is not aligned correctly, there are other problems besides the loop termination condition. For example, the garbage collector will miss pointers on the stack and collect objects that are actually in use. So while this patch is harmless for platforms where things are already correct, it's not actually sufficient to fix your port. I suggest further investigation of why you don't have alignment. A larger change may be necessary if the storage on the stack is not guaranteed to be aligned -- you may need to walk the stack 2 bytes at a time or even 1 byte at a time, in which case you'll need to change more than just this loop termination condition.
I applied David's advice and got an ASSERT on the e pointer, so it seems the Haiku stack base is not aligned. I have emailed the Haiku development list to see what they say. Based on this I don't think this patch should be applied now. Therefore I'm marking it won't fix.