Skip to content

Add support for Windows Secure Hot-Patching #138972

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/CodeGenOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,13 @@ class CodeGenOptions : public CodeGenOptionsBase {

/// A list of functions that are replacable by the loader.
std::vector<std::string> LoaderReplaceableFunctionNames;
/// The name of a file that contains functions which will be compiled for
/// hotpatching. See -fms-secure-hotpatch-functions-file.
std::string MSSecureHotPatchFunctionsFile;

/// A list of functions which will be compiled for hotpatching.
/// See -fms-secure-hotpatch-functions-list.
std::vector<std::string> MSSecureHotPatchFunctionsList;

public:
// Define accessors/mutators for code generation options of enumeration type.
Expand Down
18 changes: 18 additions & 0 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -3837,6 +3837,24 @@ def fms_hotpatch : Flag<["-"], "fms-hotpatch">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Ensure that all functions can be hotpatched at runtime">,
MarshallingInfoFlag<CodeGenOpts<"HotPatch">>;

// See llvm/lib/CodeGen/WindowsSecureHotPatching.cpp
def fms_secure_hotpatch_functions_file
: Joined<["-"], "fms-secure-hotpatch-functions-file=">,
Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption]>,
MarshallingInfoString<CodeGenOpts<"MSSecureHotPatchFunctionsFile">>,
HelpText<"Path to a file that contains a list of mangled names of "
"functions that should be hot-patched for Windows Secure "
"Hot-Patching">;
def fms_secure_hotpatch_functions_list
: CommaJoined<["-"], "fms-secure-hotpatch-functions-list=">,
Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption]>,
MarshallingInfoStringVector<CodeGenOpts<"MSSecureHotPatchFunctionsList">>,
HelpText<"List of mangled symbol names of functions that should be "
"hot-patched for Windows Secure Hot-Patching">;

def fpcc_struct_return : Flag<["-"], "fpcc-struct-return">, Group<f_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"Override the default ABI to return all structs on the stack">;
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/CodeGen/CGCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2660,6 +2660,13 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
// CPU/feature overrides. addDefaultFunctionDefinitionAttributes
// handles these separately to set them based on the global defaults.
GetCPUAndFeaturesAttributes(CalleeInfo.getCalleeDecl(), FuncAttrs);

// Windows hotpatching support
if (!MSHotPatchFunctions.empty()) {
bool IsHotPatched = llvm::binary_search(MSHotPatchFunctions, Name);
if (IsHotPatched)
FuncAttrs.addAttribute("marked_for_windows_hot_patching");
}
}

