This patch (ongoing) adds support for compilation on POSIX platforms as the default. Apple, Win32, Linux, etc specific code is specialized. Fixes #if tests to be more specific and accurate, fixes warnings, compile errors, etc. Work remains, but this allows compilation on Linux. Needs testing on Apple and Win32 again as I haven't had time to test on those platforms yet.
Created attachment 4085 [details] Patch for compilation
The following snipped makes collector work properly on Linux: void *stackBase = 0; pthread_attr_t sattr; int rc; rc = pthread_getattr_np(pthread_self(), &sattr); // Fails on Linux (?) //rc = pthread_attr_getstack(&sattr, &stackBase, &stackSize); rc = pthread_attr_getstackaddr(&sattr, &stackBase); assert(stackBase); With this applied, the patch now passes everything in the regression suite properly on Linux except Date stuff. Failures: ecma/Date/15.9.3.1-1.js ecma/Date/15.9.3.1-2.js ecma/Date/15.9.3.1-3.js ecma/Date/15.9.3.1-4.js ecma/Date/15.9.3.1-5.js ecma/Date/15.9.3.2-1.js ecma/Date/15.9.3.2-2.js ecma/Date/15.9.3.2-3.js ecma/Date/15.9.3.2-4.js ecma/Date/15.9.3.2-5.js ecma/Date/15.9.3.8-1.js ecma/Date/15.9.3.8-2.js ecma/Date/15.9.3.8-3.js ecma/Date/15.9.3.8-4.js ecma/Date/15.9.3.8-5.js ecma/Date/15.9.4.2.js ecma/Date/15.9.5.10-1.js ecma/Date/15.9.5.10-6.js ecma/Date/15.9.5.10-9.js ecma/Date/15.9.5.12-1.js ecma/Date/15.9.5.12-6.js ecma/Date/15.9.5.14.js ecma/Date/15.9.5.2-1.js ecma/Date/15.9.5.2.js ecma/Date/15.9.5.23-1.js ecma/Date/15.9.5.23-10.js ecma/Date/15.9.5.23-11.js ecma/Date/15.9.5.23-12.js ecma/Date/15.9.5.23-13.js ecma/Date/15.9.5.23-14.js ecma/Date/15.9.5.23-15.js ecma/Date/15.9.5.23-16.js ecma/Date/15.9.5.23-17.js ecma/Date/15.9.5.23-18.js ecma/Date/15.9.5.24-1.js ecma/Date/15.9.5.24-2.js ecma/Date/15.9.5.24-3.js ecma/Date/15.9.5.24-4.js ecma/Date/15.9.5.24-5.js ecma/Date/15.9.5.24-6.js ecma/Date/15.9.5.24-7.js ecma/Date/15.9.5.24-8.js ecma/Date/15.9.5.25-1.js ecma/Date/15.9.5.26-1.js ecma/Date/15.9.5.27-1.js ecma/Date/15.9.5.29-1.js ecma/Date/15.9.5.30-1.js ecma/Date/15.9.5.31-1.js ecma/Date/15.9.5.32-1.js ecma/Date/15.9.5.33-1.js ecma/Date/15.9.5.34-1.js ecma/Date/15.9.5.35-1.js ecma/Date/15.9.5.36-1.js ecma/Date/15.9.5.36-2.js ecma/Date/15.9.5.36-3.js ecma/Date/15.9.5.36-4.js ecma/Date/15.9.5.36-5.js ecma/Date/15.9.5.36-6.js ecma/Date/15.9.5.36-7.js ecma/Date/15.9.5.37-1.js ecma/Date/15.9.5.37-2.js ecma/Date/15.9.5.37-3.js ecma/Date/15.9.5.37-4.js ecma/Date/15.9.5.37-5.js ecma/Date/15.9.5.5.js ecma/Date/15.9.5.6.js ecma/Date/15.9.5.8.js ecma_3/Date/15.9.5.3.js ecma_3/Date/15.9.5.4.js ecma_3/Date/15.9.5.6.js
Created attachment 4134 [details] Updated patch This fixes almost all regressions and cleans up the code a bit more. There are 7 regressions remaining in Date and none elsewhere. They need further investigation because the actual bug seems to be quite strange.
My tests in Mozilla and extended testing of the code here indicates that the testcases are wrong. How are these passing in JSCore on OS X?
Comment on attachment 4134 [details] Updated patch Great direction to go, being portable to POSIX! The switches to using __APPLE__ for some things that are currently WIN32 seem great, although I'm not certain __APPLE__ is the best way to check for the Mac OS X platform. But we can easily switch to something else with a global replace, so it seems fine to use it for now. On the other hand, I'm not at all happy with all the changes to use #if defined X rather than #if X. Is there really a compiler in use where you get warnings for using #if X when X is not defined? If so, then I suppose we can switch uniformly to use #if defined X, but we've intentionally not used that style up until now, and I don't want to change unless there's a good reason. Is this some feature in a new version of gcc? I'm also not clear on why the seemingly not-entirely-portable code is the "else" case. Is this final case truly a "POSIX" case, or is it simply Linux being treated as the default? I don't understand why pthread_getattr_np is more portable than pthread_get_stack_addr_np. I sense Linux bias here, and similarly in the config.h header. Maybe we should seek an ifdef about the presence of some non-portable pthreads extensions, or invent our own and put it in config.h. I see some left-in fprintf and fflush calls. We don't want to land that, I don't think. Must the tm_year/tm_mon call go inside an __APPLE__ ifdef? I can see how it's not needed on all platforms, but is it harmful on any?
On Monday 03 October 2005 00:03, you wrote: > The switches to using __APPLE__ for some things that are currently WIN32 > seem great, although I'm not certain __APPLE__ is the best way to check for > the Mac OS X platform. But we can easily switch to something else with a > global replace, so it seems fine to use it for now. Just tell me which one you want and I'm perfectly happy to switch to it. There should be a system-level one already defined, right? > On the other hand, I'm not at all happy with all the changes to use #if > defined X rather than #if X. Is there really a compiler in use where you > get warnings for using #if X when X is not defined? If so, then I suppose > we can switch uniformly to use #if defined X, but we've intentionally not > used that style up until now, and I don't want to change unless there's a > good reason. Is this some feature in a new version of gcc? Older gcc versions definitely warn (at best) for #if X when X is not defined. There is also the case of the #defines not being set causing somewhat unpredictable behavior. I had at least one case where code was being compiled that really shouldn't have been. > I'm also not clear on why the seemingly not-entirely-portable code is the > "else" case. Is this final case truly a "POSIX" case, or is it simply Linux > being treated as the default? Which case do you see this? I'm checking the manpages for calls in there to make sure they're portable. I was trying to put the "fallback" or "least common denominator" case into the "else". Win32 and OSX specific code surely don't fall into that category. > I don't understand why pthread_getattr_np is > more portable than pthread_get_stack_addr_np. pthread_attr_getstack is completely broken on my Linux system and pthread_get_stackaddr_np isn't available. I get a warning that pthread_attr_getstackaddr is deprecated, but the alternative is simply not useful. Hence the comment... I haven't had a chance to check OS X, but is pthread_attr_getstackaddr available and functional? It's apparently in IEEE 1003.1, but does have issues. > I sense Linux bias here, and > similarly in the config.h header. Maybe we should seek an ifdef about the > presence of some non-portable pthreads extensions, or invent our own and > put it in config.h. That config.h header is quite the hack and needs to be cleaned up IMHO. Anyway there is no Linux bias. I was trying to restructure so that anything that isn't very basic POSIX-compliant is considered the "else" case. In most cases Mac OS X should also take this case, and WIN32 should be most often the "special" case. My goal is to make this code compile on every platform I can get access to. > I see some left-in fprintf and fflush calls. We don't want to land that, I > don't think. Ooops, unclean patch. Sorry. Please remove those entirely. They're a relic of further debugging work I was doing. > Must the tm_year/tm_mon call go inside an __APPLE__ ifdef? I can see how > it's not needed on all platforms, but is it harmful on any? Yes, testcases fail with it in there. At least on Linux they do. That code seems entirely related to CF*.
Created attachment 4185 [details] Updated patch This addresses the printfs, reduces the amount of code that is APPLE_CHANGES, fixes almost all testcases that pass on OS X and didn't on Linux, and cleans it up a bit more. I think it addresses the major issues Darin mentioned.
FYI, here is a run of JSCore on Linux with the latest patch: ** Danger, Will Robinson! Danger! The following failures have been introduced: ecma_3/Date/15.9.5.6.js You fixed the following test: ecma_3/Date/15.9.5.7.js 1 regression found. 1 test fixed. The regression is toLocale* which is still using the old Linux implementation, known broken, and also an issue in KJS. I guess there is a bug in the JSCore Apple toLocale* code though, since we somehow also "fixed" one of those bugs. :-) It would be nice to get this patch in soon so that I can move on to fixing bugs and creating a build system. I'm building patches upon patches at this point.
I intergrated this with a few tweaks. Going to land shortly. I got rid of the use of defined() for the postive case, got rid of /*exec*/ entirely, and clearly flagged the nonportable function used in the portable posix case of the thread code.