Skip to content

GH-131691: fix SEH for clang-cl on Windows #131730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 25, 2025
Merged

GH-131691: fix SEH for clang-cl on Windows #131730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e1e4f1
use /EHa when compiling with clang-cl
chris-eibl Mar 25, 2025
01335b1
blurb it
chris-eibl Mar 25, 2025
bd83c14
temporarily touch (fix) tail-call.yml to trigger CI
chris-eibl Mar 25, 2025
78cb6d9
Revert "temporarily touch (fix) tail-call.yml to trigger CI"
chris-eibl Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2025-03-25-19-14-24.gh-issue-131691.2BgHU5.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
clang-cl on Windows needs option ``/EHa`` to support SEH (structured
exception handling) correctly. Fix by Chris Eibl.
8 changes: 8 additions & 0 deletions PCbuild/pyproject-clangcl.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@

<ItemDefinitionGroup>
<ClCompile>
<!--
ExceptionHandling=Async sets MSVC's /EHa (-fasync-exceptions in clang)
For clang, /EHa just has a slightly different meaning compared to MSVC,
so this option is needed to handle SEH correctly
(even though MSVC does not need it).
Please see GH-131691 for details.
-->
<ExceptionHandling>Async</ExceptionHandling>
<AdditionalOptions>-Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="'$(Platform)' == 'Win32'">-m32 %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="'$(Platform)' == 'x64'">-m64 %(AdditionalOptions)</AdditionalOptions>
Expand Down
10 changes: 7 additions & 3 deletions PCbuild/pyproject.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@
<LinkIncremental Condition="$(Configuration) != 'Debug'">false</LinkIncremental>
</PropertyGroup>

<!-- We need the above overridden OutDir, so this must be imported after PropertyGroup -->
<Import Project="pyproject-clangcl.props" Condition="$(PlatformToolset) == 'ClangCL' and $(__PyprojectClangCl_Props_Imported) != 'true'" />

<PropertyGroup Condition="$(TargetExt) != ''">
<TargetNameExt>$(TargetName)$(TargetExt)</TargetNameExt>
<_TargetNameSep>$(TargetNameExt.LastIndexOf(`.`))</_TargetNameSep>
Expand Down Expand Up @@ -130,6 +127,13 @@
</Midl>
</ItemDefinitionGroup>

<!--
We need the overridden OutDir (PropertyGroup Label="Globals"),
and want to be able to override some ClCompile parameters,
so this must not be included erlier.
-->
<Import Project="pyproject-clangcl.props" Condition="$(PlatformToolset) == 'ClangCL' and $(__PyprojectClangCl_Props_Imported) != 'true'" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to move this further down. Otherwise, pyproject-clangcl.props would not be able to override ExceptionHandling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up needing to put it earlier, then it could override ClCompile.ExceptionHandling in a Target (with an ItemGroup, rather than an ItemDefinitionGroup). Or we can probably remove the ExceptionHandling metadata from the current ItemDefinitionGroup and let it use the default.

If you prefer it this way, then let me know. This is mergeable as is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many ways to skin that cat. Before I found out, why using ExceptionHandling did not work, I used /EHa in AdditionalOptions.

I moved that down, just in case we'd like to override more in pyproject-clangcl.props in the future.

Like you mentioned, when we need to move it back further up - we'll find ways.

So let's merge? Up to your liking ...


<UsingTask TaskName="KillPython" TaskFactory="CodeTaskFactory" AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.v4.0.dll">
<ParameterGroup>
<FileName Required="true" />
Expand Down
Loading