Opened 3 years ago

Closed 20 months ago

#13607 closed build error (fixed)

Warning about using _set_se_translator() without /EHa with MSVC

Reported by: guggy Owned by:
Priority: low Milestone:
Component: wxMSW Version: stable-latest
Keywords: warning thread bakefile-ng Cc:
Blocked By: Blocking:
Patch: no

Description

When I compile the src\msw\thread.cpp in VS2010 / Windows Vista, the warning

warning C4535: calling _set_se_translator() requires /EHa

is thrown in line 569.

Attachments (1)

seh-patch download (4.0 KB) - added by staticinline 21 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 3 years ago by vadz

  • Keywords bakefile-ng added
  • Priority changed from normal to low
  • Status changed from new to confirmed
  • Summary changed from Warning when compiling thread.cpp to Warning about using _set_se_translator() without /EHa with MSVC

This warning is harmless and can't be avoided with current bakefile which doesn't support using per-file compilation options (and using /EHa globally might be not a good idea).

comment:2 Changed 21 months ago by staticinline

I was unsure about this too. Look at DisableAutomaticSETranslator in src/msw/seh.h:

#if wxUSE_ON_FATAL_EXCEPTION && defined(__VISUALC__) && !defined(__WXWINCE__)
    #include <eh.h>

    // C++ exception to structured exceptions translator: we need it in order
    // to prevent VC++ from "helpfully" translating structured exceptions (such
    // as division by 0 or access violation) to C++ pseudo-exceptions
    extern void wxSETranslator(unsigned int code, EXCEPTION_POINTERS *ep);

    // up to VC 9 this warning ("calling _set_se_translator() requires /EHa")
    // is harmless and it's easier to suppress it than use different makefiles
    // for VC5 and 6 (which don't support /EHa at all) and VC7 (which does
    // accept it but it seems to change nothing for it anyhow)
    #if __VISUALC__ < 1600
        #pragma warning(disable: 4535)
    #endif

    // note that the SE translator must be called wxSETranslator!
    #define DisableAutomaticSETranslator() _set_se_translator(wxSETranslator)
#else // !__VISUALC__
    // the other compilers do nothing as stupid by default so nothing to do for
    // them
    #define DisableAutomaticSETranslator()
#endif // __VISUALC__/!__VISUALC__

That is four years old, so the "< 1600" could be updated to "<= 1700" to avoid the warning about _set_se_translator requiring /EHa under VC10/VC11. Moreover, this code is technically invalid because reference [ 1 ] says "You must use /EHa when using _set_se_translator." The above code seems to expect _set_se_translator does nothing under /EHsc, and my tests under VC11 seem to confirm this, though it's undefined behavior. Unfortunately, I don't see a compile define to test whether /EHa is enabled (the closest I found was _CPPUNWIND).

Also look at wxSETranslator in src/msw/main.cpp:

#ifdef __VISUALC__

void wxSETranslator(unsigned int WXUNUSED(code), EXCEPTION_POINTERS *ep)
{
    switch ( wxGlobalSEHandler(ep) )
    {
        default:
            wxFAIL_MSG( wxT("unexpected wxGlobalSEHandler() return value") );
            // fall through

        case EXCEPTION_EXECUTE_HANDLER:
            // if wxApp::OnFatalException() had been called we should exit the
            // application -- but we shouldn't kill our host when we're a DLL
#ifndef WXMAKINGDLL
            wxFatalExit();
#endif // not a DLL
            break;

        case EXCEPTION_CONTINUE_SEARCH:
            // we're called for each "catch ( ... )" and if we (re)throw from
            // here, the catch handler body is not executed, so the effect is
            // as if had inhibited translation of SE to C++ ones because the
            // handler will never see any structured exceptions
            throw;
    }
}

#endif // __VISUALC__

It wasn't apparent to me from [ 1 ] what a "throw;" with no arguments in the translator function would do, but it indeed does seem to follow the comments given above. Here's a test:

#include <stdio.h>
#include <windows.h>
#include <eh.h>

void trans_func( unsigned int u, EXCEPTION_POINTERS* pExp ) {
  printf("in trans_func\n");
  //throw 1;
  throw;
}
static void terminate() { 
  printf("term\n");
}
void g() {
  try {
    _set_se_translator( trans_func );
    try { int x = 0; x /= x; }
    catch(int)  { printf("catch int\n"); }
    catch (...) { printf("catch ...\n"); }
    printf("done\n");
  }
  catch(int) { printf("catch2 int\n"); }
  catch(...) { printf("catch2 ...\n"); }
}
int main() {
  set_terminate (terminate);
  __try { g(); }
  __except(EXCEPTION_EXECUTE_HANDLER) { printf("__except\n"); }
  return 0;
}

That prints "in trans_func\nexcept" under /EHa and "except" under /EHs (tested with VC10 and VC11).

Using "catch(...)" in user code is a bad idea anyway, and if "catch(...)" doesn't exist or /EHa is not used, I see no need for DisableAutomaticSETranslator().

[1] http://msdn.microsoft.com/en-us/library/5z4bw5h5.aspx

comment:3 Changed 21 months ago by vadz

So, if I understand correctly, your tests seem to show that everything is working as expected? Or am I missing something?

We can indeed bump up the VC version check then.

Changed 21 months ago by staticinline

comment:4 Changed 21 months ago by staticinline

Yes, though ugly and probably usually unnecessary, it seems to be working as intended but needs a version bump. Attached is a patch with improved comments.

BTW, here's a good background article for anyone interested:
http://www.codeproject.com/Articles/207464/Exception-Handling-in-Visual-Cplusplus .

comment:5 Changed 20 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [73295]) Suppress warning about _set_se_translator() for VC++ 10 and 11 too.

Previously we disabled this warning for VC versions up to 9 but the warning
still seems to be as harmless as before for the newer versions too.

Closes #13607.

Note: See TracTickets for help on using tickets.