Open
Bug 263952
Opened 21 years ago
Updated 2 years ago
mmap64, shm_open and shm_unlink compiler warnings
Categories
(NSPR :: NSPR, defect, P3)
Tracking
(Not tracked)
NEW
4.7
People
(Reporter: julien.pierre, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [build_warning])
Attachments
(2 files, 2 obsolete files)
2.03 KB,
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
1.43 KB,
patch
|
Details | Diff | Splinter Review |
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* .
Reporter | ||
Updated•21 years ago
|
Assignee: wchang0222 → julien.pierre.bugs
Priority: -- → P3
Updated•21 years ago
|
OS: SunOS → Solaris
Comment 1•21 years ago
|
||
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
Reporter | ||
Comment 2•21 years ago
|
||
Wan-Teh,
Thank you ! Those defines worked. I did ask somebody in the Solaris group, but
you answered earlier ;).
Reporter | ||
Comment 3•21 years ago
|
||
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
Reporter | ||
Comment 4•21 years ago
|
||
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
Reporter | ||
Comment 5•20 years ago
|
||
Gets rid of warnings on Solaris for mmap64, shm_open and shm_unlink
Tested on x86 / AMD64 / sparcv8 / sparcv9
Attachment #193773 -
Flags: review?(wtchang)
Reporter | ||
Updated•20 years ago
|
Target Milestone: 4.6 → 4.6.1
Comment 6•20 years ago
|
||
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-
Reporter | ||
Comment 7•20 years ago
|
||
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.
Reporter | ||
Comment 8•20 years ago
|
||
Attachment #193773 -
Attachment is obsolete: true
Attachment #193776 -
Flags: review?(wtchang)
Updated•20 years ago
|
Attachment #193776 -
Flags: review?(wtchang) → review+
Reporter | ||
Comment 9•20 years ago
|
||
Thanks for the quick review, Wan-Teh. Could you please check the patch in to the
trunk and mark the bug fixed ?
Comment 10•20 years ago
|
||
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-
Reporter | ||
Comment 11•20 years ago
|
||
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)
Comment 12•20 years ago
|
||
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.
Reporter | ||
Comment 13•20 years ago
|
||
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 14•20 years ago
|
||
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+
Reporter | ||
Comment 15•20 years ago
|
||
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 16•20 years ago
|
||
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.
Reporter | ||
Comment 17•20 years ago
|
||
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'
Comment 18•20 years ago
|
||
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?
Updated•20 years ago
|
Target Milestone: 4.6.1 → 4.6.3
Comment 19•20 years ago
|
||
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
Comment 20•20 years ago
|
||
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.
Reporter | ||
Comment 21•20 years ago
|
||
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 → ---
Comment 22•20 years ago
|
||
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.
Comment 23•20 years ago
|
||
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?
Reporter | ||
Comment 24•20 years ago
|
||
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.
Comment 25•20 years ago
|
||
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
Comment 26•20 years ago
|
||
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.
Comment 27•20 years ago
|
||
(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.
Reporter | ||
Comment 28•20 years ago
|
||
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 ?
Comment 29•20 years ago
|
||
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 ?
>
Reporter | ||
Updated•19 years ago
|
Severity: normal → minor
Reporter | ||
Updated•19 years ago
|
Target Milestone: --- → 4.7
Reporter | ||
Updated•19 years ago
|
Priority: P1 → P3
Reporter | ||
Comment 30•19 years ago
|
||
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
Updated•19 years ago
|
QA Contact: wtchang → nspr
Comment 31•15 years ago
|
||
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
Updated•14 years ago
|
Blocks: buildwarning
Whiteboard: [build_warning]
Updated•3 years ago
|
Severity: minor → S4
Comment 32•2 years ago
|
||
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.
Description
•