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

[Andrew Paverd via Phabricator via cfe-commits]

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 an 
> LLVM IR pass it should be tested as such I believe. Can you provide tests for 
> this?


Thanks for the feedback. D82447  now links 
this into `opt` and adds lit tests in `test/Transforms/CFGuard/`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

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 Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Hubert Tong via Phabricator via cfe-commits]

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 also including it in the `LibraryDependencies.inc` file for 
> other targets, such as PowerPC, which I think is causing this error. I'm not 
> sure how to indicate that the library is only needed for a specified subset 
> of targets. @rnk or @hans  any suggestions?


Is the CFGuard library meaningful to provide for the other targets? Is 
consistency in the availability of the library (e.g., in case of hardcoded `-l` 
in the Makefile of some LLVM-dependent project) desired? It may make sense to 
treat the library as not target-specific.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Andrew Paverd via Phabricator via cfe-commits]

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 
> > .
>
>
> @ajpaverd, is a fix forthcoming for the issue I mentioned with this patch?


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 also including it in the `LibraryDependencies.inc` file for 
other targets, such as PowerPC, which I think is causing this error. I'm not 
sure how to indicate that the library is only needed for a specified subset of 
targets. @rnk or @hans  any suggestions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Hubert Tong via Phabricator via cfe-commits]

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 forthcoming for the issue I mentioned with this patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Hubert Tong via Phabricator via cfe-commits]

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 building of the library added in this patch; which, in turn, 
leads to test failures.

  /src/llvm/test/tools/llvm-config/booleans.test:27:24: error: 
CHECK-SHARED-MODE-NOT: excluded string found in input
  CHECK-SHARED-MODE-NOT: error:
 ^
  :3:14: note: found here
  llvm-config: error: missing: /build/lib/libLLVMCFGuard.a
   ^~

I have confirmed that the case I mentioned fails with rGd157a9b 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Andrew Paverd via Phabricator via cfe-commits]

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 functions from cpp in a CFG/non-CET 
> world.


Apologies for the delayed reply! This code was indeed reusing `nocf_check`, but 
I now think it would be better to use a separate attribute for Control Flow 
Guard. Please see the proposed improvement at D72167: Add support for 
__declspec(guard(nocf)) 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[dmajor via Phabricator via cfe-commits]

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 point it's "patches welcome". =S


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 functions from cpp in a CFG/non-CET world.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Reid Kleckner via Phabricator via cfe-commits]

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. (https://bugs.llvm.org/show_bug.cgi?id=44096)


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 point it's "patches welcome". =S


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[dmajor via Phabricator via cfe-commits]

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 LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[dmajor via Phabricator via cfe-commits]

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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via Phabricator via cfe-commits]

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
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[David Chisnall via Phabricator via cfe-commits]

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).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-fallback.c
  clang/test/Driver/cl-options.c
  llvm/docs/LangRef.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/LLVMBuild.txt
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/LLVMBuild.txt
  llvm/lib/Target/X86/LLVMBuild.txt
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/CFGuard/CMakeLists.txt
  llvm/lib/Transforms/CFGuard/LLVMBuild.txt
  llvm/lib/Transforms/CMakeLists.txt
  llvm/lib/Transforms/LLVMBuild.txt
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/calling-conventions.3.2.ll.bc
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll
  llvm/test/CodeGen/X86/cfguard-x86-64-vectorcall.ll
  llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll

Index: llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
@@ -0,0 +1,43 @@
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; Control Flow Guard is currently only available on Windows
+
+
+; Test that Control Flow Guard checks are correctly added for x86 vector calls.
+define void @func_cf_vector_x86(void (%struct.HVA)* %0, %struct.HVA* %1) #0 {
+entry:
+  %2 = alloca %struct.HVA, align 8
+  %3 = bitcast %struct.HVA* %2 to i8*
+  %4 = bitcast %struct.HVA* %1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %3, i8* align 8 %4, i32 32, i1 false)
+  %5 = load %struct.HVA, %struct.HVA* %2, align 8
+  call x86_vectorcallcc void %0(%struct.HVA inreg %5)
+  ret void
+
+  ; X32-LABEL: func_cf_vector_x86
+  ; X32: 	 movl 12(%ebp), %eax
+  ; X32: 	 movl 8(%ebp), %ecx
+  ; X32: 	 movsd 24(%eax), %xmm4 # xmm4 = mem[0],zero
+  ; X32: 	 movsd %xmm4, 24(%esp)
+  ; X32: 	 movsd 16(%eax), %xmm5 # xmm5 = mem[0],zero
+  ; X32: 	 movsd %xmm5, 16(%esp)
+  ; X32: 	 movsd (%eax), %xmm6   # xmm6 = mem[0],zero
+  ; X32: 	 movsd 8(%eax), %xmm7  # xmm7 = mem[0],zero
+  ; X32: 	 movsd %xmm7, 8(%esp)
+  ; X32: 	 movsd %xmm6, (%esp)
+  ; X32: 

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

