From 5af2433e1794ebf7e58e848aa612c7912d71dc78 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Thu, 20 Jan 2022 11:04:46 -0500 Subject: [PATCH] [clang-cl] Support the /HOTPATCH flag This patch adds support for the MSVC /HOTPATCH flag: https://docs.microsoft.com/sv-se/cpp/build/reference/hotpatch-create-hotpatchable-image?view=msvc-170&viewFallbackFrom=vs-2019 The flag is translated to a new -fms-hotpatch flag, which in turn adds a 'patchable-function' attribute for each function in the TU. This is then picked up by the PatchableFunction pass which would generate a TargetOpcode::PATCHABLE_OP of minsize = 2 (which means the target instruction must resolve to at least two bytes). TargetOpcode::PATCHABLE_OP is only implemented for x86/x64. When targetting ARM/ARM64, /HOTPATCH isn't required (instructions are always 2/4 bytes and suitable for hotpatching). Additionally, when using /Z7, we generate a 'hot patchable' flag in the CodeView debug stream, in the S_COMPILE3 record. This flag is then picked up by LLD (or link.exe) and is used in conjunction with the linker /FUNCTIONPADMIN flag to generate extra space before each function, to accommodate for live patching long jumps. Please see: https://github.com/llvm/llvm-project/blob/d703b922961e0d02a5effdd4bfbb23ad50a3cc9f/lld/COFF/Writer.cpp#L1298 The outcome is that we can finally use Live++ or Recode along with clang-cl. NOTE: It seems that MSVC cl.exe always enables /HOTPATCH on x64 by default, although if we did the same I thought we might generate sub-optimal code (if this flag was active by default). Additionally, MSVC always generates a .debug$S section and a S_COMPILE3 record, which Clang doesn't do without /Z7. Therefore, the following MSVC command-line "cl /c file.cpp" would have to be written with Clang such as "clang-cl /c file.cpp /HOTPATCH /Z7" in order to obtain the same result. Depends on D43002, D80833 and D81301 for the full feature. Differential Revision: https://reviews.llvm.org/D116511 --- clang/docs/ReleaseNotes.rst | 10 +++++++++ clang/include/clang/Basic/CodeGenOptions.def | 3 +++ clang/include/clang/Driver/Options.td | 6 ++++- clang/lib/CodeGen/BackendUtil.cpp | 1 + clang/lib/CodeGen/CodeGenFunction.cpp | 7 ++++++ clang/lib/Driver/ToolChains/Clang.cpp | 2 ++ clang/lib/Driver/ToolChains/MSVC.cpp | 5 +++++ clang/test/CodeGen/patchable-function-entry.c | 5 +++++ clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp | 26 ++++++++++++++++++++++ clang/test/CodeGenCXX/debug-info-hotpatch.cpp | 20 +++++++++++++++++ clang/test/Driver/cl-options.c | 4 +++- llvm/include/llvm/Target/TargetOptions.h | 9 +++++--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | 6 +++++ .../DebugInfo/COFF/ARMNT/arm-register-variables.ll | 3 ++- llvm/test/MC/AArch64/coff-debug.ll | 3 ++- 15 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp create mode 100644 clang/test/CodeGenCXX/debug-info-hotpatch.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index de395b5..08e4d75 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -136,6 +136,16 @@ Windows Support or pass ``/permissive`` to disable C++ operator names altogether. See `PR42427 ` for more info. +- Add support for MSVC-compatible ``/hotpatch`` flag in clang-cl, and equivalent + -cc1 flag ``-fms-hotpatch``. Along with the linker flag ``/functionpadmin`` + this creates executable images suitable for runtime code patching. This flag + is only required for x86/x64 targets; ARM/ARM64 simply needs the linker + ``/functionpadmin``. + + With this addition, clang-cl can be used in live code patching scenarios, + along with tools such as Live++ or Recode. Microsoft Edit and Continue isn't + currently supported. + C Language Changes in Clang --------------------------- diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index d4742cd..3526b8a 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -139,6 +139,9 @@ VALUE_CODEGENOPT(XRaySelectedFunctionGroup, 32, 0) VALUE_CODEGENOPT(PatchableFunctionEntryCount , 32, 0) ///< Number of NOPs at function entry VALUE_CODEGENOPT(PatchableFunctionEntryOffset , 32, 0) +CODEGENOPT(HotPatch, 1, 0) ///< Supports the Microsoft /HOTPATCH flag and + ///< generates a 'patchable-function' attribute. + CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled. CODEGENOPT(CallFEntry , 1, 0) ///< Set when -mfentry is enabled. CODEGENOPT(MNopMCount , 1, 0) ///< Set when -mnop-mcount is enabled. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 1a9fd61..b66363b 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2498,6 +2498,9 @@ defm pascal_strings : BoolFOption<"pascal-strings", def fpatchable_function_entry_EQ : Joined<["-"], "fpatchable-function-entry=">, Group, Flags<[CC1Option]>, MetaVarName<"">, HelpText<"Generate M NOPs before function entry and N-M NOPs after function entry">, MarshallingInfoInt>; +def fms_hotpatch : Flag<["-"], "fms-hotpatch">, Group, Flags<[CC1Option, CoreOption]>, + HelpText<"Ensure that all functions can be hotpatched at runtime">, + MarshallingInfoFlag>; def fpcc_struct_return : Flag<["-"], "fpcc-struct-return">, Group, Flags<[CC1Option]>, HelpText<"Override the default ABI to return all structs on the stack">; def fpch_preprocess : Flag<["-"], "fpch-preprocess">, Group; @@ -6124,6 +6127,8 @@ def _SLASH_Gw_ : CLFlag<"Gw-">, def _SLASH_help : CLFlag<"help">, Alias, HelpText<"Display available options">; def _SLASH_HELP : CLFlag<"HELP">, Alias; +def _SLASH_hotpatch : CLFlag<"hotpatch">, Alias, + HelpText<"Create hotpatchable image">; def _SLASH_I : CLJoinedOrSeparate<"I">, HelpText<"Add directory to include search path">, MetaVarName<"">, Alias; @@ -6480,7 +6485,6 @@ def _SLASH_headerUnit : CLJoinedOrSeparate<"headerUnit">; def _SLASH_headerUnitAngle : CLJoinedOrSeparate<"headerUnit:angle">; def _SLASH_headerUnitQuote : CLJoinedOrSeparate<"headerUnit:quote">; def _SLASH_homeparams : CLFlag<"homeparams">; -def _SLASH_hotpatch : CLFlag<"hotpatch">; def _SLASH_kernel : CLFlag<"kernel">; def _SLASH_LN : CLFlag<"LN">; def _SLASH_MP : CLJoined<"MP">; diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 67fee7f..6b8e052 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -645,6 +645,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags, Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs; Options.DebugStrictDwarf = CodeGenOpts.DebugStrictDwarf; Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug; + Options.Hotpatch = CodeGenOpts.HotPatch; return true; } diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 54ddbff..50e1638 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -882,6 +882,13 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy, if (Offset) Fn->addFnAttr("patchable-function-prefix", std::to_string(Offset)); } + // Instruct that functions for COFF/CodeView targets should start with a + // patchable instruction, but only on x86/x64. Don't forward this to ARM/ARM64 + // backends as they don't need it -- instructions on these architectures are + // always atomically patchable at runtime. + if (CGM.getCodeGenOpts().HotPatch && + getContext().getTargetInfo().getTriple().isX86()) + Fn->addFnAttr("patchable-function", "prologue-short-redirect"); // Add no-jump-tables value. if (CGM.getCodeGenOpts().NoUseJumpTables) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index c48c6fd..a22f03a 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6001,6 +6001,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, } } + Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch); + if (TC.SupportsProfiling()) { Args.AddLastArg(CmdArgs, options::OPT_pg); diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index 4e15c3a..7e8636a 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -511,6 +511,11 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7)) CmdArgs.push_back("-debug"); + // If we specify /hotpatch, let the linker add padding in front of each + // function, like MSVC does. + if (Args.hasArg(options::OPT_fms_hotpatch, options::OPT__SLASH_hotpatch)) + CmdArgs.push_back("-functionpadmin"); + // Pass on /Brepro if it was passed to the compiler. // Note that /Brepro maps to -mno-incremental-linker-compatible. bool DefaultIncrementalLinkerCompatible = diff --git a/clang/test/CodeGen/patchable-function-entry.c b/clang/test/CodeGen/patchable-function-entry.c index 3065eb2..6e8d0d7 100644 --- a/clang/test/CodeGen/patchable-function-entry.c +++ b/clang/test/CodeGen/patchable-function-entry.c @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s // RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -o - | FileCheck --check-prefixes=CHECK,OPT %s +// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fms-hotpatch -o - | FileCheck --check-prefixes=HOTPATCH %s // CHECK: define{{.*}} void @f0() #0 __attribute__((patchable_function_entry(0))) void f0() {} @@ -34,3 +35,7 @@ void f() {} // CHECK: attributes #2 = { {{.*}} "patchable-function-entry"="0" "patchable-function-prefix"="4" // CHECK: attributes #3 = { {{.*}} "patchable-function-entry"="3" "patchable-function-prefix"="2" // OPT: attributes #4 = { {{.*}} "patchable-function-entry"="1" +// HOTPATCH: attributes #0 = { {{.*}} "patchable-function"="prologue-short-redirect" +// HOTPATCH: attributes #1 = { {{.*}} "patchable-function"="prologue-short-redirect" +// HOTPATCH: attributes #2 = { {{.*}} "patchable-function"="prologue-short-redirect" +// HOTPATCH: attributes #3 = { {{.*}} "patchable-function"="prologue-short-redirect" diff --git a/clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp b/clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp new file mode 100644 index 0000000..6176f17 --- /dev/null +++ b/clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp @@ -0,0 +1,26 @@ +// REQUIRES: aarch64-registered-target || arm-registered-target +/// +/// Check that using /hotpatch doesn't generate an error. +/// Binaries are always hotpatchable on ARM/ARM64. +/// +// RUN: %clang_cl --target=arm-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1 +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1 +/// +/// Ensure that we set the hotpatchable flag in the debug information. +/// +// RUN: %clang_cl --target=arm-pc-windows-msvc /c /Z7 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /Z7 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// HOTPATCH: S_COMPILE3 [size = [[#]]] +// HOTPATCH: flags = hot patchable +/// +/// Unfortunately we need /Z7, Clang does not systematically generate S_COMPILE3. +/// +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH +// NO-HOTPATCH-NOT: flags = hot patchable + +int main() { + return 0; +} diff --git a/clang/test/CodeGenCXX/debug-info-hotpatch.cpp b/clang/test/CodeGenCXX/debug-info-hotpatch.cpp new file mode 100644 index 0000000..fde1a6a --- /dev/null +++ b/clang/test/CodeGenCXX/debug-info-hotpatch.cpp @@ -0,0 +1,20 @@ +// REQUIRES: x86-registered-target +/// +// RUN: %clang_cl --target=x86_64-windows-msvc /c /hotpatch /Z7 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// HOTPATCH: S_COMPILE3 [size = [[#]]] +// HOTPATCH: flags = hot patchable +/// +// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH +// NO-HOTPATCH-NOT: flags = hot patchable +/// +// RUN: %clang_cl --target=x86_64-windows-msvc /hotpatch -### -- %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=FUNCTIONPADMIN +// FUNCTIONPADMIN: clang{{.*}} +// FUNCTIONPADMIN: {{link.*"}} +// FUNCTIONPADMIN: -functionpadmin + +int main() { + return 0; +} diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c index 618be2d..f39db87 100644 --- a/clang/test/Driver/cl-options.c +++ b/clang/test/Driver/cl-options.c @@ -118,6 +118,9 @@ // RUN: %clang_cl /Gw /Gw- -### -- %s 2>&1 | FileCheck -check-prefix=Gw_ %s // Gw_-NOT: -fdata-sections +// RUN: %clang_cl /hotpatch -### -- %s 2>&1 | FileCheck -check-prefix=hotpatch %s +// hotpatch: -fms-hotpatch + // RUN: %clang_cl /Imyincludedir -### -- %s 2>&1 | FileCheck -check-prefix=SLASH_I %s // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck -check-prefix=SLASH_I %s // SLASH_I: "-I" "myincludedir" @@ -483,7 +486,6 @@ // RUN: /GZ \ // RUN: /H \ // RUN: /homeparams \ -// RUN: /hotpatch \ // RUN: /JMC \ // RUN: /kernel \ // RUN: /LN \ diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h index c639f32..a636c48 100644 --- a/llvm/include/llvm/Target/TargetOptions.h +++ b/llvm/include/llvm/Target/TargetOptions.h @@ -140,9 +140,9 @@ namespace llvm { EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false), EmitAddrsig(false), EmitCallSiteInfo(false), SupportsDebugEntryValues(false), EnableDebugEntryValues(false), - ValueTrackingVariableLocations(false), - ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false), - DebugStrictDwarf(false), + ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false), + XRayOmitFunctionIndex(false), DebugStrictDwarf(false), + Hotpatch(false), FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {} /// DisableFramePointerElim - This returns true if frame pointer elimination @@ -342,6 +342,9 @@ namespace llvm { /// By default, it is set to false. unsigned DebugStrictDwarf : 1; + /// Emit the hotpatch flag in CodeView debug. + unsigned Hotpatch : 1; + /// Name of the stack usage file (i.e., .su file) if user passes /// -fstack-usage. If empty, it can be implied that -fstack-usage is not /// passed on the command line. diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp index 9a8188e..52c7471 100644 --- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp @@ -846,6 +846,12 @@ void CodeViewDebug::emitCompilerInformation() { if (MMI->getModule()->getProfileSummary(/*IsCS*/ false) != nullptr) { Flags |= static_cast(CompileSym3Flags::PGO); } + using ArchType = llvm::Triple::ArchType; + ArchType Arch = Triple(MMI->getModule()->getTargetTriple()).getArch(); + if (Asm->TM.Options.Hotpatch || Arch == ArchType::thumb || + Arch == ArchType::aarch64) { + Flags |= static_cast(CompileSym3Flags::HotPatch); + } OS.AddComment("Flags and language"); OS.emitInt32(Flags); diff --git a/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll b/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll index d19cca3..781a4c6 100644 --- a/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll +++ b/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll @@ -24,7 +24,8 @@ ; OBJ: Compile3Sym { ; OBJ-NEXT: Kind: S_COMPILE3 (0x113C) ; OBJ-NEXT: Language: C (0x0) -; OBJ-NEXT: Flags [ (0x0) +; OBJ-NEXT: Flags [ (0x4000) +; OBJ-NEXT: HotPatch (0x4000) ; OBJ-NEXT: ] ; OBJ-NEXT: Machine: ARMNT (0xF4) diff --git a/llvm/test/MC/AArch64/coff-debug.ll b/llvm/test/MC/AArch64/coff-debug.ll index 6099b3d..af3459a 100644 --- a/llvm/test/MC/AArch64/coff-debug.ll +++ b/llvm/test/MC/AArch64/coff-debug.ll @@ -77,7 +77,8 @@ attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-ma ; CHECK: Compile3Sym { ; CHECK: Kind: S_COMPILE3 (0x113C) ; CHECK: Language: C (0x0) -; CHECK: Flags [ (0x0) +; CHECK: Flags [ (0x4000 +; CHECK: HotPatch (0x4000) ; CHECK: ] ; CHECK: } ; CHECK: ] -- 2.7.4