clang-cl incorrect SEH exception handling

[djelinski]

The following code is supposed to read from potentially inaccessible memory, and return the provided default value if the memory is not accessible. It works fine when compiled with with cl (outputs except and done), but crashes when compiled with clang-cl:

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

//#define WORKAROUND

int SafeFetch32(const int* adr, int errValue) {
  int v = 0;
  __try {
#ifdef WORKAROUND
      printf("");
#endif
    v = *adr;
#ifdef WORKAROUND
      printf("");
#endif
  }
  __except(EXCEPTION_EXECUTE_HANDLER) {
    printf("except\n");
    v = errValue;
  }
  return v;
}
int main() {
    SafeFetch32(0, -1);
    printf("done");
    return 0;
}

When compiled with -DWORKAROUND, the code generated by clang-cl works as expected.

Clang downloaded with MS Visual Studio 2022 Community:

> clang-cl --version
clang version 15.0.1
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\x64\bin

Not sure if that's relevant, but the results were collected on Windows 11 21H2.

[efriedma-quic]

This should work correctly using latest development branch with /EHa. (This isn't supported without /EHa. And I don't think LLVM 16 has the relevant fixes.)

[djelinski]

Thanks for the info. I'm hoping it will not actually require /EHa; the provided example works just fine with clang-cl -DWORKAROUND -EHsc. The code is specifically using __try / __except to make sure it doesn't require /EHa.

[efriedma-quic]

You can handle SEH exceptions without /EHa... the problem is that without /EHa, the compiler assumes memory accesses don't trap. So an exception raised with RaiseException would work fine, but a load is assumed not to cause an exception.

As far as I know, this is the same model MSVC uses (https://learn.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-170). Maybe extremely simple cases like your testcase somehow work without /EHa, but in general you won't get the expected result.

If there's an important use-case that involves not using /EHa, please open a feature request explaining what cases you need to handle, and why it's important it works without /EHa.

[djelinski]

Thanks for the additional information.

without /EHa, the compiler assumes memory accesses don't trap

That's a dangerous assumption. /EHa is only documented to change the behavior of catch(...) C++ clause to also catch structured exceptions. The only mention of __try / __except in /EHa docs states that:

To implement SEH without specifying /EHa, you may use the try, except, and __finally syntax.

i. e. __try / __except is supposed to catch structured exceptions even when /EHa is absent.

Further, you can find a list of typical structured exception codes in GetExceptionCode documentation. I'm sure you'll notice that there's quite a few codes that can be triggered by memory accesses:

• EXCEPTION_ACCESS_VIOLATION
• EXCEPTION_ARRAY_BOUNDS_EXCEEDED
• EXCEPTION_DATATYPE_MISALIGNMENT
• EXCEPTION_GUARD_PAGE
• EXCEPTION_IN_PAGE_ERROR
• EXCEPTION_STACK_OVERFLOW

Given the above, I'd assume that the compiler is not allowed to move any instructions into or out of a __try block, and is not allowed to optimize that block away.

The code in question was taken from the OpenJDK; it was introduced in 2005 and has been working reliably ever since. The code does not use catch (...), so I may be able to use /EHa as a workaround. Still, given that Microsoft states that using /EHa can be dangerous, I'd rather not do that.

How do I open a feature request? Couldn't find that information in the "contributing" guide.

[efriedma-quic]

Feature requests are the same as bug reports... with the discussion here, I can just reopen this, I guess.

From the documentation:

When you use /EHs or /EHsc, the compiler assumes that exceptions can only occur at a throw statement or at a function call

Use /EHa if you want to catch an exception that's raised by something other than a throw

Maybe we can carve out a narrow exception for load/store ops that are contained directly in an __try...?

[djelinski]

I checked the compilation results for __try / __except and try / catch (...). The following code:

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

int SafeFetch32(const int* adr, int errValue) {
  int v = 0;
  __try {
    v = *adr;
  }
  __except(EXCEPTION_EXECUTE_HANDLER) {
    v = errValue;
  }
  return v;
}
int main() {
    return SafeFetch32(0, -1);
}

produces the same code (SEH table) with /O2 and any of the following: /EHa and /EHs, and no EH parameter.

The following code:

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

int SafeFetch32(const int* adr, int errValue) {
  int v = 0;
  try {
    v = *adr;
  }
  catch(...) {
    v = errValue;
  }
  return v;
}
int main() {
    return SafeFetch32(0, -1);
}

produces stack unwinding code (much longer code than the first example) with either /O2 /EHa or /O2 alone; it completely optimizes away the exception handling when compiled with /O2 /EHs.

This confirms that __try / __except is not affected by /EH options.

EDIT: apparently godbolt implicitly adds /EHs to the command line; repeated the test locally and confirmed that the default behaves like /EHa on the sample code. Corrected the results above.

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Daniel Jelinski (djelinski)

The following code is supposed to read from potentially inaccessible memory, and return the provided default value if the memory is not accessible. It works fine when compiled with with `cl` (outputs `except` and `done`), but crashes when compiled with clang-cl: ```c #include <stdio.h> #include <windows.h> //#define WORKAROUND int SafeFetch32(const int* adr, int errValue) { int v = 0; __try { #ifdef WORKAROUND printf(""); #endif v = *adr; #ifdef WORKAROUND printf(""); #endif } __except(EXCEPTION_EXECUTE_HANDLER) { printf("except\n"); v = errValue; } return v; } int main() { SafeFetch32(0, -1); printf("done"); return 0; } ``` When compiled with `-DWORKAROUND`, the code generated by `clang-cl` works as expected. Clang downloaded with MS Visual Studio 2022 Community: ```console > clang-cl --version clang version 15.0.1 Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\x64\bin ``` Not sure if that's relevant, but the results were collected on Windows 11 21H2.

[skoulik]

I was scratching my head facing the same issue. This example from MS: https://learn.microsoft.com/en-us/cpp/cpp/try-except-statement?view=msvc-170#example works as expected when compiling with cl.exe regardless of /EH{x}, while clang-cl.exe fails to catch the AV regardless of /EH{x}. I also confirm that the workaround works for me. Tested with Visual Studio 2019 (clang 12.0.0). Interestingly though, on a much complex project involving AV originating inside many levels on indirection, the __try ... __except block works as expected and catches the SE. This makes me feel that clang/llvm is trying to play too smart here and the observed behavior is a bug that must be addressed. Also, /EHa is irrelevant - it is to catch SEs with C++ try ... catch rather than with MS specific __try __except.