This is a placeholder bug to track all the things that need to be fixed to get Mozilla compiling with Microsoft's C++ 7.0 (The C++ compiler that comes with Visual Studio.NET) -dave

Didn't Hyatt & Ben already get this working as a precursor to mantic0re?

I used /GL (optimize across obj boundaries)compiler switch along with /LTCG (link time code generation) linker flag to get a static build It's working but code generation took more than 1 hour (PIII 400), and it's only link time ! It took 650M of memory so be sure you have at least 512MB and enough paging space... problems found: 1) work around xpidl crash by commenting out some code 2) replace #include "ver.h" by #include "winver.h" (ver.h is gone) 3) remove precompiled header option (/YX) in xpconnect makefile.win 4) replace /O1 with its equivalent from MS doc and use /Gf instead of /GF the problem here is that /O1 (and even /O2) now defaults to /GF instead of /Gf It means that string constants are stored in read-only memory, and I know by experience that this causes mozilla to crash on startup... sometimes (several profiles ?) :-(. I got this crash in nsClipboard.cpp and I could have a stack trace if someone is interested. I think that this needs more investigation.

for those who want to check this out, here is the program: #include <stdio.h> #include <tchar.h> int main(int argc, char* argv[]) { const char *test = "a"; char *p = (char *)test; *p = 'b'; // strlen() here to examine the code produced by the compiler for inline functions int l = strlen(p); // strdup here so that the code above is not optimized away return (int)strdup(p) + l; } the same source compiled with the same option (/O1 or /O2) will produce different results with VC70 and VC60: this will crash with VC7.0 and succeed with VC60, just because /GF is now the default instead of /Gf. remark: the code produced by the compiler for inline functions like strlen, memcpy, strcpy, ... has been improved, they are 2 times faster now because rep string instructions aren't used anymore ('repne scasb' for instance is replaced by a simple loop) VC60 /O1 is /Og /Os /Oy /Ob1 /Gs /Gf /Gy VC70 /O1 is /Og /Os /Oy /Ob2 /Gs /GF /Gy like I already said, I experience random crashes with /GF so I would recommend to use /O1 - /GF + /Gf when compiling with VC70. So for VC70 I'd use /Og /Os /Oy /Ob2 /Gs /Gf /Gy explicitly. personally I would add /Oi too :-), even if the code will be (little) bigger please note that /Ob1 (ony expand functions marked 'inline') is now /Ob2 (inline any suitable functions) so this could have a footprint impact

I'd be really interested in some startup/page-load-time comparisons between VC6 and VC7. Is anyone capable and willing to do these?

Bernard, can you attach your patches to make the build go with vc7? It sounds like you've done some good work there, and it'd be a shame to have to redo it.

bug 123743: crash in xpidl.exe(xpidl_write_comment()) when building with VC++7.0

mozilla/config/WIN32 : the code is generated at link time to allow better optimization between all objs linked together /O1 is expanded and /GF is replaced with /Gf to match VC60 behaviour /Oi is added because VC70 now generate good code for compiler intrinsic functions like strlen, strcpy, memcpy, ... mozilla/js/src/xpconnect/src/makefile.win: pch conflicts with new linker options mozilla/xpcom/typelib/xpidl/xpidl_util.c: work around the crash reported in bug 123743 by commenting out the faulty code Comments: 1) you can also build static with this patch 2) once this patch is applied I don't know if you can use VC60 again later (this will depend on how VC60 treat unknown compiler and linker option) 3) this is not appropriate if you want to redistribute the generated library files (*.lib) because only VC70 will understand them 4) 2) and 3) can be solved by removing /GL and /LTCG: options but my understanding is that it will not produce the best code

oops, and this patch is just for nmake builds

preliminary tests at: http://bugzilla.mozilla.org/show_bug.cgi?id=136999#c11 shows that VC7 O1 is a small footprint win over VC6 O1 (current default settings)

Isn't it a *good* thing that VC++.Net puts strings into readonly space? Modifying literal string constants is (almost) as evil as old Fortran programs that could modify literal number constants so that, for instance, PI == E after an ill-behaved function call. Shouldn't these evildoers be hunted down and "fixed"?

