by Peter Zijlstra

[permalink] [raw]

[permalink] [raw]

Subject: Re: [RFC][PATCH] objtool,x86_64: Replace recordmcount with objtool

On Thu, 23 Jul 2020 01:56:20 +0200
Peter Zijlstra <[email protected]> wrote:

> Anyway, what do you prefer, I suppose I can make objtool whatever we
> need, that patch is trivial. Simply recording the sites and not
> rewriting them should be simple enough.

Either way. If objtool turns it into nops, just make it where we can
enable -DCC_USING_NOP_MCOUNT set, and the kernel will be unaware.

Or if you just add the locations, then that would work too.

-- Steve

2020-08-06 22:12:56

by Sami Tolvanen

[permalink] [raw]

Subject: Re: [RFC][PATCH] objtool,x86_64: Replace recordmcount with objtool

On Wed, Jul 22, 2020 at 08:06:08PM -0400, Steven Rostedt wrote:
> On Thu, 23 Jul 2020 01:56:20 +0200
> Peter Zijlstra <[email protected]> wrote:
>
> > Anyway, what do you prefer, I suppose I can make objtool whatever we
> > need, that patch is trivial. Simply recording the sites and not
> > rewriting them should be simple enough.
>
> Either way. If objtool turns it into nops, just make it where we can
> enable -DCC_USING_NOP_MCOUNT set, and the kernel will be unaware.
>
> Or if you just add the locations, then that would work too.

I took Peter's earlier patch, rebased it on top of the current mainline
tree for easier testing, and tweaked the makefiles to only use objtool
--mcount when CONFIG_STACK_VALIDATION is enabled and the compiler
supports -mfentry. This works for me with both gcc and clang. Thoughts?

Sami

---
Makefile | 38 ++++++++++++----
arch/x86/Kconfig | 1 +
kernel/trace/Kconfig | 5 +++
scripts/Makefile.build | 9 ++--
tools/objtool/builtin-check.c | 3 +-
tools/objtool/builtin.h | 2 +-
tools/objtool/check.c | 83 +++++++++++++++++++++++++++++++++++
tools/objtool/check.h | 1 +
tools/objtool/objtool.h | 1 +
9 files changed, 129 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index 5cfc3481207f..2d23b6b6c4c9 100644
--- a/Makefile
+++ b/Makefile
@@ -864,17 +864,34 @@ ifdef CONFIG_HAVE_FENTRY
ifeq ($(call cc-option-yn, -mfentry),y)
CC_FLAGS_FTRACE += -mfentry
CC_FLAGS_USING += -DCC_USING_FENTRY
+ export CC_USING_FENTRY := 1
endif
endif
export CC_FLAGS_FTRACE
-KBUILD_CFLAGS += $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
-KBUILD_AFLAGS += $(CC_FLAGS_USING)
ifdef CONFIG_DYNAMIC_FTRACE
- ifdef CONFIG_HAVE_C_RECORDMCOUNT
- BUILD_C_RECORDMCOUNT := y
- export BUILD_C_RECORDMCOUNT
- endif
+ ifndef CC_USING_RECORD_MCOUNT
+ ifndef CC_USING_PATCHABLE_FUNCTION_ENTRY
+ # use objtool or recordmcount to generate mcount tables
+ ifdef CONFIG_HAVE_OBJTOOL_MCOUNT
+ ifdef CC_USING_FENTRY
+ USE_OBJTOOL_MCOUNT := y
+ CC_FLAGS_USING += -DCC_USING_NOP_MCOUNT
+ export USE_OBJTOOL_MCOUNT
+ endif
+ endif
+ ifndef USE_OBJTOOL_MCOUNT
+ USE_RECORDMCOUNT := y
+ export USE_RECORDMCOUNT
+ ifdef CONFIG_HAVE_C_RECORDMCOUNT
+ BUILD_C_RECORDMCOUNT := y
+ export BUILD_C_RECORDMCOUNT
+ endif
+ endif
+ endif
+ endif
endif
+KBUILD_CFLAGS += $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
+KBUILD_AFLAGS += $(CC_FLAGS_USING)
endif

