2022-03-20 02:14:28

by Sathvika Vasireddy

[permalink] [raw]
Subject: [RFC PATCH 0/3] objtool: Add mcount sub-command

This patchset adds support to implement 'objtool mcount' command.

Right now, objtool is built if CONFIG_STACK_VALIDATION is enabled.
And, '__mcount_loc' section is generated by objtool when --mcount
option is passed to check sub-command.

For architectures to be able to generate '__mcount_loc' section
without having to do stack validation, introduce 'mcount' as
a sub-command to objtool. This way, objtool is built for mcount
if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL is enabled. Additionally,
architectures can select HAVE_NOP_MCOUNT to be able to nop out
mcount call sites.

TODO: Enable "objtool mcount" for clang LTO builds.

Sathvika Vasireddy (3):
objtool: Move common code to utils.c
objtool: Enable and implement 'mcount' subcommand
objtool/mcount: Add powerpc specific functions

Makefile | 6 +
arch/powerpc/Kconfig | 1 +
arch/x86/Kconfig | 3 +-
scripts/Makefile.build | 12 +
tools/objtool/Build | 3 +
tools/objtool/Makefile | 8 +-
tools/objtool/arch/powerpc/Build | 1 +
tools/objtool/arch/powerpc/decode.c | 51 +++++
.../arch/powerpc/include/arch/cfi_regs.h | 37 +++
tools/objtool/arch/powerpc/include/arch/elf.h | 8 +
tools/objtool/builtin-mcount.c | 74 ++++++
tools/objtool/check.c | 178 +--------------
tools/objtool/include/objtool/builtin.h | 4 +-
tools/objtool/include/objtool/check.h | 2 -
tools/objtool/include/objtool/objtool.h | 1 +
tools/objtool/include/objtool/utils.h | 28 +++
tools/objtool/mcount.c | 138 ++++++++++++
tools/objtool/objtool.c | 1 +
tools/objtool/orc_gen.c | 1 +
tools/objtool/utils.c | 210 ++++++++++++++++++
tools/objtool/weak.c | 5 +
21 files changed, 590 insertions(+), 182 deletions(-)
create mode 100644 tools/objtool/arch/powerpc/Build
create mode 100644 tools/objtool/arch/powerpc/decode.c
create mode 100644 tools/objtool/arch/powerpc/include/arch/cfi_regs.h
create mode 100644 tools/objtool/arch/powerpc/include/arch/elf.h
create mode 100644 tools/objtool/builtin-mcount.c
create mode 100644 tools/objtool/include/objtool/utils.h
create mode 100644 tools/objtool/mcount.c
create mode 100644 tools/objtool/utils.c

--
2.31.1


2022-03-21 11:23:08

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [RFC PATCH 0/3] objtool: Add mcount sub-command

On Fri, Mar 18, 2022 at 04:21:37PM +0530, Sathvika Vasireddy wrote:
> This patchset adds support to implement 'objtool mcount' command.
>
> Right now, objtool is built if CONFIG_STACK_VALIDATION is enabled.
> And, '__mcount_loc' section is generated by objtool when --mcount
> option is passed to check sub-command.
>
> For architectures to be able to generate '__mcount_loc' section
> without having to do stack validation, introduce 'mcount' as
> a sub-command to objtool. This way, objtool is built for mcount
> if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL is enabled. Additionally,
> architectures can select HAVE_NOP_MCOUNT to be able to nop out
> mcount call sites.
>
> TODO: Enable "objtool mcount" for clang LTO builds.
>
> Sathvika Vasireddy (3):
> objtool: Move common code to utils.c
> objtool: Enable and implement 'mcount' subcommand
> objtool/mcount: Add powerpc specific functions

Hi Sathvika,

Thanks for the patches!

I have some other patches in progress which will rework the objtool
interface by modularizing the cmdline options, so that each option can
be specified either individually or in combination. Even stack
validation itself will be its own separate option.

I think it will help your situation as well: "objtool run --mcount" will
only do '__mcount_loc' generation and nothing else.

Something like so:

$ ./objtool run --help

Usage: objtool run [<options>] file.o

