Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release/19.x: Fix KCFI types for generated functions with integer normalization (#104826) #113938

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Oct 28, 2024

Backport e1c36bd

Requested by: @samitolvanen

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Oct 28, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Oct 28, 2024

@MaskRay What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Oct 28, 2024

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport e1c36bd

Requested by: @samitolvanen


Full diff: https://github.com/llvm/llvm-project/pull/113938.diff

5 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+5)
  • (modified) clang/test/CodeGen/kcfi-normalize.c (+1)
  • (modified) llvm/lib/Transforms/Utils/ModuleUtils.cpp (+7-5)
  • (added) llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll (+35)
  • (modified) llvm/test/Transforms/GCOVProfiling/kcfi.ll (+5-3)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cf5e29e5a3db8d..49266f3e3982e1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1137,6 +1137,11 @@ void CodeGenModule::Release() {
                               CodeGenOpts.SanitizeCfiCanonicalJumpTables);
   }
 
+  if (CodeGenOpts.SanitizeCfiICallNormalizeIntegers) {
+    getModule().addModuleFlag(llvm::Module::Override, "cfi-normalize-integers",
+                              1);
+  }
+
   if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
     getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
     // KCFI assumes patchable-function-prefix is the same for all indirectly
diff --git a/clang/test/CodeGen/kcfi-normalize.c b/clang/test/CodeGen/kcfi-normalize.c
index 7660c908a7bdd5..b9150e88f6ab5f 100644
--- a/clang/test/CodeGen/kcfi-normalize.c
+++ b/clang/test/CodeGen/kcfi-normalize.c
@@ -28,6 +28,7 @@ void baz(void (*fn)(int, int, int), int arg1, int arg2, int arg3) {
     fn(arg1, arg2, arg3);
 }
 
+// CHECK: ![[#]] = !{i32 4, !"cfi-normalize-integers", i32 1}
 // CHECK: ![[TYPE1]] = !{i32 -1143117868}
 // CHECK: ![[TYPE2]] = !{i32 -460921415}
 // CHECK: ![[TYPE3]] = !{i32 -333839615}