[Andrew Paverd via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D65761

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-fallback.c
  clang/test/Driver/cl-options.c
  llvm/docs/LangRef.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/LLVMBuild.txt
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/LLVMBuild.txt
  llvm/lib/Target/X86/LLVMBuild.txt
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/CFGuard/CMakeLists.txt
  llvm/lib/Transforms/CFGuard/LLVMBuild.txt
  llvm/lib/Transforms/CMakeLists.txt
  llvm/lib/Transforms/LLVMBuild.txt
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/calling-conventions.3.2.ll.bc
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll
  llvm/test/CodeGen/X86/cfguard-x86-64-vectorcall.ll
  llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll

Index: llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
@@ -0,0 +1,43 @@
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; Control Flow Guard is currently only available on Windows
+
+
+; Test that Control Flow Guard checks are correctly added for x86 vector calls.
+define void @func_cf_vector_x86(void (%struct.HVA)* %0, %struct.HVA* %1) #0 {
+entry:
+  %2 = alloca %struct.HVA, align 8
+  %3 = bitcast %struct.HVA* %2 to i8*
+  %4 = bitcast %struct.HVA* %1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %3, i8* align 8 %4, i32 32, i1 false)
+  %5 = load %struct.HVA, %struct.HVA* %2, align 8
+  call x86_vectorcallcc void %0(%struct.HVA inreg %5)
+  ret void
+
+  ; X32-LABEL: func_cf_vector_x86
+  ; X32: 	 movl 12(%ebp), %eax
+  ; X32: 	 movl 8(%ebp), %ecx
+  ; X32: 	 movsd 24(%eax), %xmm4 # xmm4 = mem[0],zero
+  ; X32: 	 movsd %xmm4, 24(%esp)
+  ; X32: 	 movsd 16(%eax), %xmm5 # xmm5 = mem[0],zero
+  ; X32: 	 movsd %xmm5, 16(%esp)
+  ; X32: 	 movsd (%eax), %xmm6   # xmm6 = mem[0],zero
+  ; X32: 	 movsd 8(%eax), %xmm7  # xmm7 = mem[0],zero
+  ; X32: 	 movsd %xmm7, 8(%esp)
+  ; X32: 	 movsd %xmm6, (%esp)
+  ; X32: 	 calll *___guard_check_icall_fptr
+  ; X32: 	 movaps %xmm6, %xmm0
+  ; X32: 	 movaps 

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

[Andrew Paverd via Phabricator via cfe-commits]

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

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp

Index: llvm/lib/Transforms/CFGuard/CFGuard.cpp
===
--- llvm/lib/Transforms/CFGuard/CFGuard.cpp
+++ llvm/lib/Transforms/CFGuard/CFGuard.cpp
@@ -17,8 +17,8 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/CallingConv.h"
-#include "llvm/IR/Instruction.h"
 #include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Instruction.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 
@@ -196,8 +196,7 @@
 GuardFnGlobal = ConstantExpr::getBitCast(GuardFnGlobal, PTy);
 
   // Load the global as a pointer to a function of the same type.
-  LoadInst *GuardDispatchLoad =
-  B.CreateLoad(CalledOperandType, GuardFnGlobal);
+  LoadInst *GuardDispatchLoad = B.CreateLoad(CalledOperandType, GuardFnGlobal);
 
   // Add the original call target as a cfguardtarget operand bundle.
   SmallVector Bundles;
@@ -206,11 +205,11 @@
 
   // Create a copy of the call/invoke instruction and add the new bundle.
   CallBase *NewCB;