I believe they are fixed in XP code, the problem is for Windows only code for instance, I had to fix this: Index: nsClipboard.cpp =================================================================== RCS file: /cvsroot/mozilla/widget/src/windows/nsClipboard.cpp,v retrieving revision 1.77 diff -u -r1.77 nsClipboard.cpp --- nsClipboard.cpp 6 Aug 2002 00:53:00 -0000 1.77 +++ nsClipboard.cpp 9 Aug 2002 14:59:30 -0000 @@ -93,7 +93,8 @@ //------------------------------------------------------------------------- UINT nsClipboard::GetFormat(const char* aMimeStr) { - nsCAutoString mimeStr ( CBufDescriptor(NS_CONST_CAST(char*,aMimeStr), PR_TRUE , PL_strlen(aMimeStr)+1) ); + nsDependentCString mimeStr(aMimeStr); + UINT format = 0; if (mimeStr.Equals(kTextMime)) maybe CBufDescriptor should be removed, see bug 98195, which has the patch

This can't be nobody, we've got to get VC7 working. It looks like the work's basically done, what's the holdup?

Having someone who actually uses MSVC 7 to verify the patch would help. As would a more recent patch against the gmake build system. Not to mention having someone who is actually familiar with the MSVC compiler could verify that all of those funky optimization flags are what we want/need for the entire tree. Did I mention how the random commenting out of a troublesome IDL_tree_to_IDL() call in xpidl_util.c just looks suspicious? Over to dveditz since he actually has MSVC7.

I think this was commented out due to bug 123743. There's lib's in that bug that can be used, so I suspect with these two bugs, the patch could be updated to remove the commenting of the "comment" code.

I think we should try and get Windows building with /GF (I thought we were already doing this on VC++6). We should be able to store string literals in read-only memory, anything that tries to modify them is evil (even though it might happen to work). In this case, someone is casting the const-ness away before constructing the CBufDescriptor. We store the const-ness to determine whether or not to add a null-terminator in nsCAutoString's constructor[0], the writing of this null-terminator causes us to crash. The smallest fix to nsClipboard.cpp is to remove the cast. The better change is the one you have, r=/sr=jag on that. Have you run into any other crashes because of /CF? 0: http://lxr.mozilla.org/seamonkey/source/string/obsolete/nsString.cpp#1266

Comment on attachment 80061 [details] [diff] [review] patch to build mozilla with VC++7.0 sorry my patch is a mess, it tries to address, or is related to several problems: 1) Build mozilla with /GF 2) Remove precompiled header build option for xpconnect because it blocks experiment with /GL 3) Build mozilla with /Oi (inline functions like memcpy, memset, strcpy ,...) 4) Build mozilla with /Ob2 (inline any suitable function not only those marked "inline") all that is *really* needed to build with VC7.0 is VC 7.0 version of libidl and glib (this is bug 123743). Commenting out the code in xpidl_util.c is a workaround until we have the proper libidl and glib version. Strictly speaking, mozilla will build with /O1 and VC7.0 but if you want to avoid potential problems with /GF and use the equivalent to VC6.0 current default settings, you can set --enable-optimize=-Og -Os -Oy -Ob1 -Gs -Gf -Gy in your mozconfig

"Have you run into any other crashes because of /GF?" No, only nsClipboard (and this should be another bug, I think) I use Mozilla built with VC7.0 as my main browser since VC7.0 has been released I also use Mail/News I don't use Composer, Venkman, etc ...

I like your fix to nsClipboard.cpp though. Anyone here wanna r= that so it can be checked in?

I decided to keep the standard names for the VC7 built libs and add support for searching for the libs in an alternate path. GLIB_PREFIX/bin & LIBIDL_PREFIX/bin must be in your path before MOZ_TOOLS/bin or xpidl will still segfault when it uses the dlls from MOZ_TOOLS/bin.

Btw, if you try to build xpidl using VC6 and the VC7 libs, it will link but it won't run. It complains about a missing mspdb70.dll.

Comment on attachment 103134 [details] [diff] [review] Add GLIB_PREFIX & LIBIDL_PREFIX support to win32 r=bryner

Comment on attachment 103134 [details] [diff] [review] Add GLIB_PREFIX & LIBIDL_PREFIX support to win32 a=asa for checkin to 1.2 (on behalf of drivers).

Patch has been checked in. The updated build instructions are at http://www.mozilla.org/build/win32.html .