Enable Control Flow Guard

CommunityToolkit / Microsoft.Toolkit.Win32

ARCHIVE - This repository contained XAML Islands wrapper controls and tooling for XAML Islands with WinUI 2, see readme for more info about XAML Islands with WinUI 3 and the WindowsAppSDK.

https://aka.ms/windowsappsdk

Other

383 stars 89 forks source link

Enable Control Flow Guard #271

Closed sylveon closed 4 years ago

sylveon commented 4 years ago

Fixes #263

PR Type

What kind of change does this PR introduce?

Bugfix
Build or CI related changes

What is the current behavior?

The DLL fails to load if strict Control Flow Guard is enforced

What is the new behavior?

The DLL uses Control Flow Guard and does not fail to load

PR Checklist

Please check if your PR fulfills the following requirements:

[x] Tested code with current supported SDKs
[x] Contains NO breaking changes

ghost commented 4 years ago

Thanks sylveon for opening a Pull Request! The reviewers will test the PR and highlight if there is any merge conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

sylveon commented 4 years ago

Welp, apparently this doesn't solve the issue, will need to investigate

sylveon commented 4 years ago

Found the issue... VCRT forwarders aren't marked as CFG-aware

sylveon commented 4 years ago

This works if combined with #251

marb2000 commented 4 years ago

I've concerns about adding the Control Flow Guard (CFG) directive because it's a scenario out of the test matrix, and we need to validate all the top scenarios.

@sylveon, did you tested the XAML Islands Samples with this configuration? This is the minimum bar.

sylveon commented 4 years ago

Well, my application works with this and anything else that is broken by CFG is supposed to be a compiler bug. I checked anyways, and here's a build of NativeXamlIslands.sln running fine with a CFG-enabled XamlHost DLL

marb2000 commented 4 years ago

This is cool 😊. What about the WPF and WinForms samples? We need to make sure that we don't break other scenarios.

By the way, the C++ sample we did is awful. It hurts my eyes every time I run it.

sylveon commented 4 years ago

marb2000 commented 4 years ago

Thanks so much @sylveon. I'm testing this on my machine as well, before approving the PR.