Closed
Bug 568782
Opened 11 years ago
Closed 11 years ago
Failed build js/src/jscntxt.cpp on Solaris
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
1.78 KB,
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jscntxt.cpp", line 157: Error: Formal argument 1 of type char* in call to munmap(char*, unsigned) is being passed int*.
Attachment #447956 -
Flags: review?(jim)
Comment 2•11 years ago
|
||
Ginn: IIRC caddr_t isn't universally available (although I know it's been in Solaris since at least 2.5.1). How about a cast to void * instead? That would match the SUSv2 spec.
caddr_t is available on Mac OS X, Linux, Solaris and BSD. The point here is we cannot use (void *) because munmap is using caddr_t (char *) on Solaris unless _POSIX_C_SOURCE > 2 or _XPG4_2 is defined.
Comment 4•11 years ago
|
||
Thanks Ginn -- I missed caddr_t on Mac because it's tucked away in /X11/Xw32defs.h and my test program only included sys/mman.h; it should have also included sys/types.h. I didn't test Linux. I was surprised to see this issue biting you, though -- perhaps it "goes away" for me because I'm running g++ and hence modified system headers. Carry on, forget I said anything! :) (except, does this need patching in jsgcchunk.cpp:228 too then?: JS_ALWAYS_TRUE(munmap(p, size) == 0); You might not spot it until you do a DEBUG build)
(In reply to comment #4) > (except, does this need patching in jsgcchunk.cpp:228 too then?: > JS_ALWAYS_TRUE(munmap(p, size) == 0); > You might not spot it until you do a DEBUG build) it is in #else /* XP_UNIX */ so ...
Comment 6•11 years ago
|
||
Whoops, it's actually unreached because it's not in JS_GC_HAS_MAP_ALIGN and we're on Solaris. Thanks, Ginn - that's doubly ironic because I'm the original author of the JS_GC_HAS_MAP_ALIGN code. I think I should just stop talking now :)
Attachment #447956 -
Flags: review?(jim) → review?(igor)
Comment on attachment 447956 [details] [diff] [review] patch jim is away
Comment 8•11 years ago
|
||
Comment on attachment 447956 [details] [diff] [review] patch >diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp >--- a/js/src/jscntxt.cpp >+++ b/js/src/jscntxt.cpp >@@ -149,17 +149,17 @@ StackSpace::init() > > void > StackSpace::finish() > { > #ifdef XP_WIN > VirtualFree(base, (commitEnd - base) * sizeof(jsval), MEM_DECOMMIT); > VirtualFree(base, 0, MEM_RELEASE); > #else >- munmap(base, CAPACITY_BYTES); >+ munmap((caddr_t)base, CAPACITY_BYTES); I would prefer to have explicit ifdef SOLARIS as AFAIK only Solaris has this non void* munmap. > #endif > } > > #ifdef XP_WIN > JS_FRIEND_API(bool) > StackSpace::bumpCommit(jsval *from, ptrdiff_t nvals) const > { > JS_ASSERT(end - from >= nvals);
Attachment #447956 -
Flags: review?(igor)
Igor, we didn't use ifdef in elsewhere, do you think I should change them? e.g. http://mxr.mozilla.org/mozilla-central/source/js/src/jsgcchunk.cpp#238 It was suggested at Bug 529764 comment 2
Comment 10•11 years ago
|
||
(In reply to comment #9) > Igor, we didn't use ifdef in elsewhere, do you think I should change them? Yes, please change them. Otherwise it is hard for a reader to figure out what exactly caddr_t is doing instead of void * from the documentation.
Assignee | ||
Comment 11•11 years ago
|
||
The old patch was committed to t-m in another bug by Leon Sha (perhaps accidentally) So, add #ifdef SOLARIS.
Attachment #447956 -
Attachment is obsolete: true
Attachment #460484 -
Flags: review?(igor)
Updated•11 years ago
|
Attachment #460484 -
Flags: review?(igor) → review+
Assignee | ||
Comment 12•11 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/26f5eea96492
Whiteboard: fixed-in-tracemonkey
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•