Most of the time objtool warnings are useless without the
absolute address within the section.
Today there is --sec-address option to get it printed, but
that option is nowhere used and requires a change in Makefile
to use it.
Having the address inside the section at all time in addition
to the address within the object doesn't hurt and will help.
Remove the --sec-address option and print it at all time.
Signed-off-by: Christophe Leroy <[email protected]>
---
tools/objtool/builtin-check.c | 1 -
tools/objtool/include/objtool/builtin.h | 1 -
tools/objtool/include/objtool/warn.h | 6 ++----
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 7c175198d09f..d5024a95467a 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -91,7 +91,6 @@ static const struct option check_options[] = {
OPT_BOOLEAN(0, "module", &opts.module, "object is part of a kernel module"),
OPT_BOOLEAN(0, "mnop", &opts.mnop, "nop out mcount call sites"),
OPT_BOOLEAN(0, "no-unreachable", &opts.no_unreachable, "skip 'unreachable instruction' warnings"),
- OPT_BOOLEAN(0, "sec-address", &opts.sec_address, "print section addresses in warnings"),
OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"),
OPT_END(),
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 2a108e648b7a..af79618cf6ab 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -35,7 +35,6 @@ struct opts {
bool mnop;
bool module;
bool no_unreachable;
- bool sec_address;
bool stats;
};
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index b1c920dc9516..2db9717d0558 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -21,7 +21,6 @@ static inline char *offstr(struct section *sec, unsigned long offset)
bool is_text = (sec->sh.sh_flags & SHF_EXECINSTR);
struct symbol *sym = NULL;
char *str;
- int len;
if (is_text)
sym = find_func_containing(sec, offset);
@@ -30,9 +29,8 @@ static inline char *offstr(struct section *sec, unsigned long offset)
if (sym) {
str = malloc(strlen(sym->name) + strlen(sec->name) + 40);
- len = sprintf(str, "%s+0x%lx", sym->name, offset - sym->offset);
- if (opts.sec_address)
- sprintf(str+len, " (%s+0x%lx)", sec->name, offset);
+ sprintf(str, "%s+0x%lx (%s+0x%lx)", sym->name,
+ offset - sym->offset, sec->name, offset);
} else {
str = malloc(strlen(sec->name) + 20);
sprintf(str, "%s+0x%lx", sec->name, offset);
--
2.40.1
On Wed, Jun 21, 2023 at 05:20:31PM +0200, Christophe Leroy wrote:
> Most of the time objtool warnings are useless without the
> absolute address within the section.
>
> Today there is --sec-address option to get it printed, but
> that option is nowhere used and requires a change in Makefile
> to use it.
>
> Having the address inside the section at all time in addition
> to the address within the object doesn't hurt and will help.
>
> Remove the --sec-address option and print it at all time.
This option actually feels like clutter to me. The func+offset format
works fine, combined with scripts like objdump-func and faddr2line. And
we also have a new OBJTOOL_VERBOSE=1 option which auto-disassembles the
affected function.
On x86 we've been using func+offset for console stack traces for many
years, due to KASLR. So maybe we're just more used to it. But the
scripts make it fine.
Also it helps with identifying the same warning across different
configs/compilers.
--
Josh
Hi Christophe,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.4-rc7 next-20230623]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christophe-Leroy/objtool-Make-sec-address-always-on/20230621-232305
base: linus/master
patch link: https://lore.kernel.org/r/e7e1de1d01194df3ff4053cb0815fc2ddba33213.1687360711.git.christophe.leroy%40csgroup.eu
patch subject: [PATCH] objtool: Make 'sec-address' always on
config: x86_64-randconfig-c002-20230622 (https://download.01.org/0day-ci/archive/20230624/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230624/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> vmlinux.o: warning: objtool: ibt_selftest+0x14 (.text+0x92b54): sibling call from callable instruction with modified stack frame
vmlinux.o: warning: objtool: .altinstr_replacement+0x19a4: redundant UACCESS disable
vmlinux.o: warning: objtool: iovec_from_user.part.0+0xb1 (.text+0x1c47761): call to copy_iovec_from_user.part.0() with UACCESS enabled
vmlinux.o: warning: objtool: ibt_selftest+0x1e (.text+0x92b5e): return with modified stack frame
objdump-func vmlinux.o ibt_selftest:
0000 0000000000092b40 <ibt_selftest>:
0000 92b40: f3 0f 1e fa endbr64
0004 92b44: e8 00 00 00 00 call 92b49 <ibt_selftest+0x9> 92b45: R_X86_64_PLT32 __fentry__-0x4
0009 92b49: 55 push %rbp
000a 92b4a: 48 89 e5 mov %rsp,%rbp
000d 92b4d: 48 8d 05 02 00 00 00 lea 0x2(%rip),%rax # 92b56 <ibt_selftest_ip>
0014 92b54: ff e0 jmp *%rax
0000 0000000000092b56 <ibt_selftest_ip>:
0000 92b56: 90 nop
0001 92b57: 48 85 c0 test %rax,%rax
0004 92b5a: 5d pop %rbp
0005 92b5b: 0f 94 c0 sete %al
0008 92b5e: e9 00 00 00 00 jmp 92b63 <ibt_selftest_ip+0xd> 92b5f: R_X86_64_PLT32 __x86_return_thunk-0x4
000d 92b63: 66 2e 0f 1f 84 00 00 00 00 00 cs nopw 0x0(%rax,%rax,1)
0017 92b6d: 0f 1f 00 nopl (%rax)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Le 24/06/2023 à 09:33, kernel test robot a écrit :
> Hi Christophe,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.4-rc7 next-20230623]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Christophe-Leroy/objtool-Make-sec-address-always-on/20230621-232305
> base: linus/master
> patch link: https://lore.kernel.org/r/e7e1de1d01194df3ff4053cb0815fc2ddba33213.1687360711.git.christophe.leroy%40csgroup.eu
> patch subject: [PATCH] objtool: Make 'sec-address' always on
> config: x86_64-randconfig-c002-20230622 (https://download.01.org/0day-ci/archive/20230624/[email protected]/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20230624/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
>>> vmlinux.o: warning: objtool: ibt_selftest+0x14 (.text+0x92b54): sibling call from callable instruction with modified stack frame
> vmlinux.o: warning: objtool: .altinstr_replacement+0x19a4: redundant UACCESS disable
> vmlinux.o: warning: objtool: iovec_from_user.part.0+0xb1 (.text+0x1c47761): call to copy_iovec_from_user.part.0() with UACCESS enabled
> vmlinux.o: warning: objtool: ibt_selftest+0x1e (.text+0x92b5e): return with modified stack frame
>
I can't really see any link between that warning and the changes in the
patch.
Whatever, this patch will be discarded as pointed out by Josh.
Christophe
>
> objdump-func vmlinux.o ibt_selftest:
> 0000 0000000000092b40 <ibt_selftest>:
> 0000 92b40: f3 0f 1e fa endbr64
> 0004 92b44: e8 00 00 00 00 call 92b49 <ibt_selftest+0x9> 92b45: R_X86_64_PLT32 __fentry__-0x4
> 0009 92b49: 55 push %rbp
> 000a 92b4a: 48 89 e5 mov %rsp,%rbp
> 000d 92b4d: 48 8d 05 02 00 00 00 lea 0x2(%rip),%rax # 92b56 <ibt_selftest_ip>
> 0014 92b54: ff e0 jmp *%rax
> 0000 0000000000092b56 <ibt_selftest_ip>:
> 0000 92b56: 90 nop
> 0001 92b57: 48 85 c0 test %rax,%rax
> 0004 92b5a: 5d pop %rbp
> 0005 92b5b: 0f 94 c0 sete %al
> 0008 92b5e: e9 00 00 00 00 jmp 92b63 <ibt_selftest_ip+0xd> 92b5f: R_X86_64_PLT32 __x86_return_thunk-0x4
> 000d 92b63: 66 2e 0f 1f 84 00 00 00 00 00 cs nopw 0x0(%rax,%rax,1)
> 0017 92b6d: 0f 1f 00 nopl (%rax)
>
Le 22/06/2023 à 08:31, Josh Poimboeuf a écrit :
> On Wed, Jun 21, 2023 at 05:20:31PM +0200, Christophe Leroy wrote:
>> Most of the time objtool warnings are useless without the
>> absolute address within the section.
>>
>> Today there is --sec-address option to get it printed, but
>> that option is nowhere used and requires a change in Makefile
>> to use it.
>>
>> Having the address inside the section at all time in addition
>> to the address within the object doesn't hurt and will help.
>>
>> Remove the --sec-address option and print it at all time.
>
> This option actually feels like clutter to me. The func+offset format
> works fine, combined with scripts like objdump-func and faddr2line. And
> we also have a new OBJTOOL_VERBOSE=1 option which auto-disassembles the
> affected function.
>
> On x86 we've been using func+offset for console stack traces for many
> years, due to KASLR. So maybe we're just more used to it. But the
> scripts make it fine.
Ah right, I didn't know that script, I was struggling with objtool -dr
Therefore the patch is not needed, and neither is the --sec-address
option, maybe we can remove it completely.
>
> Also it helps with identifying the same warning across different
> configs/compilers.
>
Good point.
Christophe
On Sat, Jun 24, 2023 at 08:30:48AM +0000, Christophe Leroy wrote:
> >>> vmlinux.o: warning: objtool: ibt_selftest+0x14 (.text+0x92b54): sibling call from callable instruction with modified stack frame
> > vmlinux.o: warning: objtool: .altinstr_replacement+0x19a4: redundant UACCESS disable
> > vmlinux.o: warning: objtool: iovec_from_user.part.0+0xb1 (.text+0x1c47761): call to copy_iovec_from_user.part.0() with UACCESS enabled
> > vmlinux.o: warning: objtool: ibt_selftest+0x1e (.text+0x92b5e): return with modified stack frame
> >
>
> I can't really see any link between that warning and the changes in the
> patch.
I suspect it's a pre-existing warning, but because the patch made a
change to the default formatting (adding .text+off), it looks like a new
warning to the bots.
--
Josh