Closed Bug 556886 Opened 10 years ago Closed 7 years ago

New warnings from STL wrappers: "warning C4275: non dll-interface class 'stdext::exception' used as base for dll-interface class"

Categories

(Core :: XPCOM, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
blocking2.0 --- -

People

(Reporter: cjones, Assigned: emk)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [build_warning])

Attachments

(1 file)

For example

d:\msvs8\VC\INCLUDE\typeinfo(160) : warning C4275: non dll-interface class 'stdext::exception' used as base for dll-interface class 'std::bad_typeid'
        d:\msvs8\VC\INCLUDE\exception(241) : see declaration of 'stdext::exception'
        d:\msvs8\VC\INCLUDE\typeinfo(159) : see declaration of 'std::bad_typeid'

The problem is that for |#if _HAS_EXCEPTIONS| builds, std::exception is _CRTIMP_PURE, with |#if !_HAS_EXCEPTIONS|, it's not, whereas std::bad_typeid is always _CRTIMP2.  I'm not sure what difference this makes, so I'd propose just suppressing warning 4275 in the STL wrappers.  Objections?
Version: unspecified → Trunk
So, let me construct a scenario which could cause problems with this warning.  Let's say that the compiler sees:

// exe.cpp

class B {
  int foo;
  // methods...
};

class __declspec(dllimport) D : public B {
  void bar() { /* access foo */ }
  // methods...
};

// dll.cpp

class __declspec(dllexport) {
  double foo;
  // methods...
};

class __declspec(dllexport) D : public B {
  void bar() { /* access foo */ }
  // methods...
};

Now, let's say that you call D::bar at some point in the exe.  In that case, D::foo will end up touching B::foo, thinking that it's a double, whereas the actual D object created in the exe code has B::foo as an int.  You will crash, if you're lucky, and debugging this problem would be unimaginably hard.

Now, in this particular case, we're already doomed if an std::bad_typeid object is ever created (because presumably it will be thrown next), therefore it is safe to ignore the warning *only for this header*.  If there are other headers with similar problems, we should ignore the warning for them individually as well.  We can't however just take a general patch which disables the warning for all code, because we want to be warned if we're actually ever making the mistake I demonstrated above for code which would actually get executed.
Duplicate of this bug: 575020
FWIW, I posted a patch to globally disable this warning in duplicate Bug 575020.  (nothing special, just adds |CXXFLAGS="$CXXFLAGS -wd4275"| in two configure.in files)

Not sure that's the solution we want, but it's one way to avoid this warning.
blocking2.0: --- → -
status2.0: ? → ---
cjones, any further thoughts on this given comment #1? Still a major source of warning spam.
As Ehsan points out, we've already lost if the problem this warning refers to surfaces (hey, a non-vacuous warning for once!).  Best would be to push a warning-disable before including this file then popping it (http://msdn.microsoft.com/en-us/library/2c8f766e%28v=VS.71%29.aspx).
(In reply to comment #5)
> As Ehsan points out, we've already lost if the problem this warning refers to
> surfaces (hey, a non-vacuous warning for once!).  Best would be to push a
> warning-disable before including this file then popping it
> (http://msdn.microsoft.com/en-us/library/2c8f766e%28v=VS.71%29.aspx).

/me agrees!
I'm having a hard time understanding where to suppress the warning. As far as I can tell by reading memory/mozalloc, the STL wrappers encompass numerous files, but I don't see where "exception" and "typeinfo" come into play.
For VS9/VS8, I think we would want to put the push/pop around the include of <exception> from msvc_throw_wrapper.h.
+#  pragma warning(disable:4275) //Suppress spurious MSVC warning
 #  include <exception>
+#  pragma warning(default:4275)

Didn't work on VS10 anyway.
AHA! From the earlier-linked KB article:
For warning numbers greater than 4699, those associated with code generation, the warning pragma has effect only when placed outside function definitions.
gah, nevermind. Was thinking 4725 for some reason...
Ben, do you have any idea what to try here?
Globally disable it with a -wd4275 compile flag?
Doesn't comment #1 recommend against globally disabling the warning?
I'm pretty sure I know why the #pragma approach hasn't been working so far. At least on my system, the following mozalloc files actually get built:
mozalloc.cpp
mozalloc_abort.cpp
msvc_raise_wrappers.cpp
mozalloc_oom.cpp
msvc_raise_wrappers.cpp
mozalloc_abort.cpp

There is no sign of msvc_throw_wrapper.cpp getting used at all. Furthermore, I put some junk in msvc_throw_wrapper.h that would break the build if hit, and the build went fine with the same warning as always. For that reason, I think that this warning is actually coming in from somewhere else.
Adding the push/pop like below in throw_msvc.h didn't work either.

#pragma warning( push )             //Suppress MSVC warning spam
#pragma warning( disable : 4275 )

#if defined(MOZ_MSVC_STL_WRAP__RAISE)
#  include "msvc_raise_wrappers.h"
#elif defined(MOZ_MSVC_STL_WRAP__Throw)
#  include "msvc_throw_wrapper.h"
#else
#  error "Unknown STL wrapper tactic"
#endif

#pragma warning( pop )
(In reply to comment #15)
> There is no sign of msvc_throw_wrapper.cpp getting used at all. Furthermore, I
> put some junk in msvc_throw_wrapper.h that would break the build if hit, and
> the build went fine with the same warning as always. For that reason, I think
> that this warning is actually coming in from somewhere else.

The compiler output should say where the warnings are coming from, right?
Here's the chain that exception comes in from:

Note: including file:  c:\mozbuild\mozilla-central\objdir-fx\dist\include\nscore.h
Note: including file:   c:\mozbuild\mozilla-central\objdir-fx\dist\include\mozilla/mozalloc.h
Note: including file:    c:\mozbuild\mozilla-central\objdir-fx\dist\stl_wrappers\new
Note: including file:     c:\mozbuild\mozilla-central\objdir-fx\dist\include\mozilla/throw_msvc.h
Note: including file:      c:\mozbuild\mozilla-central\objdir-fx\dist\include\mozilla\msvc_raise_wrappers.h
Note: including file:       c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\INCLUDE\xutility
Note: including file:        c:\mozbuild\mozilla-central\objdir-fx\dist\stl_wrappers\climits
Note: including file:         c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\INCLUDE\new
Note: including file:          c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\INCLUDE\exception
Blocks: buildwarning
Whiteboard: [build_warning]
This bug biten me when I'm working on bug 826231.
Blocks: 826231
Attached patch patchSplinter Review
This pragma should safe (unless CRT has a bug) because warning-disabled block only includes a system stl header.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #697470 - Flags: review?(ehsan)
Comment on attachment 697470 [details] [diff] [review]
patch

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

::: config/msvc-stl-wrapper.template.h
@@ +62,5 @@
>  
>  // We know that code won't be able to catch exceptions, but that's OK
>  // because we're not throwing them.
>  #pragma warning( push )
> +#pragma warning( disable : 4275 4530 )

Again, please add a comment explaining warning C4275.
Attachment #697470 - Flags: review?(ehsan) → review+
Pushed with adding the comment.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e003bb70f9c
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/2e003bb70f9c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Blocks: 836185
You need to log in before you can comment on or make changes to this bug.