-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support alternative sections for patchable function entries #131230
base: main
Are you sure you want to change the base?
Support alternative sections for patchable function entries #131230
Conversation
With -fpatchable-function-entry (or the patchable_function_entry function attribute), we emit records of patchable entry locations to the __patchable_function_entries section. Add an additional parameter to the command line option that allows one to specify a different default section name for the records, and an identical parameter to the function attribute that allows one to override the section used. The main use case for this change is the Linux kernel using prefix NOPs for ftrace, and thus depending on__patchable_function_entries to locate traceable functions. Functions that are not traceable currently disable entry NOPs using the function attribute, but this creates a compatibility issue with -fsanitize=kcfi, which expects all indirectly callable functions to have a type hash prefix at the same offset from the function entry. Adding a section parameter would allow the kernel to distinguish between traceable and non-traceable functions by adding entry records to separate sections while maintaining a stable function prefix layout for all functions. LKML discussion: https://lore.kernel.org/lkml/Y1QEzk%[email protected]/
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang-driver Author: Sami Tolvanen (samitolvanen) ChangesWith -fpatchable-function-entry (or the patchable_function_entry function attribute), we emit records of patchable entry locations to the __patchable_function_entries section. Add an additional parameter to the command line option that allows one to specify a different default section name for the records, and an identical parameter to the function attribute that allows one to override the section used. The main use case for this change is the Linux kernel using prefix NOPs for ftrace, and thus depending on__patchable_function_entries to locate traceable functions. Functions that are not traceable currently disable entry NOPs using the function attribute, but this creates a compatibility issue with -fsanitize=kcfi, which expects all indirectly callable functions to have a type hash prefix at the same offset from the function entry. Adding a section parameter would allow the kernel to distinguish between traceable and non-traceable functions by adding entry records to separate sections while maintaining a stable function prefix layout for all functions. LKML discussion: https://lore.kernel.org/lkml/Y1QEzk%2FA41PKLEPe@hirez.programming.kicks-ass.net/ Full diff: https://github.com/llvm/llvm-project/pull/131230.diff 15 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 4d34346460561..19e63ba9c4ad7 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -932,7 +932,8 @@ def PatchableFunctionEntry
"riscv64", "x86", "x86_64", "ppc", "ppc64"]>> {
let Spellings = [GCC<"patchable_function_entry">];
let Subjects = SubjectList<[Function, ObjCMethod]>;
- let Args = [UnsignedArgument<"Count">, DefaultIntArgument<"Offset", 0>];
+ let Args = [UnsignedArgument<"Count">, DefaultIntArgument<"Offset", 0>,
+ StringArgument<"Section", /* optional */ 1>];
let Documentation = [PatchableFunctionEntryDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 34e7ff9612859..c354d95b515e3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6502,10 +6502,12 @@ only N==1 is supported.
def PatchableFunctionEntryDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-``__attribute__((patchable_function_entry(N,M)))`` is used to generate M NOPs
-before the function entry and N-M NOPs after the function entry. This attribute
-takes precedence over the command line option ``-fpatchable-function-entry=N,M``.
-``M`` defaults to 0 if omitted.
+``__attribute__((patchable_function_entry(N,M,Section)))`` is used to generate M
+NOPs before the function entry and N-M NOPs after the function entry, with a record of
+the entry stored in section ``Section``. This attribute takes precedence over the
+command line option ``-fpatchable-function-entry=N,M,Section``. ``M`` defaults to 0
+if omitted.``Section`` defaults to the ``-fpatchable-function-entry`` section name if
+set, or to ``__patchable_function_entries`` otherwise.
This attribute is only supported on
aarch64/aarch64-be/loongarch32/loongarch64/riscv32/riscv64/i386/x86-64/ppc/ppc64 targets.
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index c531c656f42b7..e39a73bdb13ac 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -281,6 +281,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// -fprofile-generate, and -fcs-profile-generate.
std::string InstrProfileOutput;
+ /// Name of the patchable function entry section with
+ /// -fpatchable-function-entry.
+ std::string PatchableFunctionEntrySection;
+
/// Name of the profile file to use with -fprofile-sample-use.
std::string SampleProfileFile;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e6e6e892cdd7..e076419ebc015 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3598,6 +3598,10 @@ def err_conflicting_codeseg_attribute : Error<
def warn_duplicate_codeseg_attribute : Warning<
"duplicate code segment specifiers">, InGroup<Section>;
+def err_attribute_patchable_function_entry_invalid_section
+ : Error<"section argument to 'patchable_function_entry' attribute is not "
+ "valid for this target: %0">;
+
def err_anonymous_property: Error<
"anonymous property is not supported">;
def err_property_is_variably_modified : Error<
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 66ae8f1c7f064..c83099e70af11 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3749,10 +3749,16 @@ defm pascal_strings : BoolFOption<"pascal-strings",
// Note: This flag has different semantics in the driver and in -cc1. The driver accepts -fpatchable-function-entry=M,N
// and forwards it to -cc1 as -fpatchable-function-entry=M and -fpatchable-function-entry-offset=N. In -cc1, both flags
// are treated as a single integer.
-def fpatchable_function_entry_EQ : Joined<["-"], "fpatchable-function-entry=">, Group<f_Group>,
- Visibility<[ClangOption, CC1Option]>,
- MetaVarName<"<N,M>">, HelpText<"Generate M NOPs before function entry and N-M NOPs after function entry">,
- MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryCount">>;
+def fpatchable_function_entry_EQ
+ : Joined<["-"], "fpatchable-function-entry=">,
+ Group<f_Group>,
+ Visibility<[ClangOption, CC1Option]>,
+ MetaVarName<"<N,M,Section>">,
+ HelpText<"Generate M NOPs before function entry and N-M NOPs after "
+ "function entry. "
+ "If section is specified, use it instead of "
+ "__patchable_function_entries.">,
+ MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryCount">>;
def fms_hotpatch : Flag<["-"], "fms-hotpatch">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Ensure that all functions can be hotpatched at runtime">,
@@ -7577,6 +7583,11 @@ def fpatchable_function_entry_offset_EQ
: Joined<["-"], "fpatchable-function-entry-offset=">, MetaVarName<"<M>">,
HelpText<"Generate M NOPs before function entry">,
MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryOffset">>;
+def fpatchable_function_entry_section_EQ
+ : Joined<["-"], "fpatchable-function-entry-section=">,
+ MetaVarName<"<Section>">,
+ HelpText<"Use Section instead of __patchable_function_entries">,
+ MarshallingInfoString<CodeGenOpts<"PatchableFunctionEntrySection">>;
def fprofile_instrument_EQ : Joined<["-"], "fprofile-instrument=">,
HelpText<"Enable PGO instrumentation">, Values<"none,clang,llvm,csllvm">,
NormalizedValuesScope<"CodeGenOptions">,
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 447192bc7f60c..901cffd834840 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -959,18 +959,24 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
}
unsigned Count, Offset;
+ StringRef Section;
if (const auto *Attr =
D ? D->getAttr<PatchableFunctionEntryAttr>() : nullptr) {
Count = Attr->getCount();
Offset = Attr->getOffset();
+ Section = Attr->getSection();
} else {
Count = CGM.getCodeGenOpts().PatchableFunctionEntryCount;
Offset = CGM.getCodeGenOpts().PatchableFunctionEntryOffset;
}
+ if (Section.empty())
+ Section = CGM.getCodeGenOpts().PatchableFunctionEntrySection;
if (Count && Offset <= Count) {
Fn->addFnAttr("patchable-function-entry", std::to_string(Count - Offset));
if (Offset)
Fn->addFnAttr("patchable-function-prefix", std::to_string(Offset));
+ if (!Section.empty())
+ Fn->addFnAttr("patchable-function-entry-section", Section);
}
// 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
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index fe172d923ac07..f68cf5a7689d0 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6909,8 +6909,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getAsString(Args) << TripleStr;
else if (S.consumeInteger(10, Size) ||
- (!S.empty() && (!S.consume_front(",") ||
- S.consumeInteger(10, Offset) || !S.empty())))
+ (!S.empty() &&
+ (!S.consume_front(",") || S.consumeInteger(10, Offset))) ||
+ (!S.empty() && (!S.consume_front(",") || S.empty())))
D.Diag(diag::err_drv_invalid_argument_to_option)
<< S0 << A->getOption().getName();
else if (Size < Offset)
@@ -6919,6 +6920,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back(Args.MakeArgString(A->getSpelling() + Twine(Size)));
CmdArgs.push_back(Args.MakeArgString(
"-fpatchable-function-entry-offset=" + Twine(Offset)));
+ if (!S.empty())
+ CmdArgs.push_back(
+ Args.MakeArgString("-fpatchable-function-entry-section=" + S));
}
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index bc858c63f69b6..2868ee271e151 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5759,9 +5759,10 @@ static void handlePatchableFunctionEntryAttr(Sema &S, Decl *D,
return;
}
uint32_t Count = 0, Offset = 0;
+ StringRef Section;
if (!S.checkUInt32Argument(AL, AL.getArgAsExpr(0), Count, 0, true))
return;
- if (AL.getNumArgs() == 2) {
+ if (AL.getNumArgs() >= 2) {
Expr *Arg = AL.getArgAsExpr(1);
if (!S.checkUInt32Argument(AL, Arg, Offset, 1, true))
return;
@@ -5771,8 +5772,25 @@ static void handlePatchableFunctionEntryAttr(Sema &S, Decl *D,
return;
}
}
- D->addAttr(::new (S.Context)
- PatchableFunctionEntryAttr(S.Context, AL, Count, Offset));
+ if (AL.getNumArgs() == 3) {
+ SourceLocation LiteralLoc;
+ if (!S.checkStringLiteralArgumentAttr(AL, 2, Section, &LiteralLoc))
+ return;
+ if (llvm::Error E = S.isValidSectionSpecifier(Section)) {
+ S.Diag(LiteralLoc,
+ diag::err_attribute_patchable_function_entry_invalid_section)
+ << toString(std::move(E));
+ return;
+ }
+ if (Section.empty()) {
+ S.Diag(LiteralLoc,
+ diag::err_attribute_patchable_function_entry_invalid_section)
+ << "section must not be empty";
+ return;
+ }
+ }
+ D->addAttr(::new (S.Context) PatchableFunctionEntryAttr(S.Context, AL, Count,
+ Offset, Section));
}
static void handleBuiltinAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
diff --git a/clang/test/CodeGen/patchable-function-entry-section.c b/clang/test/CodeGen/patchable-function-entry-section.c
new file mode 100644
index 0000000000000..4c0d2a1baf77b
--- /dev/null
+++ b/clang/test/CodeGen/patchable-function-entry-section.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck --check-prefixes=COMMON,NODEFAULT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -fpatchable-function-entry-section=__default_section -o - | FileCheck --check-prefixes=COMMON,DEFAULT %s
+
+// COMMON: define{{.*}} void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0(void) {}
+
+// COMMON: define{{.*}} void @f00() #0
+__attribute__((patchable_function_entry(0, 0, "__unused_section"))) void f00(void) {}
+
+// COMMON: define{{.*}} void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2(void) {}
+
+// COMMON: define{{.*}} void @f20() #2
+__attribute__((patchable_function_entry(2, 0, "__attr_section"))) void f20(void) {}
+
+// COMMON: define{{.*}} void @f44() #3
+__attribute__((patchable_function_entry(4, 4))) void f44(void) {}
+
+// COMMON: define{{.*}} void @f52() #4
+__attribute__((patchable_function_entry(5, 2, "__attr_section"))) void f52(void) {}
+
+// OPT: define{{.*}} void @f() #5
+void f(void) {}
+
+/// No need to emit "patchable-function-entry" and thus also "patchable-function-entry-section"
+// COMMON: attributes #0 = { {{.*}}
+// COMMON-NOT: "patchable-function-entry-section"
+
+// NODEFAULT: attributes #1 = { {{.*}} "patchable-function-entry"="2"
+// NODEFAULT-NOT: "patchable-function-entry-section"
+// DEFAULT: attributes #1 = { {{.*}} "patchable-function-entry"="2" "patchable-function-entry-section"="__default_section"
+
+// COMMON: attributes #2 = { {{.*}} "patchable-function-entry"="2" "patchable-function-entry-section"="__attr_section"
+
+// NODEFAULT: attributes #3 = { {{.*}} "patchable-function-entry"="0" "patchable-function-prefix"="4"
+// NODEFAULT-NOT: "patchable-function-entry-section"
+// DEFAULT: attributes #3 = { {{.*}} "patchable-function-entry"="0" "patchable-function-entry-section"="__default_section" "patchable-function-prefix"="4"
+
+// COMMON: attributes #4 = { {{.*}} "patchable-function-entry"="3" "patchable-function-entry-section"="__attr_section" "patchable-function-prefix"="2"
+
+// DEFAULT: attributes #5 = { {{.*}} "patchable-function-entry"="1" "patchable-function-entry-section"="__default_section"
diff --git a/clang/test/Driver/fpatchable-function-entry.c b/clang/test/Driver/fpatchable-function-entry.c
index 5f07ca99a69de..43be6c5a47e47 100644
--- a/clang/test/Driver/fpatchable-function-entry.c
+++ b/clang/test/Driver/fpatchable-function-entry.c
@@ -15,6 +15,9 @@
// RUN: %clang --target=aarch64 -fsyntax-only %s -fpatchable-function-entry=2,1 -c -### 2>&1 | FileCheck --check-prefix=21 %s
// 21: "-fpatchable-function-entry=2" "-fpatchable-function-entry-offset=1"
+// RUN: %clang --target=aarch64 -fsyntax-only %s -fpatchable-function-entry=1,1,__section_name -c -### 2>&1 | FileCheck --check-prefix=SECTION %s
+// SECTION: "-fpatchable-function-entry=1" "-fpatchable-function-entry-offset=1" "-fpatchable-function-entry-section=__section_name"
+
// RUN: not %clang --target=powerpc64-ibm-aix-xcoff -fsyntax-only %s -fpatchable-function-entry=1 2>&1 | FileCheck --check-prefix=AIX64 %s
// AIX64: error: unsupported option '-fpatchable-function-entry=1' for target 'powerpc64-ibm-aix-xcoff'
diff --git a/clang/test/Sema/patchable-function-entry-attr.c b/clang/test/Sema/patchable-function-entry-attr.c
index 89e4380c36230..f453e134ab625 100644
--- a/clang/test/Sema/patchable-function-entry-attr.c
+++ b/clang/test/Sema/patchable-function-entry-attr.c
@@ -3,9 +3,15 @@
// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
__attribute__((patchable_function_entry)) void f(void);
-// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+// expected-error@+1 {{expected string literal as argument of 'patchable_function_entry' attribute}}
__attribute__((patchable_function_entry(0, 0, 0))) void f(void);
+// expected-error@+1 {{section argument to 'patchable_function_entry' attribute is not valid for this target}}
+__attribute__((patchable_function_entry(0, 0, ""))) void f(void);
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 3 arguments}}
+__attribute__((patchable_function_entry(0, 0, "__section", 0))) void f(void);
+
// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
__attribute__((patchable_function_entry(-1))) void f(void);
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 6d99cb3a516cc..ce1fe9b05ef6b 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -4583,7 +4583,13 @@ void AsmPrinter::emitPatchableFunctionEntries() {
if (TM.getTargetTriple().isOSBinFormatELF()) {
auto Flags = ELF::SHF_WRITE | ELF::SHF_ALLOC;
const MCSymbolELF *LinkedToSym = nullptr;
- StringRef GroupName;
+ StringRef GroupName, SectionName;
+
+ if (F.hasFnAttribute("patchable-function-entry-section"))
+ SectionName = F.getFnAttribute("patchable-function-entry-section")
+ .getValueAsString();
+ if (SectionName.empty())
+ SectionName = "__patchable_function_entries";
// GNU as < 2.35 did not support section flag 'o'. GNU ld < 2.36 did not
// support mixed SHF_LINK_ORDER and non-SHF_LINK_ORDER sections.
@@ -4596,8 +4602,8 @@ void AsmPrinter::emitPatchableFunctionEntries() {
LinkedToSym = cast<MCSymbolELF>(CurrentFnSym);
}
OutStreamer->switchSection(OutContext.getELFSection(
- "__patchable_function_entries", ELF::SHT_PROGBITS, Flags, 0, GroupName,
- F.hasComdat(), MCSection::NonUniqueID, LinkedToSym));
+ SectionName, ELF::SHT_PROGBITS, Flags, 0, GroupName, F.hasComdat(),
+ MCSection::NonUniqueID, LinkedToSym));
emitAlignment(Align(PointerSize));
OutStreamer->emitSymbolValue(CurrentPatchableFunctionEntrySym, PointerSize);
}
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 524e9647bfd35..3b91ed12b70f0 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2390,6 +2390,11 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-prefix", V);
checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-entry", V);
+ if (Attrs.hasFnAttr("patchable-function-entry-section"))
+ Check(!Attrs.getFnAttr("patchable-function-entry-section")
+ .getValueAsString()
+ .empty(),
+ "\"patchable-function-entry-section\" must not be empty");
checkUnsignedBaseTenFuncAttr(Attrs, "warn-stack-size", V);
if (auto A = Attrs.getFnAttr("sign-return-address"); A.isValid()) {
diff --git a/llvm/test/CodeGen/X86/patchable-function-entry.ll b/llvm/test/CodeGen/X86/patchable-function-entry.ll
index 54ecd8b1e5daf..d6dfe00c74991 100644
--- a/llvm/test/CodeGen/X86/patchable-function-entry.ll
+++ b/llvm/test/CodeGen/X86/patchable-function-entry.ll
@@ -98,3 +98,16 @@ define void @f3_2() "patchable-function-entry"="1" "patchable-function-prefix"="
%frame = alloca i8, i32 16
ret void
}
+
+define void @s1() "patchable-function-entry"="1" "patchable-function-entry-section"=".entries" {
+; CHECK-LABEL: s1:
+; CHECK-NEXT: .Lfunc_begin6:
+; CHECK: nop
+; CHECK-NEXT: ret
+; CHECK: .section .entries,"awo",@progbits,s1{{$}}
+; X86: .p2align 2
+; X86-NEXT: .long .Lfunc_begin6
+; X64: .p2align 3
+; X64-NEXT: .quad .Lfunc_begin6
+ ret void
+}
diff --git a/llvm/test/Verifier/invalid-patchable-function-entry.ll b/llvm/test/Verifier/invalid-patchable-function-entry.ll
index e74037a28abe6..a86cd89ae7ef9 100644
--- a/llvm/test/Verifier/invalid-patchable-function-entry.ll
+++ b/llvm/test/Verifier/invalid-patchable-function-entry.ll
@@ -19,3 +19,7 @@ define void @g() "patchable-function-prefix" { ret void }
define void @ga() "patchable-function-prefix"="a" { ret void }
define void @g_1() "patchable-function-prefix"="-1" { ret void }
define void @g3comma() "patchable-function-prefix"="3," { ret void }
+
+; CHECK: "patchable-function-entry-section" must not be empty
+
+define void @s1() "patchable-function-entry"="1" "patchable-function-entry-section" { ret void }
|
@llvm/pr-subscribers-clang Author: Sami Tolvanen (samitolvanen) ChangesWith -fpatchable-function-entry (or the patchable_function_entry function attribute), we emit records of patchable entry locations to the __patchable_function_entries section. Add an additional parameter to the command line option that allows one to specify a different default section name for the records, and an identical parameter to the function attribute that allows one to override the section used. The main use case for this change is the Linux kernel using prefix NOPs for ftrace, and thus depending on__patchable_function_entries to locate traceable functions. Functions that are not traceable currently disable entry NOPs using the function attribute, but this creates a compatibility issue with -fsanitize=kcfi, which expects all indirectly callable functions to have a type hash prefix at the same offset from the function entry. Adding a section parameter would allow the kernel to distinguish between traceable and non-traceable functions by adding entry records to separate sections while maintaining a stable function prefix layout for all functions. LKML discussion: https://lore.kernel.org/lkml/Y1QEzk%2FA41PKLEPe@hirez.programming.kicks-ass.net/ Full diff: https://github.com/llvm/llvm-project/pull/131230.diff 15 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 4d34346460561..19e63ba9c4ad7 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -932,7 +932,8 @@ def PatchableFunctionEntry
"riscv64", "x86", "x86_64", "ppc", "ppc64"]>> {
let Spellings = [GCC<"patchable_function_entry">];
let Subjects = SubjectList<[Function, ObjCMethod]>;
- let Args = [UnsignedArgument<"Count">, DefaultIntArgument<"Offset", 0>];
+ let Args = [UnsignedArgument<"Count">, DefaultIntArgument<"Offset", 0>,
+ StringArgument<"Section", /* optional */ 1>];
let Documentation = [PatchableFunctionEntryDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 34e7ff9612859..c354d95b515e3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6502,10 +6502,12 @@ only N==1 is supported.
def PatchableFunctionEntryDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-``__attribute__((patchable_function_entry(N,M)))`` is used to generate M NOPs
-before the function entry and N-M NOPs after the function entry. This attribute
-takes precedence over the command line option ``-fpatchable-function-entry=N,M``.
-``M`` defaults to 0 if omitted.
+``__attribute__((patchable_function_entry(N,M,Section)))`` is used to generate M
+NOPs before the function entry and N-M NOPs after the function entry, with a record of
+the entry stored in section ``Section``. This attribute takes precedence over the
+command line option ``-fpatchable-function-entry=N,M,Section``. ``M`` defaults to 0
+if omitted.``Section`` defaults to the ``-fpatchable-function-entry`` section name if
+set, or to ``__patchable_function_entries`` otherwise.
This attribute is only supported on
aarch64/aarch64-be/loongarch32/loongarch64/riscv32/riscv64/i386/x86-64/ppc/ppc64 targets.
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index c531c656f42b7..e39a73bdb13ac 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -281,6 +281,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// -fprofile-generate, and -fcs-profile-generate.
std::string InstrProfileOutput;
+ /// Name of the patchable function entry section with
+ /// -fpatchable-function-entry.
+ std::string PatchableFunctionEntrySection;
+
/// Name of the profile file to use with -fprofile-sample-use.
std::string SampleProfileFile;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e6e6e892cdd7..e076419ebc015 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3598,6 +3598,10 @@ def err_conflicting_codeseg_attribute : Error<
def warn_duplicate_codeseg_attribute : Warning<
"duplicate code segment specifiers">, InGroup<Section>;
+def err_attribute_patchable_function_entry_invalid_section
+ : Error<"section argument to 'patchable_function_entry' attribute is not "
+ "valid for this target: %0">;
+
def err_anonymous_property: Error<
"anonymous property is not supported">;
def err_property_is_variably_modified : Error<
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 66ae8f1c7f064..c83099e70af11 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3749,10 +3749,16 @@ defm pascal_strings : BoolFOption<"pascal-strings",
// Note: This flag has different semantics in the driver and in -cc1. The driver accepts -fpatchable-function-entry=M,N
// and forwards it to -cc1 as -fpatchable-function-entry=M and -fpatchable-function-entry-offset=N. In -cc1, both flags
// are treated as a single integer.
-def fpatchable_function_entry_EQ : Joined<["-"], "fpatchable-function-entry=">, Group<f_Group>,
- Visibility<[ClangOption, CC1Option]>,
- MetaVarName<"<N,M>">, HelpText<"Generate M NOPs before function entry and N-M NOPs after function entry">,
- MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryCount">>;
+def fpatchable_function_entry_EQ
+ : Joined<["-"], "fpatchable-function-entry=">,
+ Group<f_Group>,
+ Visibility<[ClangOption, CC1Option]>,
+ MetaVarName<"<N,M,Section>">,
+ HelpText<"Generate M NOPs before function entry and N-M NOPs after "
+ "function entry. "
+ "If section is specified, use it instead of "
+ "__patchable_function_entries.">,
+ MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryCount">>;
def fms_hotpatch : Flag<["-"], "fms-hotpatch">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Ensure that all functions can be hotpatched at runtime">,
@@ -7577,6 +7583,11 @@ def fpatchable_function_entry_offset_EQ
: Joined<["-"], "fpatchable-function-entry-offset=">, MetaVarName<"<M>">,
HelpText<"Generate M NOPs before function entry">,
MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryOffset">>;
+def fpatchable_function_entry_section_EQ
+ : Joined<["-"], "fpatchable-function-entry-section=">,
+ MetaVarName<"<Section>">,
+ HelpText<"Use Section instead of __patchable_function_entries">,
+ MarshallingInfoString<CodeGenOpts<"PatchableFunctionEntrySection">>;
def fprofile_instrument_EQ : Joined<["-"], "fprofile-instrument=">,
HelpText<"Enable PGO instrumentation">, Values<"none,clang,llvm,csllvm">,
NormalizedValuesScope<"CodeGenOptions">,
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 447192bc7f60c..901cffd834840 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -959,18 +959,24 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
}
unsigned Count, Offset;
+ StringRef Section;
if (const auto *Attr =
D ? D->getAttr<PatchableFunctionEntryAttr>() : nullptr) {
Count = Attr->getCount();
Offset = Attr->getOffset();
+ Section = Attr->getSection();
} else {
Count = CGM.getCodeGenOpts().PatchableFunctionEntryCount;
Offset = CGM.getCodeGenOpts().PatchableFunctionEntryOffset;
}
+ if (Section.empty())
+ Section = CGM.getCodeGenOpts().PatchableFunctionEntrySection;
if (Count && Offset <= Count) {
Fn->addFnAttr("patchable-function-entry", std::to_string(Count - Offset));
if (Offset)
Fn->addFnAttr("patchable-function-prefix", std::to_string(Offset));
+ if (!Section.empty())
+ Fn->addFnAttr("patchable-function-entry-section", Section);
}
// 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
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index fe172d923ac07..f68cf5a7689d0 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6909,8 +6909,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getAsString(Args) << TripleStr;
else if (S.consumeInteger(10, Size) ||
- (!S.empty() && (!S.consume_front(",") ||
- S.consumeInteger(10, Offset) || !S.empty())))
+ (!S.empty() &&
+ (!S.consume_front(",") || S.consumeInteger(10, Offset))) ||
+ (!S.empty() && (!S.consume_front(",") || S.empty())))
D.Diag(diag::err_drv_invalid_argument_to_option)
<< S0 << A->getOption().getName();
else if (Size < Offset)
@@ -6919,6 +6920,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back(Args.MakeArgString(A->getSpelling() + Twine(Size)));
CmdArgs.push_back(Args.MakeArgString(
"-fpatchable-function-entry-offset=" + Twine(Offset)));
+ if (!S.empty())
+ CmdArgs.push_back(
+ Args.MakeArgString("-fpatchable-function-entry-section=" + S));
}
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index bc858c63f69b6..2868ee271e151 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5759,9 +5759,10 @@ static void handlePatchableFunctionEntryAttr(Sema &S, Decl *D,
return;
}
uint32_t Count = 0, Offset = 0;
+ StringRef Section;
if (!S.checkUInt32Argument(AL, AL.getArgAsExpr(0), Count, 0, true))
return;
- if (AL.getNumArgs() == 2) {
+ if (AL.getNumArgs() >= 2) {
Expr *Arg = AL.getArgAsExpr(1);
if (!S.checkUInt32Argument(AL, Arg, Offset, 1, true))
return;
@@ -5771,8 +5772,25 @@ static void handlePatchableFunctionEntryAttr(Sema &S, Decl *D,
return;
}
}
- D->addAttr(::new (S.Context)
- PatchableFunctionEntryAttr(S.Context, AL, Count, Offset));
+ if (AL.getNumArgs() == 3) {
+ SourceLocation LiteralLoc;
+ if (!S.checkStringLiteralArgumentAttr(AL, 2, Section, &LiteralLoc))
+ return;
+ if (llvm::Error E = S.isValidSectionSpecifier(Section)) {
+ S.Diag(LiteralLoc,
+ diag::err_attribute_patchable_function_entry_invalid_section)
+ << toString(std::move(E));
+ return;
+ }
+ if (Section.empty()) {
+ S.Diag(LiteralLoc,
+ diag::err_attribute_patchable_function_entry_invalid_section)
+ << "section must not be empty";
+ return;
+ }
+ }
+ D->addAttr(::new (S.Context) PatchableFunctionEntryAttr(S.Context, AL, Count,
+ Offset, Section));
}
static void handleBuiltinAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
diff --git a/clang/test/CodeGen/patchable-function-entry-section.c b/clang/test/CodeGen/patchable-function-entry-section.c
new file mode 100644
index 0000000000000..4c0d2a1baf77b
--- /dev/null
+++ b/clang/test/CodeGen/patchable-function-entry-section.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck --check-prefixes=COMMON,NODEFAULT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -fpatchable-function-entry-section=__default_section -o - | FileCheck --check-prefixes=COMMON,DEFAULT %s
+
+// COMMON: define{{.*}} void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0(void) {}
+
+// COMMON: define{{.*}} void @f00() #0
+__attribute__((patchable_function_entry(0, 0, "__unused_section"))) void f00(void) {}
+
+// COMMON: define{{.*}} void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2(void) {}
+
+// COMMON: define{{.*}} void @f20() #2
+__attribute__((patchable_function_entry(2, 0, "__attr_section"))) void f20(void) {}
+
+// COMMON: define{{.*}} void @f44() #3
+__attribute__((patchable_function_entry(4, 4))) void f44(void) {}
+
+// COMMON: define{{.*}} void @f52() #4
+__attribute__((patchable_function_entry(5, 2, "__attr_section"))) void f52(void) {}
+
+// OPT: define{{.*}} void @f() #5
+void f(void) {}
+
+/// No need to emit "patchable-function-entry" and thus also "patchable-function-entry-section"
+// COMMON: attributes #0 = { {{.*}}
+// COMMON-NOT: "patchable-function-entry-section"
+
+// NODEFAULT: attributes #1 = { {{.*}} "patchable-function-entry"="2"
+// NODEFAULT-NOT: "patchable-function-entry-section"
+// DEFAULT: attributes #1 = { {{.*}} "patchable-function-entry"="2" "patchable-function-entry-section"="__default_section"
+
+// COMMON: attributes #2 = { {{.*}} "patchable-function-entry"="2" "patchable-function-entry-section"="__attr_section"
+
+// NODEFAULT: attributes #3 = { {{.*}} "patchable-function-entry"="0" "patchable-function-prefix"="4"
+// NODEFAULT-NOT: "patchable-function-entry-section"
+// DEFAULT: attributes #3 = { {{.*}} "patchable-function-entry"="0" "patchable-function-entry-section"="__default_section" "patchable-function-prefix"="4"
+
+// COMMON: attributes #4 = { {{.*}} "patchable-function-entry"="3" "patchable-function-entry-section"="__attr_section" "patchable-function-prefix"="2"
+
+// DEFAULT: attributes #5 = { {{.*}} "patchable-function-entry"="1" "patchable-function-entry-section"="__default_section"
diff --git a/clang/test/Driver/fpatchable-function-entry.c b/clang/test/Driver/fpatchable-function-entry.c
index 5f07ca99a69de..43be6c5a47e47 100644
--- a/clang/test/Driver/fpatchable-function-entry.c
+++ b/clang/test/Driver/fpatchable-function-entry.c
@@ -15,6 +15,9 @@
// RUN: %clang --target=aarch64 -fsyntax-only %s -fpatchable-function-entry=2,1 -c -### 2>&1 | FileCheck --check-prefix=21 %s
// 21: "-fpatchable-function-entry=2" "-fpatchable-function-entry-offset=1"
+// RUN: %clang --target=aarch64 -fsyntax-only %s -fpatchable-function-entry=1,1,__section_name -c -### 2>&1 | FileCheck --check-prefix=SECTION %s
+// SECTION: "-fpatchable-function-entry=1" "-fpatchable-function-entry-offset=1" "-fpatchable-function-entry-section=__section_name"
+
// RUN: not %clang --target=powerpc64-ibm-aix-xcoff -fsyntax-only %s -fpatchable-function-entry=1 2>&1 | FileCheck --check-prefix=AIX64 %s
// AIX64: error: unsupported option '-fpatchable-function-entry=1' for target 'powerpc64-ibm-aix-xcoff'
diff --git a/clang/test/Sema/patchable-function-entry-attr.c b/clang/test/Sema/patchable-function-entry-attr.c
index 89e4380c36230..f453e134ab625 100644
--- a/clang/test/Sema/patchable-function-entry-attr.c
+++ b/clang/test/Sema/patchable-function-entry-attr.c
@@ -3,9 +3,15 @@
// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
__attribute__((patchable_function_entry)) void f(void);
-// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+// expected-error@+1 {{expected string literal as argument of 'patchable_function_entry' attribute}}
__attribute__((patchable_function_entry(0, 0, 0))) void f(void);
+// expected-error@+1 {{section argument to 'patchable_function_entry' attribute is not valid for this target}}
+__attribute__((patchable_function_entry(0, 0, ""))) void f(void);
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 3 arguments}}
+__attribute__((patchable_function_entry(0, 0, "__section", 0))) void f(void);
+
// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
__attribute__((patchable_function_entry(-1))) void f(void);
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 6d99cb3a516cc..ce1fe9b05ef6b 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -4583,7 +4583,13 @@ void AsmPrinter::emitPatchableFunctionEntries() {
if (TM.getTargetTriple().isOSBinFormatELF()) {
auto Flags = ELF::SHF_WRITE | ELF::SHF_ALLOC;
const MCSymbolELF *LinkedToSym = nullptr;
- StringRef GroupName;
+ StringRef GroupName, SectionName;
+
+ if (F.hasFnAttribute("patchable-function-entry-section"))
+ SectionName = F.getFnAttribute("patchable-function-entry-section")
+ .getValueAsString();
+ if (SectionName.empty())
+ SectionName = "__patchable_function_entries";
// GNU as < 2.35 did not support section flag 'o'. GNU ld < 2.36 did not
// support mixed SHF_LINK_ORDER and non-SHF_LINK_ORDER sections.
@@ -4596,8 +4602,8 @@ void AsmPrinter::emitPatchableFunctionEntries() {
LinkedToSym = cast<MCSymbolELF>(CurrentFnSym);
}
OutStreamer->switchSection(OutContext.getELFSection(
- "__patchable_function_entries", ELF::SHT_PROGBITS, Flags, 0, GroupName,
- F.hasComdat(), MCSection::NonUniqueID, LinkedToSym));
+ SectionName, ELF::SHT_PROGBITS, Flags, 0, GroupName, F.hasComdat(),
+ MCSection::NonUniqueID, LinkedToSym));
emitAlignment(Align(PointerSize));
OutStreamer->emitSymbolValue(CurrentPatchableFunctionEntrySym, PointerSize);
}
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 524e9647bfd35..3b91ed12b70f0 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2390,6 +2390,11 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-prefix", V);
checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-entry", V);
+ if (Attrs.hasFnAttr("patchable-function-entry-section"))
+ Check(!Attrs.getFnAttr("patchable-function-entry-section")
+ .getValueAsString()
+ .empty(),
+ "\"patchable-function-entry-section\" must not be empty");
checkUnsignedBaseTenFuncAttr(Attrs, "warn-stack-size", V);
if (auto A = Attrs.getFnAttr("sign-return-address"); A.isValid()) {
diff --git a/llvm/test/CodeGen/X86/patchable-function-entry.ll b/llvm/test/CodeGen/X86/patchable-function-entry.ll
index 54ecd8b1e5daf..d6dfe00c74991 100644
--- a/llvm/test/CodeGen/X86/patchable-function-entry.ll
+++ b/llvm/test/CodeGen/X86/patchable-function-entry.ll
@@ -98,3 +98,16 @@ define void @f3_2() "patchable-function-entry"="1" "patchable-function-prefix"="
%frame = alloca i8, i32 16
ret void
}
+
+define void @s1() "patchable-function-entry"="1" "patchable-function-entry-section"=".entries" {
+; CHECK-LABEL: s1:
+; CHECK-NEXT: .Lfunc_begin6:
+; CHECK: nop
+; CHECK-NEXT: ret
+; CHECK: .section .entries,"awo",@progbits,s1{{$}}
+; X86: .p2align 2
+; X86-NEXT: .long .Lfunc_begin6
+; X64: .p2align 3
+; X64-NEXT: .quad .Lfunc_begin6
+ ret void
+}
diff --git a/llvm/test/Verifier/invalid-patchable-function-entry.ll b/llvm/test/Verifier/invalid-patchable-function-entry.ll
index e74037a28abe6..a86cd89ae7ef9 100644
--- a/llvm/test/Verifier/invalid-patchable-function-entry.ll
+++ b/llvm/test/Verifier/invalid-patchable-function-entry.ll
@@ -19,3 +19,7 @@ define void @g() "patchable-function-prefix" { ret void }
define void @ga() "patchable-function-prefix"="a" { ret void }
define void @g_1() "patchable-function-prefix"="-1" { ret void }
define void @g3comma() "patchable-function-prefix"="3," { ret void }
+
+; CHECK: "patchable-function-entry-section" must not be empty
+
+define void @s1() "patchable-function-entry"="1" "patchable-function-entry-section" { ret void }
|
@MaskRay here's a draft implementation of the patchable function entry extension discussed in LKML a couple of years ago. Could you please take a look and see if this seems reasonable? |
With -fpatchable-function-entry (or the patchable_function_entry function attribute), we emit records of patchable entry locations to the __patchable_function_entries section. Add an additional parameter to the command line option that allows one to specify a different default section name for the records, and an identical parameter to the function attribute that allows one to override the section used.
The main use case for this change is the Linux kernel using prefix NOPs for ftrace, and thus depending on__patchable_function_entries to locate traceable functions. Functions that are not traceable currently disable entry NOPs using the function attribute, but this creates a compatibility issue with -fsanitize=kcfi, which expects all indirectly callable functions to have a type hash prefix at the same offset from the function entry.
Adding a section parameter would allow the kernel to distinguish between traceable and non-traceable functions by adding entry records to separate sections while maintaining a stable function prefix layout for all functions. LKML discussion:
https://lore.kernel.org/lkml/Y1QEzk%[email protected]/