-  if (CallInst* CI = dyn_cast(CB)) {
+  if (CallInst *CI = dyn_cast(CB)) {
 NewCB = CallInst::Create(CI, Bundles, CB);
   } else {
 assert(isa(CB) && "Unknown indirect call type");
-InvokeInst* II = cast(CB);
+InvokeInst *II = cast(CB);
 NewCB = llvm::InvokeInst::Create(II, Bundles, CB);
   }
 
@@ -237,17 +236,17 @@
 
   // Set up prototypes for the guard check and dispatch functions.
   GuardFnType = FunctionType::get(Type::getVoidTy(M.getContext()),
-  {Type::getInt8PtrTy(M.getContext())}, false);
+  {Type::getInt8PtrTy(M.getContext())}, false);
   GuardFnPtrType = PointerType::get(GuardFnType, 0);
 
   // Get or insert the guard check or dispatch global symbols.
   if (GuardMechanism == CF_Check) {
-GuardFnGlobal = M.getOrInsertGlobal(
-  "__guard_check_icall_fptr", GuardFnPtrType);
+GuardFnGlobal =
+M.getOrInsertGlobal("__guard_check_icall_fptr", GuardFnPtrType);
   } else {
 assert(GuardMechanism == CF_Dispatch && "Invalid CFGuard mechanism");
-GuardFnGlobal = M.getOrInsertGlobal(
-  "__guard_dispatch_icall_fptr", GuardFnPtrType);
+GuardFnGlobal =
+M.getOrInsertGlobal("__guard_dispatch_icall_fptr", GuardFnPtrType);
   }
 
   return true;
Index: llvm/lib/Target/X86/X86TargetMachine.cpp
===
--- llvm/lib/Target/X86/X86TargetMachine.cpp
+++ llvm/lib/Target/X86/X86TargetMachine.cpp
@@ -419,7 +419,7 @@
   // Add Control Flow Guard checks.
   const Triple  = TM->getTargetTriple();
   if (TT.isOSWindows()) {
-if(TT.getArch() == Triple::x86_64) {
+if (TT.getArch() == Triple::x86_64) {
   addPass(createCFGuardDispatchPass());
 } else {
   addPass(createCFGuardCheckPass());
Index: llvm/lib/Target/X86/X86RegisterInfo.cpp
===
--- llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -343,8 +343,8 @@
 }
   case CallingConv::CFGuard_Check:
 assert(!Is64Bit && "CFGuard check mechanism only used on 32-bit X86");
-return (HasSSE ? CSR_Win32_CFGuard_Check_SaveList :
-   CSR_Win32_CFGuard_Check_NoSSE_SaveList);
+return (HasSSE ? CSR_Win32_CFGuard_Check_SaveList
+   : CSR_Win32_CFGuard_Check_NoSSE_SaveList);
   case CallingConv::Cold:
 if (Is64Bit)
   return CSR_64_MostRegs_SaveList;
@@ -461,8 +461,8 @@
 }
   case CallingConv::CFGuard_Check:
 assert(!Is64Bit && "CFGuard check mechanism only used on 32-bit X86");
-return (HasSSE ? CSR_Win32_CFGuard_Check_RegMask :
-   CSR_Win32_CFGuard_Check_NoSSE_RegMask);
+return (HasSSE ? CSR_Win32_CFGuard_Check_RegMask
+   : CSR_Win32_CFGuard_Check_NoSSE_RegMask);
   case CallingConv::Cold:
 if (Is64Bit)
   return CSR_64_MostRegs_RegMask;
Index: llvm/lib/Target/ARM/ARMCallingConv.h
===
--- llvm/lib/Target/ARM/ARMCallingConv.h
+++ llvm/lib/Target/ARM/ARMCallingConv.h
@@ -33,8 +33,8 @@
  CCValAssign::LocInfo LocInfo, ISD::ArgFlagsTy ArgFlags,
  CCState );
 bool 

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

[Andrew Paverd via Phabricator via cfe-commits]

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

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-fallback.c
  clang/test/Driver/cl-options.c
  llvm/docs/LangRef.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/LLVMBuild.txt
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/LLVMBuild.txt
  llvm/lib/Target/X86/LLVMBuild.txt
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/CFGuard/CMakeLists.txt
  llvm/lib/Transforms/CFGuard/LLVMBuild.txt
  llvm/lib/Transforms/CMakeLists.txt
  llvm/lib/Transforms/LLVMBuild.txt
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/calling-conventions.3.2.ll.bc
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard-setjmp.ll
  llvm/test/CodeGen/WinCFGuard/cfguard.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll
  llvm/test/CodeGen/X86/cfguard-x86-64-vectorcall.ll
  llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll

