Skip to content

Conversation

@kees
Copy link
Contributor

@kees kees commented Dec 4, 2025

We can't use "#" in the middle of the asm output, so switch to a /* */ pair instead.

We can't use "#" in the middle of the asm output, so switch to a
/* */ pair instead.
@kees kees requested a review from nathanchance December 4, 2025 20:39
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Dec 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2025

@llvm/pr-subscribers-clang

Author: Kees Cook (kees)

Changes

We can't use "#" in the middle of the asm output, so switch to a /* */ pair instead.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2-2)
  • (modified) clang/test/CodeGen/cfi-salt.c (+2-2)
  • (modified) clang/test/CodeGen/kcfi.c (+1-1)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 319e10c93c517..1dcf94fc35e07 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3212,8 +3212,8 @@ void CodeGenModule::finalizeKCFITypes() {
       continue;
 
     std::string Asm = (".weak __kcfi_typeid_" + Name + "\n.set __kcfi_typeid_" +
-                       Name + ", " + Twine(Type->getZExtValue()) + " # " +
-                       Twine(Type->getSExtValue()) + "\n")
+                       Name + ", " + Twine(Type->getZExtValue()) + " /* " +
+                       Twine(Type->getSExtValue()) + " */\n")
                           .str();
     M.appendModuleInlineAsm(Asm);
   }
diff --git a/clang/test/CodeGen/cfi-salt.c b/clang/test/CodeGen/cfi-salt.c
index 8363236869013..2a90dc31d939b 100644
--- a/clang/test/CodeGen/cfi-salt.c
+++ b/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]], {{[0-9]+}} # [[#%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]], {{[0-9]+}} # [[#%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"
diff --git a/clang/test/CodeGen/kcfi.c b/clang/test/CodeGen/kcfi.c
index b2856b5149be9..da32859dd3d88 100644
--- a/clang/test/CodeGen/kcfi.c
+++ b/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]], {{[0-9]+}} # [[#%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}}"
 

@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2025

@llvm/pr-subscribers-clang-codegen

Author: Kees Cook (kees)

Changes

We can't use "#" in the middle of the asm output, so switch to a /* */ pair instead.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2-2)
  • (modified) clang/test/CodeGen/cfi-salt.c (+2-2)
  • (modified) clang/test/CodeGen/kcfi.c (+1-1)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 319e10c93c517..1dcf94fc35e07 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3212,8 +3212,8 @@ void CodeGenModule::finalizeKCFITypes() {
       continue;
 
     std::string Asm = (".weak __kcfi_typeid_" + Name + "\n.set __kcfi_typeid_" +
-                       Name + ", " + Twine(Type->getZExtValue()) + " # " +
-                       Twine(Type->getSExtValue()) + "\n")
+                       Name + ", " + Twine(Type->getZExtValue()) + " /* " +
+                       Twine(Type->getSExtValue()) + " */\n")
                           .str();
     M.appendModuleInlineAsm(Asm);
   }
diff --git a/clang/test/CodeGen/cfi-salt.c b/clang/test/CodeGen/cfi-salt.c
index 8363236869013..2a90dc31d939b 100644
--- a/clang/test/CodeGen/cfi-salt.c
+++ b/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]], {{[0-9]+}} # [[#%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]], {{[0-9]+}} # [[#%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"
diff --git a/clang/test/CodeGen/kcfi.c b/clang/test/CodeGen/kcfi.c
index b2856b5149be9..da32859dd3d88 100644
--- a/clang/test/CodeGen/kcfi.c
+++ b/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]], {{[0-9]+}} # [[#%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}}"
 

@kees
Copy link
Contributor Author

kees commented Dec 4, 2025

This passes "check-llvm" and "check-clang" for me, as well as builds and boots x86 and aarch64 with KCFI enabled.

@kees kees merged commit 421399d into llvm:main Dec 4, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants