216021 - chekesp error if i link against a msvcrt that is built with strict calling rules

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Lukas Ramach]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/actual sourcecode

If i build mozilla and link it with my own msvcrt which i built under the strict
rules. I get a lot of chekesp errors on startup of mozilla! I tracked them down
to the file primpl.h which is in the directory nsprpub/pr/include/private .
in line 1010 there is tis function:
extern PRStatus _PR_MD_CREATE_THREAD(
                        PRThread *thread,
                        void (*start) (void *),
                        PRThreadPriority priority,
                        PRThreadScope scope,
                        PRThreadState state,
                        PRUint32 stackSize);
now i added a platform specific define which explicit sets the *start pointer on
the __stdcall calling convention for winnt/2000, and the checkesp errors were
gone! so here is my fixed example of this function.
#if WINNT == 1
#define MY_CALL_PREFIX __stdcall
#else
#define MY_CALL_PREFIX
#endif

extern PRStatus _PR_MD_CREATE_THREAD(
                        PRThread *thread,
                        void (MY_CALL_PREFIX *start) (void *),
                        PRThreadPriority priority,
                        PRThreadScope scope,
                        PRThreadState state,
                        PRUint32 stackSize);

Reproducible: Always

Steps to Reproduce:
1.build an msvcrt with the -GZ option in windows
2link mozilla with the new msvcrt
3.start mozilla


Actual Results:  
Click away a lot of checkesp error...

Expected Results:  
do this little change in primpl.h please!

i don't know if this calling convetion efects any other platforms, so check it
please.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Thank you for the bug report and patch.  We have been
relying on an ugly type cast in
mozilla/nsprpub/pr/src/md/windows/w95thred.c.  Apparently
that doesn't work with your custom msvcrt.  I'd like to
suggest a different patch.  Could you test this patch and
see if it works for you?

Comment 2

[Wan-Teh Chang]

If I add __stdcall in front of "* start" in the declaration
of _PR_MD_CREATE_THREAD in primpl.h, I get the following
compilation error:

g:/nspr-tip/win95.dbg/pr/src/threads/combined/../../../../../mozilla/nsprpub/pr/
src/threads/combined/pruthr.c(1090) : error C2152: 'function' : pointers to func
tions with different attributes
g:/nspr-tip/win95.dbg/pr/src/threads/combined/../../../../../mozilla/nsprpub/pr/
src/threads/combined/pruthr.c(1090) : warning C4024: '_PR_MD_CREATE_THREAD' : di
fferent types for formal and actual parameter 2

Comment 3

[Wan-Teh Chang]

Attachment: Proposed patch v1.1

Please try this patch instead.

Comment 4

[Wan-Teh Chang]

Attachment: Proposed patch v1.2

Patch v1.1 has a syntax error.	Sorry about that.
Please try this patch.

Comment 5

[Lukas Ramach]

thanks for your fast reply
i applied your patch wich my sourcecode 
i was only able to compile it if i kept my change in primpl.h(the stdcall)
otherwise i hav the same error you got with my change only:
src/threads/combined/pruthr.c(1090) : error C2152: 'function' : pointers to func
but when i started mozilla after the build i got a debug assertion in my run
time which i was not able to track down yet (something with the heapallocation
goes to nowhere)
on thing i forgot to mention is that i build the nspr from within mozilla but
changed the platform from win95 to winnt becausde i need the winntthreads with
my application, but i got the checkesp with both .
I'm not sure what the problem is anymore.
i hope you got any additonal ideas...

Comment 6

[Wan-Teh Chang]

Did you change pruthr.c, too?  I am confused by your result.
I suspect that you also added __stdcall to _PR_NativeRunThread
in pruthr.c.

Comment 7

[Wan-Teh Chang]

The debug assertion regarding heapallocation most likely
means that there are two instances of msvcrt (maybe the
standard msvcrt and your own msvcrt) and memory allocated
by one msvcrt is freed by a different msvcrt.  The best
solution is to make sure that everything in Mozilla uses
the same msvcrt (probably your own).  The standard msvcrt
may be called msvcrt.dll or msvcrtd.dll (note the extra d)
depending on whether you compile with /MD or /MDd.

Comment 8

[Lukas Ramach]

thanks for the hint with pruthr.c
this solved my problems
it works fine now!

Comment 9

[Lukas Ramach]

patch 1.2 resolves the error. It would be great to add it to the mozilla 1.6
since it is not included in 1.5.

Comment 10

[Wan-Teh Chang]

Thank you for verifying that the fix works.  I am
reopening this bug because I haven't checked in the
fix yet.

Comment 11

[Wan-Teh Chang]

Comment on attachment 129776 [details] [diff] [review]
Proposed patch v1.2

Darin, could you review and check in this patch?

The problem is that the prototypes of the thread
start routines for PR_CreateThread and Windows'
_beginthreadex are different.  We used to use a
typecast to cast one prototype to another.  This
patch does the "impedance matching" correctly.

Comment 12

[Darin Fisher]

Comment on attachment 129776 [details] [diff] [review]
Proposed patch v1.2

ok, i think i grok this patch.	to summarize, beginthreadex expects a function
that uses the __stdcall calling convention.  PR_CALLBACK does not use __stdcall
under WIN32.  to deal with this, we currently cast to __stdcall.  i would have
expected trouble from this, but apparently it hasn't been an issue.  maybe the
caller of the thread function doesn't access anything else on the stack after
the thread function returns.  that seems reasonable.  in fact, a quick
inspection of THREADEX.C from the MSVC CRT source code reveals that this is
indeed the case.

that said, adding this thunk is definitely a good idea.  it's too bad NSPR
doesn't use __stdcall in its callback methods on WIN32! ;-)

Comment 13

[Darin Fisher]

fixed on NSPR trunk and NSPRPUB_PRE_4_2_CLIENT_BRANCH

i touched prdepend.h since a depend build was failing in my local tree (NSPR
compiled but crashed on startup).

Comment 14

[Wan-Teh Chang]

*** Bug 230137 has been marked as a duplicate of this bug. ***