KCFI sanitizer

[samitolvanen]

Summary

The KCFI sanitizer, enabled with -fsanitize=kcfi, implements a
forward-edge control flow integrity scheme for indirect calls. It
uses a !kcfi_type metadata node to attach a type identifier for each
function and injects verification code before indirect calls.

Unlike the current CFI schemes implemented in LLVM, KCFI does not
require LTO, does not alter function references to point to a jump
table, and never breaks function address equality. KCFI is intended
to be used in low-level code, such as operating system kernels,
where the existing schemes can cause undue complications because
of the aforementioned properties. However, unlike the existing
schemes, KCFI is limited to validating only indirect calls and is
not compatible with executable-only memory.

KCFI does not provide runtime support, but always traps when a
type mismatch is encountered. Users of the scheme are expected
to handle the trap. With -fsanitize=kcfi, Clang emits a kcfi
operand bundle to indirect calls, and LLVM lowers this to a
known architecture-specific sequence of instructions for each
callsite to make runtime patching easier for users who require this
functionality.

A KCFI type identifier is a 32-bit constant produced by taking the
lower half of xxHash64 from a C++ mangled typename. If a program
contains indirect calls to assembly functions, they must be
manually annotated with the expected type identifiers to prevent
errors. To make this easier, Clang generates a weak SHN_ABS
__kcfi_typeid_<function> symbol for each address-taken function
declaration, which can be used to annotate functions in assembly
as long as at least one C translation unit linked into the program
takes the function address. For example on AArch64, we might have
the following code:

.c:
  int f(void);
  int (*p)(void) = f;
  p();

.s:
  .4byte __kcfi_typeid_f
  .global f
  f:
    ...

Note that X86 uses a different preamble format for compatibility
with Linux kernel tooling. See the comments in
X86AsmPrinter::emitKCFITypeId for details.

As users of KCFI may need to locate trap locations for binary
validation and error handling, LLVM can additionally emit the
locations of traps to a .kcfi_traps section.

Similarly to other sanitizers, KCFI checking can be disabled for
a function with a no_sanitize("kcfi") function attribute.

[samitolvanen]

llvm/lib/Target/X86/X86ISelLowering.h

83 ↗

(On Diff #433795)

Is there a reason why you really need specific KCFI_ nodes instead of only embedding the hash information into an attribute at the Machine Instruction?

This implementation is similar to CALL_RVMARKER, CALL_BTI and basically all other pseudo call instructions in LLVM. Is adding an attribute to MachineInstr the preferred approach instead?

I would also suggest that you at least don't name these as "KCFI_something", as in the future others might want to reuse the same structure for other CFI approaches.

Always happy to hear suggestions for alternative naming. Did you have something in mind?

[joaomoreira]

llvm/lib/Target/X86/X86ISelLowering.h

83 ↗

(On Diff #433795)

This implementation is similar to CALL_RVMARKER, CALL_BTI and basically all other pseudo call instructions in LLVM. Is adding an attribute to MachineInstr the preferred approach instead?

My understanding is that if, every time a new mitigation or optimization comes in, you create a new opcode for it, it will eventually bloat to non-feasibility.

Imagine you have some mitigation like kguard being implemented. Now you can have calls which are KCFI checked but not KGUARD checked; then KCFI not-checked but KGUARD checked; then KCFI and KGUARD checked.; then none-checked. And then you need all these variations for tail calls (which imho is a first, minor, instance of the problem)...

So, in general, my understanding is that this approach works, yeah, but that in the long term it could become a hassle... so ideally we should use attributes to define these sub-specific instructions instead of opcodes.

I would also suggest that you at least don't name these as "KCFI_something", as in the future others might want to reuse the same structure for other CFI approaches.

Always happy to hear suggestions for alternative naming. Did you have something in mind?

I think switching from KCFI into CFI would already be good enough, as in the end these are all implementing the control-flow integrity concept.

[samitolvanen]

llvm/lib/Target/X86/X86ISelLowering.h

83 ↗

(On Diff #433795)

My understanding is that if, every time a new mitigation or optimization comes in, you create a new opcode for it, it will eventually bloat to non-feasibility.

I do agree, if there are enough call variants that can be combined, this will quickly get messy. So far this probably just hasn't been an issue. I'm not particularly happy about the multiple pseudo instructions here either.

@pcc, any thoughts about the best way to make this cleaner? Should we switch to a MachineInstr attribute, or perhaps pass another operand with the specific call type, or something else?

so ideally we should use attributes to define these sub-specific instructions instead of opcodes.

One concern I have with using an attribute is that we have to ensure that no pass accidentally drops it while transforming the call instruction or replacing it with something else. That's not an issue if we have a separate pseudo instruction with the type as an operand. Did you run into any issues with this?

Also, how do you serialize the type attribute in MIR? Something similar to debug-instr-number?

I think switching from KCFI into CFI would already be good enough

Sure, sounds good to me. I'll change the naming once we have a consensus on the correct approach here.

[MaskRay]

Please don't land the patch this state. There are several questions which should be sorted out first.

a) Naming

About the 'k' prefix: this is generic and does not need to be coupled with "kernel",
but perhaps an argument can be made that the 'k' does not need to refer to "kernel".

b) -fsanitize=function (D1338)

It is a very similar feature but uses prolog data (after the function label)
instead of prefix data (before the function label), with some limitation that
only available on x86 and restricted to C++.
I am thinking whether we can lift the C++ restriction (we'll need to move away from
CGM.GetAddrOfRTTIDescriptor) and switch to prefix data (easier to add more ports). @pcc

c) inliner

Add this change to enabling inlining

--- i/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ w/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1776,6 +1776,8 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
         continue;
       if (Tag == LLVMContext::OB_clang_arc_attachedcall)
         continue;
+      if (Tag == LLVMContext::OB_kcfi)
+        continue;
 
       return InlineResult::failure("unsupported operand bundle");
     }

d) x86-64 scheme

Why is the following scheme picked? How are the double-int3 before and after movl useful?

        .type   test,@function
        .globl  __cfi_test                      # @test
        .type   __cfi_test,@function
__cfi_test:
        int3
        int3
        movl    $917620134, %eax                # imm = 0x36B1C5A6
        int3
        int3

e) __kcfi_typeid_*

This deserves a better description in the summary.

Is it useful to suppress the typeid symbol for _Z (if some C++ projects want to use this feature)? A mangled name has encoded the type information.

IIUC the current scheme only works when two TUs have address-taken declarations of the same name but with different signatures,
and done by a linker warning.

ELF linkers don't error for two SHN_ABS STB_GLOBAL symbols of the same st_value.
When the st_value fields differ, there will be a diagnostic. If needed, the specialized diagnostic can be added there.
But I'd prefer the lld patch to be separate and be properly tested (I add myself as a reviewer for lld/ELF changes to catch possibly unintended usage, sorry!)

% cat a.s
.globl foo
.set foo, 1
% cat b.s
.globl foo
.set foo, 2
% ld.lld a.o b.o
ld.lld: error: duplicate symbol: foo
>>> defined in a.o
>>> defined in b.o

clang/docs/ControlFlowIntegrity.rst

314 ↗