diff --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
index 122279160cc7e8..95bf9f06bc331c 100644
--- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,11 +161,13 @@ void llvm::setKCFIType(Module &M, Function &F, StringRef MangledType) {
   // Matches CodeGenModule::CreateKCFITypeId in Clang.
   LLVMContext &Ctx = M.getContext();
   MDBuilder MDB(Ctx);
-  F.setMetadata(
-      LLVMContext::MD_kcfi_type,
-      MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
-                           Type::getInt32Ty(Ctx),
-                           static_cast<uint32_t>(xxHash64(MangledType))))));
+  std::string Type = MangledType.str();
+  if (M.getModuleFlag("cfi-normalize-integers"))
+    Type += ".normalized";
+  F.setMetadata(LLVMContext::MD_kcfi_type,
+                MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
+                                     Type::getInt32Ty(Ctx),
+                                     static_cast<uint32_t>(xxHash64(Type))))));
   // If the module was compiled with -fpatchable-function-entry, ensure
   // we use the same patchable-function-prefix.
   if (auto *MD = mdconst::extract_or_null<ConstantInt>(
diff --git a/llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll b/llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll
new file mode 100644
index 00000000000000..19122b920d1ca4
--- /dev/null
+++ b/llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll
@@ -0,0 +1,35 @@
+;; Ensure __llvm_gcov_(writeout|reset|init) have the correct !kcfi_type
+;; with integer normalization.
+; RUN: mkdir -p %t && cd %t
+; RUN: opt < %s -S -passes=insert-gcov-profiling | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local void @empty() !dbg !5 {
+entry:
+  ret void, !dbg !8
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !9, !10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "a.c", directory: "")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "empty", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null}
+!8 = !DILocation(line: 2, column: 1, scope: !5)
+!9 = !{i32 4, !"kcfi", i32 1}
+!10 = !{i32 4, !"cfi-normalize-integers", i32 1}
+
+; CHECK: define internal void @__llvm_gcov_writeout()
+; CHECK-SAME: !kcfi_type ![[#TYPE:]]
+; CHECK: define internal void @__llvm_gcov_reset()
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
+; CHECK: define internal void @__llvm_gcov_init()
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
+
+; CHECK: ![[#TYPE]] = !{i32 -440107680}
diff --git a/llvm/test/Transforms/GCOVProfiling/kcfi.ll b/llvm/test/Transforms/GCOVProfiling/kcfi.ll
index b25f40f05d5bc4..1b97d25294cd65 100644
--- a/llvm/test/Transforms/GCOVProfiling/kcfi.ll
+++ b/llvm/test/Transforms/GCOVProfiling/kcfi.ll
@@ -24,8 +24,10 @@ entry:
 !9 = !{i32 4, !"kcfi", i32 1}
 
 ; CHECK: define internal void @__llvm_gcov_writeout()
-; CHECK-SAME: !kcfi_type
+; CHECK-SAME: !kcfi_type ![[#TYPE:]]
 ; CHECK: define internal void @__llvm_gcov_reset()
-; CHECK-SAME: !kcfi_type
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
 ; CHECK: define internal void @__llvm_gcov_init()
-; CHECK-SAME: !kcfi_type
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
+
+; CHECK: ![[#TYPE]] = !{i32 -1522505972}

@llvmbot
Copy link
Member Author

llvmbot commented Oct 28, 2024

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport e1c36bd

Requested by: @samitolvanen


Full diff: https://github.com/llvm/llvm-project/pull/113938.diff

5 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+5)
  • (modified) clang/test/CodeGen/kcfi-normalize.c (+1)
  • (modified) llvm/lib/Transforms/Utils/ModuleUtils.cpp (+7-5)
  • (added) llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll (+35)
  • (modified) llvm/test/Transforms/GCOVProfiling/kcfi.ll (+5-3)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cf5e29e5a3db8d..49266f3e3982e1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1137,6 +1137,11 @@ void CodeGenModule::Release() {
                               CodeGenOpts.SanitizeCfiCanonicalJumpTables);
   }
 
+  if (CodeGenOpts.SanitizeCfiICallNormalizeIntegers) {
+    getModule().addModuleFlag(llvm::Module::Override, "cfi-normalize-integers",
+                              1);
+  }
+
   if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
     getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
     // KCFI assumes patchable-function-prefix is the same for all indirectly
diff --git a/clang/test/CodeGen/kcfi-normalize.c b/clang/test/CodeGen/kcfi-normalize.c
index 7660c908a7bdd5..b9150e88f6ab5f 100644
--- a/clang/test/CodeGen/kcfi-normalize.c
+++ b/clang/test/CodeGen/kcfi-normalize.c
@@ -28,6 +28,7 @@ void baz(void (*fn)(int, int, int), int arg1, int arg2, int arg3) {
     fn(arg1, arg2, arg3);
 }
 
+// CHECK: ![[#]] = !{i32 4, !"cfi-normalize-integers", i32 1}
 // CHECK: ![[TYPE1]] = !{i32 -1143117868}
 // CHECK: ![[TYPE2]] = !{i32 -460921415}
 // CHECK: ![[TYPE3]] = !{i32 -333839615}
diff --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
index 122279160cc7e8..95bf9f06bc331c 100644
--- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,11 +161,13 @@ void llvm::setKCFIType(Module &M, Function &F, StringRef MangledType) {
   // Matches CodeGenModule::CreateKCFITypeId in Clang.
   LLVMContext &Ctx = M.getContext();
   MDBuilder MDB(Ctx);
-  F.setMetadata(
-      LLVMContext::MD_kcfi_type,
-      MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
-                           Type::getInt32Ty(Ctx),
-                           static_cast<uint32_t>(xxHash64(MangledType))))));
+  std::string Type = MangledType.str();
+  if (M.getModuleFlag("cfi-normalize-integers"))
+    Type += ".normalized";
+  F.setMetadata(LLVMContext::MD_kcfi_type,
+                MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
+                                     Type::getInt32Ty(Ctx),
+                                     static_cast<uint32_t>(xxHash64(Type))))));
   // If the module was compiled with -fpatchable-function-entry, ensure
   // we use the same patchable-function-prefix.
   if (auto *MD = mdconst::extract_or_null<ConstantInt>(
diff --git a/llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll b/llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll
new file mode 100644
index 00000000000000..19122b920d1ca4
--- /dev/null
+++ b/llvm/test/Transforms/GCOVProfiling/kcfi-normalize.ll
@@ -0,0 +1,35 @@
+;; Ensure __llvm_gcov_(writeout|reset|init) have the correct !kcfi_type
+;; with integer normalization.
+; RUN: mkdir -p %t && cd %t
+; RUN: opt < %s -S -passes=insert-gcov-profiling | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local void @empty() !dbg !5 {
+entry:
+  ret void, !dbg !8
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !9, !10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "a.c", directory: "")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "empty", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null}
+!8 = !DILocation(line: 2, column: 1, scope: !5)
+!9 = !{i32 4, !"kcfi", i32 1}
+!10 = !{i32 4, !"cfi-normalize-integers", i32 1}
+
+; CHECK: define internal void @__llvm_gcov_writeout()
+; CHECK-SAME: !kcfi_type ![[#TYPE:]]
+; CHECK: define internal void @__llvm_gcov_reset()
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
+; CHECK: define internal void @__llvm_gcov_init()
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
+
+; CHECK: ![[#TYPE]] = !{i32 -440107680}
diff --git a/llvm/test/Transforms/GCOVProfiling/kcfi.ll b/llvm/test/Transforms/GCOVProfiling/kcfi.ll
index b25f40f05d5bc4..1b97d25294cd65 100644
--- a/llvm/test/Transforms/GCOVProfiling/kcfi.ll
+++ b/llvm/test/Transforms/GCOVProfiling/kcfi.ll
@@ -24,8 +24,10 @@ entry:
 !9 = !{i32 4, !"kcfi", i32 1}
 
 ; CHECK: define internal void @__llvm_gcov_writeout()
-; CHECK-SAME: !kcfi_type
+; CHECK-SAME: !kcfi_type ![[#TYPE:]]
 ; CHECK: define internal void @__llvm_gcov_reset()
-; CHECK-SAME: !kcfi_type
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
 ; CHECK: define internal void @__llvm_gcov_init()
-; CHECK-SAME: !kcfi_type
+; CHECK-SAME: !kcfi_type ![[#TYPE]]
+
+; CHECK: ![[#TYPE]] = !{i32 -1522505972}

Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@tru tru merged commit def7925 into llvm:release/19.x Oct 29, 2024
2 of 3 checks passed
…vm#104826)

With -fsanitize-cfi-icall-experimental-normalize-integers, Clang
appends ".normalized" to KCFI types in CodeGenModule::CreateKCFITypeId,
which changes type hashes also for functions that don't have integer
types in their signatures. However, llvm::setKCFIType does not take
integer normalization into account, which means LLVM generated
functions with KCFI types, e.g. sanitizer constructors, will fail KCFI
checks when integer normalization is enabled in Clang.

Add a cfi-normalize-integers module flag to indicate integer
normalization is used, and append ".normalized" to KCFI types also in
llvm::setKCFIType to fix the type mismatch.

(cherry picked from commit e1c36bd)
Copy link

@samitolvanen (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Darksonn added a commit to Darksonn/linux that referenced this pull request Oct 30, 2024
The llvm fix [1] did not make it for 19.0.0, but ended up getting
backported to llvm 19.1.3 [2]. Thus, fix the version requirement to
correctly specify which versions have the bug.

Link: llvm/llvm-project#104826 [1]
Link: llvm/llvm-project#113938 [2]
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-lkp/[email protected]
Fixes: 8b8ca9c ("cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS")
Signed-off-by: Alice Ryhl <[email protected]>
Darksonn added a commit to Darksonn/linux that referenced this pull request Oct 30, 2024
The llvm fix [1] did not make it for 19.0.0, but ended up getting
backported to llvm 19.1.3 [2]. Thus, fix the version requirement to
correctly specify which versions have the bug.

Link: llvm/llvm-project#104826 [1]
Link: llvm/llvm-project#113938 [2]
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-lkp/[email protected]
Fixes: 8b8ca9c ("cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS")
Signed-off-by: Alice Ryhl <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Oct 30, 2024
The llvm fix [1] did not make it for 19.0.0, but ended up getting
backported to llvm 19.1.3 [2]. Thus, fix the version requirement to
correctly specify which versions have the bug.

Link: llvm/llvm-project#104826 [1]
Link: llvm/llvm-project#113938 [2]
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-lkp/[email protected]
Fixes: 8b8ca9c ("cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS")
Signed-off-by: Alice Ryhl <[email protected]>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this pull request Oct 30, 2024
The llvm fix [1] did not make it for 19.0.0, but ended up getting
backported to llvm 19.1.3 [2]. Thus, fix the version requirement to
correctly specify which versions have the bug.

Link: llvm/llvm-project#104826 [1]
Link: llvm/llvm-project#113938 [2]
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-lkp/[email protected]
Fixes: 8b8ca9c ("cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS")
Signed-off-by: Alice Ryhl <[email protected]>
Reviewed-by: Sami Tolvanen <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
TroyMitchell911 pushed a commit to Troyself/k230-linux-kernel that referenced this pull request Nov 30, 2024
The llvm fix [1] did not make it for 19.0.0, but ended up getting
backported to llvm 19.1.3 [2]. Thus, fix the version requirement to
correctly specify which versions have the bug.

Link: llvm/llvm-project#104826 [1]
Link: llvm/llvm-project#113938 [2]
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-lkp/[email protected]
Fixes: 8b8ca9c ("cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS")
Signed-off-by: Alice Ryhl <[email protected]>
Reviewed-by: Sami Tolvanen <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category llvm:transforms
Projects
Development

Successfully merging this pull request may close these issues.

4 participants