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.

[jaykrell]

Some of this discussion is confused, and hard to explain.
Roughly speaking, __try implies like-/EHa, in the code within the __try.
And presumably in the "filter"and the __finally/__except.

Now I realize, if you interpret /EHa as something to do with destructors, then this is a contradiction.
Functions with __try cannot have locals with destructors. Or at least won't run them if there is an exception. The metadata and runtime support for C EH and C++ EH are separate, and cannot be used in the same function.

And /EHa has more meaning than someone said.
It doesn't just make catch(...) catch "foreign" exceptions.
It is sorta a "stricter" definition of "what is constructed must be destructed".

Let's say we have:
class A { A(); ~A(); };

A a1;
// something here
A a2;

There is the question as to if "something here" can raise an exception.
Under /EHa, pretty much anything can. Or anything can.
And "no matter what" if a1 is constructed, then a1 will be destroyed.

But with regular /EH, the compiler can try to prove that "something here" cannot raise exceptions.
It can then merge the "states" of a1 and a2. And the guarantee can sorta be loosened.
That only if a2 is constructed, will a1 be destroyed.
Well, you know, thing is, these are meant to be equivalent statements.
If a1 is constructed, then a1 will be destroyed.

But it is a question as to if the program is guaranteed to progress from constructing a1 through to constructing a2, or if it can fail part way through.

If it cannot fail part away through, there are optimizations to be had.
If it can fail part way through, less optimization.

And /EHa drastically increases what can fail part way through.

[jaykrell]

That is:
"Also, /EHa is irrelevant - it is to catch SEs with C++ try ... catch rather than with MS specific __try __except."

Agreed, that was my point too.

[phoebewang]

The SEH support in LLVM is not rail to rail with MSVC..

I did a little archeology. The initial support of __try in LLVM was written by @rnk. The commit said:

The lowering looks a lot like normal EH lowering, with the exception
that the exceptions are caught by executing filter expression code
instead of matching typeinfo globals. The filter expressions are
outlined into functions which are used in landingpad clauses where
typeinfo would normally go.

Especially,

Non-call exceptions in __try bodies won't work yet. The plan is to
outline the __try block in the frontend to keep things simple.

I didn't dig much on following changes, my impression is the non-call exceptions were never supported before @tentzen 's solution. It took a long time before all patches merged. So only LLVM19 and forward support it.

The /EHa is relevant in LLVM because only when it's specified that enables the new behavior of __try. We don't change the behavior silently.

In a word, the mismatch between LLVM and MSVC is a long existing problem. The suggestion is to specify /EHa explicitly when user want to get the all SEH behavior.

[rnk]

Yep, this is the history I remember. I feel like I have some responsibility for not engaging with the reviews for these patches, which I guess have dragged on as recently as 2024.

I think it makes sense for /EHa to control the emission of llvm.seh.try/scope.begin/end for C++ personality functions, but for __try functions, we should go ahead and always use the new intrinsics to catch non-call exceptions. This is such a common compatibility issue. People using __try really do want to catch non-call exceptions, while most vanilla C++ programmers probably aren't.