Commands (at least one required):
-a, --uaccess validate uaccess
-c, --static-call annotate static calls
-i, --ibt validate and annotate IBT
-m, --mcount generate '__mcount_loc' section
-n, --noinstr validate noinstr
-o, --orc generate ORC metadata
-r, --retpoline validate retpoline usage
-S, --sls validate straight-line-speculation mitigation
-s, --stack-val validate stack metadata

Options:
--backtrace unwind on error
--backup create .orig files before modification
--dry-run don't write the modifications
--fp object uses frame pointers
--module object will be part of a kernel module
--no-unreachable skip 'unreachable instruction' warnings
--stats print statistics
--vmlinux object is vmlinux.o


Hopefully I'll have the patches ready soon.

--
Josh

2022-03-21 14:12:13

by Sathvika Vasireddy

[permalink] [raw]
Subject: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

This patch adds 'mcount' as a subcommand to objtool, and enables
the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
if they choose to nop out mcount call sites. If that config option is
selected, then --mnop is passed as an option to 'objtool mcount'

Signed-off-by: Sathvika Vasireddy <[email protected]>
---
Makefile | 6 ++
arch/x86/Kconfig | 3 +-
scripts/Makefile.build | 12 +++
tools/objtool/Build | 2 +
tools/objtool/Makefile | 4 +-
tools/objtool/builtin-mcount.c | 74 +++++++++++++
tools/objtool/include/objtool/builtin.h | 4 +-
tools/objtool/include/objtool/objtool.h | 1 +
tools/objtool/mcount.c | 138 ++++++++++++++++++++++++
tools/objtool/objtool.c | 1 +
tools/objtool/weak.c | 5 +
11 files changed, 247 insertions(+), 3 deletions(-)
create mode 100644 tools/objtool/builtin-mcount.c
create mode 100644 tools/objtool/mcount.c

diff --git a/Makefile b/Makefile
index 55a30ca69350..316f7d08b30a 100644
--- a/Makefile
+++ b/Makefile
@@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
endif
endif
ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+ ifdef CONFIG_HAVE_NOP_MCOUNT
CC_FLAGS_USING += -DCC_USING_NOP_MCOUNT
+ endif
endif
ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
ifdef CONFIG_HAVE_C_RECORDMCOUNT
@@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
prepare: tools/objtool
endif

+ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+prepare: tools/objtool
+endif
+
ifdef CONFIG_BPF
ifdef CONFIG_DEBUG_INFO_BTF
prepare: tools/bpf/resolve_btfids
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9f5bd41bf660..eecfe588cd9f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -185,7 +185,8 @@ config X86
select HAVE_CONTEXT_TRACKING if X86_64
select HAVE_CONTEXT_TRACKING_OFFSTACK if HAVE_CONTEXT_TRACKING
select HAVE_C_RECORDMCOUNT
- select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
+ select HAVE_OBJTOOL_MCOUNT
+ select HAVE_NOP_MCOUNT if HAVE_OBJTOOL_MCOUNT
select HAVE_BUILDTIME_MCOUNT_SORT
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a4b89b757287..6a09b4ba14a1 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -242,6 +242,18 @@ cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(o

endif # CONFIG_STACK_VALIDATION

+ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+
+objtool := $(objtree)/tools/objtool/objtool
+
+objtool_args := mcount
+objtool_args += $(if $(CONFIG_HAVE_NOP_MCOUNT), --mnop)
+
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+
+endif # CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+
ifdef CONFIG_LTO_CLANG

# Skip objtool for LLVM bitcode
diff --git a/tools/objtool/Build b/tools/objtool/Build
index 161fd451241a..0b9b8bd0ee92 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -2,12 +2,14 @@ objtool-y += arch/$(SRCARCH)/

objtool-y += weak.o

+objtool-$(SUBCMD_MCOUNT) += mcount.o
objtool-$(SUBCMD_CHECK) += check.o
objtool-$(SUBCMD_CHECK) += special.o
objtool-$(SUBCMD_ORC) += check.o
objtool-$(SUBCMD_ORC) += orc_gen.o
objtool-$(SUBCMD_ORC) += orc_dump.o

+objtool-y += builtin-mcount.o
objtool-y += builtin-check.o
objtool-y += builtin-orc.o
objtool-y += elf.o
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 92ce4fce7bc7..8404cf696954 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -41,13 +41,15 @@ AWK = awk

SUBCMD_CHECK := n
SUBCMD_ORC := n
+SUBCMD_MCOUNT := n

ifeq ($(SRCARCH),x86)
SUBCMD_CHECK := y
SUBCMD_ORC := y
+ SUBCMD_MCOUNT := y
endif

-export SUBCMD_CHECK SUBCMD_ORC
+export SUBCMD_CHECK SUBCMD_ORC SUBCMD_MCOUNT
export srctree OUTPUT CFLAGS SRCARCH AWK
include $(srctree)/tools/build/Makefile.include

diff --git a/tools/objtool/builtin-mcount.c b/tools/objtool/builtin-mcount.c
new file mode 100644
index 000000000000..f203d118cffa
--- /dev/null
+++ b/tools/objtool/builtin-mcount.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <subcmd/parse-options.h>
+#include <string.h>
+#include <stdlib.h>
+#include <objtool/builtin.h>
+#include <objtool/objtool.h>
+
+bool mnop;
+
+static const char * const mcount_usage[] = {
+ "objtool mcount [<options>] file.o",
+ NULL,
+};
+
+static const char * const env_usage[] = {
+ "OBJTOOL_ARGS=\"<options>\"",
+ NULL,
+};
+
+const struct option mcount_options[] = {
+ OPT_BOOLEAN('N', "mnop", &mnop, "nop mcount call sites"),
+ OPT_END(),
+};
+
+int cmd_parse_options_mcount(int argc, const char **argv, const char * const usage[])
+{
+ const char *envv[16] = { };
+ char *env;
+ int envc;
+
+ env = getenv("OBJTOOL_ARGS");
+ if (env) {
+ envv[0] = "OBJTOOL_ARGS";
+ for (envc = 1; envc < ARRAY_SIZE(envv); ) {
+ envv[envc++] = env;
+ env = strchr(env, ' ');
+ if (!env)
+ break;
+ *env = '\0';
+ env++;
+ }
+
+ parse_options(envc, envv, mcount_options, env_usage, 0);
+ }
+
+ argc = parse_options(argc, argv, mcount_options, usage, 0);
+ if (argc != 1)
+ usage_with_options(usage, mcount_options);
+ return argc;
+}
+
+int cmd_mcount(int argc, const char **argv)
+{
+ const char *objname;
+ struct objtool_file *file;
+ int ret;
+
+ argc = cmd_parse_options_mcount(argc, argv, mcount_usage);
+ objname = argv[0];
+
+ file = objtool_open_read(objname);
+ if (!file)
+ return 1;
+
+ ret = objtool_mcount(file);
+ if (ret)
+ return ret;
+
+ if (file->elf->changed)
+ return elf_write(file->elf);
+
+ return 0;
+}
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 89ba869ed08f..d7ed6ff65729 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -9,11 +9,13 @@

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

extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
+extern int cmd_parse_options_mcount(int argc, const char **argv, const char * const usage[]);

extern int cmd_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv);
+extern int cmd_mcount(int argc, const char **argv);

#endif /* _BUILTIN_H */
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index f99fbc6078d5..edcf28d4407c 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -41,5 +41,6 @@ void objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
int check(struct objtool_file *file);
int orc_dump(const char *objname);
int orc_create(struct objtool_file *file);
+int objtool_mcount(struct objtool_file *file);