Index: llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
@@ -0,0 +1,43 @@
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; Control Flow Guard is currently only available on Windows
+
+
+; Test that Control Flow Guard checks are correctly added for x86 vector calls.
+define void @func_cf_vector_x86(void (%struct.HVA)* %0, %struct.HVA* %1) #0 {
+entry:
+  %2 = alloca %struct.HVA, align 8
+  %3 = bitcast %struct.HVA* %2 to i8*
+  %4 = bitcast %struct.HVA* %1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %3, i8* align 8 %4, i32 32, i1 false)
+  %5 = load %struct.HVA, %struct.HVA* %2, align 8
+  call x86_vectorcallcc void %0(%struct.HVA inreg %5)
+  ret void
+
+  ; X32-LABEL: func_cf_vector_x86
+  ; X32: 	 movl 12(%ebp), %eax
+  ; X32: 	 movl 8(%ebp), %ecx
+  ; X32: 	 movsd 24(%eax), %xmm4 # xmm4 = mem[0],zero
+  ; X32: 	 movsd %xmm4, 24(%esp)
+  ; X32: 	 movsd 16(%eax), %xmm5 # xmm5 = mem[0],zero
+  ; X32: 	 movsd %xmm5, 16(%esp)
+  ; X32: 	 movsd (%eax), %xmm6   # xmm6 = mem[0],zero
+  ; X32: 	 movsd 8(%eax), %xmm7  # xmm7 = mem[0],zero
+  ; X32: 	 movsd %xmm7, 8(%esp)
+  ; X32: 	 movsd %xmm6, (%esp)
+  ; X32: 	 calll *___guard_check_icall_fptr
+  ; X32: 	 movaps %xmm6, %xmm0
+  

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

[Reid Kleckner via Phabricator via cfe-commits]

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. When you upload the next diff, 
make sure to use the merge point with origin/master as the base, so that the 
patch will apply cleanly.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5959-5960
   CmdArgs.push_back("-cfguard");
-else if (NoChecks)
-  //Emit only the table of address-taken functions.
+}
+else if (GuardArgs.equals_lower("cf,nochecks")) {
+  // Emit only the table of address-taken functions.

Style nit: LLVM puts the close bracket on the same line as else pretty 
consistently. clang-format will make it so.



Comment at: llvm/lib/CodeGen/CFGuardLongjmp.cpp:101
+  // targets.
+  for (MachineInstr *Setjmp : SetjmpCalls) {
+MachineBasicBlock *OldMBB = Setjmp->getParent();

ajpaverd wrote:
> rnk wrote:
> > Rather than changing the machine CFG, I would suggest using 
> > MachineInstr::setPostInstrSymbol, which I've been planning to use for 
> > exactly this purpose. :) You can make labels with MCContext::createSymbol, 
> > and you'll want to come up with symbols that won't clash with C or C++ 
> > symbols. I see MSVC makes labels like `$LN4`, so maybe `$cfgsjN` or 
> > something like that. I think MCContext will add numbers for you.
> Thanks for the suggestion! It looks like MCContext::createSymbol is private, 
> so I'm generating a new symbol name and number for 
> MCContext::getOrCreateSymbol. Does this look OK? 
I guess I was thinking that createTempSymbol would work, but I see you are 
taking steps to make these labels public, so that one is not appropriate. If 
the name matters, then yes, I think getOrCreateSymbol is the right API.



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:220
+GuardDispatch =
+B.CreateCall(GuardDispatchType, GuardDispatchLoad, GuardDispatchArgs);
+  } else if (InvokeInst::classof(CB)) {

rnk wrote:
> You'd want to set the tail call kind here to forward `tail` or `musttail`. 
> This matters for things like member pointer function thunks. This C++ makes a 
> musttail thunk, for example:
>   struct A { virtual void f(); };
>   auto mp = ::f;
> Which gives this IR:
>   define linkonce_odr void @"??_9A@@$BA@AA"(%struct.A* %this, ...) #0 comdat 
> align 2 {
>   entry:
> %0 = bitcast %struct.A* %this to void (%struct.A*, ...)***
> %vtable = load void (%struct.A*, ...)**, void (%struct.A*, ...)*** %0, 
> align 8, !tbaa !3
> %1 = load void (%struct.A*, ...)*, void (%struct.A*, ...)** %vtable, 
> align 8
> musttail call void (%struct.A*, ...) %1(%struct.A* %this, ...)
> ret void
>   }
> This does an indirect virtual call through the 0th slot, and if we lose 
> musttail, it won't perfectly forward arguments.
> 
Thanks, I see the test case for this.



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:254
+  // register (i.e. RAX on 64-bit X86 targets)
+  GuardDispatch->setCallingConv(CallingConv::CFGuard_Dispatch);
+

ajpaverd wrote:
> rnk wrote:
> > So, this isn't going to work if the original calling convention was 
> > something other than the default. For x64, the only commonly used 
> > non-standard convention would be vectorcall. Changing the convention here 
> > in the IR is going to make it hard to pass that along.
> > 
> > I think as you can see from how much code you have here already, replacing 
> > call instructions in general is really hard. These days there are also 
> > bundles, which I guess is something else missing from the above.
> > 
> > Here's a sketch of an alternative approach:
> > - load __guard_dispatch_icall_fptr as normal
> > - get the pre-existing function type of the callee
> > - cast the loaded pointer to the original function pointer type
> > - use `CB->setCalledOperand` to replace the call target
> > - add the original call target as an "argument bundle" to the existing 
> > instruction
> > - change SelectionDAGBuilder.cpp to recognize the new bundle kind and 
> > create a new ArgListEntry in the argument list
> > - make and set a new bit in ArgListEntry, something like isCFGTarget
> > - change CC_X86_Win64_C to put CFGTarget arguments in RAX, similar to how 
> > CCIfNest puts things in R10
> > 
> > This way, the original call instruction remains with exactly the same 
> > arrangement of args, attributes, callbr successors, tail call kind, etc. 
> > All you're doing is changing the callee, and passing the original callee as 
> > an extra argument on the side. Basically, this approach leverages bundles 
> > to avoid messing with the function type, which more or less requires 
> > replacing the call. :)
> > 
> > There are probably issues with this design 

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

