74459 - NSS autoconf build under OS/2

Status: VERIFIED WONTFIX

(SeaMonkey :: Build Config, defect, P3)

Description

[jhp (no longer active)]

Changes necessary to get NSS autoconf building under OS/2.

Comment 1

[jhp (no longer active)]

Attachment: first patch

Comment 2

[jhp (no longer active)]

Added patch with mostly os2 changes, with some of the changes based on 
72693.  Except...

NOTES:
1) Moved NSINSTALL to configure/autoconf.mk, since we use a main 
nsinstall rather than supporting multiple one across components.
2) Rearranged 'compiler-based dependencies' check to be more like that 
in mozilla/configure.in -- COMPILER_DEPENDS was always getting defined 
for me using old check.

Comment 3

[jhp (no longer active)]

Attachment: patch updated with bryner's suggestion

Comment 4

[jhp (no longer active)]

Updated the patch to include bryner's fix for the --disable-md check 
in configure.in.

Comment 5

[Mike Kaply [:mkaply]]

Attachment: Just some build stuff and the callback enablement

Comment 6

[Brian Ryner (not reading)]

This bug should probably go to me, since it's the autoconf branch.

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Final set of diffs for NSS autoconf

Comment 8

[Mike Kaply [:mkaply]]

I have these diffs ready to go on NSS_CLIENT_BRANCH. Just 
say the word.

Comment 9

[Brian Ryner (not reading)]

I hate to impose more administrative overhead, but you really need to separate
the NSS patches (security/nss) from the PSM2 patches (security/manager) here. 
Please file a bug on building PSM2 on OS/2 and mark it dependent on this one,
and attach just the PSM2 diffs there.  The PSM2 directory is _not_ branched on
the autoconf branch, so those changes will need a full review from the PSM team.

Thanks.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I don't see why you need to make the callbacks in nsNSSComponent and
nsPKCS12Blob non-members of the classes.  There are existing callback
functions in the tree that are static members of classes.  See
http://lxr.mozilla.org/seamonkey/source/rdf/chrome/src/nsChromeProtocolHandler.cpp
for an example.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also, should the changes to NSS that aren't in the build system find their way
back to the NSS trunk as well?

Comment 12

[Brian Ryner (not reading)]

Getting the equivalent NSS changes in on the trunk/NSS_CLIENT_BRANCH will be a
prerequisite for the PSM changes as well, since we don't have any definite plans
for turning on the autoconf branch yet.

Comment 13

[Mike Kaply [:mkaply]]

Anytime you see a class member function (even if it static) passed as a callback 
to a function that expects an extern "C", it is wrong.

These APIs and their typedefs are inside extern "C" so any function passed to 
them MUST be extern "C". Take a look at the build log for SunOS on the ports 
tinderbox page and do a search on extern "C".

I have a list of 1600 instances of this particular problem in Mozilla and I am 
going to work to get them all fixed.

This is essential for real portable code.

As far as separating the changes, I am getting confused as to what is PSM 2.0 
and what is not. Should I only put build specific changes in here?

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK... I read up on the relevant parts of the C++ standard (3.5 and 7.5).  What
makes the pref API callback type extern "C"?  And what makes the linkage on your
static functions "C" rather than "C++" linkage?  Shouldn't you have to declare
them within an |extern "C"| block?  I'm speaking here only from a quick reading
of the standard, so I could easily be wrong...

Comment 15

[Mike Kaply [:mkaply]]

In the pref case, the code looks like this in prefapi.h:

NSPR_BEGIN_EXTERN_C
.
.
typedef int (*PrefChangedFunc) (const char *, void *);

void PREF_RegisterCallback( const char* domain,
                            PrefChangedFunc callback, void* instance_data );
PrefResult PREF_UnregisterCallback( const char* domain,
                            PrefChangedFunc callback, void* instance_data );
.
.
NSPR_END_EXTERN_C

Note there should probably be a PR_CALLBACK (or come callback) in the typedef.

So this code means that any function that is used as a PrefChangedFunc callback 
has to be defined in an extern "C".

