Add support for Windows Secure Hot-Patching (redo) #145565
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@qinkunbao - reapplying this after fixing warning |
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: None (sivadeilra) Changes(This is a re-do of #138972, which had a minor warning in This PR adds some of the support needed for Windows hot-patching. Windows implements a form of hot-patching. This allows patches to be applied to Windows apps, drivers, and the kernel, without rebooting or restarting any of these components. Hot-patching is a complex technology and requires coordination between the OS, compilers, linkers, and additional tools. This PR adds support to Clang and LLVM for part of the hot-patching process. It enables LLVM to generate the required code changes and to generate CodeView symbols which identify hot-patched functions. The PR provides new command-line arguments to Clang which allow developers to identify the list of functions that need to be hot-patched. This PR also allows LLVM to directly receive the list of functions to be modified, so that language front-ends which have not yet been modified (such as Rust) can still make use of hot-patching. This PR:
Although the flags are redundant between Clang and LLVM, this allows additional languages (such as Rust) to take advantage of hot-patching support before they have been modified to generate the required attributes. Credit to @dpaoliello, who wrote the original form of this patch. Patch is 56.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145565.diff 31 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 7ba21fca6dd6b..77a0c559f7689 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -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.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 4f91b82a3bfa6..26e953f7ac613 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3838,6 +3838,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">;
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index fd75de42515da..c8c3d6b20c496 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -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.
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 16688810d0685..96fdab212beb1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -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() {}
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 1b67d4354efc0..cb013feb769fc 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -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,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 87d04a42fcd70..25c65ab52fbaf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6803,6 +6803,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);
+ if (Args.hasArg(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);
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
new file mode 100644
index 0000000000000..839dd44f7ff61
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
@@ -0,0 +1,18 @@
+// REQUIRES: x86-registered-target
+
+// 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;
+}
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp b/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
new file mode 100644
index 0000000000000..3dc75c95d76f7
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp b/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
new file mode 100644
index 0000000000000..69704626c8cb6
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// 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"
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
new file mode 100644
index 0000000000000..d76d2aa6d8acc
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
@@ -0,0 +1,135 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
new file mode 100644
index 0000000000000..6adb0b1818e31
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// 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;
+}
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch.c b/clang/test/CodeGen/X86/ms-secure-hotpatch.c
new file mode 100644
index 0000000000000..b829e5acc5c83
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch.c
@@ -0,0 +1,23 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/llvm/include/llvm/CodeGen/Passes.h b/llvm/include/llvm/CodeGen/Passes.h
index 990452fa11fec..18df5d657064a 100644
--- a/llvm/include/llvm/CodeGen/Passes.h
+++ b/llvm/include/llvm/CodeGen/Passes.h
@@ -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
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
index 9d85acc49fa02..b38bdb482df43 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
@@ -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
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
index 5b4f0d31e6427..f5f6fe69430cc 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
@@ -177,6 +177,21 @@ class CallerSym : public SymbolRecord {
uint32_t RecordOffset = 0;
};
+class HotPatchFuncSym : public SymbolRecord {
+public:
+ explicit HotPatchFuncSym(SymbolRecordKind Kind) : SymbolRecord(Kind) {}
+ HotPatchFuncSym(uint32_t RecordOffset)
+ : SymbolRecord(SymbolRecordKind::HotPatchFuncSym),
+ RecordOffset(RecordOffset) {}
+
+ // This is an ItemID in the IPI stream, which points to an LF_FUNC_ID or
+ // LF_MFUNC_ID record.
+ TypeIndex Function;
+ StringRef Name;
+
+ uint32_t RecordOffset = 0;
+};
+
struct DecodedAnnotation {
StringRef Name;
ArrayRef<uint8_t> Bytes;
diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td
index d488c5f419b82..0bcd15eeed879 100644
--- a/llvm/include/llvm/IR/Attributes.td
+++ b/llvm/include/llvm/IR/Attributes.td
@@ -389,6 +389,16 @@ def CoroDestroyOnlyWhenComplete : EnumAttr<"coro_only_destroy_when_complete", In
/// pipeline to perform elide on the call or invoke instruction.
def CoroElideSafe : EnumAttr<"coro_elide_safe", IntersectPreserve, [FnAttr]>;
+/// Function is marked for Windows Hot Patching
+def MarkedForWindowsSecureHotPatching
+ : StrBoolAttr<"marked_for_windows_hot_patching">;
+
+/// Global variable should not be accessed through a "__ref_" global variable in
+/// a hot patching function This attribute is applied to the global variable
+/// decl, not the hotpatched function.
+def AllowDirectAccessInHotPatchFunction
+ : StrBoolAttr<"allow_direct_access_in_hot_patch_function">;
+
/// Target-independent string attributes.
def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;
def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 1b5b1d5888824..1c4ed3843b390 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -336,6 +336,7 @@ LLVM_ABI void initializeVerifierLegacyPassPass(PassRegistry &);
LLVM_ABI void initializeVirtRegMapWrapperLegacyPass(PassRegistry &);
LLVM_ABI void initializeVirtRegRewriterLegacyPass(PassRegistry &);
LLVM_ABI void initializeWasmEHPreparePass(PassRegistry &);
+LLVM_ABI void initializeWindowsSecureHotPatchingPass(PassRegistry &);
LLVM_ABI void initializeWinEHPreparePass(PassRegistry &);
LLVM_ABI void initializeWriteBitcodePassPass(PassRegistry &);
LLVM_ABI void initializeXRayInstrumentationLegacyPass(PassRegistry &);
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index ea57a8fa1f793..5e1b313b4d2fa 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -669,6 +669,8 @@ void CodeViewDebug::endModule() {
if (!Asm)
return;
+ emitSecureHotPatchInformation();
+
emitInlineeLinesSubsection();
// Emit per-function debug information.
@@ -823,6 +825,28 @@ void CodeViewDebug::emitObjName() {
endSymbolRecord(CompilerEnd);
}
+void CodeViewDebug::emitSecureHotPatchInformation() {
+ MCSymbol *hotPatchInfo = nullptr;
+
+ for (const auto &F : MMI->getModule()->functions()) {
+ if (!F.isDeclarationForLinker() &&
+ F.hasFnAttribute("marked_for_windows_hot_patching")) {
+ if (hotPatchInfo == nullptr)
+ hotPatchInfo = beginCVSubsection(DebugSubsectionKind::Symbols);
+ MCSymbol *HotPatchEnd = beginSymbolRecord(SymbolKind::S_HOTPATCHFUNC);
+ auto *SP...
[truncated]
|
|
@llvm/pr-subscribers-backend-x86 Author: None (sivadeilra) Changes(This is a re-do of #138972, which had a minor warning in This PR adds some of the support needed for Windows hot-patching. Windows implements a form of hot-patching. This allows patches to be applied to Windows apps, drivers, and the kernel, without rebooting or restarting any of these components. Hot-patching is a complex technology and requires coordination between the OS, compilers, linkers, and additional tools. This PR adds support to Clang and LLVM for part of the hot-patching process. It enables LLVM to generate the required code changes and to generate CodeView symbols which identify hot-patched functions. The PR provides new command-line arguments to Clang which allow developers to identify the list of functions that need to be hot-patched. This PR also allows LLVM to directly receive the list of functions to be modified, so that language front-ends which have not yet been modified (such as Rust) can still make use of hot-patching. This PR:
Although the flags are redundant between Clang and LLVM, this allows additional languages (such as Rust) to take advantage of hot-patching support before they have been modified to generate the required attributes. Credit to @dpaoliello, who wrote the original form of this patch. Patch is 56.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145565.diff 31 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 7ba21fca6dd6b..77a0c559f7689 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -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.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 4f91b82a3bfa6..26e953f7ac613 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3838,6 +3838,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">;
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index fd75de42515da..c8c3d6b20c496 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -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.
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 16688810d0685..96fdab212beb1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -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() {}
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 1b67d4354efc0..cb013feb769fc 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -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,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 87d04a42fcd70..25c65ab52fbaf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6803,6 +6803,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);
+ if (Args.hasArg(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);
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
new file mode 100644
index 0000000000000..839dd44f7ff61
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
@@ -0,0 +1,18 @@
+// REQUIRES: x86-registered-target
+
+// 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;
+}
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp b/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
new file mode 100644
index 0000000000000..3dc75c95d76f7
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp b/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
new file mode 100644
index 0000000000000..69704626c8cb6
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// 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"
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
new file mode 100644
index 0000000000000..d76d2aa6d8acc
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
@@ -0,0 +1,135 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
new file mode 100644
index 0000000000000..6adb0b1818e31
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// 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;
+}
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch.c b/clang/test/CodeGen/X86/ms-secure-hotpatch.c
new file mode 100644
index 0000000000000..b829e5acc5c83
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch.c
@@ -0,0 +1,23 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/llvm/include/llvm/CodeGen/Passes.h b/llvm/include/llvm/CodeGen/Passes.h
index 990452fa11fec..18df5d657064a 100644
--- a/llvm/include/llvm/CodeGen/Passes.h
+++ b/llvm/include/llvm/CodeGen/Passes.h
@@ -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
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
index 9d85acc49fa02..b38bdb482df43 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
@@ -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
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
index 5b4f0d31e6427..f5f6fe69430cc 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
@@ -177,6 +177,21 @@ class CallerSym : public SymbolRecord {
uint32_t RecordOffset = 0;
};
+class HotPatchFuncSym : public SymbolRecord {
+public:
+ explicit HotPatchFuncSym(SymbolRecordKind Kind) : SymbolRecord(Kind) {}
+ HotPatchFuncSym(uint32_t RecordOffset)
+ : SymbolRecord(SymbolRecordKind::HotPatchFuncSym),
+ RecordOffset(RecordOffset) {}
+
+ // This is an ItemID in the IPI stream, which points to an LF_FUNC_ID or
+ // LF_MFUNC_ID record.
+ TypeIndex Function;
+ StringRef Name;
+
+ uint32_t RecordOffset = 0;
+};
+
struct DecodedAnnotation {
StringRef Name;
ArrayRef<uint8_t> Bytes;
diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td
index d488c5f419b82..0bcd15eeed879 100644
--- a/llvm/include/llvm/IR/Attributes.td
+++ b/llvm/include/llvm/IR/Attributes.td
@@ -389,6 +389,16 @@ def CoroDestroyOnlyWhenComplete : EnumAttr<"coro_only_destroy_when_complete", In
/// pipeline to perform elide on the call or invoke instruction.
def CoroElideSafe : EnumAttr<"coro_elide_safe", IntersectPreserve, [FnAttr]>;
+/// Function is marked for Windows Hot Patching
+def MarkedForWindowsSecureHotPatching
+ : StrBoolAttr<"marked_for_windows_hot_patching">;
+
+/// Global variable should not be accessed through a "__ref_" global variable in
+/// a hot patching function This attribute is applied to the global variable
+/// decl, not the hotpatched function.
+def AllowDirectAccessInHotPatchFunction
+ : StrBoolAttr<"allow_direct_access_in_hot_patch_function">;
+
/// Target-independent string attributes.
def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;
def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 1b5b1d5888824..1c4ed3843b390 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -336,6 +336,7 @@ LLVM_ABI void initializeVerifierLegacyPassPass(PassRegistry &);
LLVM_ABI void initializeVirtRegMapWrapperLegacyPass(PassRegistry &);
LLVM_ABI void initializeVirtRegRewriterLegacyPass(PassRegistry &);
LLVM_ABI void initializeWasmEHPreparePass(PassRegistry &);
+LLVM_ABI void initializeWindowsSecureHotPatchingPass(PassRegistry &);
LLVM_ABI void initializeWinEHPreparePass(PassRegistry &);
LLVM_ABI void initializeWriteBitcodePassPass(PassRegistry &);
LLVM_ABI void initializeXRayInstrumentationLegacyPass(PassRegistry &);
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index ea57a8fa1f793..5e1b313b4d2fa 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -669,6 +669,8 @@ void CodeViewDebug::endModule() {
if (!Asm)
return;
+ emitSecureHotPatchInformation();
+
emitInlineeLinesSubsection();
// Emit per-function debug information.
@@ -823,6 +825,28 @@ void CodeViewDebug::emitObjName() {
endSymbolRecord(CompilerEnd);
}
+void CodeViewDebug::emitSecureHotPatchInformation() {
+ MCSymbol *hotPatchInfo = nullptr;
+
+ for (const auto &F : MMI->getModule()->functions()) {
+ if (!F.isDeclarationForLinker() &&
+ F.hasFnAttribute("marked_for_windows_hot_patching")) {
+ if (hotPatchInfo == nullptr)
+ hotPatchInfo = beginCVSubsection(DebugSubsectionKind::Symbols);
+ MCSymbol *HotPatchEnd = beginSymbolRecord(SymbolKind::S_HOTPATCHFUNC);
+ auto *SP...
[truncated]
|
|
@llvm/pr-subscribers-clang-driver Author: None (sivadeilra) Changes(This is a re-do of #138972, which had a minor warning in This PR adds some of the support needed for Windows hot-patching. Windows implements a form of hot-patching. This allows patches to be applied to Windows apps, drivers, and the kernel, without rebooting or restarting any of these components. Hot-patching is a complex technology and requires coordination between the OS, compilers, linkers, and additional tools. This PR adds support to Clang and LLVM for part of the hot-patching process. It enables LLVM to generate the required code changes and to generate CodeView symbols which identify hot-patched functions. The PR provides new command-line arguments to Clang which allow developers to identify the list of functions that need to be hot-patched. This PR also allows LLVM to directly receive the list of functions to be modified, so that language front-ends which have not yet been modified (such as Rust) can still make use of hot-patching. This PR:
Although the flags are redundant between Clang and LLVM, this allows additional languages (such as Rust) to take advantage of hot-patching support before they have been modified to generate the required attributes. Credit to @dpaoliello, who wrote the original form of this patch. Patch is 56.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145565.diff 31 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 7ba21fca6dd6b..77a0c559f7689 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -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.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 4f91b82a3bfa6..26e953f7ac613 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3838,6 +3838,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">;
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index fd75de42515da..c8c3d6b20c496 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -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.
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 16688810d0685..96fdab212beb1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -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() {}
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 1b67d4354efc0..cb013feb769fc 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -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,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 87d04a42fcd70..25c65ab52fbaf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6803,6 +6803,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);
+ if (Args.hasArg(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);
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
new file mode 100644
index 0000000000000..839dd44f7ff61
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-bad-file.c
@@ -0,0 +1,18 @@
+// REQUIRES: x86-registered-target
+
+// 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;
+}
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp b/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
new file mode 100644
index 0000000000000..3dc75c95d76f7
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-cpp.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp b/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
new file mode 100644
index 0000000000000..69704626c8cb6
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-eh.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// 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"
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
new file mode 100644
index 0000000000000..d76d2aa6d8acc
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-globals.c
@@ -0,0 +1,135 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c b/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
new file mode 100644
index 0000000000000..6adb0b1818e31
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch-lto.c
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// 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;
+}
diff --git a/clang/test/CodeGen/X86/ms-secure-hotpatch.c b/clang/test/CodeGen/X86/ms-secure-hotpatch.c
new file mode 100644
index 0000000000000..b829e5acc5c83
--- /dev/null
+++ b/clang/test/CodeGen/X86/ms-secure-hotpatch.c
@@ -0,0 +1,23 @@
+// REQUIRES: x86-registered-target
+
+// 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
diff --git a/llvm/include/llvm/CodeGen/Passes.h b/llvm/include/llvm/CodeGen/Passes.h
index 990452fa11fec..18df5d657064a 100644
--- a/llvm/include/llvm/CodeGen/Passes.h
+++ b/llvm/include/llvm/CodeGen/Passes.h
@@ -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
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
index 9d85acc49fa02..b38bdb482df43 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def
@@ -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
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
index 5b4f0d31e6427..f5f6fe69430cc 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
@@ -177,6 +177,21 @@ class CallerSym : public SymbolRecord {
uint32_t RecordOffset = 0;
};
+class HotPatchFuncSym : public SymbolRecord {
+public:
+ explicit HotPatchFuncSym(SymbolRecordKind Kind) : SymbolRecord(Kind) {}
+ HotPatchFuncSym(uint32_t RecordOffset)
+ : SymbolRecord(SymbolRecordKind::HotPatchFuncSym),
+ RecordOffset(RecordOffset) {}
+
+ // This is an ItemID in the IPI stream, which points to an LF_FUNC_ID or
+ // LF_MFUNC_ID record.
+ TypeIndex Function;
+ StringRef Name;
+
+ uint32_t RecordOffset = 0;
+};
+
struct DecodedAnnotation {
StringRef Name;
ArrayRef<uint8_t> Bytes;
diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td
index d488c5f419b82..0bcd15eeed879 100644
--- a/llvm/include/llvm/IR/Attributes.td
+++ b/llvm/include/llvm/IR/Attributes.td
@@ -389,6 +389,16 @@ def CoroDestroyOnlyWhenComplete : EnumAttr<"coro_only_destroy_when_complete", In
/// pipeline to perform elide on the call or invoke instruction.
def CoroElideSafe : EnumAttr<"coro_elide_safe", IntersectPreserve, [FnAttr]>;
+/// Function is marked for Windows Hot Patching
+def MarkedForWindowsSecureHotPatching
+ : StrBoolAttr<"marked_for_windows_hot_patching">;
+
+/// Global variable should not be accessed through a "__ref_" global variable in
+/// a hot patching function This attribute is applied to the global variable
+/// decl, not the hotpatched function.
+def AllowDirectAccessInHotPatchFunction
+ : StrBoolAttr<"allow_direct_access_in_hot_patch_function">;
+
/// Target-independent string attributes.
def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;
def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 1b5b1d5888824..1c4ed3843b390 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -336,6 +336,7 @@ LLVM_ABI void initializeVerifierLegacyPassPass(PassRegistry &);
LLVM_ABI void initializeVirtRegMapWrapperLegacyPass(PassRegistry &);
LLVM_ABI void initializeVirtRegRewriterLegacyPass(PassRegistry &);
LLVM_ABI void initializeWasmEHPreparePass(PassRegistry &);
+LLVM_ABI void initializeWindowsSecureHotPatchingPass(PassRegistry &);
LLVM_ABI void initializeWinEHPreparePass(PassRegistry &);
LLVM_ABI void initializeWriteBitcodePassPass(PassRegistry &);
LLVM_ABI void initializeXRayInstrumentationLegacyPass(PassRegistry &);
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index ea57a8fa1f793..5e1b313b4d2fa 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -669,6 +669,8 @@ void CodeViewDebug::endModule() {
if (!Asm)
return;
+ emitSecureHotPatchInformation();
+
emitInlineeLinesSubsection();
// Emit per-function debug information.
@@ -823,6 +825,28 @@ void CodeViewDebug::emitObjName() {
endSymbolRecord(CompilerEnd);
}
+void CodeViewDebug::emitSecureHotPatchInformation() {
+ MCSymbol *hotPatchInfo = nullptr;
+
+ for (const auto &F : MMI->getModule()->functions()) {
+ if (!F.isDeclarationForLinker() &&
+ F.hasFnAttribute("marked_for_windows_hot_patching")) {
+ if (hotPatchInfo == nullptr)
+ hotPatchInfo = beginCVSubsection(DebugSubsectionKind::Symbols);
+ MCSymbol *HotPatchEnd = beginSymbolRecord(SymbolKind::S_HOTPATCHFUNC);
+ auto *SP...
[truncated]
|
|
Both the original patch and the redo break the Solaris/amd64 buildbot. I wonder if somehow the fact that the source lives in |
Yes, the files names in the tests have to come after hyphens (—) otherwise they can be treated like flags on some systems, here on Solaris or on MacOS. @dpaoliello @sivadeilra |
|
Looking at this now. Would it be sufficient to change the Also, why was this not caught during the CI jobs for the PR? Can something be changed so that it would be caught during CI? |
You actually have to add (two) hyphens before %s in the tests, such as:
This needs a unique file system setup, such as running inside a /opt folder, but I assume this could be replicated on the pre-commit CI. Would you mind filling an issue please? |
|
Ok, I've created #145713 for this. So if I understand correctly, all of the tests that I just added for Is there a way I could have detected this during my local builds, before submitting a PR? |
I assume checking out the repo on a Linux box under |
|
I just tried building and running tests under Ubuntu 24.04. The tests passed, i.e. they didn't hit the same problem. So I don't have an easy way to verify a fix. |
I recall on MacOS the issue happens in a |
Merged
|
#145737 for fix. I tried moving my checkout dir to |
You can also check the test command lines and the outputs by running a single test in a more verbose mode, such as: And try to tweak the base folder or the command lines accordingly until you hit the issue. |
dpaoliello
pushed a commit
that referenced
this pull request
Jun 25, 2025
#145565 broke the Solaris buildbot, due to a subtlety in how command-lines are parsed, which is different between Windows and non-Windows platforms. The fix is to use `--` to force passing the rest of args without interpretation. This is similar to existing tests for `%clang_cl`, such as `clang/test/CodeGen/debug-info-codeview-buildinfo.c`. Currently, CI jobs for PRs do not detect this problem. Fixing that is tracked in issue #145713.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/136/builds/4376 Here is the relevant piece of the build log for the reference |
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
(This is a re-do of llvm#138972, which had a minor warning in `Clang.cpp`.) This PR adds some of the support needed for Windows hot-patching. Windows implements a form of hot-patching. This allows patches to be applied to Windows apps, drivers, and the kernel, without rebooting or restarting any of these components. Hot-patching is a complex technology and requires coordination between the OS, compilers, linkers, and additional tools. This PR adds support to Clang and LLVM for part of the hot-patching process. It enables LLVM to generate the required code changes and to generate CodeView symbols which identify hot-patched functions. The PR provides new command-line arguments to Clang which allow developers to identify the list of functions that need to be hot-patched. This PR also allows LLVM to directly receive the list of functions to be modified, so that language front-ends which have not yet been modified (such as Rust) can still make use of hot-patching. This PR: * Adds a `MarkedForWindowsHotPatching` LLVM function attribute. This attribute indicates that a function should be _hot-patched_. This generates a new CodeView symbol, `S_HOTPATCHFUNC`, which identifies any function that has been hot-patched. This attribute also causes accesses to global variables to be indirected through a `_ref_*` global variable. This allows hot-patched functions to access the correct version of a global variable; the hot-patched code needs to access the variable in the _original_ image, not the patch image. * Adds a `AllowDirectAccessInHotPatchFunction` LLVM attribute. This attribute may be placed on global variable declarations. It indicates that the variable may be safely accessed without the `_ref_*` indirection. * Adds two Clang command-line parameters: `-fms-hotpatch-functions-file` and `-fms-hotpatch-functions-list`. The `-file` flag may point to a text file, which contains a list of functions to be hot-patched (one function name per line). The `-list` flag simply directly identifies functions to be patched, using a comma-separated list. These two command-line parameters may also be combined; the final set of functions to be hot-patched is the union of the two sets. * Adds similar LLVM command-line parameters: `--ms-hotpatch-functions-file` and `--ms-hotpatch-functions-list`. * Adds integration tests for both LLVM and Clang. * Adds support for dumping the new `S_HOTPATCHFUNC` CodeView symbol. Although the flags are redundant between Clang and LLVM, this allows additional languages (such as Rust) to take advantage of hot-patching support before they have been modified to generate the required attributes. Credit to @dpaoliello, who wrote the original form of this patch.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
llvm#145565 broke the Solaris buildbot, due to a subtlety in how command-lines are parsed, which is different between Windows and non-Windows platforms. The fix is to use `--` to force passing the rest of args without interpretation. This is similar to existing tests for `%clang_cl`, such as `clang/test/CodeGen/debug-info-codeview-buildinfo.c`. Currently, CI jobs for PRs do not detect this problem. Fixing that is tracked in issue llvm#145713.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(This is a re-do of #138972, which had a minor warning in
Clang.cpp.)This PR adds some of the support needed for Windows hot-patching.
Windows implements a form of hot-patching. This allows patches to be applied to Windows apps, drivers, and the kernel, without rebooting or restarting any of these components. Hot-patching is a complex technology and requires coordination between the OS, compilers, linkers, and additional tools.
This PR adds support to Clang and LLVM for part of the hot-patching process. It enables LLVM to generate the required code changes and to generate CodeView symbols which identify hot-patched functions. The PR provides new command-line arguments to Clang which allow developers to identify the list of functions that need to be hot-patched. This PR also allows LLVM to directly receive the list of functions to be modified, so that language front-ends which have not yet been modified (such as Rust) can still make use of hot-patching.
This PR:
MarkedForWindowsHotPatchingLLVM function attribute. This attribute indicates that a function should be hot-patched. This generates a new CodeView symbol,S_HOTPATCHFUNC, which identifies any function that has been hot-patched. This attribute also causes accesses to global variables to be indirected through a_ref_*global variable. This allows hot-patched functions to access the correct version of a global variable; the hot-patched code needs to access the variable in the original image, not the patch image.AllowDirectAccessInHotPatchFunctionLLVM attribute. This attribute may be placed on global variable declarations. It indicates that the variable may be safely accessed without the_ref_*indirection.-fms-hotpatch-functions-fileand-fms-hotpatch-functions-list. The-fileflag may point to a text file, which contains a list of functions to be hot-patched (one function name per line). The-listflag simply directly identifies functions to be patched, using a comma-separated list. These two command-line parameters may also be combined; the final set of functions to be hot-patched is the union of the two sets.--ms-hotpatch-functions-fileand--ms-hotpatch-functions-list.S_HOTPATCHFUNCCodeView symbol.Although the flags are redundant between Clang and LLVM, this allows additional languages (such as Rust) to take advantage of hot-patching support before they have been modified to generate the required attributes.
Credit to @dpaoliello, who wrote the original form of this patch.