# We trigger additional mismatches with less inlining
@@ -1211,11 +1228,16 @@ uapi-asm-generic:
PHONY += prepare-objtool prepare-resolve_btfids
prepare-objtool: $(objtool_target)
ifeq ($(SKIP_STACK_VALIDATION),1)
+objtool-lib-prompt := "please install libelf-dev, libelf-devel or elfutils-libelf-devel"
+ifdef USE_OBJTOOL_MCOUNT
+ @echo "error: Cannot generate __mcount_loc for CONFIG_DYNAMIC_FTRACE=y, $(objtool-lib-prompt)" >&2
+ @false
+endif
ifdef CONFIG_UNWINDER_ORC
- @echo "error: Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
+ @echo "error: Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, $(objtool-lib-prompt)" >&2
@false
else
- @echo "warning: Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
+ @echo "warning: Cannot use CONFIG_STACK_VALIDATION=y, $(objtool-lib-prompt)" >&2
endif
endif

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9a2849527dd7..149c94a44cf0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -163,6 +163,7 @@ config X86
select HAVE_CMPXCHG_LOCAL
select HAVE_CONTEXT_TRACKING if X86_64
select HAVE_C_RECORDMCOUNT
+ select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index a4020c0b4508..b510af5b216c 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -56,6 +56,11 @@ config HAVE_C_RECORDMCOUNT
help
C version of recordmcount available?

+config HAVE_OBJTOOL_MCOUNT
+ bool
+ help
+ Arch supports objtool --mcount
+
config TRACER_MAX_TRACE
bool

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2e8810b7e5ed..f66f8c0ef294 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -175,8 +175,7 @@ cmd_modversions_c = \
fi
endif

-ifdef CONFIG_FTRACE_MCOUNT_RECORD
-ifndef CC_USING_RECORD_MCOUNT
+ifdef USE_RECORDMCOUNT
# compiler will not generate __mcount_loc use recordmcount or recordmcount.pl
ifdef BUILD_C_RECORDMCOUNT
ifeq ("$(origin RECORDMCOUNT_WARN)", "command line")
@@ -203,8 +202,7 @@ recordmcount_source := $(srctree)/scripts/recordmcount.pl
endif # BUILD_C_RECORDMCOUNT
cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)), \
$(sub_cmd_record_mcount))
-endif # CC_USING_RECORD_MCOUNT
-endif # CONFIG_FTRACE_MCOUNT_RECORD
+endif # USE_RECORDMCOUNT

ifdef CONFIG_STACK_VALIDATION
ifneq ($(SKIP_STACK_VALIDATION),1)
@@ -227,6 +225,9 @@ endif
ifdef CONFIG_X86_SMAP
objtool_args += --uaccess
endif
+ifdef USE_OBJTOOL_MCOUNT
+ objtool_args += --mcount
+endif

# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 7a44174967b5..71595cf4946d 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -18,7 +18,7 @@
#include "builtin.h"
#include "objtool.h"

-bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux;
+bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux, mcount;

static const char * const check_usage[] = {
"objtool check [<options>] file.o",
@@ -35,6 +35,7 @@ const struct option check_options[] = {
OPT_BOOLEAN('s', "stats", &stats, "print statistics"),
OPT_BOOLEAN('d', "duplicate", &validate_dup, "duplicate validation for vmlinux.o"),
OPT_BOOLEAN('l', "vmlinux", &vmlinux, "vmlinux.o validation"),
+ OPT_BOOLEAN('M', "mcount", &mcount, "generate __mcount_loc"),
OPT_END(),
};

diff --git a/tools/objtool/builtin.h b/tools/objtool/builtin.h
index 85c979caa367..94565a72b701 100644
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -8,7 +8,7 @@
#include <subcmd/parse-options.h>

extern const struct option check_options[];
-extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux;
+extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux, mcount;

extern int cmd_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv);
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e034a8f24f46..6e0b478dc065 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -433,6 +433,65 @@ static int add_dead_ends(struct objtool_file *file)
return 0;
}

