340443 - Not possible to use the XPCOM glue when linking against static CRT, crash on startup with tests enabled

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Brian Ryner (not reading)]

On Windows, the XPCOM glue is compiled against the dynamic CRT (/MD).  It ends up referencing at least one imported symbol from the dynamic CRT (snprintf, from nsAString::AppendInt.  As a result, if you try to link with the glue and use the static CRT, the DLL fails to link.

A couple of possible solutions are to build a version of the glue library that's compiled against the static CRT (/MT), or to remove the dependency on dynamic CRT imports (seems to be just snprintf at the moment).

Comment 1

[Benjamin Smedberg]

snprintf or _snprintf? The static CRT should have one of those exports. If we compile the glue(s) with /MT (USE_STATIC_LIBS in the makefile) and then remove the directives this should probably work (I mean to try it at some point).

Comment 2

[Brian Ryner (not reading)]

_snprintf

You're right, compiling the glue with /MT _should_ solve the problem, but I'm not sure if we want to do this in the general case.  Maybe it makes sense to compile two versions of the .lib?

Comment 3

[[e-mail]]

Note: while this works for snprintf, it does not work for stdin, stdout or stderr (the code for accessing DLL imported data references is completely different) so that if you mix static and dynamic linkage and you're compiling debug then the fprintf(stderr, ...) in nsGenericModule.cpp crashes.

Comment 4

[Benjamin Smedberg]

Attachment: Don't use stderr, rev. 1

This bug was mostly fixed in bug 334528, but introduced a crash when compiling debug builds and linking against the dynamic glue. This is because the calling pattern for dllexport *data* is incompatible when compiling with /MT. Neil, can you test?

Comment 5

[[e-mail]]

Regarding the concern about leaking FILEs, you could use
FILE *fp = fdopen(dup(2));
fprintf(fp, ...);
fclose(fp);

Comment 6

[[e-mail]]

Comment on attachment 237031 [details] [diff] [review]
Don't use stderr, rev. 1

This doesn't crash, but the created FILE is buffered, so you don't see the output until shutdown.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This bug is making it impossible to build debug builds with tests enabled which is a pretty big deal to me.

Comment 8

[Benjamin Smedberg]

Attachment: Don't use stderr, rev. 2

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 237377 [details] [diff] [review]
Don't use stderr, rev. 2

if you dup stderr you really should close fd...

Is there a STDERR_FILENO constant on windows?

Comment 10

[Benjamin Smedberg]

Attachment: Don't use stderr, rev. 3

Comment 11

[[e-mail]]

Comment on attachment 237521 [details] [diff] [review]
Don't use stderr, rev. 3

>+void
>+printf_stderr(const char *fmt, ...)
>+{
>+  FILE *fd = _fdopen(_dup(2), "a");
>+
>+  va_list args;
>+  va_start(args, fmt);
>+  vfprintf(fd, fmt, args);
>+  va_end(args);
>+}
You need to fclose(fp); for two reasons: a) to avoid leaking FILEs b) to actually get anything printed before shutdown. r=me with that fixed.

Nit: fd is an abbreviation for file descriptor (i.e. 2); the corresponding abbreviation of FILE pointer (which is what you have here) is fp.

Comment 12

[Benjamin Smedberg]

Attachment: Don't use stderr, rev. 3.1

Comment 13

[Darin Fisher]

Comment on attachment 237820 [details] [diff] [review]
Don't use stderr, rev. 3.1

>Index: xpcom/glue/nsDebug.h

>+/* When compiling the XPCOM Glue on Windows, we pretend that it's going to be
>+ * be linked with a static CRT (-MT) even when it's not. This means that we

"be be"


>+ * cannot link to data exports from the CRT, only function exports. So,
>+ * instead of referencing "stderr" directly, use fdopen.
>+ */
>+PR_BEGIN_EXTERN_C
>+
>+void
>+printf_stderr(const char *fmt, ...);

Wow, this is unfortunate.  OK.


sr=darin

Comment 14

[Benjamin Smedberg]

Fixed on trunk.