Open Bug 263952 Opened 21 years ago Updated 2 years ago

mmap64, shm_open and shm_unlink compiler warnings

Categories

(NSPR :: NSPR, defect, P3)

Sun
Solaris

Tracking

(Not tracked)

People

(Reporter: julien.pierre, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [build_warning])

Attachments

(2 files, 2 obsolete files)

mmap64 on Solaris is defined to return and take char*, rather than void* . This causes a compiler warning in 64-bit builds, both Sparc and AMD64, in pr/src/md/unix.c:2790 _md_iovector._mmap64 = mmap64; pointer to function(pointer to void, unsigned int, int, int, int, long long) returning pointer to void "=" pointer to function(pointer to char, unsigned int, int, int, int, long long) returning pointer to char The prototype for the function mmap64 does not agree with the definition of _MD_Mmap64 . I would rather not cast mmap64 to _MD_Mmap64 in unix.c, because this could hide real problems in the future. The proper solution is probably to ifdef the _MD_Mmap64 definition for Solaris . On other unix platforms (Linux), the mmap64 definition uses void* .
Assignee: wchang0222 → julien.pierre.bugs
Priority: -- → P3
OS: SunOS → Solaris
The proper declaration of mmap and mmap64 should use void *. See http://www.opengroup.org/onlinepubs/009695399/functions/mmap.html. I think the solution is to compile NSPR with the appropriate macros so that we get the right declaration of mmap and mmap64 in <sys/mman.h>. By looking at the standards(5) man page on Solaris 8, I believe we should use -D_XOPEN_SOURCE=500 -D__EXTENSIONS__. These in turn require that we use -DHAVE_SOCKLEN_T. You might want to ask someone in the Solaris or compiler team about the right macros to define. You should also test the above changes under gcc.
Status: NEW → ASSIGNED
Wan-Teh, Thank you ! Those defines worked. I did ask somebody in the Solaris group, but you answered earlier ;).
Hmm. Actually, these defines fixed the warning on Solaris 9 / sparc, but on S10/AMD64, I got : "/usr/include/sys/feature_tests.h", line 332: #error: "Compiler or options invalid for pre-UNIX 03 X/Open applications and pre-2001 POSIX applications" I had to set the version to 600 to make it work. This also fixes another problem with implicit definitions for shm_open and shm_unlink . "../../../../../pr/src/md/unix/uxshm.c", line 358: warning: implicit function declaration: shm_open "../../../../../pr/src/md/unix/uxshm.c", line 465: warning: implicit function declaration: shm_unlink
Summary: mmap64 compile warning → mmap64, shm_open and shm_unlink compiler warnings
It looks like the patch will require checking specific minor versions of Solaris in order to define the correct XOPEN version. We can't define a higher version. Defining version 600 and compiling on Solaris 9 results in failure. We have to define version 500 exactly to build on that platform. In addition, the gcc compiler behaves differently with the various defines. This bug does not have to be fixed short term, so targetting NSPR 4.6.
Keywords: sun-orion4
Target Milestone: --- → 4.6
Attached patch working patch (obsolete) — Splinter Review
Gets rid of warnings on Solaris for mmap64, shm_open and shm_unlink Tested on x86 / AMD64 / sparcv8 / sparcv9
Attachment #193773 - Flags: review?(wtchang)
Target Milestone: 4.6 → 4.6.1
Comment on attachment 193773 [details] [diff] [review] working patch The __EXTENSIONS__ and _XPG4_2 macros should be defined in configure.in so that they are only used when compiling NSPR itself. _solaris.cfg will be included by NSPR customers. We don't want to force our customers to define those two macros. Could you explain how you determined that these two macros are the right ones to define? Have you tested this on Solaris 8-10 and with GCC?
Attachment #193773 - Flags: review?(wtchang) → review-
I tested with gcc. I tried Solaris 9 and 10, I didn't check with 8. I'll make a new patch for configure / configure.in so that this change doesn't affect customers. I found that these are the right macros by examining the tests in the Solaris header files, mostly /usr/include/sys/mman.h . See the following section in particular : #ifdef __STDC__ #if (_POSIX_C_SOURCE > 2) || defined(_XPG4_2) extern void *mmap(void *, size_t, int, int, int, off_t); extern int munmap(void *, size_t); extern int mprotect(void *, size_t, int); extern int msync(void *, size_t, int); #if (!defined(_XPG4_2) || (_POSIX_C_SOURCE > 2)) || defined(__EXTENSIONS__) extern int mlock(const void *, size_t); extern int munlock(const void *, size_t); extern int shm_open(const char *, int, mode_t); extern int shm_unlink(const char *); We don't set POSIX_C_SOURCE, and I had no luck playing with that macro. Short of that, the only possibility that allows shm_open and shm_unlink to be defined is for both _XPG4_2 and __EXTENSIONS__ to be set . This happened to also fix mmap64 at the same time, and didn't change any other warnings for my NSPR build.
Attachment #193773 - Attachment is obsolete: true
Attachment #193776 - Flags: review?(wtchang)
Attachment #193776 - Flags: review?(wtchang) → review+
Thanks for the quick review, Wan-Teh. Could you please check the patch in to the trunk and mark the bug fixed ?
Comment on attachment 193776 [details] [diff] [review] make all the changes in configure / configure.in Julien: sorry, I found that _XPG4_2 is a private feature test macro and applications must not use it. See the comments in <sys/feature_tests.h>. I looked at Solaris 10's <sys/feature_tests.h> and couldn't figure out a solution without using autoconf tests.
Attachment #193776 - Flags: review+ → review-
Wan-Teh, FYI, the comments about _XPG4_2 sys/feature_tests.h Solaris 10 are different than for 8 and 9, and say nothing about it being an internal Sun macro ... In fact, the s10 header says : * _XPG4_2 X/Open CAE Specification, Issue 4, Version 2 (XPG4v2/UNIX 95/SUS)
See lines 247-264 in <sys/feature_tests.h>, which I quote below: (http://cvs.opensolaris.org/source/xref/usr/src/uts/common/sys/feature_tests.h) 247 * In order to simplify the guards within the headers, the following 248 * implementation private test macros have been created. Applications 249 * must NOT use these private test macros as unexpected results will 250 * occur. 251 * 252 * Note that in general, the use of these private macros is cumulative. 253 * For example, the use of _XPG3 with no other restrictions on the X/Open 254 * namespace will make the symbols visible for XPG3 through XPG6 255 * compilation environments. The use of _XPG4_2 with no other X/Open 256 * namespace restrictions indicates that the symbols were introduced in 257 * XPG4v2 and are therefore visible for XPG4v2 through XPG6 compilation 258 * environments, but not for XPG3 or XPG4 compilation environments. 259 * 260 * _XPG3 X/Open Portability Guide, Issue 3 (XPG3) 261 * _XPG4 X/Open CAE Specification, Issue 4 (XPG4) 262 * _XPG4_2 X/Open CAE Specification, Issue 4, Version 2 (XPG4v2/UNIX 95/SUS) 263 * _XPG5 X/Open CAE Specification, Issue 5 (XPG5/UNIX 98/SUSv2) 264 * _XPG6 Open Group Technical Standard, Issue 6 (XPG6/UNIX 03/SUSv3) So _XPG4_2 is one of the "private test macros" the comments at lines 247-250 refer to. Also, do "man -s 5 standards" on Solaris to look at the standards(5) man page. These private test macros are not documented in that man page.
This patch only uses public, blessed macros, and works on s9 and s10, with both studio and gcc .
Attachment #193776 - Attachment is obsolete: true
Attachment #193876 - Flags: review?(wtchang)
Comment on attachment 193876 [details] [diff] [review] use public macros r=wtc. The only thing in this patch I'm worried about is socklen_t. I don't know in which Solaris release socklen_t was added. Let's at least compile the code on Solaris 8. The standards(5) man page seems to say that XPG4v2 support was added in Solaris 2.6, which means the _XOPEN_SOURCE and _XOPEN_SOURCE_EXTENDED defines are definitely okay.
Attachment #193876 - Flags: review?(wtchang) → review+
Wan-Teh, I just compiled NSPR with this patch on Solaris 8 x86, and it built successfully, as well as without the warnings reported in this bug.
Comment on attachment 193876 [details] [diff] [review] use public macros I checked in this patch on the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9 development) for some real world testing. I suspect the HAVE_SOCKLEN_T define will break builds on old Solaris releases (such as Solaris 2.6). I will check this patch on the NSPR trunk when things settle down.
This patch does break the build on Solaris 2.6, but not because of HAVE_SOCKLEN_T : cc -o prfdcach.o -c -xstrconst -xs -g -KPIC -UNDEBUG -DDEBUG_jp96085 -DDEBUG=1 -DXP_UNIX=1 -DSVR4=1 -DSYSV=1 -D__svr4=1 -D__svr4__=1 -DSOLARIS=1 -DHAVE_FCNTL_FILE_LOCKING=1 -DHAVE_SOCKLEN_T=1 -D__EXTENSIONS__=1 -D_XOPEN_SOURCE=1 -D_XOPEN_SOURCE_EXTENDED=1 -D_PR_HAVE_OFF64_T=1 -DHAVE_LCHOWN=1 -DHAVE_STRERROR=1 -D_REENTRANT=1 -DHAVE_POINTER_LOCALTIME_R=1 -DFORCE_PR_LOG -D_PR_PTHREADS -UHAVE_CVAR_BUILT_ON_SEM -D_NSPR_BUILD_ -I/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include -I../../../../pr/include -I../../../../pr/include/private ../../../../pr/src/io/prfdcach.c "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 600: syntax error before or at: _MDOff64_t "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 600: warning: old-style declaration or incorrect type for: _MDOff64_t "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 618: syntax error before or at: * "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 618: syntax error before or at: _MDOff64_t "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OB J/include/md/_unixos.h", line 618: warning: undefined or missing type for: _MDOff64_t "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 618: warning: undefined or missing type for: PRIntn "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 618: function cannot return function or array "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 618: identifier redeclared: _MDOff64_t current : function() returning int previous: int : "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 600 "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 628: syntax error before or at: _MD_Lseek64 "/h/monstre/export/home/nss/perf/mozilla/security/nss/../../dist/SunOS5.6_DBG.OBJ/include/md/_unixos.h", line 628: cannot recover from previous errors cc: acomp failed for ../../../../pr/src/io/prfdcach.c gmake[4]: *** [prfdcach.o] Error 2 gmake[4]: Leaving directory `/h/monstre/export/home/nss/perf/mozilla/nsprpub/SunOS5.6_DBG.OBJ/pr/src/io' gmake[3]: *** [export] Error 2 gmake[3]: Leaving directory `/h/monstre/export/home/nss/perf/mozilla/nsprpub/SunOS5.6_DBG.OBJ/pr/src' gmake[2]: *** [export] Error 2 gmake[2]: Leaving directory `/h/monstre/export/home/nss/perf/mozilla/nsprpub/SunOS5.6_DBG.OBJ/pr' gmake[1]: *** [export] Error 2 gmake[1]: Leaving directory `/h/monstre/export/home/nss/perf/mozilla/nsprpub/SunOS5.6_DBG.OBJ'
The build would fail still later with socklen_t if it could continue. I don't have any Solaris 2.6 machines. Julien, do you know how to fix the off64_t problem? If not, we will need to update the configure script to only define those macros on Solaris 8 or higher. Could you email me the Solaris 2.6 header that defines off64_t?
Target Milestone: 4.6.1 → 4.6.3
Patch 193876, which was checked in to the trunk, caused a large performance regression (~2.5%) in specweb performance using the current NSS trunk build. I am raising the priority of this bug because this regression must be fixed by NSS 3.11 RTM. Judging by the difficulty of authoring another patch which both fixes the warnings and does not cause a performance regression, I propose we revert the change for now.
Priority: P3 → P1
NSS 3.11 should be built against the NSPR_4_6_BRANCH. Julien's patch is only on the NSPR trunk; it is not on the NSPR_4_6_BRANCH. So the performance regression does not need to be fixed before NSS 3.11 RTM.
Unfortunately, I don't have the time to do the investigation of the performance regression caused by this patch at the moment. I'm unsetting the target for this fix since there is no 4.7 target yet, and the fix isn't on the NSPR_4_6_BRANCH.
Target Milestone: 4.6.3 → ---
This is an alternate way to fix the compiler warning for mmap64. I can't reproduce the compiler warnings for shm_open and shm_unlink, so this patch may not address that issue.
This can't be P1 if it has no target milestone. P1 means we'd hold a release for it. Which release would we hold?
Nelson, The release we would hold for this is NSPR 4.7 . The reason the target is not set on this bug is that the 4.7 version number is not currently in the available targets in bugzilla.
Now I can not build firefox trunk on solaris 10. The error message is below. cd config; gmake -j1 export gmake[1]: Entering directory `/export/home/leon/work/mozilla/firefox/src/build1009/mozilla/nsprpub/config' /opt/SUNWspro/bin/cc -o nsinstall.o -c -xlibmil -xstrconst -xs -g -KPIC -UNDEBUG -DDEBUG_leon -DDEBUG=1 -DXP_UNIX=1 -DSVR4=1 -DSYSV=1 -D__svr4=1 -D__svr4__=1 -DSOLARIS=1 -DHAVE_FCNTL_FILE_LOCKING=1 -D_XOPEN_SOURCE=1 -D_XOPEN_SOURCE_EXTENDED=1 -D__EXTENSIONS__=1 -DHAVE_SOCKLEN_T=1 -D_PR_HAVE_OFF64_T=1 -D_PR_INET6=1 -DHAVE_LCHOWN=1 -DHAVE_STRERROR=1 -D_REENTRANT=1 -DHAVE_POINTER_LOCALTIME_R=1 -DFORCE_PR_LOG -D_PR_PTHREADS -UHAVE_CVAR_BUILT_ON_SEM nsinstall.c "/usr/include/sys/feature_tests.h", line 332: #error: "Compiler or options invalid for pre-UNIX 03 X/Open applications and pre-2001 POSIX applications" cc: acomp failed for nsinstall.c gmake[1]: *** [nsinstall.o] Error 2 gmake[1]: Leaving directory `/export/home/leon/work/mozilla/firefox/src/build1009/mozilla/nsprpub/config' gmake: *** [export] Error 2
Leon: There may be a conflict between the -xlibmil flag that you use use and the -D_XOPEN_SOURCE=1 -D_XOPEN_SOURCE_EXTENDED=1 feature test macros we added. Could you try configuring Firefox without -xlibmil in your CFLAGS? In any case, I backed out the checkin to define the Solaris feature test macros for SUS (XPG4v2) with extensions. I'll try the other patch (attachment 196874 [details] [diff] [review]) to define a type for an address and use that type in the prototype of the function type _MD_Mmap64.
(In reply to comment #26) > Leon: > > There may be a conflict between the -xlibmil flag that you use > use and the -D_XOPEN_SOURCE=1 -D_XOPEN_SOURCE_EXTENDED=1 feature > test macros we added. Could you try configuring Firefox without > -xlibmil in your CFLAGS? This does not help. Still the same error. > > In any case, I backed out the checkin to define the Solaris feature > test macros for SUS (XPG4v2) with extensions. I'll try the other > patch (attachment 196874 [details] [diff] [review] [edit]) to define a type for an address and use > that type in the prototype of the function type _MD_Mmap64.
Leon, I tried the exact same compiler command you used in comment #25 on my tree, prior to Wan-Teh backing out the change. It compiled just fine, both on Sparc and x86 on Solaris 10 . I was using the Studio 10 compiler. I also tried with Forte 6 update 2 on Sparc. Which compiler were you using that got you the failure ?
I have tried Sun stadio 8 and Sun stadio 9. Both of them have problem to build this. And I have also tried Sun Stadio 10 and forte 6. There is no problem to build this. Since there are some problems when build firefox 1.0.x using Sun Stadio 10. I am always using Sun Stadio 8 to build it. Maybe I should use Sun Stadio 10 to build firefox 1.5 later. (In reply to comment #28) > Leon, > > I tried the exact same compiler command you used in comment #25 on my tree, > prior to Wan-Teh backing out the change. It compiled just fine, both on Sparc > and x86 on Solaris 10 . I was using the Studio 10 compiler. I also tried with > Forte 6 update 2 on Sparc. > > Which compiler were you using that got you the failure ? >
Severity: normal → minor
Target Milestone: --- → 4.7
Priority: P1 → P3
Reassigning to Nelson who is Sun's main NSPR contact now. This is a minor bug . Unfortunately, we were not able to find a single combination of macros that works accross all of Sun's compilers and platforms to resolve these compiler warnings. This would need to be passed to a Solaris expert, if it needs to be resolved. I'm not sure if it does - this is really an aesthetic thing - warnings show up in build logs when looking for warnings and errors, and that makes us look bad since these warnings are legit.
Assignee: bugzilla → nelson
Status: ASSIGNED → NEW
QA Contact: wtchang → nspr
Reassigning to Alexei, since I am no longer at Sun and no longer have access to Sun systems with which to test any possible fixes.
Assignee: nelson → alexei.volkov.bugs
Blocks: buildwarning
Whiteboard: [build_warning]
Severity: minor → S4

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: alvolkov.bgs → nobody
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: