Windows hotpatching support

move hotpatch tests to X86 subdir

move llvm hotpatch tests to X86 dir

switch to strbool attributes

cleanup

require x86 target for tests

Author: sivadeilra

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.

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">;

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.

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() {}

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,

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 (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);
 

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;
+}

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

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"

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

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;
+}

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

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

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