[Andrew Paverd via Phabricator via cfe-commits]

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]>,
+  HelpText<"Emit Windows Control Flow Guard tables only (no checks).">;

hans wrote:
> rnk wrote:
> > @hans, WDYT about the -cc1 interface? I think this is fine.
> The -cc1 interface looks good to me, but since these are cc1 options only, I 
> think they should be in CC1Options.td, not Options.td (I realize this is a 
> pre-existing issue with -cfguard).
Thanks for all the helpful feedback @hans. I think I've addressed all your 
comments in the latest revision.



Comment at: llvm/lib/CodeGen/CFGuardLongjmp.cpp:101
+  // targets.
+  for (MachineInstr *Setjmp : SetjmpCalls) {
+MachineBasicBlock *OldMBB = Setjmp->getParent();

rnk wrote:
> Rather than changing the machine CFG, I would suggest using 
> MachineInstr::setPostInstrSymbol, which I've been planning to use for exactly 
> this purpose. :) You can make labels with MCContext::createSymbol, and you'll 
> want to come up with symbols that won't clash with C or C++ symbols. I see 
> MSVC makes labels like `$LN4`, so maybe `$cfgsjN` or something like that. I 
> think MCContext will add numbers for you.
Thanks for the suggestion! It looks like MCContext::createSymbol is private, so 
I'm generating a new symbol name and number for MCContext::getOrCreateSymbol. 
Does this look OK? 



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:
+///

rnk wrote:
> hans wrote:
> > Naive question: Why do we generate code as in the examples in the first 
> > place, and can't some general optimization pass do this folding? From the 
> > examples it looks like straight-forward constant propagation.
> Actually, I used this test IR, LLVM seems to always fold the memory operand 
> into the call:
> ```
> @fptr = external dso_local global void()*
> define i32 @foo() {
>   %fp1 = load void()*, void()** @fptr
>   call void %fp1()
>   %fp2 = load void()*, void()** @fptr
>   call void %fp2()
>   ret i32 0
> }
> ```
> 
> Maybe it won't do it if there are more parameters, I'm not sure.
> 
> I ran llc with both isels for x64 and ia32, and it always folded the load 
> into the call. Maybe it's best to make this a verification pass that emits an 
> error via MCContext if there is an unfolded load of the CFG check function 
> pointer?
I'm seeing this when compiling with optimizations disabled. When I run llc with 
`-fast-isel=false`, the following slightly modified IR does not fold the memory 
operand into the call:

```
@fptr = external dso_local global void()*
define i32 @foo() #0 {
  %fp1 = load void()*, void()** @fptr
  call void %fp1()
  %fp2 = load void()*, void()** @fptr
  call void %fp2()
  ret i32 0
}
attributes #0 = { noinline optnone }
```

It looks like this is caused by checks for `OptLevel == CodeGenOpt::None` in:

  - SelectionDAGISel::IsLegalToFold
  - X86DAGToDAGISel::IsProfitableToFold
  - X86DAGToDAGISel::PreprocessISelDAG (in this case OptLevel != 
CodeGenOpt::None)

I guess this is not high priority as it only happens at -O0. Should I look into 
enabling these specific optimizations when CFG is enabled, or just emit a 
warning when this happens?



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:254
+  // register (i.e. RAX on 64-bit X86 targets)
+  GuardDispatch->setCallingConv(CallingConv::CFGuard_Dispatch);
+