+static int create_mcount_loc_sections(struct objtool_file *file)
+{
+ struct section *sec, *reloc_sec;
+ struct reloc *reloc;
+ unsigned long *loc;
+ struct instruction *insn;
+ int idx;
+
+ sec = find_section_by_name(file->elf, "__mcount_loc");
+ if (sec) {
+ INIT_LIST_HEAD(&file->mcount_loc_list);
+ WARN("file already has __mcount_loc section, skipping");
+ return 0;
+ }
+
+ if (list_empty(&file->mcount_loc_list))
+ return 0;
+
+ idx = 0;
+ list_for_each_entry(insn, &file->mcount_loc_list, mcount_loc_node)
+ idx++;
+
+ sec = elf_create_section(file->elf, "__mcount_loc", sizeof(unsigned long), idx);
+ if (!sec)
+ return -1;
+
+ reloc_sec = elf_create_reloc_section(file->elf, sec, SHT_RELA);
+ if (!reloc_sec)
+ return -1;
+
+ idx = 0;
+ list_for_each_entry(insn, &file->mcount_loc_list, mcount_loc_node) {
+
+ loc = (unsigned long *)sec->data->d_buf + idx;
+ memset(loc, 0, sizeof(unsigned long));
+
+ reloc = malloc(sizeof(*reloc));
+ if (!reloc) {
+ perror("malloc");
+ return -1;
+ }
+ memset(reloc, 0, sizeof(*reloc));
+
+ reloc->sym = insn->sec->sym;
+ reloc->addend = insn->offset;
+ reloc->type = R_X86_64_64;
+ reloc->offset = idx * sizeof(unsigned long);
+ reloc->sec = reloc_sec;
+ elf_add_reloc(file->elf, reloc);
+
+ idx++;
+ }
+
+ if (elf_rebuild_reloc_section(file->elf, reloc_sec))
+ return -1;
+
+ return 0;
+}
+
/*
* Warnings shouldn't be reported for ignored functions.
*/
@@ -784,6 +843,22 @@ static int add_call_destinations(struct objtool_file *file)
insn->type = INSN_NOP;
}

+ if (mcount && !strcmp(insn->call_dest->name, "__fentry__")) {
+ if (reloc) {
+ reloc->type = R_NONE;
+ elf_write_reloc(file->elf, reloc);
+ }
+
+ elf_write_insn(file->elf, insn->sec,
+ insn->offset, insn->len,
+ arch_nop_insn(insn->len));
+
+ insn->type = INSN_NOP;
+
+ list_add_tail(&insn->mcount_loc_node,
+ &file->mcount_loc_list);
+ }
+
/*
* Whatever stack impact regular CALLs have, should be undone
* by the RETURN of the called function.
@@ -2791,6 +2866,7 @@ int check(const char *_objname, bool orc)

INIT_LIST_HEAD(&file.insn_list);
hash_init(file.insn_hash);
+ INIT_LIST_HEAD(&file.mcount_loc_list);
file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment");
file.ignore_unreachables = no_unreachable;
file.hints = false;
@@ -2838,6 +2914,13 @@ int check(const char *_objname, bool orc)
warnings += ret;
}

+ if (mcount) {
+ ret = create_mcount_loc_sections(&file);
+ if (ret < 0)
+ goto out;
+ warnings += ret;
+ }
+
if (orc) {
ret = create_orc(&file);
if (ret < 0)
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 061aa96e15d3..b62afd3d970b 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -22,6 +22,7 @@ struct insn_state {
struct instruction {
struct list_head list;
struct hlist_node hash;
+ struct list_head mcount_loc_node;
struct section *sec;
unsigned long offset;
unsigned int len;
diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h
index 528028a66816..427806079540 100644
--- a/tools/objtool/objtool.h
+++ b/tools/objtool/objtool.h
@@ -16,6 +16,7 @@ struct objtool_file {
struct elf *elf;
struct list_head insn_list;
DECLARE_HASHTABLE(insn_hash, 20);
+ struct list_head mcount_loc_list;
bool ignore_unreachables, c_file, hints, rodata;
};