You cannot put extern "C" declarations inside of classes. They have to be moved 
outside of the class and made extern "C", and in most cases you make them 
friends as well because they usually call class functions.

Note by the way that because of the way extern "C" was architected, static and 
extern "C" are mutually exclusive, so the functions end up being externed. This 
is why I made the names look so strange, to guarantee uniqueness.

By the way, SEC_BEGIN_PROTOS and SEC_END_PROTOS correspond to extern "C". In 
this case, I should have probably used the NSPR_EXTERN_C stuff to be more clear.

Comment 16

[Brian Ryner (not reading)]

PSM 2 is everything under security/manager, NSS is everything under
security/coreconf and security/nss.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So there are two definitions of PrefChangedFunc, one in prefapi.h and one in
nsIPref.idl, and only the former is within |extern "C"| and only the latter has
PR_CALLBACK.

Are |static| and |extern "C"| mutually exclusive?  I don't think they should
be according to the C++ spec, and gcc assembler output agrees with that.  (I
think a |static| function in an |extern "C"| block should have internal linkage
with C calling conventions.)  When I looked at gcc3 assembler output for a test
with 4 functions:

extern "C" { static int static_function_in_extern_C() { return 0; } }
static int static_function() { return 0; }
extern "C" { int function_in_extern_C() { return 0; } }
int function() { return 0; }

the third and fourth had ".globl <function-name>" lines (which I assume means
they have external linkage) and the second and fourth had their names mangled
(which seems to be the only difference in the C++ calling convention).

You're right that C calling convention callbacks can't be inside classes -- I
learned that from C++ section 7.5 this morning.

Comment 18

[Mike Kaply [:mkaply]]

You just have to love stuff like that.

I'll have to think about which one is correct.

As far as static AND extern "C", while logically it should work, and it does 
compile in gcc, it is not correct. 

Unfortunately, the C/C++ architects here decided to overload the meaning of 
extern here and so the concept of static within an extern block doesn't work.

I'd be interested to see what other compiler (particular HPUX and SunOS) do with 
this, but I know if you do this on our compiler, it basically ignores the extern 
"C" and makes them static, creating the same problem.

I'm not sure what the right thing to do is in this case. We probably need to ask 
a C++ guru about that aspect of things.

Incidentally, extern "C" isn't just about calling convention, it also prevents 
name mangling. This is obviously not an issue in the static case.

Comment 19

[cls]

Adding resident c++ guru to cc:.

What's the corresponding bug for getting the OS/2 build working on the NSS trunk
or client tag or whatever?  I want to make sure that we have the bare minimal
(preferably zero) non-build changes on the client branch that are not in the NSS
team's tree.  

Comment 20

[Dmitry Kubov]

EMX updates:

security\coreconf\rules.mk
	@cmd /C "$(FILTER) $(SUB_SHLOBJS) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 1,20,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 21,40,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 41,60,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 61,80,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 81,100,$(SUB_SHLOBJS)) >>$@.def"

security\nss\configure.in
add this define:
    *-*-os2_emx)
        MKSHLIB = $(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -o $@

Comment 21

[jhp (no longer active)]

Attachment: NSS only patch

Comment 22

[jhp (no longer active)]

Removed PSM files to make this bug refer only to NSS autoconf issues.  
The PSM 2.0 files can be found at bug 72693.

Comment 23

[jhp (no longer active)]

Requires prtypes.h change in bug 76896.

Comment 24

[Dmitry Kubov]

Please add this for EMX:

security\nss\configure.in


    *-*-os2_emx)

        MKSHLIB='$(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -o $@'

Comment 25

[jhp (no longer active)]

Attachment: new patch with EMX changes

Comment 26

[Brian Ryner (not reading)]

cls, can you take this one?

Comment 27

[Jon Granrose]

changing to browser/build config so I can triage and it doesn't show up as an
untargetted bug.

Comment 28

[cls]

I think we've given up on the nss autoconf branch for now.  Marking WONTFIX.

Comment 29

[jhp (no longer active)]

verified