Closed
Bug 86323
Opened 24 years ago
Closed 23 years ago
mozilla using non-standard zlib.dll conflicts with third party product in Windows platform
Categories
(SeaMonkey :: Build Config, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.3
People
(Reporter: edxu, Assigned: leaf)
Details
(Whiteboard: [needed for embedding])
Attachments
(8 files)
50.46 KB,
application/octet-stream
|
Details | |
131.75 KB,
application/octet-stream
|
Details | |
1.66 KB,
text/plain
|
Details | |
104.30 KB,
application/octet-stream
|
Details | |
5.53 KB,
patch
|
Details | Diff | Splinter Review | |
15.13 KB,
application/octet-stream
|
Details | |
741 bytes,
patch
|
Details | Diff | Splinter Review | |
812 bytes,
patch
|
Details | Diff | Splinter Review |
On Windows platform mozilla/modules/zlib is compiled with __cdecl for exported functions instead of WINAPI or __stdcall as expected in standard zlib.dll. This cause conflicts with third party product such as plug-ins or gecko embedding application that use standard zlib.dll because the non-standard one is loaded in address space and hence the crash. Compiling with ZLIB_DLL and _WINDOWS will not solve problem because mozilla hard-code PR_API types in the zlib source files. As a matter of fact mozilla MUST uses standard zlib so that third party products will not complain.
Reporter | ||
Comment 1•24 years ago
|
||
![]() |
||
Updated•24 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: adamlock → leaf
Component: Embedding: Docshell → Build Config
Comment 4•24 years ago
|
||
leaf/seawood, can you do the r= and sr= on this? until we get this in its causing a big embedding customer a lot of headaches... if this patch looks good it would be good to get it in today.
Updated•24 years ago
|
Whiteboard: [needed for embedding]
Target Milestone: --- → mozilla0.9.2
Assignee | ||
Comment 6•24 years ago
|
||
can you repost the diff as a text file, rather than application/octet-stream, please?
Updated•24 years ago
|
Priority: -- → P2
Assignee | ||
Comment 7•24 years ago
|
||
alright, it's a zip file; patch is barfing for me on a simple diff, so it could be until tomorrow night (when i predict to have another windows machine up and building) before i get a windows build with this patch. What i'm concerned with more is the effect on the unix platforms, which i bet hasn't been tested yet. Someone feel free to correct me before i waste time testing it.
Ugh. So rather than taking these patches, why don't we just import a copy of the the latest release of zlib or the original release that was checked in? I don't know about leaf but outside of the PR_API changes, I have no clue what this relatively huge (184k) patch is doing.
Reporter | ||
Comment 9•24 years ago
|
||
Turn out the gecko version of zlib is two years older than the current version 1.1.3 (which Todd patched from). Version 1.1.3 is the current version and has been out for several years now. If we simply grab the source code of 1.1.3 and drop it into mozilla (and define ZLIB_DLL for Windows), will it work for Unix without using prtypes? If it will then I will go with Christopher on this and not use the patch, which *might* not work for UNIX due to the same problem. As a temparory solution I am using the pre-built version of zlib.dll/zlib.lib for Gecko, of course this will only work for Windows.
Comment 10•24 years ago
|
||
On unix, by default, we use the system installed version of zlib and it appears to work fine.
Updated•24 years ago
|
Summary: mozilla using none standard zlib.dll conflicts with third party product in Windows platform → mozilla using non-standard zlib.dll conflicts with third party product in Windows platform
Comment 11•24 years ago
|
||
Does anyone know if the mac requires us to use the prtypes or can we just plop in the latest zlib release without mods?
Comment 12•24 years ago
|
||
Mac requires some PR_ macro stuff in zlib to manage symbol exports. If we had an alternative way to control exports (e.g. .exp file), then this could go away.
Comment 13•24 years ago
|
||
Ok, I got the zlib 1.1.3 tarball (extracted from a srpm since the main zlib site is down). Dropping the source files into the tree works for the linux build. We already compile with -D_WINDOWS (set in config/WIN32) but I do see the mention of needing -DZLIB_DLL for each project that uses zlib (ugh). http://www.winimage.com/zLibDll/build.html Simon, how much work would it be to provide the alternative method for the mac?
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
You _should_ be able to move modules/zlib out of the way and unzip the attached zip file in modules. It uses pristine zlib 1.1.3 src with the mozilla build files with one exception. I modified zconf.h to automatically define ZLIB_DLL if MOZILLA_CLIENT & XP_WIN32 are set. The zip compiles on windows but doesn't create a .lib library even though nothing in the win32 build files have changed. Help! Simon, I created a zlib.exp file under macbuild using the jpeg one as a template. Any chance you could modify the project file to use it?
Assignee | ||
Comment 17•24 years ago
|
||
i'll track down where the .lib went to, chris; are you, by any chance, building with the static build branch?
Comment 18•24 years ago
|
||
I've checked about 20 times to make sure but the static build stuff is in a separate tree. :)
Assignee | ||
Comment 19•24 years ago
|
||
fair enough, i'll keep looking for other reasons why this might not be working :)
Comment 20•24 years ago
|
||
Interesting but disturbing factiod: When using the current cvs src, if I ifdef out the prtypes.h include from zconf.h & zutil.c & use the non-MOZILLA_CLIENT define of PR_PUBLIC_API, I see the same missing .lib problem. Does win32 have some link-time script that looks for specific PR_PUBLIC_API signatures or something?
Comment 21•24 years ago
|
||
Comment 22•24 years ago
|
||
Copying the zlib.dnt from the nt/ subdir of the zlib-1.1.3 dist into zlib/src and adding DEFFILE=zlib.dnt to zlib/src/makefile.win brings back the .lib . My build got past necko so I'm guessing that it works. I'll let you know if the build actually runs.
Comment 23•24 years ago
|
||
Comment 24•24 years ago
|
||
Comment 25•24 years ago
|
||
So, we're back to pristine 1.1.3 src files. The latest zip (attach 39297) should be unzipped in a current cvs pull of modules/zlib/src . Then apply the patch to make the rest of the tree build. I feel so dirty. I need a shower.
Assignee | ||
Comment 26•24 years ago
|
||
yeah, there are magic things done with linker scripts and .def files, but i didn't know it was based on .h preprocessing *sigh* I'll verify the build and we can probably get this carpooled with dougt tomorrow, assuming we get an a= ;)
Comment 27•24 years ago
|
||
We still need some mac lovin' and a project file that knows to use macbuild/zlib.exp .
Comment 29•24 years ago
|
||
Assignee | ||
Comment 30•24 years ago
|
||
<fume>my system's "patch" has decided that it needs to crash in the middle of applying any diff</fume>. Attempting to test on the verification machine.
Comment 31•24 years ago
|
||
Tried on the mac, and exercised the CRC checking code that I just wrote which uses zlib. It worked. Not an extensive test, but it did build, and didn't break me (two typically rare events :-)
Assignee | ||
Comment 33•24 years ago
|
||
checked into the branch; spinning a verification build. once verified, i'll commit on the trunk
Comment 34•24 years ago
|
||
pdt+ per chofmann (pdt2@netscape.com)
Whiteboard: [needed for embedding] → [PDT+][needed for embedding]
Assignee | ||
Comment 35•24 years ago
|
||
fixed on the branch, will merge to trunk shortly, clearing from 0.9.2 milestone radar.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Comment 36•24 years ago
|
||
This change broke the VMS build. The proto of strerror in zutil.h used to have an #ifndef MOZILLA_CLIENT around it, now it doesn't. Why are we even trying to prototype strerror when the correct thing to do is #include <string.h>??? For now I've fixed it in my tree, but I need to know what flavor of patch you want back. Do you want #ifndef MOZILLA_CLIENT or #ifndef VMS around the strerror proto?
Assignee | ||
Comment 37•24 years ago
|
||
I think we want VMS, since this didn't seem to cause problems for the platforms that embedding is on. I'm not sure the reasoning for zlib's prototyping of strerror, but i'm not willing to futz with how zlib is built on the platforms where embedding was having an issue.
Assignee | ||
Comment 38•24 years ago
|
||
colin, i'll be around for a while today (i'm merging this to the trunk), but if i don't get to review a patch today, i'll be around monday to do so. Anyone cc'd, i think, can give you an r= to checkin to either the 0.9.2 branch or the trunk.
Assignee | ||
Comment 39•24 years ago
|
||
ok, merged to trunk; i'll leave this open until we get vms building again.
Comment 40•24 years ago
|
||
reducing severity and removing PDT+ since this has been fixed on the 0.9.2 branch.
Severity: blocker → normal
Priority: P2 → P3
Whiteboard: [PDT+][needed for embedding] → [needed for embedding]
Comment 41•24 years ago
|
||
Assignee | ||
Comment 43•24 years ago
|
||
can you also include another comment line under the comment line starting with: /* @(#) $Id: zutil.h,v saying /* This file has been modified from the original distribution */ otherwise, sr=leaf for the trunk
Comment 44•24 years ago
|
||
Comment 45•24 years ago
|
||
Added the comment, as requested by Leaf. Is the trunk under driver control at the moment, or am I all clear to check in once it opens?
Comment 47•23 years ago
|
||
marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
Updated•17 years ago
|
Attachment #38860 -
Attachment description: Patch from Todd Brannum: the changes made are primarily in the "makefile.win" to set the correct flags for zlib exports - it's important to use the correct calling convention otherwise the 'return' call in a zlib function could blow up with a stack proble → Patch from Todd Brannum
You need to log in
before you can comment on or make changes to this bug.
Description
•