(On Diff #433795)

Don't repeat the heading.

clang/lib/CodeGen/CodeGenModule.cpp

2201

if (!(llvm::isAlnum(C) || C == '_' || C == '.'))
  return false;

6495

T

clang/lib/CodeGen/CodeGenModule.h

1399

Unlikely Emit*, there are many set* functions which use the correct case, so there is no excuse using the wrong case.

clang/lib/Driver/SanitizerArgs.cpp

699

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1339 ↗

(On Diff #433795)

early return

1345 ↗

(On Diff #433795)

Use getCodePointerSIze().

A 4-byte PC-relative relocation is insufficient if text and data are more than 31-bit away.

llvm/lib/MC/MCObjectFileInfo.cpp

1130 ↗

(On Diff #433795)

Remove SHF_WRITE. The section doesn't need a dynamic relocation.

llvm/lib/Target/X86/X86AsmPrinter.cpp

135 ↗

(On Diff #433795)

For a temporary symbol (STB_LOCAL), no need to add __ prefix.

llvm/test/CodeGen/AArch64/kcfi-bti.ll

4 ↗

(On Diff #433795)

Add a test with "patchable-function-entry"="2"

[samitolvanen]

In D119296#3562409, @MaskRay wrote:

a) Naming

About the 'k' prefix: this is generic and does not need to be coupled with "kernel",
but perhaps an argument can be made that the 'k' does not need to refer to "kernel".

This scheme was specifically designed for the kernel, hence the name. Clang has other CFI schemes for user space, and while this might be useful for other low-level software, everyone else is probably better off using -fsanitize=cfi. I'm always happy to hear ideas for improving naming, of course!

c) inliner

Add this change to enabling inlining

Good catch, I'll fix that in the next version.

d) x86-64 scheme

Why is the following scheme picked? How are the double-int3 before and after movl useful?

This preamble format was specifically requested by x86 kernel maintainers to avoid special casing objtool and to accommodate runtime patching. I'll add a comment to explain this.

e) __kcfi_typeid_*

This deserves a better description in the summary.

Sure, I'll add better description.

Is it useful to suppress the typeid symbol for _Z (if some C++ projects want to use this feature)? A mangled name has encoded the type information.

It's not, we still need a symbol that can be referenced directly in assembly code without having to compute the actual hash.

IIUC the current scheme only works when two TUs have address-taken declarations of the same name but with different signatures,
and done by a linker warning.

Yes, this will build, but an indirect call from the TU with an incorrect declaration will result in a runtime error.

ELF linkers don't error for two SHN_ABS STB_GLOBAL symbols of the same st_value.
When the st_value fields differ, there will be a diagnostic. If needed, the specialized diagnostic can be added there.

OK, so we could just not make the symbols weak and end up failing at link time if there's a mismatch. That sounds reasonable to me.

But I'd prefer the lld patch to be separate and be properly tested (I add myself as a reviewer for lld/ELF changes to catch possibly unintended usage, sorry!)

Sure, I'll drop the lld change from this patch and we can improve the error message later. I'll address the remaining inline comments you had in the next version as well. Thanks for taking a look!

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1345 ↗

(On Diff #433795)

Use getCodePointerSIze().

A 4-byte PC-relative relocation is insufficient if text and data are more than 31-bit away.

I did use that earlier, but the kernel maintainers specifically requested that I use a 32-bit offset instead to reduce memory overhead. The kernel uses this scheme also for static call addresses, so the 31-bit limit doesn't seem to be a concern in practise.

[samitolvanen]

• Addressed Fangrui's feedback.
• Renamed KCFI_* DAG nodes and pseudo instructions to CFI_* for now based on Joao's feedback. Still looking for more feedback on the best way to implement this part.

lld/ELF/Symbols.cpp

553 ↗

(On Diff #433795)

Dropped the lld changes for now, and will send another patch to improve the error message later.

llvm/test/CodeGen/AArch64/kcfi-bti.ll

4 ↗

(On Diff #433795)

Added the "patchable-function-entry"="2" test to AArch64/kcfi.ll.

[samitolvanen]

In D119296#3562409, @MaskRay wrote:

ELF linkers don't error for two SHN_ABS STB_GLOBAL symbols of the same st_value.
When the st_value fields differ, there will be a diagnostic. If needed, the specialized diagnostic can be added there.

After testing this a bit further, it looks like this won't work with LTO where we're not yet linking ELF object files. For example:

$ llvm-modextract -b -n 0 -o - sound/soc/sh/rcar/dma.o | llvm-dis | grep __kcfi_typeid_rsnd_mod_get_status
module asm ".globl __kcfi_typeid_rsnd_mod_get_status"
module asm ".set __kcfi_typeid_rsnd_mod_get_status, 463955820"
$ llvm-modextract -b -n 0 -o - sound/soc/sh/rcar/cmd.o | llvm-dis | grep __kcfi_typeid_rsnd_mod_get_status
module asm ".globl __kcfi_typeid_rsnd_mod_get_status"
module asm ".set __kcfi_typeid_rsnd_mod_get_status, 463955820"
$ ld.lld -r -o test.o sound/soc/sh/rcar/dma.o sound/soc/sh/rcar/cmd.o
ld.lld: error: duplicate symbol: __kcfi_typeid_rsnd_mod_get_status
>>> defined in sound/soc/sh/rcar/dma.o
>>> defined in sound/soc/sh/rcar/cmd.o

Do you have any thoughts on how to solve this without going back to a weak symbol?

[samitolvanen]

• Per Fangrui's request, added comments explaining the X86 preamble format, and a note about it to the patch summary.
• Switched back to .weak for the __kcfi_typeid_ symbols to fix compatibility with LTO. Whether we need a warning for symbol value mismatches and how this should be implemented can be addressed later. The warning isn't critical for the functionality and mismatches should be extremely rare.

[ychen]

clang/lib/CodeGen/CodeGenModule.cpp

2192

FYI: using prefix data may not work for the C++ coroutine. (https://github.com/llvm/llvm-project/issues/49689) because corosplit pass may clone coro functions and change its function type. But prefix data is opaque, so there is no way to detect this, and then drop the prefix data in the corosplit pass.

If this is a concern for now or for the near future, it might be better to use alternative approaches like D115844.

[samitolvanen]

clang/lib/CodeGen/CodeGenModule.cpp

2192

FYI: using prefix data may not work for the C++ coroutine.

Thanks for the link, that's interesting. The Linux kernel doesn't use C++ so this isn't a concern there, but I suppose there could theoretically also be C++ users for this feature. @pcc, any thoughts if we should just switch from prefix data to a metadata node here?

[samitolvanen]

Rebased after ToT member function name changes.

[samitolvanen]

Switched from prefix data + attribute to a metadata node based on previous discussion. This seems to be a cleaner solution overall.

[samitolvanen]

Fixed the debug output in InstCombine to use metadata as well.

[pcc]

clang/lib/CodeGen/CodeGenModule.cpp

2192

I think the issue with coroutines was more about the fact that the function was cloned (invalidating the relative reference in the prefix data) than that the function type was changed. I seem to recall from a discussion with the coroutine folks that the functions created by the coroutine pass with different types aren't actually intended to be called directly from C/C++, so the fact that the types are changed doesn't matter.

I think the main advantage of metadata here would be in doing things like the check for type mismatches in direct calls more easily, so it seems like a reasonable approach even though the rationale is different.

6505

It would be better to have a separate function for computing the KCFI type ids than to try and reuse this one, since you don't need the MDStrings (i.e. unnecessary string uniquing) in your new caller. It also isn't appropriate to pass MetadataIdMap in your new caller because you'll end up with inconsistent values added to MetadataIdMap if we end up calling the other caller (e.g. if both CFI and KCFI are enabled), but that's moot if you avoid it.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

2453 ↗

(On Diff #439137)

For PAuth ABI the motivation for avoiding the gap between check and call was around avoiding spilling the verified pointer, is this not possible with KCFI?

llvm/lib/Target/X86/X86AsmPrinter.cpp

119 ↗

(On Diff #439137)

If this is just about satisfying objtool do these symbols need to be exported or can they just be STB_LOCAL?

llvm/lib/Target/X86/X86ISelLowering.h

83 ↗

(On Diff #433795)

Would it make sense to add a field to MachineInstr::ExtraInfo to hold the additional information here? That way you can avoid increasing the size of MachineInstr.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

3093 ↗

(On Diff #439137)

With CFI, if this situation occurs we unconditionally crash, the rationale being that if we end up in this situation it is a situation unexpected by the developer and may even be intended to be unreachable at runtime, so it's best to crash if it does occur. The same approach is used for things like UBSan integer overflow checks if the optimizer figures out that the overflow will always happen. If I'm understanding the code correctly KCFI will just let the call occur in this case and I'm not sure if that's a good idea.

[samitolvanen]

clang/lib/CodeGen/CodeGenModule.cpp

6505

The code in SanitizerArgs.cpp doesn't allow both CFI and KCFI to be enabled at the same time, but you're right, it's probably better to just split this into a separate function for KCFI. I'll do that in the next version.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

2453 ↗

(On Diff #439137)

I don't believe that's a concern here. We might emit instructions for setting up call arguments between the check and the call, but nothing should spill the pointer or change the target register anymore. The main reason to avoid bundling the check and the call is to avoid further complications with expanding the remaining pseudo instructions.

llvm/lib/Target/X86/X86AsmPrinter.cpp

119 ↗

(On Diff #439137)

They don't have to be exported, but we ran into some objtool confusion when the parent function was weak, so it's easier to just use the same linkage for the preamble symbol.

llvm/lib/Target/X86/X86ISelLowering.h

83 ↗

(On Diff #433795)

I'll look into it. Per offline discussion, we might need a similar construction for SDNode to avoid increasing memory usage too much should we go with type hash attributes instead.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

3093 ↗

(On Diff #439137)

The difference here is that unlike with CFI, we never emit KCFI checks for direct calls in the back-end. While this code drops unnecessary kcfi operand bundles from the IR, it doesn't actually change the machine code we generate.

Also, I would argue that KCFI should focus only on runtime protection of actual indirect calls, and otherwise shouldn't trap on potential undefined behavior that already exists. Causing a runtime failure for something we can detect at compile-time and don't aim to protect against doesn't sound ideal, especially in the kernel where we prefer to avoid crashing unnecessarily. I did consider changing this to a warning though, so the developers are notified of potential issues, but opted for debug logging for now. Thoughts?

[samitolvanen]

• Moved to a standalone function for generating KCFI type hashes in Clang.
• Switched from pseudo instructions to a MachineInstr::ExtraInfo + SDNode attributes.
• Added KCFI passes for emitting the indirect call checks.

llvm/lib/Target/X86/X86ISelLowering.h

83 ↗

(On Diff #433795)

In my testing, adding a uint32_t attribute to SDNode increased max RSS by ~0.15% during a kernel build. As there's no ExtraInfo type construction in SDNode currently, adding one would increase memory usage more. Perhaps we can look into that if more attributes are needed?

[nickdesaulniers]

I see you modified the mir parser & printer; consider adding a .mir test.

Still LGTM. Might be nice to document the generated asm more for other compiler vendors to better understand the implementation, rather than having to read the tests added here.

clang/lib/CodeGen/CodeGenFunction.h

4595

Should this parameter be an ArrayRef? This might be a more precise type, but seeing an impl type kind of breaks the whole pImpl idiom; I'm pretty sure that's what ArrayRef is for.
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-arrayref-h
That said, this header is pretty inconsistent in the use of SmallVectorImp vs ArrayRef, but I *think* ArrayRef is strongly preferred.

llvm/lib/CodeGen/MachineInstr.cpp

1770–1771 ↗

(On Diff #440389)

If FirstOp is false, reassign it false? Is that right? I see that's what's done on L1763, but..."what?"

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7821–7830 ↗

(On Diff #440389)

Can we check whether the CallBase is an indirect call first before bothering with getting the operand bundle?

7822–7840 ↗

(On Diff #440389)

Are we able to reorder these?

CLI.setDebugLoc ...
  ...
if (Bundle && CB.isIndirectCall) {
  ...
  CLI.setCFIType ...

llvm/lib/Target/AArch64/AArch64KCFI.cpp

106 ↗

(On Diff #440389)

const auto &SubTarget = ...

llvm/lib/Target/X86/X86KCFI.cpp

110 ↗

(On Diff #440389)

const auto &SubTarget = ...

[samitolvanen]

Added a MIR parser test for cfi-type, address Nick's other feedback.

[samitolvanen]

In D119296#3623059, @nickdesaulniers wrote:

I see you modified the mir parser & printer; consider adding a .mir test.

I added a .mir test for parsing the cfi-type. The machine instructions generated for the various call types are covered in the .ll tests.

Might be nice to document the generated asm more for other compiler vendors to better understand the implementation, rather than having to read the tests added here.

The exact instruction sequence is also documented in the kernel patches in case other compiler vendors want to implement this, but sure, that makes sense. I assume comments in the various AsmPrinter functions would be sufficient?

clang/lib/CodeGen/CodeGenFunction.h

4595

I don't believe I can use ArrayRef when I need to append to the list, it appears to be read-only.

llvm/lib/CodeGen/MachineInstr.cpp

1770–1771 ↗

(On Diff #440389)

Ah yes, it looks like this has been copied a few times earlier already. I'll stop the cycle.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7822–7840 ↗

(On Diff #440389)

We can reorder these, but this does follow the same convention as the rest of the function. I assume CallLoweringInfo was specifically designed so that the arguments can all be initialized in one statement.

[MaskRay]

It uses LLVM prefix data to store a type identifier for each function and injects verification code before indirect calls.

Is "prefix data" stale now?

[MaskRay]

There are 30+ comments not marked as "done". Having so many is distracting to reviewers as it's unclear whether this is in a ready-for-review state.
I personally definitely don't consider this in a ready-for-land state.

clang/include/clang/Basic/Features.def

226

move before modules for an alphabetical order.

[MaskRay]

clang/lib/CodeGen/CodeGenModule.cpp

2187

delete blank line

2193

delete blank line

clang/test/CodeGen/kcfi.c

3

If -O2 has different behavior, having the test in clang/test/CodeGen is likely a layering violation.
Optimization tests should be placed in llvm/test/, in the relevant pass. Folks working on llvm optimizations generally don't want to test every frontend.

clang/test/Driver/fsanitize.c

670

Use modern spelling --target=. -target is deprecated

[MaskRay]

clang/lib/CodeGen/CodeGenModule.h

1401

End with a period.

This function can be lower-case as well. There is no strong precedent to keep it upper-case.

clang/lib/Driver/SanitizerArgs.cpp

63

This is incorrect.

If a violation is found, ud2 is executed. ud2 is not followed by normal control flow so I don't think recovery from the error is supported.

This seems like Unrecoverable

clang/test/CodeGen/kcfi.c

3

If may be useful to have a -x c++ test.

Add a not-address-taken external linkage function.

8

Optional: COM: is fine. But to make non-RUN non-CHECK lines stand out (from editor highlighting), a simpler /// suffices as well.

llvm/docs/LangRef.rst

7222 ↗

(On Diff #441546)

[MaskRay]

llvm/lib/Target/X86/X86AsmPrinter.cpp

125 ↗

(On Diff #441546)

A previous comment isn't done. This doesn't explain that the double-int3 before and after the identifier is an objtool requirement (or similar).

[samitolvanen]

Addressed Fangrui's feedback.

[samitolvanen]

In D119296#3625796, @MaskRay wrote:

It uses LLVM prefix data to store a type identifier for each function and injects verification code before indirect calls.

Is "prefix data" stale now?

Yes, I forgot to update the commit message here. Fixed.

There are 30+ comments not marked as "done". Having so many is distracting to reviewers as it's unclear whether this is in a ready-for-review state.

My bad, I wasn't familiar with the convention here. I've just been marking my replies as done.

I personally definitely don't consider this in a ready-for-land state.

I left some questions about your comments below. PTAL.

[samitolvanen]

clang/lib/Driver/SanitizerArgs.cpp

63

This variable is only used to indicate whether -fno-sanitize-recover command line parameter can be used with the sanitizer. It makes no sense to allow this with KCFI as we always emit a recoverable instruction sequence, hence it's included here.

Also, ud2 absolutely is recoverable in the kernel, and Linux specifically uses ud2 to trigger warnings in assembly code.

clang/test/CodeGen/kcfi.c

3

If -O2 has different behavior, having the test in clang/test/CodeGen is likely a layering violation.

Good point, the -O2 test isn't actually relevant anymore as we no longer rely on it. I'll drop it.

Optimization tests should be placed in llvm/test/, in the relevant pass.

Yes, the InstCombine test below covers this already.

3

If may be useful to have a -x c++ test.

Would you mind elaborating on this? The main difference with -x c++ is the name mangling, I'm not sure if that adds much value in this case.

Add a not-address-taken external linkage function.

Added.

clang/test/Driver/fsanitize.c

670

Use modern spelling --target=. -target is deprecated

It doesn't look like --target works here:

clang-15: error: unsupported option '--target'; did you mean '-target'?

Also the rest of the file uses -target, so perhaps these can all be updated at once when the flag is deprecated?

llvm/lib/Target/X86/X86AsmPrinter.cpp

125 ↗

(On Diff #441546)

A previous comment isn't done. This doesn't explain that the double-int3 before and after the identifier is an objtool requirement (or similar).

These are not objtool requirements, and I believe the comments do explain why the int3 padding is added. The first two are to reserve some space for runtime patching this code, and the latter two is to avoid call target gadgets in the checks. I'm obviously happy to improve the comments, is there something specific you'd like me to add?

[samitolvanen]

Addressed the rest of Fangrui's comments after clarifying the open questions with him offline.

[MaskRay]

clang/lib/Driver/SanitizerArgs.cpp

63

ud2 being recoverable in the kernel is insufficient. The IR should consider this recoverable. In the presence of a failure, the control flow should be transferred as if no failure happens. E.g. for an asan out-of-bounds failure, the code should behave as if the failure is ignored.

[samitolvanen]

clang/lib/Driver/SanitizerArgs.cpp

63

ud2 being recoverable in the kernel is insufficient. The IR should consider this recoverable. In the presence of a failure, the control flow should be transferred as if no failure happens. E.g. for an asan out-of-bounds failure, the code should behave as if the failure is ignored.

Sure, that's exactly now the code we emit here works as well. There are no traps emitted to the IR code, we only add an operand bundle to indirect calls, which doesn't affect the control flow as far as the IR is concerned.

[samitolvanen]

• Added a missing equivalency check to MachineInstr.

[MaskRay]

Mostly looks good.

clang/lib/CodeGen/CodeGenModule.cpp

2200

Use llvm::all_of or llvm::any_of

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

332 ↗

(On Diff #444022)

Add const. Delete blank line after the declaration.

350 ↗

(On Diff #444022)

Add const

384 ↗

(On Diff #444022)

delete blank line

401 ↗

(On Diff #444022)

delete blank line

llvm/lib/Target/AArch64/AArch64KCFI.cpp

62 ↗

(On Diff #444022)

delete blank line

103 ↗

(On Diff #444022)

delete blank line

llvm-project doesn't typically add a blank line after a variable declaration. please fix other places in this patch.

111 ↗

(On Diff #444022)

delete blank line

117 ↗

(On Diff #444022)

don't use early return if the then part is simple.

llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

1183 ↗

(On Diff #444022)

} else if (Info.CFIType) {
  ...
}

llvm/lib/Target/X86/X86KCFI.cpp

101 ↗

(On Diff #444022)

delete blank line

107 ↗

(On Diff #444022)

MaskRayUnsubmittedDone
delete blank line

115 ↗

(On Diff #444022)

delete blank line

121 ↗

(On Diff #444022)

Fix this in a way similar to AArch64.

[samitolvanen]

Addressed Fangrui's feedback.

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

332 ↗

(On Diff #444022)

Deleted the blank line, but this can't be const because it's assigned to in the if statement below.

[MaskRay]

Worth waiting a bit and checking whether @pcc has more comments.

[samitolvanen]

• Simplified AArch64 by dropping the KCFI_CHECK_BTI pseudo and just using a different caller-saved temporary register if needed.
• Added -verify-machineinstrs to llc tests.

[samitolvanen]

Based on the recent LKML discussions about X86 retbleed mitigations (https://lore.kernel.org/lkml/[e-mail]/), we're going to need some changes to the code generated for X86.

[MaskRay]

(Will read new updates.)

Note that release/15.x is branching soon (https://discourse.llvm.org/t/llvm-15-0-0-release-schedule/63495): 2022-07-26.
It seems that we can miss the branch to have more time mature the feature and have it for 16.0.

[samitolvanen]

Addressed conflicts with X86 retbleed mitigations (https://lore.kernel.org/lkml/[e-mail]/):

1. Changed the type check instruction sequence emitted by X86AsmPrinter::LowerKCFI_CHECK not to include the full constant, which allows us to freely position the function preamble without worrying about call target gadgets at indirect call sites.
2. Changed the lowering code to take patchable-function-prefix into account, and allowed -fpatchable-function-entry=N,M where M>0 to be used in Clang with KCFI.
3. As we must maintain alignment of the function entry on X86 to avoid performance regressions (https://lore.kernel.org/lkml/87ilnuuiw8.ffs@tglx/), changed the preamble padding to ensure the function entry remains aligned with KCFI, also when combined with patchable-function-prefix.

[samitolvanen]

In D119296#3673297, @MaskRay wrote:

It seems that we can miss the branch to have more time mature the feature and have it for 16.0.

Sure, that sounds reasonable.

[joaomoreira]

I really like this revision. It removes the redundancy of having KCFI passes both in CodeGen and in the backend; it detangles CALL instructions from KCFI by creating a new MIR instruction; it fixes alignment while still supporting the -fpatchable-function-entry option; it doesn't add hashes/gadgets through the code as it was before needed by the use of cmp instructions. With all this said, the patch LGTM.

I tested the patch compiling toy snippets and also compiling the kernel and found no issues.

Some minor suggestions/not so relevant:

• Change the name of CFIType to CFIHash, as it is probably more descriptive.
• Change the KCFI.* into CFI.* where the passes/structures can be re-used by a different CFI approach. For example, X86KCFI::emitCheck can be easily re-used by other CFI schemes. Similarly, the LowerKCFI_CHECK function can also be easily re-used. Because these other approaches are hypothetical by now, this is not a big deal... but may be subject to changes in the future, in case someone comes up with new schemes.
• Deny the ENDBR64/32 opcodes as valid hashes for the CFIType. There was an effort in the past to prevent it from being emitted as an immediate, thus, perhaps, we should also care. Since FineIBT may inherit hashes from KCFI, it would be great to prevent these from becoming valid indirect targets or needing to be checked.

Some relevant optimizations/improvements for future work:

• When LTO is used, hashes may be suppressed for non-local + non-address taken functions.
• As suggested by Andy Cooper, add a salt attribute that allows to differentiate hashes of functions with the same prototype.

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

617 ↗

(On Diff #447476)

I wonder if CFIHash would be a more descriptive name.

[samitolvanen]

• Ensured that we don't emit ENDBR64/32 as the type hash on X86.

[samitolvanen]

In D119296#3684022, @joaomoreira wrote:

Some minor suggestions/not so relevant:

• Change the name of CFIType to CFIHash, as it is probably more descriptive.

My thinking here was that Type is more generic than a Hash, but I don't really have a strong opinion about this.

• Change the KCFI.* into CFI.* where the passes/structures can be re-used by a different CFI approach. For example, X86KCFI::emitCheck can be easily re-used by other CFI schemes. Similarly, the LowerKCFI_CHECK function can also be easily re-used. Because these other approaches are hypothetical by now, this is not a big deal... but may be subject to changes in the future, in case someone comes up with new schemes.

Sure, it should be trivial for a future patch that wants to reuse this code to rename the functions and classes as needed.

• Deny the ENDBR64/32 opcodes as valid hashes for the CFIType. There was an effort in the past to prevent it from being emitted as an immediate, thus, perhaps, we should also care. Since FineIBT may inherit hashes from KCFI, it would be great to prevent these from becoming valid indirect targets or needing to be checked.

Good point. It should be quite unlikely that we'll end up randomly generating ENDBR as a hash, but I added a helper to the X86 code to ensure we don't end up emitting these values.

Some relevant optimizations/improvements for future work:

• When LTO is used, hashes may be suppressed for non-local + non-address taken functions.
• As suggested by Andy Cooper, add a salt attribute that allows to differentiate hashes of functions with the same prototype.

Agreed, and another request from Peter Z was to add a flag to omit ENDBR from functions with the !kcfi_type attribute. I think these can all be separate follow-up patches.

[joaomoreira]

llvm/lib/Target/X86/X86AsmPrinter.cpp

121 ↗

(On Diff #448454)

Can we use another constant blinding scheme, such as a Value++ or anything else? This way, we would prevent endbrs from being emitted in the indirect branch guards too.

Since we are using Value (prologue) and ~Value (caller/guard) for doing the checks, we also need to check if ~ENDBR was picked as a KCFIType, otherwise ENDBR will be emitted in the ibranch guards.

llvm/test/CodeGen/X86/kcfi.ll

91 ↗

(On Diff #448454)

We need to also ensure/test that these are not emitted in the caller/indirect branch guards.

I assume that in the current scheme (blinding with ~Value) would be unfeasible to do this, so maybe we need a different approach for masking (as suggested above).

[samitolvanen]

• Changed MaskKCFIType to also avoid negative invalid patterns and return Value + 1 for clarity.

llvm/lib/Target/X86/X86AsmPrinter.cpp

121 ↗

(On Diff #448454)

Can we use another constant blinding scheme, such as a Value++ or anything else? This way, we would prevent endbrs from being emitted in the indirect branch guards too.

Since we are using Value (prologue) and ~Value (caller/guard) for doing the checks, we also need to check if ~ENDBR was picked as a KCFIType, otherwise ENDBR will be emitted in the ibranch guards.

I don't mind changing this to Value + 1, but that actually doesn't change anything because we emit -Value in indirect call checks, not ~Value. Therefore, using ~Value works equally well here.

Specifically, this code currently emits ~Valuein the preamble and -(~Value) == Value + 1 in the indirect call check. Switching to Value + 1 simply reverses the order; we'll emit Value + 1 in the preamble and -(Value + 1) == ~Value in the indirect call check.

However, you are right that we also need to avoid -ENDBR in this function. I'll fix that and clarify the comment.

llvm/test/CodeGen/X86/kcfi.ll

91 ↗

(On Diff #448454)

Added a test for -ENDBR too.

[joaomoreira]

llvm/lib/Target/X86/X86AsmPrinter.cpp

121 ↗

(On Diff #448454)

Oops, got confused with the operands, tks for clearing it up. Otherwise, changes LGTM.

[MaskRay]

OK, so we could just not make the symbols weak and end up failing at link time if there's a mismatch. That sounds reasonable to me.

For STB_WEAK SHN_ABS __kcfi_typeid_*, there is no duplicate definition error. Is this behavior intentional?
Note: I don't think we should change lld to recognize some symbol prefix and enforce more rigid diagnostics.
An error must be implemented with existing ELF features.

clang/lib/CodeGen/CodeGenModule.cpp

2220–2222

if (const llvm::MDNode *MD = F.getMetadata(llvm::LLVMContext::MD_kcfi_type))
  Type = ...
else
  continue;

[MaskRay]

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

330 ↗

(On Diff #452339)

const?

331 ↗

(On Diff #452339)

delete blank line

[samitolvanen]

Addressed Fangrui's comments.

[samitolvanen]

In D119296#3733753, @MaskRay wrote:

For STB_WEAK SHN_ABS __kcfi_typeid_*, there is no duplicate definition error. Is this behavior intentional?

Yes, this is by design. Multiple translation units must be able to take the address of the same function declaration, and it must be possible to link them together.

Note: I don't think we should change lld to recognize some symbol prefix and enforce more rigid diagnostics.
An error must be implemented with existing ELF features.

Sure, that's reasonable. I don't think we need more diagnostics here as __kcfi_typeid_ value mismatches are unlikely, but should the need arise, we can figure out the best way to implement them later. Note that this must also work with LTO when linking bitcode files, so we can't rely on ELF features alone.

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

330 ↗

(On Diff #452339)

This can't be const because we assign to this array below in the for (auto &Reg : ScratchRegs) loop.

[efriedma]

clang/docs/ControlFlowIntegrity.rst

319 ↗

(On Diff #453838)

To clarify, by "indirect call checking", do you mean function pointers specifically? Or does this also work for C++ vtables/member function pointers?

[samitolvanen]

clang/docs/ControlFlowIntegrity.rst

319 ↗

(On Diff #453838)

Yes, function pointers only. I'll clarify this part.

[samitolvanen]

Clarified that KCFI checks only function pointers in the documentation.

[samitolvanen]

llvm/include/llvm/CodeGen/MachineInstr.h

265 ↗

(On Diff #455316)

This fails on 32-bit architectures as PointerEmbeddedInt doesn't allow storing 32 bits in a 32-bit pointer:

// Note: This '<' is correct; using '<=' would result in some shifts
// overflowing their storage types.
static_assert(Bits < sizeof(uintptr_t) * CHAR_BIT,
              "Cannot embed more bits than we have in a pointer!")

[samitolvanen]

llvm/include/llvm/CodeGen/MachineInstr.h

265 ↗

(On Diff #455316)

PointerSumType also needs space for a tag, which we don't have in a 32-bit pointer. Looks like we just need to always store CFIType in ExtraInfo to avoid this issue, similarly to HeapAllocMarker.

clang/docs/ClangCommandLineReference.rst

 .. option:: -fsanitize-cfi-cross-dso, -fno-sanitize-cfi-cross-dso
 
 Enable control flow integrity (CFI) checks for cross-DSO calls.
 
 .. option:: -fsanitize-cfi-icall-generalize-pointers
 
 Generalize pointers in CFI indirect call type signature checks
 
+.. option:: -fsanitize-kcfi-offset=<value>
+
+Set the number of bytes between the KCFI type identifier and the start of the function.
+
 .. option:: -fsanitize-coverage-allowlist=<arg>, -fsanitize-coverage-whitelist=<arg>
 
 Restrict sanitizer coverage instrumentation exclusively to modules and functions that match the provided special case list, except the blocked ones
 
 .. option:: -fsanitize-coverage-ignorelist=<arg>, -fsanitize-coverage-blacklist=<arg>
 
 Disable sanitizer coverage instrumentation for modules and functions that match the provided special case list, even the allowed ones
 

clang/docs/UsersManual.rst

-  .. _opt_fsanitize_undefined:
 
       ``-fsanitize=undefined``: :doc:`UndefinedBehaviorSanitizer`,
       a fast and compatible undefined behavior checker.
 
    -  ``-fsanitize=dataflow``: :doc:`DataFlowSanitizer`, a general data
       flow analysis.
    -  ``-fsanitize=cfi``: :doc:`control flow integrity <ControlFlowIntegrity>`
       checks. Requires ``-flto``.
+   -  ``-fsanitize=kcfi``: kernel indirect call forward-edge control flow
+      integrity.
    -  ``-fsanitize=safe-stack``: :doc:`safe stack <SafeStack>`
       protection against stack-based memory corruption errors.
 
    There are more fine-grained checks available: see
    the :ref:`list <ubsan-checks>` of specific kinds of
    undefined behavior that can be detected and the :ref:`list <cfi-schemes>`
    of control flow integrity schemes.
 
OPTIONS:
       -fsanitize-address-use-odr-indicator
                               Enable ODR indicator globals to avoid false ODR violation reports in partially sanitized programs at the cost of an increase in binary size
       -fsanitize-ignorelist=<value>
                               Path to ignorelist file for sanitizers
       -fsanitize-cfi-cross-dso
                               Enable control flow integrity (CFI) checks for cross-DSO calls.
       -fsanitize-cfi-icall-generalize-pointers
                               Generalize pointers in CFI indirect call type signature checks
+      -fsanitize-kcfi-offset=<value>
+                              Offset between the KCFI type identifier and the start of a function.
       -fsanitize-coverage=<value>
                               Specify the type of coverage instrumentation for Sanitizers
       -fsanitize-hwaddress-abi=<value>
                               Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor)
       -fsanitize-memory-track-origins=<value>
                               Enable origins tracking in MemorySanitizer
       -fsanitize-memory-track-origins
                               Enable origins tracking in MemorySanitizer

clang/include/clang/Basic/Attr.td

let Spellings = [
     // Intel's compiler on Windows also supports:
     // , Declspec<"align_value">
   ];
   let Args = [ExprArgument<"Alignment">];
   let Subjects = SubjectList<[Var, TypedefName]>;
   let Documentation = [AlignValueDocs];
 }
 
+def KCFIUnchecked : Attr {
+  let Spellings = [Clang<"kcfi_unchecked">];
+  let Subjects = SubjectList<[Var, TypedefName]>;
+  let Documentation = [KCFIUncheckedDocs];
+}
+
 def AlignMac68k : InheritableAttr {
   // This attribute has no spellings as it is only ever created implicitly.
   let Spellings = [];
   let SemaHandler = 0;
   let Documentation = [Undocumented];
 }
 
 def AlignNatural : InheritableAttr {

clang/include/clang/Basic/AttrDocs.td

.. code-block: c++
   int __attribute__((noderef)) &y = x;  // warning: 'noderef' can only be used on an array or pointer type
 
 .. code-block: objc
 
   id __attribute__((noderef)) obj = [NSObject new]; // warning: 'noderef' can only be used on an array or pointer type
 }];
 }
 
+def KCFIUncheckedDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{
+The ``kcfi_unchecked`` attribute causes Clang to not emit KCFI checks for indirect
+calls made through annotated function pointer variables or types.
+
+.. code-block:: c
+
+  int f1(void) { return 0; }
+
+  int (*p1)(void) = f1;
+  p1();  // checked
+
+  int (*p2)(void) __attribute__((kcfi_unchecked)) = f1;
+  p2();  // unchecked
+
+  typedef typeof(f1) * __attribute__((kcfi_unchecked) unchecked_t;
+  ((unchecked_t)p2)();  // unchecked
+
+  unchecked_t p3 = f1;
+  p3();  // unchecked
+
+``kcfi_unchecked`` is  only supported for function pointers and function pointer types.
+}];
+}
+
 def ReinitializesDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 The ``reinitializes`` attribute can be applied to a non-static, non-const C++
 member function to indicate that this member function reinitializes the entire
 object to a known state, independent of the previous state of the object.
 
 This attribute can be interpreted by static analyzers that warn about uses of an

clang/include/clang/Basic/CodeGenOptions.def

CODEGENOPT(SanitizeMemoryUseAfterDtor, 1, 0) ///< Enable use-after-delete detection
                                              ///< in MemorySanitizer
 CODEGENOPT(SanitizeCfiCrossDso, 1, 0) ///< Enable cross-dso support in CFI.
 CODEGENOPT(SanitizeMinimalRuntime, 1, 0) ///< Use "_minimal" sanitizer runtime for
                                          ///< diagnostics.
 CODEGENOPT(SanitizeCfiICallGeneralizePointers, 1, 0) ///< Generalize pointer types in
                                                      ///< CFI icall function signatures
 CODEGENOPT(SanitizeCfiCanonicalJumpTables, 1, 0) ///< Make jump table symbols canonical
                                                  ///< instead of creating a local jump table.
+CODEGENOPT(SanitizeKCFIOffset, 32, 0) ///< Define the KCFI type identifier offset from
+                                      ///< the start of the function.
 CODEGENOPT(SanitizeCoverageType, 2, 0) ///< Type of sanitizer coverage
                                        ///< instrumentation.
 CODEGENOPT(SanitizeCoverageIndirectCalls, 1, 0) ///< Enable sanitizer coverage
                                                 ///< for indirect calls.
 CODEGENOPT(SanitizeCoverageTraceBB, 1, 0) ///< Enable basic block tracing in
                                           ///< in sanitizer coverage.
 CODEGENOPT(SanitizeCoverageTraceCmp, 1, 0) ///< Enable cmp instruction tracing
                                            ///< in sanitizer coverage.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_attribute_return_pointers_only : Warning<
   "%0 attribute only applies to return values that are pointers">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_return_pointers_refs_only : Warning<
   "%0 attribute only applies to return values that are pointers or references">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_pointer_or_reference_only : Warning<
   "%0 attribute only applies to a pointer or reference (%1 is invalid)">,
   InGroup<IgnoredAttributes>;
+def err_attribute_function_pointers_only : Error<
+  "%0 attribute argument only applies to a function pointer or a function "
+  "pointer type">;
 def err_attribute_no_member_pointers : Error<
   "%0 attribute cannot be used with pointers to members">;
 def err_attribute_invalid_implicit_this_argument : Error<
   "%0 attribute is invalid for the implicit this argument">;
 def err_ownership_type : Error<
   "%0 attribute only applies to %select{pointer|integer}1 arguments">;
 def err_ownership_returns_index_mismatch : Error<
   "'ownership_returns' attribute index does not match; here it is %0">;

clang/include/clang/Basic/Features.def

 FEATURE(is_polymorphic, LangOpts.CPlusPlus)
 FEATURE(is_sealed, LangOpts.CPlusPlus &&LangOpts.MicrosoftExt)
 FEATURE(is_trivial, LangOpts.CPlusPlus)
 FEATURE(is_trivially_assignable, LangOpts.CPlusPlus)
 FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
 FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
 FEATURE(is_union, LangOpts.CPlusPlus)
 FEATURE(modules, LangOpts.Modules)
+FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI))
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))
 FEATURE(shadow_call_stack,
         LangOpts.Sanitize.has(SanitizerKind::ShadowCallStack))
 FEATURE(tls, PP.getTargetInfo().isTLSSupported())
 FEATURE(underlying_type, LangOpts.CPlusPlus)
 
 // C11 features supported by other languages as extensions.
 EXTENSION(c_alignas, true)

clang/include/clang/Basic/Sanitizers.def

 SANITIZER("cfi-mfcall", CFIMFCall)
 SANITIZER("cfi-unrelated-cast", CFIUnrelatedCast)
 SANITIZER("cfi-nvcall", CFINVCall)
 SANITIZER("cfi-vcall", CFIVCall)
 SANITIZER_GROUP("cfi", CFI,
                 CFIDerivedCast | CFIICall | CFIMFCall | CFIUnrelatedCast |
                     CFINVCall | CFIVCall)
 
+// Kernel Control Flow Integrity
+SANITIZER("kcfi", KCFI)
+
 // Safe Stack
 SANITIZER("safe-stack", SafeStack)
 
 // Shadow Call Stack
 SANITIZER("shadow-call-stack", ShadowCallStack)
 
 // -fsanitize=undefined includes all the sanitizers which have low overhead, no
 // ABI or address space layout implications, and only catch undefined behavior.

clang/include/clang/Driver/Options.td

def fsanitize_cfi_icall_generalize_pointers : Flag<["-"], "fsanitize-cfi-icall-generalize-pointers">,
                                               Group<f_clang_Group>,
                                               HelpText<"Generalize pointers in CFI indirect call type signature checks">,
                                               MarshallingInfoFlag<CodeGenOpts<"SanitizeCfiICallGeneralizePointers">>;
 defm sanitize_cfi_canonical_jump_tables : BoolOption<"f", "sanitize-cfi-canonical-jump-tables",
   CodeGenOpts<"SanitizeCfiCanonicalJumpTables">, DefaultFalse,
   PosFlag<SetTrue, [], "Make">, NegFlag<SetFalse, [CoreOption, NoXarchOption], "Do not make">,
   BothFlags<[], " the jump table addresses canonical in the symbol table">>,
   Group<f_clang_Group>;
+def fsanitize_kcfi_offset
+    : Joined<["-"], "fsanitize-kcfi-offset=">, MetaVarName<"<value>">,
+      Group<f_clang_Group>,
+      HelpText<"Assume there are <value> bytes between the KCFI type identifer and start of the function.">,
+      MarshallingInfoInt<CodeGenOpts<"SanitizeKCFIOffset">>;
 defm sanitize_stats : BoolOption<"f", "sanitize-stats",
   CodeGenOpts<"SanitizeStats">, DefaultFalse,
   PosFlag<SetTrue, [], "Enable">, NegFlag<SetFalse, [CoreOption, NoXarchOption], "Disable">,
   BothFlags<[], " sanitizer statistics gathering.">>,
   Group<f_clang_Group>;
 def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
                                      Group<f_clang_Group>,
                                      HelpText<"Enable memory access instrumentation in ThreadSanitizer (default)">;

clang/include/clang/Driver/SanitizerArgs.h

class SanitizerArgs {
   bool AsanPoisonCustomArrayCookie = false;
   bool AsanGlobalsDeadStripping = false;
   bool AsanUseOdrIndicator = false;
   bool AsanInvalidPointerCmp = false;
   bool AsanInvalidPointerSub = false;
   bool AsanOutlineInstrumentation = false;
   llvm::AsanDtorKind AsanDtorKind = llvm::AsanDtorKind::Invalid;
   std::string HwasanAbi;
+  unsigned int KCFIOffset = 0;
   bool LinkRuntimes = true;
   bool LinkCXXRuntimes = false;
   bool NeedPIE = false;
   bool SafeStackRuntime = false;
   bool Stats = false;
   bool TsanMemoryAccess = true;
   bool TsanFuncEntryExit = true;
   bool TsanAtomics = true;

clang/lib/CodeGen/CGExpr.cpp

 }
 
 const SanitizerHandlerInfo SanitizerHandlers[] = {
 #define SANITIZER_CHECK(Enum, Name, Version) {#Name, Version},
     LIST_SANITIZER_CHECKS
 #undef SANITIZER_CHECK
 };
 
+void CodeGenFunction::EmitKCFICheck(llvm::Value *Ptr, llvm::ConstantInt *Hash) {
+  uint64_t Offset = CGM.getCodeGenOpts().SanitizeKCFIOffset;
+  if (Offset)
+    Ptr = Builder
+              .CreateConstInBoundsGEP(
+                  Address(Builder.CreateBitCast(Ptr, Int8PtrTy),
+                          CGM.getPointerAlign()),
+                  -Offset)
+              .getPointer();
+
+  Address HashPtr = Builder.CreateConstInBoundsGEP(
+      Address(Builder.CreateBitCast(Ptr, Int64Ty->getPointerTo(0)),
+              CGM.getPointerAlign()),
+      -1);
+  llvm::Value *Test = Builder.CreateICmpEQ(Builder.CreateLoad(HashPtr), Hash);
+  llvm::BasicBlock *ContBB = createBasicBlock("kcfi.cont");
+  llvm::BasicBlock *FailBB = createBasicBlock("kcfi.fail");
+  llvm::Instruction *Branch = Builder.CreateCondBr(Test, ContBB, FailBB);
+
+  llvm::MDBuilder MDHelper(getLLVMContext());
+  Branch->setMetadata(llvm::LLVMContext::MD_prof,
+                      MDHelper.createBranchWeights((1U << 20) - 1, 1));
+
+  EmitBlock(FailBB);
+
+  llvm::FunctionCallee Handler = CGM.getModule().getOrInsertFunction(
+      "__kcfi_check_fail",
+      llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy}, false));
+  llvm::CallInst *Call = Builder.CreateCall(
+      Handler, {Hash, Builder.CreateBitCast(Ptr, Int8PtrTy)});
+
+  if (CGM.getCodeGenOpts().SanitizeRecover.has(SanitizerKind::KCFI))
+    Builder.CreateBr(ContBB);
+  else {
+    Call->setDoesNotReturn();
+    Call->setDoesNotThrow();
+    Builder.CreateUnreachable();
+  }
+
+  EmitBlock(ContBB);
+}
+
 static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                  llvm::FunctionType *FnType,
                                  ArrayRef<llvm::Value *> FnArgs,
                                  SanitizerHandler CheckHandler,
                                  CheckRecoverableKind RecoverKind, bool IsFatal,
                                  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
   Optional<ApplyDebugLocation> DL;
RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee,
   // Get the actual function type. The callee type will always be a pointer to
   // function type or a block pointer type.
   assert(CalleeType->isFunctionPointerType() &&
          "Call must have function pointer type!");
 
   const Decl *TargetDecl =
       OrigCallee.getAbstractInfo().getCalleeDecl().getDecl();
 
+  auto OrigType = CalleeType;
   CalleeType = getContext().getCanonicalType(CalleeType);
 
   auto PointeeType = cast<PointerType>(CalleeType)->getPointeeType();
 
   CGCallee Callee = OrigCallee;
 
   if (getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function) &&
       (!TargetDecl || !isa<FunctionDecl>(TargetDecl))) {
if (llvm::Constant *PrefixSig =
                 {CalleePtr, CalleeRTTI, FTRTTIConst});
 
       Builder.CreateBr(Cont);
       EmitBlock(Cont);
     }
   }
 
   const auto *FnType = cast<FunctionType>(PointeeType);
+  bool IsIndirectCall = !TargetDecl || !isa<FunctionDecl>(TargetDecl);
 
   // If we are checking indirect calls and this call is indirect, check that the
   // function pointer is a member of the bit set for the function type.
-  if (SanOpts.has(SanitizerKind::CFIICall) &&
-      (!TargetDecl || !isa<FunctionDecl>(TargetDecl))) {
+  if (SanOpts.has(SanitizerKind::CFIICall) && IsIndirectCall) {
     SanitizerScope SanScope(this);
     EmitSanitizerStatReport(llvm::SanStat_CFI_ICall);
 
     llvm::Metadata *MD;
     if (CGM.getCodeGenOpts().SanitizeCfiICallGeneralizePointers)
       MD = CGM.CreateMetadataIdentifierGeneralized(QualType(FnType, 0));
     else
       MD = CGM.CreateMetadataIdentifierForType(QualType(FnType, 0));
if (CGM.getCodeGenOpts().SanitizeCfiCrossDso && CrossDsoTypeId) {
                            CastedCallee, StaticData);
     } else {
       EmitCheck(std::make_pair(TypeTest, SanitizerKind::CFIICall),
                 SanitizerHandler::CFICheckFail, StaticData,
                 {CastedCallee, llvm::UndefValue::get(IntPtrTy)});
     }
   }
 
+  if (SanOpts.has(SanitizerKind::KCFI) && IsIndirectCall) {
+    auto *TargetType = dyn_cast<TypedefType>(OrigType);
+    if ((!TargetDecl || !TargetDecl->hasAttr<KCFIUncheckedAttr>()) &&
+        (!TargetType || !TargetType->getDecl()->hasAttr<KCFIUncheckedAttr>()))
+      EmitKCFICheck(Callee.getFunctionPointer(),
+                    CGM.CreateKCFITypeId(QualType(FnType, 0)));
+  }
+
   CallArgList Args;
   if (Chain)
     Args.add(RValue::get(Builder.CreateBitCast(Chain, CGM.VoidPtrTy)),
              CGM.getContext().VoidPtrTy);
 
   // C++17 requires that we evaluate arguments to a call using assignment syntax
   // right-to-left, and that we evaluate arguments to certain other operators
   // left-to-right. Note that we allow this to override the order dictated by

clang/lib/CodeGen/CodeGenFunction.h

public:
   /// Convert a value into a format suitable for passing to a runtime
   /// sanitizer handler.
   llvm::Value *EmitCheckValue(llvm::Value *V);
 
   /// Emit a description of a source location in a format suitable for
   /// passing to a runtime sanitizer handler.
   llvm::Constant *EmitCheckSourceLocation(SourceLocation Loc);
 
+  void EmitKCFICheck(llvm::Value *Ptr, llvm::ConstantInt *Hash);
+
   /// Create a basic block that will either trap or call a handler function in
   /// the UBSan runtime with the provided arguments, and create a conditional
   /// branch to it.
   void EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerMask>> Checked,
                  SanitizerHandler Check, ArrayRef<llvm::Constant *> StaticArgs,
                  ArrayRef<llvm::Value *> DynamicArgs);
 
   /// Emit a slow path cross-DSO CFI check which calls __cfi_slowpath

clang/lib/CodeGen/CodeGenModule.h

public:
   /// Emit type metadata for the given vtable using the given layout.
   void EmitVTableTypeMetadata(const CXXRecordDecl *RD,
                               llvm::GlobalVariable *VTable,
                               const VTableLayout &VTLayout);
 
   /// Generate a cross-DSO type identifier for MD.
   llvm::ConstantInt *CreateCrossDsoCfiTypeId(llvm::Metadata *MD);
 
+  /// Generate a KCFI type identifier for T.
+  llvm::ConstantInt *CreateKCFITypeId(QualType T);
+
   /// Create a metadata identifier for the given type. This may either be an
   /// MDString (for external identifiers) or a distinct unnamed MDNode (for
   /// internal identifiers).
   llvm::Metadata *CreateMetadataIdentifierForType(QualType T);
 
   /// Create a metadata identifier that is intended to be used to check virtual
   /// calls via a member function pointer.
   llvm::Metadata *CreateMetadataIdentifierForVirtualMemPtrType(QualType T);
 
   /// Create a metadata identifier for the generalization of the given type.
   /// This may either be an MDString (for external identifiers) or a distinct
   /// unnamed MDNode (for internal identifiers).
   llvm::Metadata *CreateMetadataIdentifierGeneralized(QualType T);
 
   /// Create and attach type metadata to the given function.
   void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
                                           llvm::Function *F);
 
+  /// Set type hash as prefix data to the given function
+  void SetKCFITypePrefix(const FunctionDecl *FD, llvm::Function *F);
+
+  /// Emit KCFI type identifier constants
+  void EmitKCFIConstants();
+
   /// Whether this function's return type has no side effects, and thus may
   /// be trivially discarded if it is unused.
   bool MayDropFunctionReturn(const ASTContext &Context, QualType ReturnType);
 
   /// Returns whether this module needs the "all-vtables" type identifier.
   bool NeedAllVtablesTypeId() const;
 
   /// Create and attach type metadata for the given vtable.
private:
   /// Helper function for ConstructAttributeList and
   /// addDefaultFunctionDefinitionAttributes.  Builds a set of function
   /// attributes to add to a function with the given properties.
   void getDefaultFunctionAttributes(StringRef Name, bool HasOptnone,
                                     bool AttrOnCallSite,
                                     llvm::AttrBuilder &FuncAttrs);
 
   llvm::Metadata *CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap &Map,
-                                               StringRef Suffix);
+                                               StringRef Suffix,
+                                               bool OnlyExternal = true);
 };
 
 }  // end namespace CodeGen
 }  // end namespace clang
 
 #endif // LLVM_CLANG_LIB_CODEGEN_CODEGENMODULE_H

clang/lib/CodeGen/CodeGenModule.cpp

 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/CodeGen/ConstantInitBuilder.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ProfileSummary.h"
 #include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/X86TargetParser.h"
+#include "llvm/Support/xxhash.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 static llvm::cl::opt<bool> LimitedCoverage(
     "limited-coverage-experimental", llvm::cl::ZeroOrMore, llvm::cl::Hidden,
     llvm::cl::desc("Emit limited coverage mapping information (experimental)"),
     llvm::cl::init(false));
void CodeGenModule::Release() {
   EmitDeferredUnusedCoverageMappings();
   CodeGenPGO(*this).setValueProfilingFlag(getModule());
   if (CoverageMapping)
     CoverageMapping->emit();
   if (CodeGenOpts.SanitizeCfiCrossDso) {
     CodeGenFunction(*this).EmitCfiCheckFail();
     CodeGenFunction(*this).EmitCfiCheckStub();
   }
+  if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+    EmitKCFIConstants();
   emitAtAvailableLinkGuard();
   if (Context.getTargetInfo().getTriple().isWasm() &&
       !Context.getTargetInfo().getTriple().isOSEmscripten()) {
     EmitMainVoidAlias();
   }
 
   // Emit reference of __amdgpu_device_library_preserve_asan_functions to
   // preserve ASAN functions in bitcode libraries.
   if (LangOpts.Sanitize.has(SanitizerKind::Address) && getTriple().isAMDGPU()) {

 
 llvm::ConstantInt *CodeGenModule::CreateCrossDsoCfiTypeId(llvm::Metadata *MD) {
   llvm::MDString *MDS = dyn_cast<llvm::MDString>(MD);
   if (!MDS) return nullptr;
 
   return llvm::ConstantInt::get(Int64Ty, llvm::MD5Hash(MDS->getString()));
 }
 
+llvm::ConstantInt *CodeGenModule::CreateKCFITypeId(QualType T) {
+  llvm::MDString *MDS = dyn_cast<llvm::MDString>(CreateMetadataIdentifierImpl(
+      T, MetadataIdMap, "", /*OnlyExternal=*/false));
+  if (!MDS)
+    return nullptr;
+
+  return llvm::ConstantInt::get(
+      Int64Ty,
+      (llvm::xxHash64(MDS->getString()) & (-1ULL >> 16)) | (0x10CFULL << 48));
+}
+
 void CodeGenModule::SetLLVMFunctionAttributes(GlobalDecl GD,
                                               const CGFunctionInfo &Info,
                                               llvm::Function *F, bool IsThunk) {
   unsigned CallingConv;
   llvm::AttributeList PAL;
   ConstructAttributeList(F->getName(), Info, GD, PAL, CallingConv,
                          /*AttrOnCallSite=*/false, IsThunk);
   F->setAttributes(PAL);
void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
   F->addTypeMetadata(0, CreateMetadataIdentifierGeneralized(FD->getType()));
 
   // Emit a hash-based bit set entry for cross-DSO calls.
   if (CodeGenOpts.SanitizeCfiCrossDso)
     if (auto CrossDsoTypeId = CreateCrossDsoCfiTypeId(MD))
       F->addTypeMetadata(0, llvm::ConstantAsMetadata::get(CrossDsoTypeId));
 }
 
+void CodeGenModule::SetKCFITypePrefix(const FunctionDecl *FD,
+                                      llvm::Function *F) {
+
+  if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic())
+    return;
+
+  F->setPrefixData(CreateKCFITypeId(FD->getType()));
+}
+
+static bool allowKCFIIdentifier(const std::string &Name) {
+  // KCFI type identifier constants are only necessary for external assembly
+  // functions, which means it's safe to skip unusual names. Subset of
+  // MCAsmInfo::isAcceptableChar() and MCAsmInfoXCOFF::isAcceptableChar().
+  for (const char &C : Name) {
+    if (llvm::isAlnum(C) || C == '_' || C == '.')
+      continue;
+    return false;
+  }
+  return true;
+}
+
+void CodeGenModule::EmitKCFIConstants() {
+  llvm::Module &M = getModule();
+  // Generate a weak constant with the expected KCFI type identifier for all
+  // address-taken function declarations.
+  for (auto &F : M.functions()) {
+    if (!F.isDeclaration() || !F.hasAddressTaken() || !F.hasPrefixData())
+      continue;
+
+    auto *Id = dyn_cast<llvm::ConstantInt>(F.getPrefixData());
+    if (!Id)
+      continue;
+
+    std::string Name = "__kcfi_typeid_" + F.getName().str();
+    if (!allowKCFIIdentifier(Name))
+      continue;
+
+    std::string Asm = ".weak " + Name + "\n" + ".set " + Name + ", " +
+                      std::to_string(Id->getSExtValue()) + "\n";
+    M.appendModuleInlineAsm(Asm);
+  }
+}
+
 void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
                                           bool IsIncompleteFunction,
                                           bool IsThunk) {
 
   if (llvm::Intrinsic::ID IID = F->getIntrinsicID()) {
     // If this is an intrinsic function, set the function's attributes
     // to the intrinsic's attributes.
     F->setAttributes(llvm::Intrinsic::getAttributes(getLLVMContext(), IID));
     return;
   }
 
   const auto *FD = cast<FunctionDecl>(GD.getDecl());
 
   if (!IsIncompleteFunction)
void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
   // Don't emit entries for function declarations in the cross-DSO mode. This
   // is handled with better precision by the receiving DSO. But if jump tables
   // are non-canonical then we need type metadata in order to produce the local
   // jump table.
   if (!CodeGenOpts.SanitizeCfiCrossDso ||
       !CodeGenOpts.SanitizeCfiCanonicalJumpTables)
     CreateFunctionTypeMetadataForIcall(FD, F);
 
+  if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+    SetKCFITypePrefix(FD, F);
+
   if (getLangOpts().OpenMP && FD->hasAttr<OMPDeclareSimdDeclAttr>())
     getOpenMPRuntime().emitDeclareSimdFunction(FD, F);
 
   if (const auto *CB = FD->getAttr<CallbackAttr>()) {
     // Annotate the callback behavior as metadata:
     //  - The callback callee (as argument number).
     //  - The callback payloads (as argument numbers).
     llvm::LLVMContext &Ctx = F->getContext();
for (auto RefExpr : D->varlists()) {
     if (auto InitFunction = getOpenMPRuntime().emitThreadPrivateVarDefinition(
             VD, Addr, RefExpr->getBeginLoc(), PerformInit))
       CXXGlobalInits.push_back(InitFunction);
   }
 }
 
 llvm::Metadata *
 CodeGenModule::CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap &Map,
-                                            StringRef Suffix) {
+                                            StringRef Suffix,
+                                            bool OnlyExternal /*=true*/) {
   if (auto *FnType = T->getAs<FunctionProtoType>())
     T = getContext().getFunctionType(
         FnType->getReturnType(), FnType->getParamTypes(),
         FnType->getExtProtoInfo().withExceptionSpec(EST_None));
 
   llvm::Metadata *&InternalId = Map[T.getCanonicalType()];
   if (InternalId)
     return InternalId;
 
-  if (isExternallyVisible(T->getLinkage())) {
+  if (isExternallyVisible(T->getLinkage()) || !OnlyExternal) {
     std::string OutName;
     llvm::raw_string_ostream Out(OutName);
     getCXXABI().getMangleContext().mangleTypeName(T, Out);
     Out << Suffix;
 
     InternalId = llvm::MDString::get(getLLVMContext(), Out.str());
   } else {
     InternalId = llvm::MDNode::getDistinct(getLLVMContext(),

clang/lib/Driver/SanitizerArgs.cpp

static const SanitizerMask NeedsUbsanRt =
     SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
     SanitizerKind::ObjCCast;
 static const SanitizerMask NeedsUbsanCxxRt =
     SanitizerKind::Vptr | SanitizerKind::CFI;
 static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
 static const SanitizerMask NotAllowedWithMinimalRuntime =
     SanitizerKind::Function | SanitizerKind::Vptr;
 static const SanitizerMask RequiresPIE =
-    SanitizerKind::DataFlow | SanitizerKind::HWAddress | SanitizerKind::Scudo;
+    SanitizerKind::DataFlow | SanitizerKind::HWAddress | SanitizerKind::Scudo |
+    SanitizerKind::KCFI;
 static const SanitizerMask NeedsUnwindTables =
     SanitizerKind::Address | SanitizerKind::HWAddress | SanitizerKind::Thread |
     SanitizerKind::Memory | SanitizerKind::DataFlow;
 static const SanitizerMask SupportsCoverage =
     SanitizerKind::Address | SanitizerKind::HWAddress |
     SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress |
     SanitizerKind::MemTag | SanitizerKind::Memory |
     SanitizerKind::KernelMemory | SanitizerKind::Leak |
     SanitizerKind::Undefined | SanitizerKind::Integer | SanitizerKind::Bounds |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
     SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
     SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack |
     SanitizerKind::Thread | SanitizerKind::ObjCCast;
 static const SanitizerMask RecoverableByDefault =
     SanitizerKind::Undefined | SanitizerKind::Integer |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
 static const SanitizerMask Unrecoverable =
     SanitizerKind::Unreachable | SanitizerKind::Return;
 static const SanitizerMask AlwaysRecoverable =
     SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress;
 static const SanitizerMask NeedsLTO = SanitizerKind::CFI;
 static const SanitizerMask TrappingSupported =
     (SanitizerKind::Undefined & ~SanitizerKind::Vptr) | SanitizerKind::Integer |
     SanitizerKind::Nullability | SanitizerKind::LocalBounds |
     SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
     SanitizerKind::ObjCCast;
 static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
 static const SanitizerMask CFIClasses =

 }
 
 static void addDefaultIgnorelists(const Driver &D, SanitizerMask Kinds,
                                   std::vector<std::string> &IgnorelistFiles,
                                   bool DiagnoseErrors) {
   struct Ignorelist {
     const char *File;
     SanitizerMask Mask;
-  } Ignorelists[] = {{"asan_ignorelist.txt", SanitizerKind::Address},
-                     {"hwasan_ignorelist.txt", SanitizerKind::HWAddress},
-                     {"memtag_ignorelist.txt", SanitizerKind::MemTag},
-                     {"msan_ignorelist.txt", SanitizerKind::Memory},
-                     {"tsan_ignorelist.txt", SanitizerKind::Thread},
-                     {"dfsan_abilist.txt", SanitizerKind::DataFlow},
-                     {"cfi_ignorelist.txt", SanitizerKind::CFI},
-                     {"ubsan_ignorelist.txt",
-                      SanitizerKind::Undefined | SanitizerKind::Integer |
-                          SanitizerKind::Nullability |
-                          SanitizerKind::FloatDivideByZero}};
+  } Ignorelists[] = {
+      {"asan_ignorelist.txt", SanitizerKind::Address},
+      {"cfi_ignorelist.txt", SanitizerKind::CFI | SanitizerKind::KCFI},
+           SanitizerKind::Nullability | SanitizerKind::FloatDivideByZero}};
 
   for (auto BL : Ignorelists) {
     if (!(Kinds & BL.Mask))
       continue;
 
     clang::SmallString<64> Path(D.ResourceDir);
     llvm::sys::path::append(Path, "share", BL.File);
     if (D.getVFS().exists(Path))
       IgnorelistFiles.push_back(std::string(Path.str()));
-    else if (BL.Mask == SanitizerKind::CFI && DiagnoseErrors)
+    else if (BL.Mask & SanitizerKind::CFI && DiagnoseErrors)
       // If cfi_ignorelist.txt cannot be found in the resource dir, driver
       // should fail.
       D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
   validateSpecialCaseListFormat(
       D, IgnorelistFiles, clang::diag::err_drv_malformed_sanitizer_ignorelist,
       DiagnoseErrors);
 }
if (AllAddedKinds & SanitizerKind::CFI) {
     CfiICallGeneralizePointers =
         Args.hasArg(options::OPT_fsanitize_cfi_icall_generalize_pointers);
 
     if (CfiCrossDso && CfiICallGeneralizePointers && DiagnoseErrors)
       D.Diag(diag::err_drv_argument_not_allowed_with)
           << "-fsanitize-cfi-cross-dso"
           << "-fsanitize-cfi-icall-generalize-pointers";
 
+    if (AllAddedKinds & SanitizerKind::KCFI && DiagnoseErrors)
+      D.Diag(diag::err_drv_argument_not_allowed_with) << "-fsanitize=cfi*"
+                                                      << "-fsanitize=kcfi";
+
     CfiCanonicalJumpTables =
         Args.hasFlag(options::OPT_fsanitize_cfi_canonical_jump_tables,
                      options::OPT_fno_sanitize_cfi_canonical_jump_tables, true);
   }
 
+  if (AllAddedKinds & SanitizerKind::KCFI)
+    if (Arg *A = Args.getLastArg(options::OPT_fsanitize_kcfi_offset)) {
+      StringRef S = A->getValue();
+      if (S.getAsInteger(0, KCFIOffset) && DiagnoseErrors)
+        D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
+    }
+
   Stats = Args.hasFlag(options::OPT_fsanitize_stats,
                        options::OPT_fno_sanitize_stats, false);
 
   if (MinimalRuntime) {
     SanitizerMask IncompatibleMask =
         Kinds & ~setGroupBits(CompatibleWithMinimalRuntime);
     if (IncompatibleMask && DiagnoseErrors)
       D.Diag(clang::diag::err_drv_argument_not_allowed_with)
           << "-fsanitize-minimal-runtime"
           << lastArgumentForMask(D, Args, IncompatibleMask);
 
     SanitizerMask NonTrappingCfi = Kinds & SanitizerKind::CFI & ~TrappingKinds;
if (CfiCrossDso)
     CmdArgs.push_back("-fsanitize-cfi-cross-dso");
 
   if (CfiICallGeneralizePointers)
     CmdArgs.push_back("-fsanitize-cfi-icall-generalize-pointers");
 
   if (CfiCanonicalJumpTables)
     CmdArgs.push_back("-fsanitize-cfi-canonical-jump-tables");
 
+  if (KCFIOffset)
+    CmdArgs.push_back(
+        Args.MakeArgString("-fsanitize-kcfi-offset=" + Twine(KCFIOffset)));
+
   if (Stats)
     CmdArgs.push_back("-fsanitize-stats");
 
   if (MinimalRuntime)
     CmdArgs.push_back("-fsanitize-minimal-runtime");
 
   if (AsanFieldPadding)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-address-field-padding=" +

clang/lib/Driver/ToolChain.cpp

SanitizerMask Res =
       SanitizerKind::UnsignedShiftBase | SanitizerKind::ImplicitConversion |
       SanitizerKind::Nullability | SanitizerKind::LocalBounds;
   if (getTriple().getArch() == llvm::Triple::x86 ||
       getTriple().getArch() == llvm::Triple::x86_64 ||
       getTriple().getArch() == llvm::Triple::arm || getTriple().isWasm() ||
       getTriple().isAArch64())
     Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
+      getTriple().isAArch64(64))
+    Res |= SanitizerKind::KCFI;
+  if (getTriple().getArch() == llvm::Triple::x86_64 ||
       getTriple().isAArch64(64) || getTriple().isRISCV())
     Res |= SanitizerKind::ShadowCallStack;
   if (getTriple().isAArch64(64))
     Res |= SanitizerKind::MemTag;
   return Res;
 }
 
 void ToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,

clang/lib/Sema/SemaDeclAttr.cpp

void Sema::AddModeAttr(Decl *D, const AttributeCommonInfo &CI,
   else if (auto *ED = dyn_cast<EnumDecl>(D))
     ED->setIntegerType(NewTy);
   else
     cast<ValueDecl>(D)->setType(NewTy);
 
   D->addAttr(::new (Context) ModeAttr(Context, CI, Name));
 }
 
+static void handleKCFIUncheckedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  QualType Type;
+
+  if (auto *TD = dyn_cast<TypedefNameDecl>(D))
+    Type = TD->getUnderlyingType();
+  else if (auto *VD = dyn_cast<VarDecl>(D))
+    if (auto *TSI = VD->getTypeSourceInfo())
+      Type = TSI->getType();
+
+  if (Type.isNull() || !Type->isFunctionPointerType()) {
+    S.Diag(D->getBeginLoc(), diag::err_attribute_function_pointers_only) << AL;
+    return;
+  }
+
+  D->addAttr(::new (S.Context) KCFIUncheckedAttr(S.Context, AL));
+}
+
 static void handleNoDebugAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(::new (S.Context) NoDebugAttr(S.Context, AL));
 }
 
 AlwaysInlineAttr *Sema::mergeAlwaysInlineAttr(Decl *D,
                                               const AttributeCommonInfo &CI,
                                               const IdentifierInfo *Ident) {
   if (OptimizeNoneAttr *Optnone = D->getAttr<OptimizeNoneAttr>()) {
case ParsedAttr::AT_CUDALaunchBounds:
     handleLaunchBoundsAttr(S, D, AL);
     break;
   case ParsedAttr::AT_Restrict:
     handleRestrictAttr(S, D, AL);
     break;
   case ParsedAttr::AT_Mode:
     handleModeAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_KCFIUnchecked:
+    handleKCFIUncheckedAttr(S, D, AL);
+    break;
   case ParsedAttr::AT_NonNull:
     if (auto *PVD = dyn_cast<ParmVarDecl>(D))
       handleNonNullAttrParameter(S, PVD, AL);
     else
       handleNonNullAttr(S, D, AL);
     break;
   case ParsedAttr::AT_ReturnsNonNull:
     handleReturnsNonNullAttr(S, D, AL);

clang/test/CodeGen/kcfi.c

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck --check-prefixes=CHECK,OFFSET0,FATAL %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-recover=kcfi -o - %s | FileCheck --check-prefixes=CHECK,OFFSET0,RECOVER %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-kcfi-offset=1 -fpatchable-function-entry=1 -fpatchable-function-entry-offset=1 -o - %s | FileCheck --check-prefixes=CHECK,OFFSET1 %s
+#if !__has_feature(kcfi)
+#error Missing kcfi?
+#endif
+
+// CHECK: module asm ".weak __kcfi_typeid_f4"
+// CHECK: module asm ".set __kcfi_typeid_f4, [[#%d,HASH:]]"
+
+typedef int (*fn_t)(void);
+
+// CHECK: define dso_local i32 @f1(){{.*}} prefix i64 [[#HASH]]
+int f1(void) { return 0; }
+
+// CHECK: define dso_local i32 @f2(){{.*}} prefix i64 [[#%d,HASH2:]]
+unsigned int f2(void) { return 2; }
+
+// CHECK-LABEL: define dso_local i32 @__call(i32 ()*{{.*}} %f)
+int __call(fn_t f) __attribute__((__no_sanitize__("kcfi"))) {
+  // CHECK-NOT: kcfi.fail
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK-NOT: kcfi.cont
+  // CHECK: %call = call i32 %[[#]]()
+  return f();
+}
+
+// CHECK-LABEL: define dso_local i32 @call(i32 ()*{{.*}} %f)
+int call(fn_t f) {
+  // OFFSET1: %[[#I8CAST:]] = bitcast i32 ()* %{{.}} to i8*
+  // OFFSET1: %[[#OFFSET:]] = getelementptr inbounds i8, i8* %[[#I8CAST]], i64 -1
+  // OFFSET1: %[[#CAST:]] = bitcast i8* %[[#OFFSET]] to i64*
+  // OFFSET0: %[[#CAST:]] = bitcast i32 ()* %{{.}} to i64*
+  // CHECK: %[[#GEPI:]] = getelementptr inbounds i64, i64* %[[#CAST]], i64 -1
+  // CHECK: %[[#LOAD:]] = load i64, i64* %[[#GEPI]], align 8
+  // CHECK: %[[#ICMP:]] = icmp eq i64 %[[#LOAD]], [[#HASH]]
+  // CHECK: br i1 %[[#ICMP]], label %kcfi.cont, label %kcfi.fail
+  // CHECK-LABEL: kcfi.fail{{.*}}:
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]], i8*
+  // FATAL: unreachable
+  // RECOVER: br label %kcfi.cont
+  // CHECK-LABEL: kcfi.cont{{.*}}:
+  // CHECK: %call = call i32 %[[#]]()
+  return f();
+}
+
+// CHECK-DAG: define internal i32 @f3(){{.*}} prefix i64 [[#HASH]]
+static int f3(void) { return 1; }
+
+// CHECK-DAG: declare i32 @f4(){{.*}} prefix i64 [[#HASH]]
+extern int f4(void);
+
+// CHECK-DAG: declare void @__kcfi_check_fail(i64, i8*)
+
+int test() {
+  return call(f1) +
+         __call((fn_t)f2) +
+         call(f3) +
+         call(f4);
+}

clang/test/CodeGen/kcfi_unchecked.c

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s
+
+#if !__has_feature(kcfi)
+#error Missing kcfi
+#endif
+
+// CHECK-LABEL: define dso_local i32 @f1(){{.*}} prefix i64 [[#%d,HASH:]]
+int f1(void) { return 0; }
+
+typedef int (*fn_t)(void);
+typedef int (*fn_unchecked_t)(void) __attribute__((kcfi_unchecked));
+
+typedef typeof(f1) *fn_typeof_t;
+typedef typeof(f1) *__attribute__((kcfi_unchecked)) fn_typeof_unchecked_t;
+
+// CHECK-LABEL: define{{.*}} i32 @checked()
+int checked(void) {
+  // CHECK: kcfi.fail
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]]
+  // CHECK: kcfi.cont
+  // CHECK: call i32 %
+  return ({ &f1; })();
+}
+// CHECK-LABEL: define{{.*}} i32 @checked_typedef_cast()
+int checked_typedef_cast(void) {
+  // CHECK: kcfi.fail
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]]
+  // CHECK: kcfi.cont
+  // CHECK: call i32 %
+  return ({ (fn_t) & f1; })();
+}
+// CHECK-LABEL: define{{.*}} i32 @checked_outside_typedef_cast()
+int checked_outside_typedef_cast(void) {
+  // CHECK: kcfi.fail
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]]
+  // CHECK: kcfi.cont
+  // CHECK: call i32 %
+  return ((fn_t)({ &f1; }))();
+}
+// CHECK-LABEL: define{{.*}} i32 @checked_typeof_typedef_cast()
+int checked_typeof_typedef_cast(void) {
+  // CHECK: kcfi.fail
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]]
+  // CHECK: kcfi.cont
+  // CHECK: call i32 %
+  return ({ (fn_typeof_t) & f1; })();
+}
+// CHECK-LABEL: define{{.*}} i32 @checked_var()
+int checked_var(void) {
+  fn_t p = f1;
+  // CHECK: kcfi.fail
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]]
+  // CHECK: kcfi.cont
+  // CHECK: call i32 %
+  return p();
+}
+// CHECK-LABEL: define{{.*}} i32 @checked_param(i32 ()*
+int checked_param(fn_t p) {
+  // CHECK: kcfi.fail
+  // CHECK: call void @__kcfi_check_fail(i64 [[#HASH]]
+  // CHECK: kcfi.cont
+  // CHECK: call i32 %
+  return p();
+}
+
+// CHECK-LABEL: define{{.*}} i32 @unchecked_typedef_cast()
+int unchecked_typedef_cast(void) {
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return ({ (fn_unchecked_t) & f1; })();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_outside_typedef_cast()
+int unchecked_outside_typedef_cast(void) {
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return ((fn_unchecked_t)({ &f1; }))();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_typeof_typedef_cast()
+int unchecked_typeof_typedef_cast(void) {
+  // CHECK-NOT: call void @__kcfi_check_fail
+  return ({ (fn_typeof_unchecked_t) & f1; })();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_compound_var()
+int unchecked_compound_var(void) {
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return ({
+    fn_unchecked_t p = f1;
+    p;
+  })();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_compound_local_typedef_cast()
+int unchecked_compound_local_typedef_cast(void) {
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return ({
+    typedef typeof(f1) *__attribute__((kcfi_unchecked)) fn_local_unchecked_t;
+    (fn_local_unchecked_t) & f1;
+  })();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_var()
+int unchecked_var(void) {
+  fn_unchecked_t p = f1;
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return p();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_var_attr()
+int unchecked_var_attr(void) {
+  fn_t __attribute__((kcfi_unchecked)) p = f1;
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return p();
+}
+// CHECK-LABEL: define{{.*}} i32 @unchecked_param(i32 ()*
+int unchecked_param(fn_t __attribute__((kcfi_unchecked)) p) {
+  // CHECK-NOT: call void @__kcfi_check_fail
+  // CHECK: call i32 %
+  return p();
+}

clang/test/Driver/fsanitize.c

 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fno-sanitize-cfi-canonical-jump-tables -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-CFI-CANONICAL-JUMP-TABLES
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-CANONICAL-JUMP-TABLES
 // CHECK-CFI-CANONICAL-JUMP-TABLES: -fsanitize-cfi-canonical-jump-tables
 // CHECK-NO-CFI-CANONICAL-JUMP-TABLES-NOT: -fsanitize-cfi-canonical-jump-tables
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fsanitize-stats -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-STATS
 // CHECK-CFI-STATS: -fsanitize-stats
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fsanitize=cfi -flto -fvisibility=hidden %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-NOCFI
+// CHECK-KCFI-NOCFI: error: invalid argument '-fsanitize=cfi*' not allowed with '-fsanitize=kcfi'
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fsanitize-trap=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-NOTRAP
+// CHECK-KCFI-NOTRAP: error: unsupported argument 'kcfi' to option '-fsanitize-trap'
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI
+// CHECK-KCFI: "-fsanitize=kcfi"
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fsanitize-recover=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-RECOVER
+// CHECK-KCFI-RECOVER: "-fsanitize=kcfi"
+// CHECK-KCFI-RECOVER: "-fsanitize-recover=kcfi"
+
 // RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -LDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -MD -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -MT -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -LD -LDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // CHECK-ASAN-DEBUGRTL: error: invalid argument
 // CHECK-ASAN-DEBUGRTL: not allowed with '-fsanitize=address'

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: HIPManaged (SubjectMatchRule_variable)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
 // CHECK-NEXT: IFunc (SubjectMatchRule_function)
 // CHECK-NEXT: InitPriority (SubjectMatchRule_variable)
 // CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
+// CHECK-NEXT: KCFIUnchecked (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: LTOVisibilityPublic (SubjectMatchRule_record)
 // CHECK-NEXT: Leaf (SubjectMatchRule_function)
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: MinSize (SubjectMatchRule_function, SubjectMatchRule_objc_method)

clang/test/Sema/attr-kcfi_unchecked.c

+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+int a __attribute__((kcfi_unchecked));   // expected-error {{'kcfi_unchecked' attribute argument only applies to a function pointer or a function pointer type}}
+void *p __attribute__((kcfi_unchecked)); // expected-error {{'kcfi_unchecked' attribute argument only applies to a function pointer or a function pointer type}}
+void (*f)(void) __attribute__((kcfi_unchecked));
+
+typedef unsigned long l_unchecked_t __attribute__((kcfi_unchecked)); // expected-error {{'kcfi_unchecked' attribute argument only applies to a function pointer or a function pointer type}}
+typedef int (*f_unchecked_t)(void) __attribute__((kcfi_unchecked));
+
+void f1(unsigned long p __attribute__((kcfi_unchecked))) {} // expected-error {{'kcfi_unchecked' attribute argument only applies to a function pointer or a function pointer type}}
+void f2(void *p __attribute__((kcfi_unchecked))) {}         // expected-error {{'kcfi_unchecked' attribute argument only applies to a function pointer or a function pointer type}}
+void f3(void (*p)(void) __attribute__((kcfi_unchecked))) {}