Initial Solaris support

Initial Solaris support

[ry, 2010-01-19 22:55:15]

Initial support for Solaris. Most of this work was done by Erich Ocean
<erich.ocean@me.com> (he has just now signed the licence agreement).

Bug 575.

[Erik Corry, 2010-01-21 09:58:08]

Thanks for this.
It needs a few fixes before we can commit it.

http://codereview.chromium.org/545125/diff/1/10
File src/platform-solaris.cc (right):

http://codereview.chromium.org/545125/diff/1/10#newcode29
src/platform-solaris.cc:29: // parts the implementation is in platform-posix.cc.
Please add a comment to the effect that this doesn't work on SPARC.  If you can
add a #error then that is even better, but perhaps that's hard to test.

http://codereview.chromium.org/545125/diff/1/10#newcode76
src/platform-solaris.cc:76: int isgreater(double x, double y) {
We don't seem to use this anywhere.  It should be removed.

http://codereview.chromium.org/545125/diff/1/10#newcode81
src/platform-solaris.cc:81: // Classify floating point number - usually defined
in math.h#ifndef fpclassify
The line above looks wrong.

http://codereview.chromium.org/545125/diff/1/10#newcode125
src/platform-solaris.cc:125: // TODO: Test to see if ceil() is correct on
Solaris.
Please remove this TODO.  If you suspect the unit tests are insufficient on this
point then please file a bug in the issue database on
http://code.google.com/p/v8

http://codereview.chromium.org/545125/diff/1/10#newcode163
src/platform-solaris.cc:163: return tzname[0]; // the location of the timezone
string on Solaris
Comments should start with a capital letter and end with a comment.  Also,
please install cpplint.py from Google and run the tools/presubmit.py to catch
lint errors (in this case there's a missing space.

http://codereview.chromium.org/545125/diff/1/10#newcode176
src/platform-solaris.cc:176: days = loc->tm_yday = utc->tm_yday;
Should this '=' be a '-'?

http://codereview.chromium.org/545125/diff/1/10#newcode177
src/platform-solaris.cc:177: hours = ((days < -1 ? 24 : 1 < days ? -24 : days *
24) +
I'm not sure what the line above is trying to do, but it looks like it has sign
issues at the very least.

http://codereview.chromium.org/545125/diff/1/10#newcode212
src/platform-solaris.cc:212: return (size_t)getpagesize();
We only allow C++-style casts, not C-style casts.

http://codereview.chromium.org/545125/diff/1/10#newcode265
src/platform-solaris.cc:265: // Redirect to std abort to signal abnormal program
termination
Missing full stop.

http://codereview.chromium.org/545125/diff/1/10#newcode311
src/platform-solaris.cc:311: UNIMPLEMENTED();
Please provide an empty implementation instead, so we can at least have logging
on Solaris, even if profiling doesn't give correct results for ticks in shared
libraries.

http://codereview.chromium.org/545125/diff/1/10#newcode612
src/platform-solaris.cc:612: TickSample sample;
Please set the sample.pc, sample.sp and sample.fp to dummy values (eg 0) and
file a bug to say that profiling doesn't work on Solaris.

http://codereview.chromium.org/545125/diff/1/12
File test/cctest/test-compiler.cc (right):

http://codereview.chromium.org/545125/diff/1/12#newcode77
test/cctest/test-compiler.cc:77: printf("%lc", (wint_t)string[j]);
C-style cast.

[ry, 2010-01-21 22:13:52]

http://codereview.chromium.org/545125/diff/1/10
File src/platform-solaris.cc (right):

http://codereview.chromium.org/545125/diff/1/10#newcode29
src/platform-solaris.cc:29: // parts the implementation is in platform-posix.cc.
On 2010/01/21 09:58:08, Erik Corry wrote:
> Please add a comment to the effect that this doesn't work on SPARC.  If you
can
> add a #error then that is even better, but perhaps that's hard to test.

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode76
src/platform-solaris.cc:76: int isgreater(double x, double y) {
On 2010/01/21 09:58:08, Erik Corry wrote:
> We don't seem to use this anywhere.  It should be removed.

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode81
src/platform-solaris.cc:81: // Classify floating point number - usually defined
in math.h#ifndef fpclassify
On 2010/01/21 09:58:08, Erik Corry wrote:
> The line above looks wrong.

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode125
src/platform-solaris.cc:125: // TODO: Test to see if ceil() is correct on
Solaris.
On 2010/01/21 09:58:08, Erik Corry wrote:
> Please remove this TODO.  If you suspect the unit tests are insufficient on
this
> point then please file a bug in the issue database on
> http://code.google.com/p/v8

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode176
src/platform-solaris.cc:176: days = loc->tm_yday = utc->tm_yday;
On 2010/01/21 09:58:08, Erik Corry wrote:
> Should this '=' be a '-'?

replaced this implmentation with a simpler version

http://codereview.chromium.org/545125/diff/1/10#newcode212
src/platform-solaris.cc:212: return (size_t)getpagesize();
On 2010/01/21 09:58:08, Erik Corry wrote:
> We only allow C++-style casts, not C-style casts.

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode265
src/platform-solaris.cc:265: // Redirect to std abort to signal abnormal program
termination
On 2010/01/21 09:58:08, Erik Corry wrote:
> Missing full stop.

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode311
src/platform-solaris.cc:311: UNIMPLEMENTED();
On 2010/01/21 09:58:08, Erik Corry wrote:
> Please provide an empty implementation instead, so we can at least have
logging
> on Solaris, even if profiling doesn't give correct results for ticks in shared
> libraries.

Done.

http://codereview.chromium.org/545125/diff/1/10#newcode612
src/platform-solaris.cc:612: TickSample sample;
On 2010/01/21 09:58:08, Erik Corry wrote:
> Please set the sample.pc, sample.sp and sample.fp to dummy values (eg 0) and
> file a bug to say that profiling doesn't work on Solaris.

Done.

http://codereview.chromium.org/545125/diff/1/12
File test/cctest/test-compiler.cc (right):

http://codereview.chromium.org/545125/diff/1/12#newcode77
test/cctest/test-compiler.cc:77: printf("%lc", (wint_t)string[j]);
On 2010/01/21 09:58:08, Erik Corry wrote:
> C-style cast.

Done.

[Mads Ager (chromium), 2010-01-25 12:41:13]

Thanks for the patch.  Landed in V8 bleeding_edge revision 3688.