Build on Solaris systems without glibc compatibility

Build on Solaris systems without glibc compatibility

- Some systems don't have signbit() defined
- backtrace() is a GNUism; use the Solaris variant, walkstack().

[ry, 2010-05-14 23:56:09]

Hi Erik,

Here are a few more changes to the Solaris port. Can you please review them?

Ry

[Erik Corry, 2010-05-15 20:00:02]

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

http://codereview.chromium.org/2092007/diff/1/2#newcode65
src/platform-solaris.cc:65: return x < 0;
This will return 0 for a 'negative NaN'.  That seems to be OK, since everywhere
we use signbit we know it is not a NaN, but it at least deserves a comment. 
Alternatively i could be fixed.  if (x < x) will tell you if you have a NaN, I
think.  signbit is part of C99 so presumably it is coming to Solaris at some
point.  Does the ifndef do anything?

http://codereview.chromium.org/2092007/diff/1/2#newcode248
src/platform-solaris.cc:248: struct stack_walker {
Unless this name is constrained by Solaris, which seems unlikely, it should be
NamedLikeThis and not_like_this.

http://codereview.chromium.org/2092007/diff/1/2#newcode249
src/platform-solaris.cc:249: Vector<OS::StackFrame> &frames;
Incorrect placement of & (see below).

http://codereview.chromium.org/2092007/diff/1/2#newcode254
src/platform-solaris.cc:254: static int StackWalkCallback(uintptr_t pc, int
signo, void *data) {
Incorrect placement of asterisk.  The Google C++ style guide requires us to be
consistent and AFAIR prefers asterisk next to the type rather than the variable.
 That is what we try to stick to in V8 (even though it is wrong :-).

http://codereview.chromium.org/2092007/diff/1/2#newcode255
src/platform-solaris.cc:255: struct stack_walker * walker = static_cast<struct
stack_walker *>(data);
No space before * in types (twice in this line).

http://codereview.chromium.org/2092007/diff/1/2#newcode260
src/platform-solaris.cc:260: walker->frames[i].address = (void*)pc;
We don't allow C style casts.  You have to use static_cast, dynamic_cast or
const_cast.

http://codereview.chromium.org/2092007/diff/1/2#newcode268
src/platform-solaris.cc:268: if (dladdr((void*)pc, &info) == 0) {
C style cast.

http://codereview.chromium.org/2092007/diff/1/2#newcode271
src/platform-solaris.cc:271: // we have containing symbol info
Comments should start with a capital letter and end with a full stop (period).

http://codereview.chromium.org/2092007/diff/1/2#newcode274
src/platform-solaris.cc:274: // no local symbol info
Comment formatting.

http://codereview.chromium.org/2092007/diff/1/2#newcode278
src/platform-solaris.cc:278: (unsigned long)pc - (unsigned long)info.dli_fbase,
C style casts and %p seems inappropriate for an unsigned long.

http://codereview.chromium.org/2092007/diff/1/2#newcode292
src/platform-solaris.cc:292: if (!walkcontext(&ctx, StackWalkCallback,
(void*)(&walker))) {
C style cast.

http://codereview.chromium.org/2092007/diff/1/3
File src/platform.h (right):

http://codereview.chromium.org/2092007/diff/1/3#newcode87
src/platform.h:87: # if !defined(signbit)
ifndef?

[ry, 2010-05-17 20:42:14]

Hi Erik, 

I think I've addressed all your comments in patch set 2.

ry

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

http://codereview.chromium.org/2092007/diff/1/2#newcode65
src/platform-solaris.cc:65: return x < 0;
On 2010/05/15 20:00:02, Erik Corry wrote:
> This will return 0 for a 'negative NaN'.  That seems to be OK, since
everywhere
> we use signbit we know it is not a NaN, but it at least deserves a comment. 
> Alternatively i could be fixed.  if (x < x) will tell you if you have a NaN, I
> think.  signbit is part of C99 so presumably it is coming to Solaris at some
> point.  Does the ifndef do anything?

I added some comments about why this is here and a note about -NaN.

http://codereview.chromium.org/2092007/diff/1/2#newcode248
src/platform-solaris.cc:248: struct stack_walker {
On 2010/05/15 20:00:02, Erik Corry wrote:
> Unless this name is constrained by Solaris, which seems unlikely, it should be
> NamedLikeThis and not_like_this.

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode249
src/platform-solaris.cc:249: Vector<OS::StackFrame> &frames;
On 2010/05/15 20:00:02, Erik Corry wrote:
> Incorrect placement of & (see below).

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode254
src/platform-solaris.cc:254: static int StackWalkCallback(uintptr_t pc, int
signo, void *data) {
On 2010/05/15 20:00:02, Erik Corry wrote:
> Incorrect placement of asterisk.  The Google C++ style guide requires us to be
> consistent and AFAIR prefers asterisk next to the type rather than the
variable.
>  That is what we try to stick to in V8 (even though it is wrong :-).

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode255
src/platform-solaris.cc:255: struct stack_walker * walker = static_cast<struct
stack_walker *>(data);
On 2010/05/15 20:00:02, Erik Corry wrote:
> No space before * in types (twice in this line).

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode260
src/platform-solaris.cc:260: walker->frames[i].address = (void*)pc;
On 2010/05/15 20:00:02, Erik Corry wrote:
> We don't allow C style casts.  You have to use static_cast, dynamic_cast or
> const_cast.

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode268
src/platform-solaris.cc:268: if (dladdr((void*)pc, &info) == 0) {
On 2010/05/15 20:00:02, Erik Corry wrote:
> C style cast.

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode271
src/platform-solaris.cc:271: // we have containing symbol info
On 2010/05/15 20:00:02, Erik Corry wrote:
> Comments should start with a capital letter and end with a full stop (period).

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode274
src/platform-solaris.cc:274: // no local symbol info
On 2010/05/15 20:00:02, Erik Corry wrote:
> Comment formatting.

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode278
src/platform-solaris.cc:278: (unsigned long)pc - (unsigned long)info.dli_fbase,
On 2010/05/15 20:00:02, Erik Corry wrote:
> C style casts and %p seems inappropriate for an unsigned long.

Done.

http://codereview.chromium.org/2092007/diff/1/2#newcode292
src/platform-solaris.cc:292: if (!walkcontext(&ctx, StackWalkCallback,
(void*)(&walker))) {
On 2010/05/15 20:00:02, Erik Corry wrote:
> C style cast.

Done.

[Erik Corry, 2010-05-19 07:36:51]

Committed to bleeding edge as 4677.