Closed
Bug 216021
Opened 21 years ago
Closed 21 years ago
chekesp error if i link against a msvcrt that is built with strict calling rules
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
4.5
People
(Reporter: ramach, Assigned: wtc)
References
Details
Attachments
(1 file, 2 obsolete files)
4.60 KB,
patch
|
darin.moz
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•21 years ago
|
||
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?
Assignee | ||
Comment 2•21 years ago
|
||
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
Assignee | ||
Comment 3•21 years ago
|
||
Please try this patch instead.
Attachment #129774 -
Attachment is obsolete: true
Assignee | ||
Comment 4•21 years ago
|
||
Patch v1.1 has a syntax error. Sorry about that. Please try this patch.
Attachment #129775 -
Attachment is obsolete: true
Reporter | ||
Comment 5•21 years ago
|
||
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...
Assignee | ||
Comment 6•21 years ago
|
||
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.
Assignee | ||
Comment 7•21 years ago
|
||
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.
Reporter | ||
Comment 8•21 years ago
|
||
thanks for the hint with pruthr.c this solved my problems it works fine now!
Reporter | ||
Comment 9•21 years ago
|
||
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.
Status: UNCONFIRMED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•21 years ago
|
||
Thank you for verifying that the fix works. I am reopening this bug because I haven't checked in the fix yet.
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Target Milestone: --- → 4.5
Updated•21 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 11•21 years ago
|
||
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.
Attachment #129776 -
Flags: review?(darin)
Comment 12•21 years ago
|
||
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! ;-)
Attachment #129776 -
Flags: review?(darin) → review+
Comment 13•21 years ago
|
||
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).
Status: NEW → RESOLVED
Closed: 21 years ago → 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•21 years ago
|
||
*** Bug 230137 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•