Merge 32 and 64 bit versions of prcpucfg.h on Solaris

RESOLVED FIXED in 4.5.1

Status

defect
P1
blocker
RESOLVED FIXED
15 years ago
13 years ago

People

(Reporter: christophe.ravel.bugs, Assigned: christophe.ravel.bugs)

Tracking

4.5.1
Sun
Solaris

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Assignee

Description

15 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913

Currently, prcpucfg.h is different for 32 and 64 bit Solaris.

32: prcpucfg.h = md/_solaris32.cfg
64: prcpucfg.h = md/_solaris64.cfg

The expected behaviour is to have only one version (md/_solaris.cfg) and use the
flag _LP64 to select the right definitions.

_LP64 is the flag used by all the Solaris headers included by NSPR.


Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Actual Results:  
:-(

Expected Results:  
:-)
Assignee

Updated

15 years ago
OS: other → Solaris
Hardware: Other → Sun
Version: other → 4.5
Assignee

Comment 1

15 years ago
New file _solaris.cfg = merge of _solaris32.cfg and _solaris64.cfg

Updated

15 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

15 years ago
Assignee: wchang0222 → christophe.ravel.bugs
Assignee

Comment 2

15 years ago
MDCPUCFG_H=_solaris.cfg
Assignee

Comment 3

15 years ago
MDCPUCFG_H=_solaris.cfg
Assignee

Updated

15 years ago
Attachment #162897 - Attachment description: mozilla/nsprpub/configure.in → patch for mozilla/nsprpub/configure.in
Assignee

Updated

15 years ago
Attachment #162895 - Attachment description: mozilla/nsprpub/pr/include/md/_solaris.cfg → new file mozilla/nsprpub/pr/include/md/_solaris.cfg

Comment 4

15 years ago
Thanks, Christophe. Please also create patches for the tip of NSPR in addition
to NSPR_4_5_BRANCH.
Keywords: sun-orion3
Assignee

Comment 5

15 years ago
remove files:
mozilla/nsprpub/pr/include/md/_solaris32.cfg
mozilla/nsprpub/pr/include/md/_solaris64.cfg
Keywords: sun-orion3
Assignee

Comment 6

15 years ago
New file _solaris.cfg = merge of _solaris32.cfg and _solaris64.cfg

For Tip. Only Copyright differences with the NSPR_4_5_BRANCH version.
Assignee

Comment 7

15 years ago
Comment on attachment 162895 [details]
new file mozilla/nsprpub/pr/include/md/_solaris.cfg

For NSPR_4_5_BRANCH
Assignee

Comment 8

15 years ago
Comment on attachment 162896 [details] [diff] [review]
patch for mozilla/nsprpub/configure

For both tip and NSPR_4_5_BRANCH
Assignee

Comment 9

15 years ago
Comment on attachment 162897 [details] [diff] [review]
patch for mozilla/nsprpub/configure.in

For both tip and NSPR_4_5_BRANCH
Assignee

Updated

15 years ago
Attachment #162895 - Flags: superreview?(wchang0222)
Attachment #162895 - Flags: review?(julien.pierre.bugs)
Assignee

Updated

15 years ago
Attachment #162896 - Flags: superreview?(wchang0222)
Attachment #162896 - Flags: review?(julien.pierre.bugs)
Assignee

Updated

15 years ago
Attachment #162897 - Flags: superreview?(wchang0222)
Attachment #162897 - Flags: review?(julien.pierre.bugs)
Assignee

Updated

15 years ago
Attachment #162901 - Flags: superreview?(wchang0222)
Attachment #162901 - Flags: review?(julien.pierre.bugs)

Comment 10

15 years ago
Is it causing problems to have 32 and 64 bit versions of
prcpucfg.h on Solaris?

Comment 11

15 years ago
In a word, yes.

A Solaris component depends on NSPR/NSS, and builds both 32 and 64 bits, on
Solaris and x86 architectures, but we were only delivering a single set of
header files, the 64 bit header for Sparc, and the 32 bit header for x86.

The error went unnoticed for Sparc 32 bit and 64 bits as apparently the 64-bit
header file did not break the 32-bit Sparc compilation, although I can't imagine
how the resulting binaries ran (they probably didn't, but Solaris defaults to
booting 64 bit).

When I added 64-bit AMD support, the 32-bit header we used to package was
replaced with the 64-bit header, which broke the 32-bit x86 compilation .