rnk wrote:
> So, this isn't going to work if the original calling convention was something 
> other than the default. For x64, the only commonly used non-standard 
> convention would be vectorcall. Changing the convention here in the IR is 
> going to make it hard to pass that along.
> 
> I think as you can see from how much code you have here already, replacing 
> call instructions in general is really hard. These days there are also 
> bundles, which I guess is something else missing from the above.
> 
> Here's a sketch of an alternative approach:
> - load __guard_dispatch_icall_fptr as normal
> - get the pre-existing function type of the callee
> - cast the loaded pointer to the original function pointer type
> - use `CB->setCalledOperand` to replace the call target
> - add the original call target as an "argument bundle" to the existing 
> instruction
> - change SelectionDAGBuilder.cpp to recognize the new bundle kind and create 
> a new ArgListEntry in the argument list
> - make and set a new bit in ArgListEntry, something like isCFGTarget
> - change CC_X86_Win64_C to put CFGTarget arguments in RAX, similar to how 
> CCIfNest puts things in R10
> 
> This way, the original call instruction remains with exactly the same 
> 

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

[Andrew Paverd via Phabricator via cfe-commits]

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 think I've been able to address 
most of your comments in the latest update. Your suggested approach of using 
operand bundles for the dispatch mechanism instead of changing calling 
conventions works well and makes the design a lot cleaner.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Andrew Paverd via Phabricator via cfe-commits]

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. Instruction
selection adds this target as a new ArgListEntry and sets a new 
`isCFGuardTarget` bit. CC_X86_Win64_C puts this argument in RAX.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-fallback.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86FixupCFGuard.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard-setjmp.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll

Index: llvm/test/CodeGen/X86/cfguard-module-flag.ll
===
--- llvm/test/CodeGen/X86/cfguard-module-flag.ll
+++ llvm/test/CodeGen/X86/cfguard-module-flag.ll
@@ -1,9 +1,7 @@
 
 ; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
 ; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefix=X64
-
-; This test is disabled on Linux
-; UNSUPPORTED: linux
+; Control Flow Guard is currently only available on Windows
 
 ; Test that Control Flow Guard checks are not added in modules with the 
 ; cfguard=1 flag (emit tables but no checks).
Index: llvm/test/CodeGen/X86/cfguard-checks.ll
===
--- llvm/test/CodeGen/X86/cfguard-checks.ll
+++ llvm/test/CodeGen/X86/cfguard-checks.ll
@@ -1,8 +1,6 @@
 ; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
 ; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefix=X64
-
-; This test is disabled on Linux
-; UNSUPPORTED: linux
+; Control Flow Guard is currently only available on Windows
 
 ; Test that Control Flow Guard checks are correctly added when required.
 
@@ -32,7 +30,8 @@
 attributes #0 = { nocf_check }
 
 
-; Test that Control Flow Guard checks are correctly added as single instructions even at -O0.
+; Test that Control Flow Guard checks are added even at -O0.
+; FIXME Ideally these checks should be added as a single call instruction, as in the optimized case.
 define i32 @func_optnone_cf() #1 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
@@ -46,13 +45,15 @@
 	; X32: 	 leal  _target_func, %eax
 	; X32: 	 movl  %eax, (%esp)
 	; X32: 	 movl  (%esp), %ecx
-	; X32: 	 calll *___guard_check_icall_fptr
+	; X32: 	 movl ___guard_check_icall_fptr, %eax
+	; X32: 	 calll *%eax
 	; X32-NEXT:  calll *%ecx
 
   ; On x86_64, __guard_dispatch_icall_fptr tail calls the function, so there should be only one call instruction.
   ; X64-LABEL: func_optnone_cf
   ; X64:   leaq	target_func(%rip), %rax
-  ; X64:   callq *__guard_dispatch_icall_fptr(%rip)
+  ; X64:   movq __guard_dispatch_icall_fptr(%rip), %rcx
+  ; X64:   callq *%rcx
   ; X64-NOT:   callq
 }
 attributes #1 = { noinline optnone }
@@ -120,5 

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

[Reid Kleckner via Phabricator via cfe-commits]

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 in the first 
> place, and can't some general optimization pass do this folding? From the 
> examples it looks like straight-forward constant propagation.
Actually, I used this test IR, LLVM seems to always fold the memory operand 
into the call:
```
@fptr = external dso_local global void()*
define i32 @foo() {
%fp1 = load void()*, void()** @fptr
call void %fp1()
%fp2 = load void()*, void()** @fptr
call void %fp2()
ret i32 0
}
```

Maybe it won't do it if there are more parameters, I'm not sure.

I ran llc with both isels for x64 and ia32, and it always folded the load into 
the call. Maybe it's best to make this a verification pass that emits an error 
via MCContext if there is an unfolded load of the CFG check function pointer?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Hans Wennborg via Phabricator via cfe-commits]

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<["-"], "b">, Flags<[Unsupported]>;
+def cfguard_no_checks : Flag<["-"], "cfguard-no-checks">, Flags<[CC1Option]>,
+  HelpText<"Emit Windows Control Flow Guard tables only (no checks).">;

