[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd added a comment. Herald added a reviewer: jdoerfert. In D65761#2102525 , @mehdi_amini wrote: > It seems like this pass was added in lib/Transforms but does not have any > unit-tests (lit tests) provided. It isn't even linked into `opt`. As it is

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

mehdi_amini added a comment. It seems like this pass was added in lib/Transforms but does not have any unit-tests (lit tests) provided. It isn't even linked into `opt`. As it is an LLVM IR pass it should be tested as such I believe. Can you provide tests for this? Repository: rG LLVM

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. In D65761#1814968 , @ajpaverd wrote: > The CFGuard library shouldn't be needed for targets other than ARM, AArch64, > and X86, and it's only being built for these targets. However, it looks like > `llvm-build` is

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd added a comment. In D65761#1811097 , @hubert.reinterpretcast wrote: > In D65761#1804302 , > @hubert.reinterpretcast wrote: > > > I have confirmed that the case I mentioned fails with rGd157a9b > >

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. In D65761#1804302 , @hubert.reinterpretcast wrote: > I have confirmed that the case I mentioned fails with rGd157a9b > . @ajpaverd, is a fix

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. Is building `check-all` immediately after configuration expected to be clean? I have a build configured to build LLVM with just `clang` and just with the `PowerPC` target, and it seems `check-all` (under such a configuration) does not trigger the

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd added a comment. In D65761#1772076 , @dmajor wrote: > Well, this patch specifically has code (even a test) that `nocf_check` > prevents CFG, so it looks like the intent was there, it's just that there > isn't a way to get that attribute onto

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

dmajor added a comment. In D65761#1772031 , @rnk wrote: > I think `-fcf-protection` and `__attribute__((nocf_check))` have to do with > CET and Intel's endbranch instruction or what have you. Similar goals, > different implementation. I think at this

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a comment. In D65761#1766993 , @dmajor wrote: > Are there any plans to implement `__declspec(guard(nocf))` or an equivalent > mechanism? `__attribute__((nocf_check))` doesn't do anything without the > -fcf-protection flag.

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

dmajor added a comment. Are there any plans to implement `__declspec(guard(nocf))` or an equivalent mechanism? `__attribute__((nocf_check))` doesn't do anything without the -fcf-protection flag. (https://bugs.llvm.org/show_bug.cgi?id=44096) Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

dmajor added a comment. Hi, I filed https://bugs.llvm.org/show_bug.cgi?id=44049 for some strange crashes that we're seeing because the CFG code overwrites the lower byte of function pointers before jumping to them. (Commenting separately here because I was unable to CC @ajpaverd in Bugzilla)

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

thakis added a comment. All new files added in this patch had dos line endings. I fixed that in e59f7488 but going forward please configure your environment to create new files for LLVM with unix line endings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGd157a9bc8ba1: Add Windows Control Flow Guard checks (/guard:cf). (authored by ajpaverd, committed by theraven).

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd updated this revision to Diff 226164. ajpaverd added a comment. Split cfguard_setjmp test into target-specific tests and add missing cfguard_longjmp pass for ARM and AArch64. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65761/new/

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd updated this revision to Diff 225955. ajpaverd marked 3 inline comments as done. ajpaverd added a comment. Formatting fixes for CFGuard patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65761/new/ https://reviews.llvm.org/D65761

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd updated this revision to Diff 225943. ajpaverd marked 6 inline comments as done. ajpaverd added a comment. Final changes and tests suggested by @rnk. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65761/new/ https://reviews.llvm.org/D65761

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a comment. I think this looks pretty good, thanks! I really only noticed style nits. I think with some small fixes, we should go ahead and merge it. If you want, I can commit it on your behalf, but I know there are other folks at Microsoft with commit access to LLVM, if you'd prefer.

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd marked 9 inline comments as done. ajpaverd added inline comments. Comment at: clang/include/clang/Driver/Options.td:500 def b : JoinedOrSeparate<["-"], "b">, Flags<[Unsupported]>; +def cfguard_no_checks : Flag<["-"], "cfguard-no-checks">, Flags<[CC1Option]>, +

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd added a comment. In D65761#1660121 , @rnk wrote: > Here is some feedback, I apologize for dragging my feet. > > Also, I would really like to get feedback from @pcc. He's OOO currently, > though. Thanks for this really helpful feedback @rnk! I

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd updated this revision to Diff 223167. ajpaverd marked 22 inline comments as done. ajpaverd added a comment. Improved Control Flow Guard implementation. As suggested by @rnk, the CFGuard dispatch mechanism now uses a new `cfguardtarget` operand bundle to pass the indirect call target.

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added inline comments. Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:13 +/// for such cases and replaces the pair of instructions with a single +/// call/invoke. For example: +/// hans wrote: > Naive question: Why do we generate code as in the examples

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hans added a comment. Herald added a subscriber: ychen. This is very exciting! I didn't look closely at the actual instrumentation code, as rnk knows that better and had some good comments. Comment at: clang/include/clang/Driver/Options.td:500 def b : JoinedOrSeparate<["-"],

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a subscriber: hans. rnk added a comment. Here is some feedback, I apologize for dragging my feet. Also, I would really like to get feedback from @pcc. He's OOO currently, though. Comment at: clang/include/clang/Driver/Options.td:500 def b : JoinedOrSeparate<["-"],

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd added a comment. In D65761#1646059 , @rnk wrote: > I plan to take a look at this tomorrow, sorry for putting this off for a > while. :( Thanks @rnk! If there's anything I can clarify/fix, please let me know. Repository: rG LLVM Github

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a comment. I plan to take a look at this tomorrow, sorry for putting this off for a while. :( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65761/new/ https://reviews.llvm.org/D65761 ___

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

ajpaverd created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya, kristof.beyls, javed.absar, mgorny, mehdi_amini. Herald added projects: clang, LLVM. A new module pass (Transforms/CFGuard/CFGuard.cpp) inserts CFGuard checks on indirect