#endif /* _OBJTOOL_H */
diff --git a/tools/objtool/mcount.c b/tools/objtool/mcount.c
new file mode 100644
index 000000000000..fc3d215671bf
--- /dev/null
+++ b/tools/objtool/mcount.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <string.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+
+#include <arch/elf.h>
+#include <objtool/builtin.h>
+#include <objtool/cfi.h>
+#include <objtool/arch.h>
+#include <objtool/check.h>
+#include <objtool/utils.h>
+#include <objtool/special.h>
+#include <objtool/warn.h>
+
+#include <linux/objtool.h>
+#include <linux/hashtable.h>
+#include <linux/kernel.h>
+#include <linux/static_call_types.h>
+
+static int classify_symbols(struct objtool_file *file)
+{
+ struct section *sec;
+ struct symbol *func;
+
+ for_each_sec(file, sec) {
+ list_for_each_entry(func, &sec->symbol_list, list) {
+ if (func->bind != STB_GLOBAL)
+ continue;
+ if ((!strcmp(func->name, "__fentry__")) || (!strcmp(func->name, "_mcount")))
+ func->fentry = true;
+ }
+ }
+
+ return 0;
+}
+
+static void annotate_call_site(struct objtool_file *file,
+ struct instruction *insn, bool sibling)
+{
+ struct reloc *reloc = insn_reloc(file, insn);
+ struct symbol *sym = insn->call_dest;
+
+ if (!sym)
+ sym = reloc->sym;
+
+ if (sym->fentry) {
+ if (sibling)
+ WARN_FUNC("Tail call to _mcount !?!?", insn->sec, insn->offset);
+ if (mnop) {
+ 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->call_node, &file->mcount_loc_list);
+ return;
+ }
+}
+
+static void add_call_dest(struct objtool_file *file, struct instruction *insn,
+ struct symbol *dest, bool sibling)
+{
+ insn->call_dest = dest;
+ if (!dest)
+ return;
+
+ remove_insn_ops(insn);
+
+ annotate_call_site(file, insn, sibling);
+}
+static int add_call_destinations(struct objtool_file *file)
+{
+ struct instruction *insn;
+ unsigned long dest_off;
+ struct symbol *dest;
+ struct reloc *reloc;
+
+ for_each_insn(file, insn) {
+ if (insn->type != INSN_CALL)
+ continue;
+
+ reloc = insn_reloc(file, insn);
+ if (!reloc) {
+ dest_off = arch_jump_destination(insn);
+ dest = find_call_destination(insn->sec, dest_off);
+
+ add_call_dest(file, insn, dest, false);
+
+
+ } else {
+ add_call_dest(file, insn, reloc->sym, false);
+ }
+ }
+
+ return 0;
+}
+
+static int decode_sections(struct objtool_file *file)
+{
+ int ret;
+
+ ret = decode_instructions(file);
+ if (ret)
+ return ret;
+
+ ret = classify_symbols(file);
+ if (ret)
+ return ret;
+
+ ret = add_call_destinations(file);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+
+int objtool_mcount(struct objtool_file *file)
+{
+ int ret, warnings = 0;
+
+ ret = decode_sections(file);
+ if (ret < 0)
+ return 0;
+
+ ret = create_mcount_loc_sections(file);
+ if (ret < 0)
+ return 0;
+ warnings += ret;
+ return 0;
+}
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index bdf699f6552b..e19851813d5b 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -37,6 +37,7 @@ static const char objtool_usage_string[] =

static struct cmd_struct objtool_cmds[] = {
{"check", cmd_check, "Perform stack metadata validation on an object file" },
+ {"mcount", cmd_mcount, "Generate __mcount_loc section" },
{"orc", cmd_orc, "Generate in-place ORC unwind tables for an object file" },
};

diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c
index 8314e824db4a..19b2dfcacf2e 100644
--- a/tools/objtool/weak.c
+++ b/tools/objtool/weak.c
@@ -29,3 +29,8 @@ int __weak orc_create(struct objtool_file *file)
{
UNSUPPORTED("orc");
}
+
+int __weak objtool_mcount(struct objtool_file *file)
+{
+ UNSUPPORTED("mcount subcommand");
+}
--
2.31.1

2022-03-21 19:26:41

by Naveen N. Rao

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

Christophe Leroy wrote:
>
>
> Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit :
>> This patch adds 'mcount' as a subcommand to objtool, and enables
>> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
>> if they choose to nop out mcount call sites. If that config option is
>> selected, then --mnop is passed as an option to 'objtool mcount'
>>
>> Signed-off-by: Sathvika Vasireddy <[email protected]>
>> ---
>> Makefile | 6 ++
>> arch/x86/Kconfig | 3 +-
>> scripts/Makefile.build | 12 +++
>> tools/objtool/Build | 2 +
>> tools/objtool/Makefile | 4 +-
>> tools/objtool/builtin-mcount.c | 74 +++++++++++++
>> tools/objtool/include/objtool/builtin.h | 4 +-
>> tools/objtool/include/objtool/objtool.h | 1 +
>> tools/objtool/mcount.c | 138 ++++++++++++++++++++++++
>> tools/objtool/objtool.c | 1 +
>> tools/objtool/weak.c | 5 +
>> 11 files changed, 247 insertions(+), 3 deletions(-)
>> create mode 100644 tools/objtool/builtin-mcount.c
>> create mode 100644 tools/objtool/mcount.c
>>
>> diff --git a/Makefile b/Makefile
>> index 55a30ca69350..316f7d08b30a 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
>> endif
>> endif
>> ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>> + ifdef CONFIG_HAVE_NOP_MCOUNT
>> CC_FLAGS_USING += -DCC_USING_NOP_MCOUNT
>> + endif
>> endif
>> ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>> ifdef CONFIG_HAVE_C_RECORDMCOUNT
>> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
>> prepare: tools/objtool
>> endif
>>
>> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>> +prepare: tools/objtool
>> +endif
>
> I don't think that will work for powerpc.
>
> See arch/powerpc/Makefile
>
> powerpc builds the VDSO under prepare: , and powerpc has
> CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that
> seem to use objtool, allthough that looks odd to me. Must be something
> to fix somewhere.
>
> powerpc64-linux-gcc
> -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc
> -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include
> -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi
> -I./include/uapi -I./include/generated/uapi -include
> ./include/linux/compiler-version.h -include ./include/linux/kconfig.h
> -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc
> -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef
> -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
> -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration
> -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89
> -mbig-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv1
> -mcall-aixdesc -mcmodel=medium -mno-pointers-to-nested-functions
> -mtune=power7 -mcpu=power5 -mno-altivec -mno-vsx
> -fno-asynchronous-unwind-tables -mno-string -Wa,-maltivec -Wa,-mpower4
> -Wa,-many -mabi=elfv1 -mcall-aixdesc -mbig-endian
> -mstack-protector-guard=tls -mstack-protector-guard-reg=r13
> -fno-delete-null-pointer-checks -Wno-frame-address
> -Wno-format-truncation -Wno-format-overflow
> -Wno-address-of-packed-member -O2 -fno-allow-store-data-races
> -Wframe-larger-than=2048 -fstack-protector-strong
> -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable
> -Wno-unused-const-variable -fno-stack-clash-protection
> -Wdeclaration-after-statement -Wvla -Wno-pointer-sign
> -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds
> -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict
> -Wno-maybe-uninitialized -Wno-alloc-size-larger-than
> -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time
> -Werror=incompatible-pointer-types -Werror=designated-init
> -Wno-packed-not-aligned -mstack-protector-guard-offset=3200 -shared
> -fno-common -fno-builtin -nostdlib -Wl,--hash-style=both -include
> /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c -fno-stack-protector
> -DDISABLE_BRANCH_PROFILING -ffreestanding -fasynchronous-unwind-tables
> -ffixed-r30
> -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"'
> -DKBUILD_BASENAME='"vgettimeofday_64"'
> -DKBUILD_MODNAME='"vgettimeofday_64"'
> -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o
> arch/powerpc/kernel/vdso/vgettimeofday-64.o
> arch/powerpc/kernel/vdso/vgettimeofday.c ; ./tools/objtool/objtool
> mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o

We don't enable ftrace for vdso, so I suspect objtool run above will be
a no-op. This needs to be confirmed, of course.


- Naveen

2022-03-21 21:54:30

by Christophe Leroy

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand



Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit :
> This patch adds 'mcount' as a subcommand to objtool, and enables
> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
> if they choose to nop out mcount call sites. If that config option is
> selected, then --mnop is passed as an option to 'objtool mcount'
>
> Signed-off-by: Sathvika Vasireddy <[email protected]>
> ---
> Makefile | 6 ++
> arch/x86/Kconfig | 3 +-
> scripts/Makefile.build | 12 +++
> tools/objtool/Build | 2 +
> tools/objtool/Makefile | 4 +-
> tools/objtool/builtin-mcount.c | 74 +++++++++++++
> tools/objtool/include/objtool/builtin.h | 4 +-
> tools/objtool/include/objtool/objtool.h | 1 +
> tools/objtool/mcount.c | 138 ++++++++++++++++++++++++
> tools/objtool/objtool.c | 1 +
> tools/objtool/weak.c | 5 +
> 11 files changed, 247 insertions(+), 3 deletions(-)
> create mode 100644 tools/objtool/builtin-mcount.c
> create mode 100644 tools/objtool/mcount.c
>
> diff --git a/Makefile b/Makefile
> index 55a30ca69350..316f7d08b30a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
> endif
> endif
> ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> + ifdef CONFIG_HAVE_NOP_MCOUNT
> CC_FLAGS_USING += -DCC_USING_NOP_MCOUNT
> + endif
> endif
> ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> ifdef CONFIG_HAVE_C_RECORDMCOUNT
> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
> prepare: tools/objtool
> endif
>
> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> +prepare: tools/objtool
> +endif

I don't think that will work for powerpc.

See arch/powerpc/Makefile

powerpc builds the VDSO under prepare: , and powerpc has
CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that
seem to use objtool, allthough that looks odd to me. Must be something
to fix somewhere.

powerpc64-linux-gcc
-Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc
-I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include
-I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi
-I./include/uapi -I./include/generated/uapi -include
./include/linux/compiler-version.h -include ./include/linux/kconfig.h
-include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc
-DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef
-Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
-fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration
-Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89
-mbig-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv1
-mcall-aixdesc -mcmodel=medium -mno-pointers-to-nested-functions
-mtune=power7 -mcpu=power5 -mno-altivec -mno-vsx
-fno-asynchronous-unwind-tables -mno-string -Wa,-maltivec -Wa,-mpower4
-Wa,-many -mabi=elfv1 -mcall-aixdesc -mbig-endian
-mstack-protector-guard=tls -mstack-protector-guard-reg=r13
-fno-delete-null-pointer-checks -Wno-frame-address
-Wno-format-truncation -Wno-format-overflow
-Wno-address-of-packed-member -O2 -fno-allow-store-data-races
-Wframe-larger-than=2048 -fstack-protector-strong
-Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable
-Wno-unused-const-variable -fno-stack-clash-protection
-Wdeclaration-after-statement -Wvla -Wno-pointer-sign
-Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds
-Wno-array-bounds -Wno-stringop-overflow -Wno-restrict
-Wno-maybe-uninitialized -Wno-alloc-size-larger-than
-fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time
-Werror=incompatible-pointer-types -Werror=designated-init
-Wno-packed-not-aligned -mstack-protector-guard-offset=3200 -shared
-fno-common -fno-builtin -nostdlib -Wl,--hash-style=both -include
/home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c -fno-stack-protector
-DDISABLE_BRANCH_PROFILING -ffreestanding -fasynchronous-unwind-tables
-ffixed-r30
-DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"'
-DKBUILD_BASENAME='"vgettimeofday_64"'
-DKBUILD_MODNAME='"vgettimeofday_64"'
-D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o
arch/powerpc/kernel/vdso/vgettimeofday-64.o
arch/powerpc/kernel/vdso/vgettimeofday.c ; ./tools/objtool/objtool
mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o


> +
> ifdef CONFIG_BPF
> ifdef CONFIG_DEBUG_INFO_BTF
> prepare: tools/bpf/resolve_btfids

2022-03-21 22:28:31

by Christophe Leroy

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand



Le 21/03/2022 à 09:19, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>>
>>
>> Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit :
>>> This patch adds 'mcount' as a subcommand to objtool, and enables
>>> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>>> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
>>> if they choose to nop out mcount call sites. If that config option is
>>> selected, then --mnop is passed as an option to 'objtool mcount'
>>>
>>> Signed-off-by: Sathvika Vasireddy <[email protected]>
>>> ---
>>>   Makefile                                |   6 ++
>>>   arch/x86/Kconfig                        |   3 +-
>>>   scripts/Makefile.build                  |  12 +++
>>>   tools/objtool/Build                     |   2 +
>>>   tools/objtool/Makefile                  |   4 +-
>>>   tools/objtool/builtin-mcount.c          |  74 +++++++++++++
>>>   tools/objtool/include/objtool/builtin.h |   4 +-
>>>   tools/objtool/include/objtool/objtool.h |   1 +
>>>   tools/objtool/mcount.c                  | 138 ++++++++++++++++++++++++
>>>   tools/objtool/objtool.c                 |   1 +
>>>   tools/objtool/weak.c                    |   5 +
>>>   11 files changed, 247 insertions(+), 3 deletions(-)
>>>   create mode 100644 tools/objtool/builtin-mcount.c
>>>   create mode 100644 tools/objtool/mcount.c
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 55a30ca69350..316f7d08b30a 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
>>>     endif
>>>   endif
>>>   ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>>> + ifdef CONFIG_HAVE_NOP_MCOUNT
>>>     CC_FLAGS_USING    += -DCC_USING_NOP_MCOUNT
>>> + endif
>>>   endif
>>>   ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>>>     ifdef CONFIG_HAVE_C_RECORDMCOUNT
>>> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
>>>   prepare: tools/objtool
>>>   endif
>>> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>>> +prepare: tools/objtool
>>> +endif
>>
>> I don't think that will work for powerpc.
>>
>> See arch/powerpc/Makefile
>>
>> powerpc builds the VDSO under prepare: , and powerpc has
>> CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that
>> seem to use objtool, allthough that looks odd to me. Must be something
>> to fix somewhere.
>>
>>    powerpc64-linux-gcc
>> -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc
>> -I./arch/powerpc/include -I./arch/powerpc/include/generated
>> -I./include -I./arch/powerpc/include/uapi
>> -I./arch/powerpc/include/generated/uapi -I./include/uapi
>> -I./include/generated/uapi -include ./include/linux/compiler-version.h
>> -include ./include/linux/kconfig.h -include
>> ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc
>> -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef
>> -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
>> -fno-common -fshort-wchar -fno-PIE
>> -Werror=implicit-function-declaration -Werror=implicit-int
>> -Werror=return-type -Wno-format-security -std=gnu89 -mbig-endian -m64
>> -msoft-float -pipe -mtraceback=no -mabi=elfv1 -mcall-aixdesc
>> -mcmodel=medium -mno-pointers-to-nested-functions -mtune=power7
>> -mcpu=power5 -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables
>> -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mabi=elfv1
>> -mcall-aixdesc -mbig-endian -mstack-protector-guard=tls
>> -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks
>> -Wno-frame-address -Wno-format-truncation -Wno-format-overflow
>> -Wno-address-of-packed-member -O2 -fno-allow-store-data-races
>> -Wframe-larger-than=2048 -fstack-protector-strong
>> -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable
>> -Wno-unused-const-variable -fno-stack-clash-protection
>> -Wdeclaration-after-statement -Wvla -Wno-pointer-sign
>> -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds
>> -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict
>> -Wno-maybe-uninitialized -Wno-alloc-size-larger-than
>> -fno-strict-overflow -fno-stack-check -fconserve-stack
>> -Werror=date-time -Werror=incompatible-pointer-types
>> -Werror=designated-init -Wno-packed-not-aligned
>> -mstack-protector-guard-offset=3200 -shared -fno-common -fno-builtin
>> -nostdlib -Wl,--hash-style=both -include
>> /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c
>> -fno-stack-protector -DDISABLE_BRANCH_PROFILING -ffreestanding
>> -fasynchronous-unwind-tables -ffixed-r30
>> -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"'
>> -DKBUILD_BASENAME='"vgettimeofday_64"'
>> -DKBUILD_MODNAME='"vgettimeofday_64"'
>> -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o
>> arch/powerpc/kernel/vdso/vgettimeofday-64.o
>> arch/powerpc/kernel/vdso/vgettimeofday.c  ; ./tools/objtool/objtool
>> mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o
>
> We don't enable ftrace for vdso, so I suspect objtool run above will be
> a no-op. This needs to be confirmed, of course.
>

I just checked without the series: recordmcount isn't run for VDSO, so
objtool shouldn't be run either when the series is applied.

Christophe

2022-03-21 22:59:14

by Naveen N. Rao

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

Christophe Leroy wrote:
>
>
> Le 21/03/2022 à 09:19, Naveen N. Rao a écrit :
>> Christophe Leroy wrote:
>>
>> We don't enable ftrace for vdso, so I suspect objtool run above will be
>> a no-op. This needs to be confirmed, of course.
>>
>
> I just checked without the series: recordmcount isn't run for VDSO, so
> objtool shouldn't be run either when the series is applied.

Agree. I was only pointing out that it would be harmless, but it is good
to ensure objtool is skipped for files/directories where ftrace isn't
enabled. recordmcount keys off the presence of CC_FLAGS_FTRACE, and we
should do the same for 'objtool --mcount'.


- Naveen