rnk wrote:
> @hans, WDYT about the -cc1 interface? I think this is fine.
The -cc1 interface looks good to me, but since these are cc1 options only, I 
think they should be in CC1Options.td, not Options.td (I realize this is a 
pre-existing issue with -cfguard).



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:493
+  }
   if (CodeGenOpts.ControlFlowGuard) {
+// Function ID tables and checks for Control Flow Guard (cfguard=2).

Maybe check for this first, and then the ControlFlowGuardNoChecks one in an 
else-if, to make this one take precedence?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5971
+
+if (Instrument && !NoChecks)
+  //Emit CFG instrumentation and the table of address-taken functions.

It seems unfortunate that the Instrument and NoChecks variables allow four 
different states. whereas I think the logic should really only allow for three: 
1) do nothing, 2) tables only, 3) tables and checks. Maybe we could have an 
enum instead?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5972
+if (Instrument && !NoChecks)
+  //Emit CFG instrumentation and the table of address-taken functions.
   CmdArgs.push_back("-cfguard");

rnk wrote:
> Comment formatting needs to be fixed, you can use clang-format for this.
Also I'd suggest braces since even though there's just one statement, there are 
multiple lines here and in the else block.



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:420
+  // Flags that can simply be passed through.
+  Args.AddAllArgs(CmdArgs, options::OPT__SLASH_guard);
 

Thanks for remembering /fallback. Please add a /guard:cf option to the first 
test in clang/test/Driver/cl-fallback.c



Comment at: llvm/docs/LangRef.rst:441
+This calling convention is used for the Control Flow Guard check function,
+   which can be inserted before indirect calls to check that the call 
target 
+   is a valid function address. The check function has no return value, but

Indentation looks a little off here.

"function, which can be inserted before indirect calls" -- maybe instead 
"function, calls to which can be inserted ..."



Comment at: llvm/docs/LangRef.rst:447
+
+- On X86_32 the target address is passed in ECX.
+- On ARM the target address is passed in R0.

I don't think X86_32 is common nomenclature.. maybe just X86 is enough.



Comment at: llvm/docs/LangRef.rst:456
+(architecture-specific) register. All other function arguments are 
+   passed as usual.
+

Indentation off here too.



Comment at: llvm/docs/LangRef.rst:458
+
+- On X86-64 the target address is passed in RAX.
 "``cc ``" - Numbered convention

Maybe explicitly call out in the text that this calling convention is only used 
on x86_64, and is used instead of cfguard_checkcc (iiuc)?



Comment at: llvm/include/llvm/IR/CallingConv.h:79
+/// Special calling convention on Windows for calling the Control
+/// Guard Check ICall funtion. The function take exactly one argument
+/// (address of the target function) passed in the first argument register,

nit: s/take/takes/



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:381
 
