Bas@BAS-LAPTOP ~/Development/mozilla/firefox-rel-layers/js/ctypes/libffi $ find -name *.lib | xargs dumpbin -directives | grep "MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" /DEFAULTLIB:"MSVCRTD" This is pretty bad, it suggests we're compiling files with /MDd. This is bad, partially because we're using less optimized routines for CRT functions. But also because in the MS debug CRTs some structures have different sizes than in the release CRTs, if we build against debug, and link against release, this could cause nasty corruption in theory.

Also, if this is pulling in the debug CRT DLLs, normal users won't actually be able to run our app. This probably isn't a problem in our nightly/release builds, since they --enable-jemalloc, and that does -NODEFAULTLIB:msvcrtd, among other things: http://mxr.mozilla.org/mozilla-central/source/configure.in#6919

(In reply to comment #1) > Also, if this is pulling in the debug CRT DLLs, normal users won't actually be > able to run our app. This probably isn't a problem in our nightly/release > builds, since they --enable-jemalloc, and that does -NODEFAULTLIB:msvcrtd, > among other things: > http://mxr.mozilla.org/mozilla-central/source/configure.in#6919 It's still a problem in the sense that structures created by the libffi library might be expected to have a different structure if they're passed across to other libraries. (although I don't know if that happens)

Right, I meant the "normal users can't run the builds because they're linked to non-distributable debug CRT DLLs" problem. This is still a problem otherwise.

http://mxr.mozilla.org/mozilla-central/source/js/ctypes/libffi/msvcc.sh#72 That's where the (anti-)magic happens. Sounds like this is trivial to fix; someone want to roll a patch and push to try?

Actually, hang on. I'm not sure changing those bits will work. In debug builds, we have to use /MDd, right? And in release, we want to use /MD. The problem is, libffi has no concept of 'debug' and 'release'. It always builds with the gcc flags -g and -O1. So you get opt, with debug symbols. When we translate '-g' in msvcc.sh, we have to go with /MDd, or it breaks. I'm not sure if there are some linker flags to get around this; if not, we'll have to figure out a way to get the debug/release info into msvcc.sh so it can decide, or add a configure switch to libffi.

Can we make libffi only pass -g if you --enable-debug? That should match our behavior here.

Yes, it'll require changing http://mxr.mozilla.org/mozilla-central/source/js/ctypes/libffi/Makefile.am#176, and possibly configure.ac as well. And then regenerating them all with autotools. Thankfully, we should be able to upstream that patch. I've no idea if autotools has a built-in concept of --enable-debug, or if we'd have to burn it into configure.ac ourselves.

We really need to fix this.

OK, I'll take a look today.

Uses -DFFI_DEBUG to instruct msvcc.sh to link against the debug CRT. (I don't think we want to overload the meaning of -g, since it's legitimate that people might want an opt build with symbols.)

This is huge because automake decided to out-of-line some m4/ files present in the source tree. (My automake is v1.11.1, c.f. v1.11 previously used.)

As stated. The part 1 patch doesn't need to go into libffi.patch since I'm pushing it upstream.

I should also note that we already pass --enable-debug when configuring libffi, which defines FFI_DEBUG, so there's no configury change required on our end.

p.p.s. these only require rs=, since they're going upstream.

Comment on attachment 441175 [details] [diff] [review] part 1: split libffi into debug/opt configurations Does this work with Mozilla --enable-debug-symbols? For release builds on Windows you really want OS_CXXFLAGS += -Zi -UDEBUG -DNDEBUG OS_CFLAGS += -Zi -UDEBUG -DNDEBUG OS_LDFLAGS += -DEBUG -OPT:REF (see the MOZ_DEBUG_SYMBOL block in config.mk)

Comment on attachment 441178 [details] [diff] [review] part 2: autoreconf >-%.o: %.S >+.S.o: These local hacks were added to fix pymake, you'll keep them in our tree, right?

Comment on attachment 441179 [details] [diff] [review] part 3: apply libffi.patch oh, heh

Comment on attachment 441175 [details] [diff] [review] part 1: split libffi into debug/opt configurations Clearing request which I'm pretty sure doesn't make symbols in our nightly/release builds.

This should block.

(In reply to comment #15) > OS_CXXFLAGS += -Zi -UDEBUG -DNDEBUG > OS_CFLAGS += -Zi -UDEBUG -DNDEBUG > OS_LDFLAGS += -DEBUG -OPT:REF Does -UDEBUG -DNDEBUG have meaning just for moz code, or does it affect e.g. WIN32 headers? I assume it's OK if libffi unconditionally builds with debug symbols in opt builds (i.e. the debug info gets stripped out when we ship release builds). If not, I'll add an --enable-debug-symbols option to libffi itself.

I believe that _DEBUG is what controls the CRT selection, and DEBUG/NDEBUG are not used by the CRT. http://support.microsoft.com/kb/140584

Tested on MSVC debug and opt configurations, and confirmed that the libs are linked against MSVCRT/MSVCRTD as appropriate. I'll push this upstream.

Fixed by pushing upstream and pulling the new libffi git rev. http://hg.mozilla.org/tracemonkey/rev/e2265b714a02

Fixed by sayrer's merge of the aforementioned cset.