Skip to content

Commit c4bbb04

Browse files
committed
[CodeGen][KCFI] Emit signed comment for .set directive value
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. Signed-off-by: Kees Cook <[email protected]>
1 parent 196ea57 commit c4bbb04

File tree

3 files changed

+5
-4
lines changed

3 files changed

+5
-4
lines changed

clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3189,7 +3189,8 @@ void CodeGenModule::finalizeKCFITypes() {
31893189
continue;
31903190

31913191
std::string Asm = (".weak __kcfi_typeid_" + Name + "\n.set __kcfi_typeid_" +
3192-
Name + ", " + Twine(Type->getZExtValue()) + "\n")
3192+
Name + ", " + Twine(Type->getZExtValue()) + " # " +
3193+
Twine(Type->getSExtValue()) + "\n")
31933194
.str();
31943195
M.appendModuleInlineAsm(Asm);
31953196
}

clang/test/CodeGen/cfi-salt.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ typedef unsigned int (* __cfi_salt ufn_salt_t)(void);
2727

2828
/// Must emit __kcfi_typeid symbols for address-taken function declarations
2929
// CHECK: module asm ".weak __kcfi_typeid_[[F4:[a-zA-Z0-9_]+]]"
30-
// CHECK: module asm ".set __kcfi_typeid_[[F4]], [[#%d,LOW_SODIUM_HASH:]]"
30+
// CHECK: module asm ".set __kcfi_typeid_[[F4]], {{[0-9]+}} # [[#%d,LOW_SODIUM_HASH:]]"
3131
// CHECK: module asm ".weak __kcfi_typeid_[[F4_SALT:[a-zA-Z0-9_]+]]"
32-
// CHECK: module asm ".set __kcfi_typeid_[[F4_SALT]], [[#%d,ASM_SALTY_HASH:]]"
32+
// CHECK: module asm ".set __kcfi_typeid_[[F4_SALT]], {{[0-9]+}} # [[#%d,ASM_SALTY_HASH:]]"
3333

3434
/// Must not __kcfi_typeid symbols for non-address-taken declarations
3535
// CHECK-NOT: module asm ".weak __kcfi_typeid_f6"

clang/test/CodeGen/kcfi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

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

0 commit comments

Comments
 (0)