+  // Emit tables for any value of cfguard flag (i.e. cfguard=1 or cfguard=2)
   if (mdconst::extract_or_null(

ultra nit: period at the end, here and for other 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:
+///

Naive question: Why do we generate code as in the examples in the first place, 
and can't some general optimization pass do this folding? From the examples it 
looks like straight-forward constant propagation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Reid Kleckner via Phabricator via cfe-commits]

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<["-"], "b">, Flags<[Unsupported]>;
+def cfguard_no_checks : Flag<["-"], "cfguard-no-checks">, Flags<[CC1Option]>,
+  HelpText<"Emit Windows Control Flow Guard tables only (no checks).">;

@hans, WDYT about the -cc1 interface? I think this is fine.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5972
+if (Instrument && !NoChecks)
+  //Emit CFG instrumentation and the table of address-taken functions.
   CmdArgs.push_back("-cfguard");

Comment formatting needs to be fixed, you can use clang-format for this.



Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:179
+assert((!CachedMCSymbol || LabelIsPublic == true) && "Cannot set label to 
public after it has been accessed.");
+LabelIsPublic = true; 
+  }

I hesitate to make MBB labels public, I think it might be better to build your 
own MCSymbol label with a public name.



Comment at: llvm/lib/CodeGen/CFGuardLongjmp.cpp:101
+  // targets.
+  for (MachineInstr *Setjmp : SetjmpCalls) {
+MachineBasicBlock *OldMBB = Setjmp->getParent();

Rather than changing the machine CFG, I would suggest using 
MachineInstr::setPostInstrSymbol, which I've been planning to use for exactly 
this purpose. :) You can make labels with MCContext::createSymbol, and you'll 
want to come up with symbols that won't clash with C or C++ symbols. I see MSVC 
makes labels like `$LN4`, so maybe `$cfgsjN` or something like that. I think 
MCContext will add numbers for you.



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:11
+/// This file contains the machine instruction transform to fixup the Control
+/// Flow Guard checks inserted by /X86CFGuard.cpp. This pass searches
+/// for such cases and replaces the pair of instructions with a single

I guess it's not X86CFGuard.cpp now that you've generalized it as a 
cross-platform IR pass.



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:57-58
+
+/// MachineFunction pass to fix x86 Control Flow Guard checks that have been
+/// generated as separate load and call/invoke instructions.
+class X86FixupCFGuard : public MachineFunctionPass {

Can you elaborate on what the correctness requirement is? The correctness 
requirement is just that the loaded value is not saved in a CSR which is then 
spilled and reloaded from memory across a call site. If possible, I think it 
would be worth teaching both FastISel and SDISel to build the correct 
MachineInstrs during instruction selection, and turn this into a validation 
pass that fails if it sees indirect calls through registers in functions where 
CFG is enabled.



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:114
+OldCallOpcode = X86::CALL64r;
+NewCallOpcode = X86::CALL64m;
+  } else {

I think you probably also want to look for tail calls. I think the best way to 
do this would be to use `MI.isCall()` in the loop, and then switch on the 
opcode to handle all call variants.



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:134
+  // load instruction
+  for (auto I = LoadMI.getIterator(), E = MBB->instr_end(); I != E; ++I) {
+MachineInstr  = cast(*I);

Is it possible to use MachineRegisterInfo to iterate over all the uses of the 
load, instead of scanning the BB? It seems like this could be O(n^2) if a 
single BB contains a long sequence of indirect calls. I think it would look 
like this:
  MachineRegisterInfo *MRI = MF.getRegInfo();
  for (MachineInstr  : MRI->use_instructions(Reg))
// do something with MI
There's also use_operands.



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:168-170
+  // Erase all marked call/invoke instructions
+  for (MachineInstr *MI : MIsToErase) {
+MI->eraseFromParent();

Erasing at the end is always the safest way to do it. :)



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:209
+  for (MachineInstr *LoadMI : GuardLoads) {
+fixupGuardLoad(*LoadMI, TII);
+

This doesn't do anything with the return value, should it?



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:38
+/// architectures (e.g. X86_32 uses cf_check, X86_64 uses cf_dispatch).
+class CFGuard : public ModulePass {
+public:

Does this need to be a ModulePass? It's preferable to avoid module passes if 
possible. When the pass manager has a sequence of function passes to run, it 
runs all passes on function f, then g, then h, and so on, rather than running 
the function pass over the entire module and 

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

[Andrew Paverd via Phabricator via cfe-commits]

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 Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Reid Kleckner via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Andrew Paverd via Phabricator via cfe-commits]

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 function calls, using either the check or dispatch mechanism. These
checks require new calling conventions for the supported targets (currently
x86, ARM, and AArch64). An additional pass (Target/X86/X86FixupCFGuard.cpp) is
used on x86 to prevent stack spills between loading and calling the check, which
could be exploited. Another pass (CodeGen/CFGuardLongjmp.cpp) is used to
identify and emit valid longjmp targets, as required by /guard:cf.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65761

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-options.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/LLVMBuild.txt
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/LLVMBuild.txt
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/LLVMBuild.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86FixupCFGuard.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/CFGuard/CMakeLists.txt
  llvm/lib/Transforms/CFGuard/LLVMBuild.txt
  llvm/lib/Transforms/CMakeLists.txt
  llvm/lib/Transforms/LLVMBuild.txt
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/calling-conventions.3.2.ll.bc
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard-setjmp.ll
  llvm/test/CodeGen/WinCFGuard/cfguard.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll

Index: llvm/test/CodeGen/X86/cfguard-module-flag.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-module-flag.ll
@@ -0,0 +1,28 @@
+
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefix=X64
+
+; This test is disabled on Linux
+; UNSUPPORTED: linux
+
+; Test that Control Flow Guard checks are not added in modules with the 
+; cfguard=1 flag (emit tables but no checks).
+
+
+declare void @target_func()
+
+define void @func_in_module_without_cfguard() #0 {
+entry:
+  %func_ptr = alloca void ()*, align 8
+  store void ()* @target_func, void ()** %func_ptr, align 8
+  %0 = load void ()*, void ()** %func_ptr, align 8
+
+  call void %0()
+  ret void
+
+  ; X32-NOT: __guard_check_icall_fptr
+  ; X64-NOT: __guard_dispatch_icall_fptr
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"cfguard", i32 1}
\ No newline at end of file
Index: llvm/test/CodeGen/X86/cfguard-checks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-checks.ll
@@ -0,0 +1,124 @@
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefix=X64
+
+; This test is disabled on