560017 - Failed build js/src/jsemit.cpp js/src/jsgcchunk.cpp on Solaris with Sun Studio

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ginn Chen]

"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1239: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1249: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1286: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1299: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1319: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1508: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1574: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 1724: Warning (Anachronism): Formal argument errorCallback of type extern "C" const JSErrorFormatString*(*)(void*,const char*,const unsigned) in call to JS_ReportErrorNumber(JSContext*, extern "C" const JSErrorFormatString*(*)(void*,const char*,const unsigned), void*, const unsigned, ...) is being passed const JSErrorFormatString*(*)(void*,const char*,const unsigned).
NEXT ERROR "/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 2192: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
"/export/home/moz/tools/ff_slave/mozilla-central-solaris-sparc/build/js/src/jsemit.cpp", line 2290: Error: The name JSTreeContext is ambiguous, JSTreeContext and js::JSTreeContext.
9 Error(s) and 17 Warning(s) detected.

Comment 1

[Ginn Chen]

Attachment: patch

Comment 2

[Jim Blandy :jimb]

Why are the changes to the mmap calls necessary?  The Solaris man pages suggest that the first argument to mmap should indeed be a void *.

http://docs.sun.com/app/docs/doc/816-5167/mmap-2?a=view

The '::JSTreeContext' change looks fine.

Comment 3

[Jim Blandy :jimb]

As does the '#if' -> '#ifdef'.

Comment 4

[Ginn Chen]

(In reply to comment #2)
> Why are the changes to the mmap calls necessary?  The Solaris man pages suggest
> that the first argument to mmap should indeed be a void *.
> 
> http://docs.sun.com/app/docs/doc/816-5167/mmap-2?a=view

The sys/mman.h is here on my system.

http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/sys/mman.h

Probably I should have _XOPEN_SOURCE defined, then we will have _XPG4_2, but that would require more changes at other places.
I'm working on it as it is required to enable ipc.

Comment 5

[Jim Blandy :jimb]

I'm uncomfortable changing the type of the argument to munmap in code that is not Solaris-specific.  If you could duplicate the definition of UnmapPages into a Solaris and non-Solaris definition, that would be better. When all is POSIXy, then we can re-unify them.

Comment 6

[Jim Blandy :jimb]

Comment on attachment 439683 [details] [diff] [review]
patch

Need to split out Solaris-specific definition for UnmapPages.

Comment 7

[Ginn Chen]

Jim,

At first we did something like,
+#ifdef SOLARIS
+typedef caddr_t maddr_ptr;
+#else
+typedef void *maddr_ptr;
+#endif

The code is still there in nanojit/avmplus.cpp.

Then, I got a different suggestion at
https://bugzilla.mozilla.org/show_bug.cgi?id=529764#c2

So we just cast to caddr_ptr, I think it would not introduce extra cast if the declaration is munmap(void *, size_t).

What do you think?

Comment 8

[Jim Blandy :jimb]

Comment on attachment 439683 [details] [diff] [review]
patch

Okay --- Now that I've read Zack's review, and how the use of caddr_t was arrived at, and played with the Linux headers a bit (I didn't realize that _BSD_SOURCE was the default) it makes more sense.

Comment 9

[Ginn Chen]

http://hg.mozilla.org/tracemonkey/rev/df5f812ab8b7

The change for friend class was fixed by Bug 560504. The patch is same.

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/df5f812ab8b7