Windows hotpatching support

Address PR feedback

* Simply use F.getName() for function name.
* Fix llvm/test/CodeGen/Generic/ms-hotpatch-direct-global-access.ll
* Add negative testing for functions that are not supposed to be hotpatched

Author: sivadeilra

clang/include/clang/Basic/CodeGenOptions.h

@@ -501,6 +501,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-hot-patch-functions-file.
+  std::string MSHotPatchFunctionsFile;
+
+  /// A list of functions which will be compiled for hotpatching.
+  /// See -fms-hot-patch-functions-list.
+  std::vector<std::string> MSHotPatchFunctionsList;
 
 public:
   // Define accessors/mutators for code generation options of enumeration type.

clang/include/clang/Driver/Options.td

@@ -3830,6 +3830,20 @@ 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">>;
+def fms_hotpatch_functions_file
+    : Joined<["-"], "fms-hotpatch-functions-file=">,
+      Group<f_Group>,
+      Visibility<[ClangOption, CC1Option, CLOption]>,
+      MarshallingInfoString<CodeGenOpts<"MSHotPatchFunctionsFile">>,
+      HelpText<"Path to a file that contains a list of mangled symbol names of "
+               "functions that should be hot-patched">;
+def fms_hotpatch_functions_list
+    : CommaJoined<["-"], "fms-hotpatch-functions-list=">,
+      Group<f_Group>,
+      Visibility<[ClangOption, CC1Option, CLOption]>,
+      MarshallingInfoStringVector<CodeGenOpts<"MSHotPatchFunctionsList">>,
+      HelpText<"List of mangled symbol names of functions that should be "
+               "hot-patched">;
 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

@@ -2639,6 +2639,15 @@ 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 = std::binary_search(MSHotPatchFunctions.begin(),
+                                             MSHotPatchFunctions.end(), Name);
+      if (IsHotPatched) {
+        FuncAttrs.addAttribute(llvm::Attribute::MarkedForWindowsHotPatching);
+      }
+    }
   }
 
   // Mark functions that are replaceable by the loader.

clang/lib/CodeGen/CodeGenModule.cpp

@@ -453,6 +453,37 @@ 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 hot-patching,
+  // then build the list of functions now.
+  if (!CGO.MSHotPatchFunctionsFile.empty() ||
+      !CGO.MSHotPatchFunctionsList.empty()) {
+    if (!CGO.MSHotPatchFunctionsFile.empty()) {
+      auto BufOrErr = llvm::MemoryBuffer::getFile(CGO.MSHotPatchFunctionsFile);
+      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.MSHotPatchFunctionsFile
+                          << BufOrErr.getError().message();
+      }
+    }
+
+    for (const auto &FuncName : CGO.MSHotPatchFunctionsList) {
+      this->MSHotPatchFunctions.push_back(FuncName);
+    }
+
+    std::sort(this->MSHotPatchFunctions.begin(),
+              this->MSHotPatchFunctions.end());
+  }
 }
 
 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

@@ -6946,6 +6946,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fms_hotpatch_functions_file)) {
+    Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch_functions_file);
+  }
+
+  for (const auto &A :
+       Args.getAllArgValues(options::OPT_fms_hotpatch_functions_list)) {
+    CmdArgs.push_back(
+        Args.MakeArgString("-fms-hotpatch-functions-list=" + Twine(A)));
+  }
+
   if (TC.SupportsProfiling()) {
     Args.AddLastArg(CmdArgs, options::OPT_pg);
 

clang/test/CodeGen/ms-hotpatch-bad-file.c

@@ -0,0 +1,16 @@
+// This verifies that we correctly handle a -fms-hotpatch-functions-file argument that points
+// to a missing file.
+//
+// RUN: not %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-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/ms-hotpatch-cpp.cpp

@@ -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-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/ms-hotpatch-functions.txt

@@ -0,0 +1 @@
+this_gets_hotpatched

clang/test/CodeGen/ms-hotpatch-lto.c

@@ -0,0 +1,18 @@
+// 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-hotpatch-functions-file=%S/ms-hotpatch-functions.txt -flto /Fo%t.bc %s
+// RUN: llvm-dis %t.bc -o - | FileCheck %s
+//
+// CHECK: ; Function Attrs: marked_for_windows_hot_patching mustprogress nofree noinline norecurse nosync nounwind sspstrong willreturn memory(none) uwtable
+// CHECK-NEXT: define dso_local noundef i32 @this_gets_hotpatched() local_unnamed_addr #0 !dbg !13 {
+//
+// CHECK: ; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind sspstrong willreturn memory(none) uwtable
+// CHECK-NEXT: define dso_local noundef i32 @this_does_not_get_hotpatched() local_unnamed_addr #1 !dbg !19 {
+
+int __declspec(noinline) this_gets_hotpatched() {
+    return 42;
+}
+
+int __declspec(noinline) this_does_not_get_hotpatched() {
+    return this_gets_hotpatched() + 100;
+}