// Mark functions that are replaceable by the loader.
Expand Down
29 changes: 29 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,35 @@ CodeGenModule::CodeGenModule(ASTContext &C,
if (Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
getModule().addModuleFlag(llvm::Module::Error, "NumRegisterParameters",
CodeGenOpts.NumRegisterParameters);

// If there are any functions that are marked for Windows secure hot-patching,
// then build the list of functions now.
if (!CGO.MSSecureHotPatchFunctionsFile.empty() ||
!CGO.MSSecureHotPatchFunctionsList.empty()) {
if (!CGO.MSSecureHotPatchFunctionsFile.empty()) {
auto BufOrErr =
llvm::MemoryBuffer::getFile(CGO.MSSecureHotPatchFunctionsFile);
if (BufOrErr) {
const llvm::MemoryBuffer &FileBuffer = **BufOrErr;
for (llvm::line_iterator I(FileBuffer.getMemBufferRef(), true), E;
I != E; ++I)
this->MSHotPatchFunctions.push_back(std::string{*I});
} else {
auto &DE = Context.getDiagnostics();
unsigned DiagID =
DE.getCustomDiagID(DiagnosticsEngine::Error,
"failed to open hotpatch functions file "
"(-fms-hotpatch-functions-file): %0 : %1");
DE.Report(DiagID) << CGO.MSSecureHotPatchFunctionsFile
<< BufOrErr.getError().message();
}
}

for (const auto &FuncName : CGO.MSSecureHotPatchFunctionsList)
this->MSHotPatchFunctions.push_back(FuncName);

llvm::sort(this->MSHotPatchFunctions);
}
}

CodeGenModule::~CodeGenModule() {}
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,11 @@ class CodeGenModule : public CodeGenTypeCache {

AtomicOptions AtomicOpts;

// A set of functions which should be hot-patched; see
// -fms-hotpatch-functions-file (and -list). This will nearly always be empty.
// The list is sorted for binary-searching.
std::vector<std::string> MSHotPatchFunctions;

public:
CodeGenModule(ASTContext &C, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
const HeaderSearchOptions &headersearchopts,
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6801,6 +6801,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,

Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);

if (Arg *A = Args.getLastArg(options::OPT_fms_secure_hotpatch_functions_file))
Args.AddLastArg(CmdArgs, options::OPT_fms_secure_hotpatch_functions_file);

for (const auto &A :
Args.getAllArgValues(options::OPT_fms_secure_hotpatch_functions_list))
CmdArgs.push_back(
Args.MakeArgString("-fms-secure-hotpatch-functions-list=" + Twine(A)));

if (TC.SupportsProfiling()) {
Args.AddLastArg(CmdArgs, options::OPT_pg);

Expand Down
16 changes: 16 additions & 0 deletions clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// This verifies that we correctly handle a -fms-secure-hotpatch-functions-file argument that points
// to a missing file.
//
// RUN: not %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-file=%S/this-file-is-intentionally-missing-do-not-create-it.txt /Fo%t.obj %s 2>&1 | FileCheck %s
// CHECK: failed to open hotpatch functions file

void this_might_have_side_effects();

int __declspec(noinline) this_gets_hotpatched() {
this_might_have_side_effects();
return 42;
}

int __declspec(noinline) this_does_not_get_hotpatched() {
return this_gets_hotpatched() + 100;
}
22 changes: 22 additions & 0 deletions clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// This verifies that hotpatch function attributes are correctly propagated when compiling directly to OBJ,
// and that name mangling works as expected.
//
// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-list=?this_gets_hotpatched@@YAHXZ /Fo%t.obj %s
// RUN: llvm-readobj --codeview %t.obj | FileCheck %s

void this_might_have_side_effects();

int __declspec(noinline) this_gets_hotpatched() {
this_might_have_side_effects();
return 42;
}

// CHECK: Kind: S_HOTPATCHFUNC (0x1169)
// CHECK-NEXT: Function: this_gets_hotpatched
// CHECK-NEXT: Name: ?this_gets_hotpatched@@YAHXZ

extern "C" int __declspec(noinline) this_does_not_get_hotpatched() {
return this_gets_hotpatched() + 100;
}

// CHECK-NOT: S_HOTPATCHFUNC
24 changes: 24 additions & 0 deletions clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Global constant data such as exception handler tables should not be redirected by Windows Secure Hot-Patching
//
// RUN: %clang_cl -c --target=x86_64-windows-msvc /EHsc -O2 -fms-secure-hotpatch-functions-list=this_gets_hotpatched /Fo%t.obj /clang:-S /clang:-o- %s 2>& 1 | FileCheck %s

class Foo {
public:
int x;
};

void this_might_throw();

extern "C" int this_gets_hotpatched(int k) {
int ret;
try {
this_might_throw();
ret = 1;
} catch (Foo& f) {
ret = 2;
}
return ret;
}

// We expect that RTTI data is not redirected.
// CHECK-NOT: "__ref_??_R0?AVFoo@@@8"
133 changes: 133 additions & 0 deletions clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
// This verifies that global variable redirection works correctly when using hotpatching.
//
// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 \
// RUN: -fms-secure-hotpatch-functions-list=hp1,hp2,hp3,hp4,hp5_phi_ptr_mixed,hp_phi_ptr_both,hp_const_ptr_sub \
// RUN: /clang:-S /clang:-o- %s | FileCheck %s

#ifdef __clang__
#define NO_TAIL __attribute__((disable_tail_calls))
#else
#define NO_TAIL
#endif

extern int g_data[10];

struct SomeData {
int x;
int y;
};

const struct SomeData g_this_is_const = { 100, 200 };

struct HasPointers {
int* ptr;
int x;
};

extern struct HasPointers g_has_pointers;

void take_data(const void* p);

void do_side_effects();
void do_other_side_effects();

void hp1() NO_TAIL {
take_data(&g_data[5]);
}

// CHECK: hp1:
// CHECK: mov rcx, qword ptr [rip + __ref_g_data]
// CHECK: add rcx, 20
// CHECK: call take_data
// CHECK: .seh_endproc

void hp2() NO_TAIL {
// We do not expect string literals to be redirected.
take_data("hello, world!");
}

// CHECK: hp2:
// CHECK: lea rcx, [rip + "??_C@_0O@KJBLMJCB@hello?0?5world?$CB?$AA@"]
// CHECK: call take_data
// CHECK: .seh_endproc

void hp3() NO_TAIL {
// We do not expect g_this_is_const to be redirected because it is const
// and contains no pointers.
take_data(&g_this_is_const);
}

// CHECK: hp3:
// CHECK: lea rcx, [rip + g_this_is_const]
// CHECK: call take_data
// CHECK-NOT: __ref_g_this_is_const
// CHECK: .seh_endproc

void hp4() NO_TAIL {
take_data(&g_has_pointers);
// We expect &g_has_pointers to be redirected.
}

// CHECK: hp4:
// CHECK: mov rcx, qword ptr [rip + __ref_g_has_pointers]
// CHECK: call take_data
// CHECK: .seh_endproc

// This case checks that global variable redirection interacts correctly with PHI nodes.
// The IR for this generates a "phi ptr g_has_pointers, g_this_is_const" node.
// We expect g_has_pointers to be redirected, but not g_this_is_const.
void hp5_phi_ptr_mixed(int x) NO_TAIL {
const void* y;
if (x) {
y = &g_has_pointers;
do_side_effects();
} else {
y = &g_this_is_const;
do_other_side_effects();
}
take_data(y);
}

// CHECK: hp5_phi_ptr_mixed
// CHECK: .seh_endprologue
// CHECK: test ecx, ecx
// CHECK: mov rsi, qword ptr [rip + __ref_g_has_pointers]
// CHECK: call do_side_effects
// CHECK: jmp
// CHECK: call do_other_side_effects
// CHECK: lea rsi, [rip + g_this_is_const]
// CHECK: mov rcx, rsi
// CHECK: call take_data
// CHECK: .seh_endproc

// This case tests that global variable redirection interacts correctly with PHI nodes,
// where two (all) operands of a given PHI node are globabl variables that redirect.
void hp_phi_ptr_both(int x) NO_TAIL {
const void* y;
if (x) {
y = &g_has_pointers;
do_side_effects();
} else {
y = &g_data[5];
do_other_side_effects();
}
take_data(y);
}

// CHECK: hp_phi_ptr_both:
// CHECK: .seh_endprologue
// CHECK: test ecx, ecx
// CHECK: mov rsi, qword ptr [rip + __ref_g_has_pointers]
// CHECK: mov rsi, qword ptr [rip + __ref_g_data]
// CHECK: take_data
// CHECK: .seh_endproc

// Test a constant expression which references global variable addresses.
size_t hp_const_ptr_sub() NO_TAIL {
return (unsigned char*)&g_has_pointers - (unsigned char*)&g_data;
}

// CHECK: hp_const_ptr_sub:
// CHECK: mov rax, qword ptr [rip + __ref_g_has_pointers]
// CHECK: sub rax, qword ptr [rip + __ref_g_data]
// CHECK: ret
24 changes: 24 additions & 0 deletions clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This verifies that hotpatch function attributes are correctly propagated through LLVM IR when compiling with LTO.
//
// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-list=this_gets_hotpatched -flto /Fo%t.bc %s
// RUN: llvm-dis %t.bc -o - | FileCheck %s
//
// CHECK-LABEL: define dso_local noundef i32 @this_gets_hotpatched()
// CHECK-SAME: #0
//
// CHECK-LABEL: define dso_local noundef i32 @this_does_not_get_hotpatched()
// CHECK-SAME: #1

// CHECK: attributes #0
// CHECK-SAME: "marked_for_windows_hot_patching"

// CHECK: attributes #1
// CHECK-NOT: "marked_for_windows_hot_patching"

int __declspec(noinline) this_gets_hotpatched() {
return 42;
}

int __declspec(noinline) this_does_not_get_hotpatched() {
return this_gets_hotpatched() + 100;
}
21 changes: 21 additions & 0 deletions clang/test/CodeGen/X86/ms-secure-hotpatch.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// This verifies that hotpatch function attributes are correctly propagated when compiling directly to OBJ.
//
// RUN: echo this_gets_hotpatched > %t.patch-functions.txt
// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-file=%t.patch-functions.txt /Fo%t.obj %s
// RUN: llvm-readobj --codeview %t.obj | FileCheck %s

void this_might_have_side_effects();

int __declspec(noinline) this_gets_hotpatched() {
this_might_have_side_effects();
return 42;
}

// CHECK: Kind: S_HOTPATCHFUNC (0x1169)
// CHECK-NEXT: Function: this_gets_hotpatched

int __declspec(noinline) this_does_not_get_hotpatched() {
return this_gets_hotpatched() + 100;
}

// CHECK-NOT: S_HOTPATCHFUNC
3 changes: 3 additions & 0 deletions llvm/include/llvm/CodeGen/Passes.h
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,9 @@ LLVM_ABI FunctionPass *createSelectOptimizePass();

LLVM_ABI FunctionPass *createCallBrPass();

/// Creates Windows Secure Hot Patch pass. \see WindowsSecureHotPatching.cpp
ModulePass *createWindowsSecureHotPatchingPass();

/// Lowers KCFI operand bundles for indirect calls.
LLVM_ABI FunctionPass *createKCFIPass();
} // namespace llvm
Expand Down
2 changes: 2 additions & 0 deletions llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ SYMBOL_RECORD_ALIAS(S_GTHREAD32 , 0x1113, GlobalTLS, ThreadLocalDataSym)
SYMBOL_RECORD(S_UNAMESPACE , 0x1124, UsingNamespaceSym)
SYMBOL_RECORD(S_ANNOTATION , 0x1019, AnnotationSym)

SYMBOL_RECORD(S_HOTPATCHFUNC , 0x1169, HotPatchFuncSym)

#undef CV_SYMBOL
#undef SYMBOL_RECORD
#undef SYMBOL_RECORD_ALIAS
Loading