One possible solution that I was advocating was to ship two sets of include
directories, and let the Solaris component choose which one to use accordingly,
but this went against the common usage in Solaris, so in the end it was
preferred to ship just one header that would work for all 4 cases (Sparc/x86,
32/64 bits). I must say it wasn't very logical to have one header for 32-bit
Sparc and 32-bit x86, and another header for 64-bit Sparc and 64-bit AMD.
Historic NSPR usage would probably dictate to have 4 files I suppose ...

Updated

15 years ago
Priority: -- → P1
Target Milestone: --- → 4.5.1

Comment 12

15 years ago
I reviewed and checked in the patch, added the new file,
and removed the two files on NSPR tip and NSPR_4_5_BRANCH.

I regenerated configure using autoconf as opposed to
modifying it directly.

I made some minor white space and comment changes to
_solaris.cfg and moved the definition of the PR_AF_INET6
macro outside the ifdef _LP64.
Priority: P1 → --
Target Milestone: 4.5.1 → ---

Updated

15 years ago
Priority: -- → P1
Target Milestone: --- → 4.5.1

Comment 13

15 years ago
Wan-Teh,

Not so quick :-(

It doesn't appear that _LP64 is properly defined by the 64-bit compilers, but
only in the Solaris build rather. We are looking for an alternate 32/64 bit
differentiator .

Comment 14

15 years ago
Turns out there isn't a generic compiler define for 32 / 64 bit check. _LP64 is
only defined after one includes a Solaris header file.

Christophe, I believe the defines we need to check for are :

__sparc and __sparcv9 : 64 bit sparc
__sparc and __sparcv8 : 32 bit sparc

__x86_64 or __amd_64 : 64-bit AMD
__i386 or i386 : 32-bit x86

Be careful with the logic in the x86 case, as gcc defines both __x86_64 and
__i386 in 64-bit mode.

The above defines should work with either gcc or studio in the 4 different
platform cases. The only case I didn't test was gcc with 64-bit Solaris, as the
old version of gcc I have on sparc only does 32 bit.

Updated

15 years ago
Attachment #162896 - Flags: review?(julien.pierre.bugs) → review+

Updated

15 years ago
Attachment #162897 - Flags: review?(julien.pierre.bugs) → review+

Updated

15 years ago
Attachment #162895 - Flags: review?(julien.pierre.bugs) → review-

Updated

15 years ago
Attachment #162901 - Flags: review?(julien.pierre.bugs) → review-

Comment 15

15 years ago
I tested 12 variants of Solaris NSPR builds successfully with this update.
Attachment #162895 - Attachment is obsolete: true

Updated

15 years ago
Attachment #162957 - Flags: review?(christophe.ravel.bugs)
Assignee

Comment 16

15 years ago
Comment on attachment 162957 [details] [diff] [review]
updated _solaris.cfg for NSPR_4_5_BRANCH

Julien,

You could put
#define PR_AF_INET6 26	/* same as AF_INET6 */
outside the 32/64 bit area as suggested by WTC.
Otherwise OK.
Attachment #162957 - Flags: review?(christophe.ravel.bugs) → review+
Assignee

Comment 17

15 years ago
Comment on attachment 162957 [details] [diff] [review]
updated _solaris.cfg for NSPR_4_5_BRANCH

Checked in NSPR_4_5_BRANCH with #define PR_AF_INET6 26 outside the IS_64 if and
else areas.

Updated

15 years ago
Attachment #162895 - Flags: superreview?(wchang0222)

Updated

15 years ago
Attachment #162896 - Flags: superreview?(wchang0222)

Updated

15 years ago
Attachment #162901 - Flags: superreview?(wchang0222)

Updated

15 years ago
Attachment #162897 - Flags: superreview?(wchang0222)

Comment 18

15 years ago
Christophe, thanks for the review and branch check-in.

Wan-Teh, could you please check the equivalent _solaris.cfg (it should differ
from the branch only by license and your comment changes) to the tip ?

Thanks.

Comment 19

15 years ago
I checked in the patch for _solaris.cfg on the
NSPR trunk (NSPR 4.6) and NSPRPUB_PRE_4_2_CLIENT_BRANCH
(post Mozilla 1.8 Alpha 4).

Julien, could you email me the <sys/isa_defs.h> header
from Solaris 10?
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED

Comment 20

15 years ago
Wan-Teh,

Thanks for checking in this patch. I have sent you the header file.

Updated

15 years ago
Keywords: sun-orion3

Comment 21

15 years ago
Julien, the _solaris.cfg I checked in on the NSPR
trunk is equivalent to but slightly different from
the _solaris.cfg you checked in on the NSPR_4_5_BRANCH.
You need to log in before you can comment on or make changes to this bug.