Closed Bug 732124 Opened 12 years ago Closed 12 years ago

Build errors with VC11 Beta - mixing MTd libs with MDd exes fail to link

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla15

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(1 file, 4 obsolete files)

Typical error:

xpcomglue_s.lib(nsISupportsImpl.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MTd_StaticDebug' doesn't match value 'MDd_DynamicDebug' in xpcshell.obj

I've managed ot hack my way around this by commenting out various

USE_STATIC_LIBS = 1

declarations in make files. I'll post a work around patch. I'm not 100% on why we do this so I'm not sure how to go about fixing the problem the right way.

For example, in xpcom/glue we have:

# Pretend we're statically linking the CRT, even though we might not be: this
# avoids "msvcrp" and assembly dependencies from creeping into the directives
# for this library on Windows.
USE_STATIC_LIBS = 1
Attached patch work around (obsolete) — Splinter Review
Also, in browsercomps, AFAICT, MOZ_MEMORY isn't used in our build.
Comment on attachment 602055 [details] [diff] [review]
work around

The ultimate goal here was to make xpcomglue_s.lib capable of being linked using either the static CRT or the dynamic CRT. That used to work, I'm guessing from this error message that it doesn't work any more?

The basic goal is that the standalone glue (i.e. what gets linked into "firefox" and probably also the WebRT launcher) shouldn't have to carry around msvcrt.dll. But we want to use that for the main runtime.
Blocks: 732518
Blocks: 737975
Attached file example failure - xpshell (obsolete) —
We can fix this one of two ways - link the static runtime into affected exes (for example, adding USE_STATIC_LIBS = 1 to the xpshell makefile gets past the attached failure), or ifdef the USE_STATIC_LIBS = 1 out when building using the new dev kit. I'm not sure what effect that will have on xulrunner builds though, but it solves the problem for fx/fennec. I can test build xul runner to see.

Note you don't have to remove the ifdef _MSC_VER gunk that landed in bug 334528, just USE_STATIC_LIBS.
This affects win7 builds as well.
OS: Windows 8 → Windows 7
Summary: Build errors with VC11 Beta on Win8 - mixing MTd libs with MDd exes fail to link → Build errors with VC11 Beta - mixing MTd libs with MDd exes fail to link
The error message is coming from a pragma directive [http://msdn.microsoft.com/en-us/library/ee956429.aspx].  The directive is included by the compiler and its value is based on whether the "/MD[d]" or the "/MT[d]" compiler option is present.  Note that there is no way to specify "neither," as the default is equivalent to "/MT" [http://msdn.microsoft.com/en-us/library/abx4dbyh%28v=vs.110%29.aspx - "If you link your program from the command line without a compiler option that specifies a C run-time library, the linker will use LIBCMT.LIB."]

As Jim points out, the right way to fix this is to match static/dynamic CRT usage in objects that get linked together.

From comment #3, it sounds like we want our various "stubs" (firefox.exe, webapprt.exe, probably xulrunner?) to link with the static CRT, but that we want our "main runtime" (which I believe is mostly xul.dll) to link with the dynamic CRT.  These also have to link against xpcomglue.  I believe we can accomplish our goals if we build two versions of xpcomglue; one using the dynamic CRT ("/MD[d]") and one using the static CRT ("/MT[d]").  For each module we build, we would link against the matching xpcomglue, and be careful about passing CRT objects between the various stubs and the runtime [http://msdn.microsoft.com/en-us/library/ms235460%28v=vs.110%29.aspx].

I think we could have done this instead of the "xpcomglue_s.lib capable of being linked using either the static CRT or the dynamic CRT" approach, but we didn't, so I suspect I'm missing or misunderstanding something here.  Let me know if I'm way off base :)
The stubs either do or should link with the static CRT. They also use independent linkage and link with xpcomglue.lib (built from xpcom/glue/standalone). So as far as I can see we only ever need to build xpcomglue.lib against the static CRT.

xul.dll is not involved in this issue because it includes its own copy of the glue code (built from xpcom/build).

Non-libxul code, including browsercomps.dll in our build as well as binary components in extensions, use the dependent glue (xpcomglue_s.lib). browsercomps.dll wants to use the dynamic CRT. Most extensions should however use a static CRT, unless they know that they are compiled against the exact same CRT as Firefox is. So to solve this correctly you might need two copies of xpcomglue_s.

However, can we just use a linker flag or #define or something to suppress_mismatch checking for these libraries?
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> The stubs either do or should link with the static CRT. They also use
> independent linkage and link with xpcomglue.lib (built from
> xpcom/glue/standalone). So as far as I can see we only ever need to build
> xpcomglue.lib against the static CRT.
>
> xul.dll is not involved in this issue because it includes its own copy of
> the glue code (built from xpcom/build).
> 
> Non-libxul code, including browsercomps.dll in our build as well as binary
> components in extensions, use the dependent glue (xpcomglue_s.lib).
> browsercomps.dll wants to use the dynamic CRT. Most extensions should
> however use a static CRT, unless they know that they are compiled against
> the exact same CRT as Firefox is. So to solve this correctly you might need
> two copies of xpcomglue_s.
> 

I didn't realize that there is an "xpcomglue" and a "dependent glue (xpcomglue_s)."  In my previous comment I was talking exclusively about xpcomglue_s (even when I said "xpcomglue").  I think we're in agreement here; two `xpcomglue_s.lib`s would need to be generated (one MD and one MT) and consumers would link to one or the other.

> However, can we just use a linker flag or #define or something to
> suppress_mismatch checking for these libraries?

I haven't found anything yet, but I imagine that this should be possible.  This would be the quickest and easiest workaround.
Assignee: nobody → tabraldes
Attached patch build fix (obsolete) — Splinter Review
I think this is headed in the right direction - build extra copies of the glue libs and link one set statically and one set dynamically. Then touch up various configs to use the right library. Since dynamic linking is the most prevalent, I made the existing libs dynamic and added static libs.

With these changes fx build. I haven't worked on any other projects. I also haven't tested a jemalloc build yet.

One thing I'm not sure of - the components in browser comps were all static but the dll was dynamic, so I switch the underlying components to dynamic. Not sure if that's right. Also the unichar lib was the same way, so I did the same thing there.
Attachment #608418 - Attachment is obsolete: true
Attachment #621150 - Flags: feedback?(benjamin)
Attachment #602055 - Attachment is obsolete: true
Minor update - jemalloc works. I've also pushed this to try - 

https://tbpl.mozilla.org/?noignore=0&tree=Try&rev=0ae4d32dd809
I think it's safe to say you've taken this bug, Jim :)
Assignee: tabraldes → jmathies
Comment on attachment 621150 [details] [diff] [review]
build fix

The reason all the browser/components components linked against the CRT statically is because of the manifests for the CRT: we ship the CRT in the main Firefox directory, and the embedded manifest said to look for the CRT in the same directory as the binary (because the only other option is to look in system locations, but we can't rely on the CRT being in any system locations). But because the browser components are in a subdirectory, Windows wouldn't find the CRT assembly.

That may have changed with MSVC2010, which I've heard doesn't use an assembly any more. If so, then this is probably fine. But we'll need to test this carefully on a winXP system without MSVC installed, which is where this problem shows up.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #13)
> Comment on attachment 621150 [details] [diff] [review]
> build fix
> 
> The reason all the browser/components components linked against the CRT
> statically is because of the manifests for the CRT: we ship the CRT in the
> main Firefox directory, and the embedded manifest said to look for the CRT
> in the same directory as the binary (because the only other option is to
> look in system locations, but we can't rely on the CRT being in any system
> locations). But because the browser components are in a subdirectory,
> Windows wouldn't find the CRT assembly.
> 
> That may have changed with MSVC2010, which I've heard doesn't use an
> assembly any more. If so, then this is probably fine. But we'll need to test
> this carefully on a winXP system without MSVC installed, which is where this
> problem shows up.

Not a problem then, I switched to a dynamic link because I wasn't sure why we set it up this way. I can just as easily flip the dll back to static.
ted was the expert about manifests being awful. Ted, are the days of manifests behind us?
It doesn't look like the VC10 CRT uses manifests, so I'd guess we no longer have to worry about those arcane rules.
Attachment #621150 - Flags: feedback?(benjamin)
Attached patch build fix (obsolete) — Splinter Review
browser comps now using static linking.

Not sure who would be the best reviewer for this, ted r? to you, feel free to change it up. bsmedberg might like to review as well.
Attachment #621150 - Attachment is obsolete: true
Attachment #621700 - Flags: review?(ted.mielczarek)
Attached patch build configSplinter Review
Minor update for this - the new glue configure.in vars I was creating weren't getting set up correctly on unix systems.
Attachment #621700 - Attachment is obsolete: true
Attachment #621936 - Flags: review?(ted.mielczarek)
Attachment #621700 - Flags: review?(ted.mielczarek)
Thanks for putting this patch together, Jim!  It's making my life much easier as I build with VC11.

If you make one more small change, this patch also allows xulrunner to build:
  In xulrunner/stub/Makefile.in, "$(XPCOM_STANDALONE_GLUE_LDOPTS)" needs to be changed to "$(XPCOM_STANDALONE_STATICRUNTIME_GLUE_LDOPTS)"

I imagine that other projects (e.g. Thunderbird) will have to make similar changes?
(In reply to Tim Abraldes from comment #19)
> Thanks for putting this patch together, Jim!  It's making my life much
> easier as I build with VC11.
> 
> If you make one more small change, this patch also allows xulrunner to build:
>   In xulrunner/stub/Makefile.in, "$(XPCOM_STANDALONE_GLUE_LDOPTS)" needs to
> be changed to "$(XPCOM_STANDALONE_STATICRUNTIME_GLUE_LDOPTS)"
> 
> I imagine that other projects (e.g. Thunderbird) will have to make similar
> changes?

I actually did a test tb build and didn't have any problems, so I'm hoping all the apps built off of cc will be ok.
Blocks: 751541
Comment on attachment 621936 [details] [diff] [review]
build config

Review of attachment 621936 [details] [diff] [review]:
-----------------------------------------------------------------

Overall this patch looks fine, I just have a few nits.

My only real concern is that this might break certain build configurations with VC8/VC9, but I *think* it's only likely to be very fringe cases like Firefox-on-Xulrunner, so it's probably not something we should worry about.

::: intl/unicharutil/tests/Makefile.in
@@ +59,4 @@
>  		$(DIST)/lib/$(LIB_PREFIX)unicharutil_external_s.$(LIB_SUFFIX) \
>  		$(XPCOM_LIBS) \
>  		$(NSPR_LIBS) \
>  		$(NULL)

Mind reformatting these to two-space indent while you're here?

::: xpcom/glue/Makefile.in
@@ +147,5 @@
>  		$(LIB_PREFIX)xpcomglue_s.$(LIB_SUFFIX) \
>  		$(NULL)
>  
>  
> +# Create a static lib

Sort of a redundant comment given the variable name...

::: xpcom/glue/standalone/staticruntime/Makefile.in
@@ +1,3 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +# You can obtain one at http://mozilla.org/MPL/2.0/.

Is this file the same as xpcom/glue/standalone/Makefile.in, except for a few fiddly bits? If so, it might be nicer to create a common.mk that both Makefiles could include.

@@ +17,5 @@
> +DIST_INSTALL	= 1
> +
> +LOCAL_INCLUDES	= \
> +		-I$(srcdir)/../../../build \
> +		$(NULL)

I know these are copy-pasted from the other file, but please use a two-space indent.

::: xpcom/glue/staticruntime/Makefile.in
@@ +1,3 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +# You can obtain one at http://mozilla.org/MPL/2.0/.

Same question here re: common bits. It'd suck for people to have to make mechanical changes in both files.

@@ +17,5 @@
> +DIST_INSTALL	= 1
> +
> +LOCAL_INCLUDES	= \
> +		-I$(srcdir)/../../build \
> +		$(NULL)

Also indents.
Attachment #621936 - Flags: review?(ted.mielczarek) → review+
(In reply to Ted Mielczarek [:ted] from comment #21)
> ::: xpcom/glue/standalone/staticruntime/Makefile.in
> @@ +1,3 @@
> > +# This Source Code Form is subject to the terms of the Mozilla Public
> > +# License, v. 2.0. If a copy of the MPL was not distributed with this file,
> > +# You can obtain one at http://mozilla.org/MPL/2.0/.
> 
> Is this file the same as xpcom/glue/standalone/Makefile.in, except for a few
> fiddly bits? If so, it might be nicer to create a common.mk that both
> Makefiles could include.

Took a shot at this but ran into problems due to the src being in the obj dirs. A little bit of data could move to a common.mk file, but it would need to be broken up between a header common and a footer common split by the rules.mk include. Entries like CPPSRCS would still need to be unique AFAICT. Doesn't seem as though there's much to gain in doing this.
Okay, thanks for looking.
(In reply to Jim Mathies [:jimm] from comment #22)
> (In reply to Ted Mielczarek [:ted] from comment #21)
> > ::: xpcom/glue/standalone/staticruntime/Makefile.in
> > @@ +1,3 @@
> > > +# This Source Code Form is subject to the terms of the Mozilla Public
> > > +# License, v. 2.0. If a copy of the MPL was not distributed with this file,
> > > +# You can obtain one at http://mozilla.org/MPL/2.0/.
> > 
> > Is this file the same as xpcom/glue/standalone/Makefile.in, except for a few
> > fiddly bits? If so, it might be nicer to create a common.mk that both
> > Makefiles could include.
> 
> Took a shot at this but ran into problems due to the src being in the obj
> dirs. A little bit of data could move to a common.mk file, but it would need
> to be broken up between a header common and a footer common split by the
> rules.mk include. Entries like CPPSRCS would still need to be unique AFAICT.
> Doesn't seem as though there's much to gain in doing this.

Actually, it's not the src in obj, it's the obj files themselves which need to be linked to different crts.
https://hg.mozilla.org/mozilla-central/rev/925c8302f34a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
I'm trying to interpret comment 8 "Most extensions should however use a static CRT, unless they know that they are compiled against the exact same CRT as Firefox is" in the terms of the landed patch.

What I think that I understand is this:

1) for my binary extensions, where "compiled against the exact same CRT as (Thunderbird)" is false, then I should use xpcomglue_staticruntime_s.lib, and set USE_STATIC_LIBS = 1

2) for Lightning, where "compiled against the exact same CRT as (Thunderbird)" is true, then they should use xpcomglue_s.lib, and NOT set USE_STATIC_LIBS = 1.

Is that correct?

The only symptom that I am currently having is that Lightning will not link currently with Moz15/VS2008. The Lightning team has yet to build successfully under Moz
(oops, hit return before finishing)

... The Lightning team has yet to build successfully under Moz15, but they believe that they will work OK under VS 2010 (with xpcomglue_s.lib, and USE_STATIC_LIBS = 1.) But that seems incorrect to me based on my understanding of this bug.

Could one of you confirm or correct my understanding here?
It would be better to diagnose these build issues elsewhere.  Please join #windev on IRC or perhaps try one of the newsgroups.

There are 4 .lib files that you can possibly link against. I will try to list the requirements for linking against each one here

  xpcomglue.lib:
    At runtime, the XPCOM glue DLL must be accessible by your module
    Your other .lib files must be built with /MD
    Your module must be compiled against the same CRT as the EXE that will load your module
  xpcomglue_staticruntime.lib:
    At runtime, the XPCOM glue DLL must be accessible by your module
    Your other .lib files must be built with /MT (USE_STATIC_LIBS)
  xpcomglue_s.lib
    Your other .lib files must be built with /MD
    Your module must be compiled against the same CRT as the EXE that will load your module
  xpcomglue_staticruntime_s.lib
    Your other .lib files must be built with /MT (USE_STATIC_LIBS)

Jim or Benjamin can jump in if I've lead you astray, but again, this discussion is better suited to IRC or a newsgroup.
No longer blocks: 737975
Sorry, my English is very very poor.

I build Seamonkey 2.12 x86 with VC11 RTM @ Win7 x64.

VC11 comes with the Windows SDK 8.0A, and it is not usable.
Because there is no more mapi header-file.

So, I edit start-msvc11.bat to use the Windows SDK 7.0A comes with VC10.

And now I got an error message and cannot finished compiler:
============
unicharutil_external_s.lib(nsUnicharUtils.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in nsSuiteModule.obj
unicharutil_external_s.lib(nsUnicodeProperties.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in nsSuiteModule.obj
   Creating library fake.lib and object fake.exp
suite.dll : fatal error LNK1319: 2 mismatches detected
============

Any idea?    Thanks very much.
Just for reference: SeaMonkey build problem from Comment 30 is handled in Bug 829850
Blocks: 829850
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.