[CodeGen][KCFI] Allow setting type hash from xxHash64 to FNV-1a (#167254)

When emitting the assembly .set directive, KCFI needs to use
getZExtValue(). However, this means that FileCheck pattern matching can't
match between the .set directive and the IR when the high bit of a 32-bit
value is set. We had gotten lucky with the existing tests happening to
just not have had the high bit set. The coming hash change will expose
this, though.

LLVM IR's default printing behavior uses APInt::operator<<, which calls
APInt::print(OS, /*isSigned=*/true). This means KCFI operand bundles in
call instructions print as signed (e.g. [ "kcfi"(i32 -1208803271) ]),
and KCFI type metadata prints as signed (e.g. !3 = !{i32 -1208803271}).
Changing the IR to print unsigned i32 values would impact hundreds of
existing tests, so it is best to just leave it be.

Update the KCFI .set direct to use getSExtValue() in a comment so that
we can both build correctly and use FileCheck with pattern matching in
tests.

KCFI generates hashes in two places. Instead of exposing the hash
implementation in both places, introduce a helper that wraps the
specific hash implementation in a single place, llvm::getKCFITypeID.

In order to transition between KCFI hash, we need to be able to specify
them. Add the Clang option -fsanitize-kcfi-hash= and a IR module option
"kcfi-hash" that can choose between xxHash64 and FNV-1a. Default to
xxHash64 to stay backward compatible, as we'll need to also update rustc
to take a new option to change the hash to FNV-1a for interop with the
coming GCC KCFI.

Author: kees

clang/include/clang/Basic/CodeGenOptions.h

@@ -21,6 +21,7 @@
 #include "llvm/Frontend/Debug/Options.h"
 #include "llvm/Frontend/Driver/CodeGenOptions.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Hash.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
@@ -514,6 +515,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// binary metadata pass should not be instrumented.
   std::vector<std::string> SanitizeMetadataIgnorelistFiles;
 
+  /// Hash algorithm to use for KCFI type IDs.
+  llvm::KCFIHashAlgorithm SanitizeKcfiHash;
+
   /// Name of the stack usage file (i.e., .su file) if user passes
   /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
   /// passed on the command line.

clang/include/clang/Options/Options.td

@@ -2719,6 +2719,16 @@ def fsanitize_kcfi_arity : Flag<["-"], "fsanitize-kcfi-arity">,
                            Group<f_clang_Group>,
                            HelpText<"Embed function arity information into the KCFI patchable function prefix">,
                            MarshallingInfoFlag<CodeGenOpts<"SanitizeKcfiArity">>;
+def fsanitize_kcfi_hash_EQ
+    : Joined<["-"], "fsanitize-kcfi-hash=">,
+      HelpText<"Select hash algorithm for KCFI type IDs (xxHash64, FNV-1a)">,
+      Visibility<[ClangOption, CC1Option]>,
+      Values<"xxHash64,FNV-1a">,
+      NormalizedValuesScope<"llvm">,
+      NormalizedValues<["KCFIHashAlgorithm::xxHash64",
+                        "KCFIHashAlgorithm::FNV1a"]>,
+      MarshallingInfoEnum<CodeGenOpts<"SanitizeKcfiHash">,
+                          "KCFIHashAlgorithm::xxHash64">;
 defm sanitize_stats : BoolOption<"f", "sanitize-stats",
   CodeGenOpts<"SanitizeStats">, DefaultFalse,
   PosFlag<SetTrue, [], [ClangOption], "Enable">,

clang/lib/CodeGen/CodeGenModule.cpp

@@ -66,11 +66,12 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Hash.h"
 #include "llvm/Support/TimeProfiler.h"
-#include "llvm/Support/xxhash.h"
 #include "llvm/TargetParser/RISCVISAInfo.h"
 #include "llvm/TargetParser/Triple.h"
 #include "llvm/TargetParser/X86TargetParser.h"
+#include "llvm/Transforms/Instrumentation/KCFI.h"
 #include "llvm/Transforms/Utils/BuildLibCalls.h"
 #include <optional>
 #include <set>
@@ -1272,6 +1273,12 @@ void CodeGenModule::Release() {
                                 CodeGenOpts.PatchableFunctionEntryOffset);
     if (CodeGenOpts.SanitizeKcfiArity)
       getModule().addModuleFlag(llvm::Module::Override, "kcfi-arity", 1);
+    // Store the hash algorithm choice for use in LLVM passes
+    getModule().addModuleFlag(
+        llvm::Module::Override, "kcfi-hash",
+        llvm::MDString::get(
+            getLLVMContext(),
+            llvm::stringifyKCFIHashAlgorithm(CodeGenOpts.SanitizeKcfiHash)));
   }
 
   if (CodeGenOpts.CFProtectionReturn &&
@@ -2450,8 +2457,8 @@ llvm::ConstantInt *CodeGenModule::CreateKCFITypeId(QualType T, StringRef Salt) {
   if (getCodeGenOpts().SanitizeCfiICallGeneralizePointers)
     Out << ".generalized";
 
-  return llvm::ConstantInt::get(Int32Ty,
-                                static_cast<uint32_t>(llvm::xxHash64(OutName)));
+  return llvm::ConstantInt::get(
+      Int32Ty, llvm::getKCFITypeID(OutName, getCodeGenOpts().SanitizeKcfiHash));
 }
 
 void CodeGenModule::SetLLVMFunctionAttributes(GlobalDecl GD,
@@ -3205,7 +3212,8 @@ void CodeGenModule::finalizeKCFITypes() {
       continue;
 
     std::string Asm = (".weak __kcfi_typeid_" + Name + "\n.set __kcfi_typeid_" +
-                       Name + ", " + Twine(Type->getZExtValue()) + "\n")
+                       Name + ", " + Twine(Type->getZExtValue()) + " # " +
+                       Twine(Type->getSExtValue()) + "\n")
                           .str();
     M.appendModuleInlineAsm(Asm);
   }

clang/test/CodeGen/cfi-salt.c

@@ -27,9 +27,9 @@ typedef unsigned int (* __cfi_salt ufn_salt_t)(void);
 
 /// Must emit __kcfi_typeid symbols for address-taken function declarations
 // CHECK: module asm ".weak __kcfi_typeid_[[F4:[a-zA-Z0-9_]+]]"
-// CHECK: module asm ".set __kcfi_typeid_[[F4]], [[#%d,LOW_SODIUM_HASH:]]"
+// CHECK: module asm ".set __kcfi_typeid_[[F4]], {{[0-9]+}} # [[#%d,LOW_SODIUM_HASH:]]"
 // CHECK: module asm ".weak __kcfi_typeid_[[F4_SALT:[a-zA-Z0-9_]+]]"
-// CHECK: module asm ".set __kcfi_typeid_[[F4_SALT]], [[#%d,ASM_SALTY_HASH:]]"
+// CHECK: module asm ".set __kcfi_typeid_[[F4_SALT]], {{[0-9]+}} # [[#%d,ASM_SALTY_HASH:]]"
 
 /// Must not __kcfi_typeid symbols for non-address-taken declarations
 // CHECK-NOT: module asm ".weak __kcfi_typeid_f6"

clang/test/CodeGen/kcfi-hash.c

@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck --check-prefix=DEFAULT %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-kcfi-hash=xxHash64 -o - %s | FileCheck --check-prefix=XXHASH %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-kcfi-hash=FNV-1a -o - %s | FileCheck --check-prefix=FNV %s
+
+void foo(void) {}
+
+// DEFAULT: ![[#]] = !{i32 4, !"kcfi-hash", !"xxHash64"}
+// XXHASH: ![[#]] = !{i32 4, !"kcfi-hash", !"xxHash64"}
+// FNV: ![[#]] = !{i32 4, !"kcfi-hash", !"FNV-1a"}

clang/test/CodeGen/kcfi.c

@@ -7,7 +7,7 @@
 
 /// Must emit __kcfi_typeid symbols for address-taken function declarations
 // CHECK: module asm ".weak __kcfi_typeid_[[F4:[a-zA-Z0-9_]+]]"
-// CHECK: module asm ".set __kcfi_typeid_[[F4]], [[#%d,HASH:]]"
+// CHECK: module asm ".set __kcfi_typeid_[[F4]], {{[0-9]+}} # [[#%d,HASH:]]"
 /// Must not __kcfi_typeid symbols for non-address-taken declarations
 // CHECK-NOT: module asm ".weak __kcfi_typeid_{{f6|_Z2f6v}}"
 

llvm/include/llvm/Support/Hash.h

@@ -0,0 +1,36 @@
+//===- llvm/Support/Hash.h - Hash functions --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides hash functions.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_HASH_H
+#define LLVM_SUPPORT_HASH_H
+
+#include "llvm/ADT/StringRef.h"
+#include <cstdint>
+
+namespace llvm {
+
+enum class KCFIHashAlgorithm { xxHash64, FNV1a };
+
+/// Parse a KCFI hash algorithm name.
+/// Returns xxHash64 if the name is not recognized.
+KCFIHashAlgorithm parseKCFIHashAlgorithm(StringRef Name);
+
+/// Convert a KCFI hash algorithm enum to its string representation.
+StringRef stringifyKCFIHashAlgorithm(KCFIHashAlgorithm Algorithm);
+
+/// Compute KCFI type ID from mangled type name.
+/// The algorithm can be xxHash64 or FNV-1a.
+uint32_t getKCFITypeID(StringRef MangledTypeName, KCFIHashAlgorithm Algorithm);
+
+} // end namespace llvm
+
+#endif // LLVM_SUPPORT_HASH_H

llvm/lib/Support/CMakeLists.txt

@@ -202,6 +202,7 @@ add_llvm_component_library(LLVMSupport
   FormatVariadic.cpp
   GlobPattern.cpp
   GraphWriter.cpp
+  Hash.cpp
   HexagonAttributeParser.cpp
   HexagonAttributes.cpp
   InitLLVM.cpp

llvm/lib/Support/Hash.cpp

@@ -0,0 +1,51 @@
+//===- Hash.cpp - Hash functions ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements hash functions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/Hash.h"
+#include "llvm/Support/xxhash.h"
+
+using namespace llvm;
+
+KCFIHashAlgorithm llvm::parseKCFIHashAlgorithm(StringRef Name) {
+  if (Name == "FNV-1a")
+    return KCFIHashAlgorithm::FNV1a;
+  // Default to xxHash64 for backward compatibility
+  return KCFIHashAlgorithm::xxHash64;
+}
+
+StringRef llvm::stringifyKCFIHashAlgorithm(KCFIHashAlgorithm Algorithm) {
+  switch (Algorithm) {
+  case KCFIHashAlgorithm::xxHash64:
+    return "xxHash64";
+  case KCFIHashAlgorithm::FNV1a:
+    return "FNV-1a";
+  }
+  llvm_unreachable("Unknown KCFI hash algorithm");
+}
+
+uint32_t llvm::getKCFITypeID(StringRef MangledTypeName,
+                             KCFIHashAlgorithm Algorithm) {
+  switch (Algorithm) {
+  case KCFIHashAlgorithm::xxHash64:
+    // Use lower 32 bits of xxHash64
+    return static_cast<uint32_t>(xxHash64(MangledTypeName));
+  case KCFIHashAlgorithm::FNV1a:
+    // FNV-1a hash (32-bit)
+    uint32_t Hash = 2166136261u; // FNV offset basis
+    for (unsigned char C : MangledTypeName) {
+      Hash ^= C;
+      Hash *= 16777619u; // FNV prime
+    }
+    return Hash;
+  }
+  llvm_unreachable("Unknown KCFI hash algorithm");
+}

llvm/lib/Transforms/Instrumentation/KCFI.cpp

@@ -23,6 +23,7 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/xxhash.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"