Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1718316 > unrolled thread
| Started by | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| First post | 2017-08-23 14:30 +0200 |
| Last post | 2017-08-24 21:20 +0200 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
gcc-8 objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-08-23 14:30 +0200
Re: gcc-8 objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-23 14:50 +0200
Re: gcc-8 objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-08-23 15:40 +0200
Re: gcc-8 objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-23 18:10 +0200
Re: gcc-8 objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-08-24 12:20 +0200
Re: gcc-8 objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-24 21:20 +0200
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-08-23 14:30 +0200 |
| Subject | gcc-8 objtool warnings |
| Message-ID | <uhCXh-u4-25@gated-at.bofh.it> |
Hi Josh,
I upgraded the compiler to gcc-8.0.0 (from earlier this week) and into one
configuration that produces countless objtool warnings, starting with
kernel/panic.o: warning: objtool: nmi_panic()+0x31: sibling call from
callable instruction with modified stack frame
kernel/panic.o: warning: objtool: __warn()+0x26: sibling call from
callable instruction with modified stack frame
kernel/panic.o: warning: objtool: nmi_panic.cold.0()+0x0: call without
frame pointer save/setup
kernel/panic.o: warning: objtool: __warn.cold.1()+0x0: call without
frame pointer save/setup
arch/x86/kernel/irq_64.o: warning: objtool: handle_irq()+0x8a: sibling
call from callable instruction with modified stack frame
arch/x86/kernel/dumpstack.o: warning: objtool: oops_end()+0x83:
sibling call from callable instruction with modified stack frame
kernel/exit.o: warning: objtool: do_exit()+0x39: sibling call from
callable instruction with modified stack frame
kernel/exit.o: warning: objtool: do_exit.cold.1()+0x0: call without
frame pointer save/setup
kernel/cred.o: warning: objtool: put_cred_rcu()+0x26: sibling call
from callable instruction with modified stack frame
kernel/cred.o: warning: objtool: put_cred_rcu.cold.1()+0x0: call
without frame pointer save/setup
mm/oom_kill.o: warning: objtool: oom_kill_process.isra.4()+0xfe:
sibling call from callable instruction with modified stack frame
mm/oom_kill.o: warning: objtool: out_of_memory()+0x556: sibling call
from callable instruction with modified stack frame
mm/oom_kill.o: warning: objtool: oom_kill_process.isra.4.cold.5()+0x0:
call without frame pointer save/setup
mm/oom_kill.o: warning: objtool: out_of_memory.cold.6()+0x0: call
without frame pointer save/setup
mm/page_alloc.o: warning: objtool: warn_alloc()+0x2f: sibling call
from callable instruction with modified stack frame
mm/page_alloc.o: warning: objtool: warn_alloc.cold.20()+0x9: call
without frame pointer save/setup
arch/x86/kernel/apic/io_apic.o: warning: objtool: mp_save_irq()+0x6c:
sibling call from callable instruction with modified stack frame
See https://pastebin.com/1EehhrcP for the .config file.
I looked at a few examples, and they all seem to involve calling panic()
at the end of a a function in .text.unlikely, e.g.
0000000000000390 <put_cred_rcu>:
390: 55 push %rbp
391: 48 89 e5 mov %rsp,%rbp
394: 41 55 push %r13
396: 41 54 push %r12
398: 53 push %rbx
399: 48 89 fb mov %rdi,%rbx
39c: 4c 8d ab 60 ff ff ff lea -0xa0(%rbx),%r13
3a3: e8 00 00 00 00 callq 3a8 <put_cred_rcu+0x18>
3a4: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4
3a8: 44 8b a3 70 ff ff ff mov -0x90(%rbx),%r12d
3af: 41 81 fc 44 61 65 44 cmp $0x44656144,%r12d
3b6: 0f 85 00 00 00 00 jne 3bc <put_cred_rcu+0x2c>
3b8: R_X86_64_PC32 .text.unlikely-0x4
...
0000000000000000 <put_cred_rcu.cold.1>:
0: e8 00 00 00 00 callq 5 <put_cred_rcu.cold.1+0x5>
1: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4
5: 44 8b 8b 64 ff ff ff mov -0x9c(%rbx),%r9d
c: 48 8b 8b 68 ff ff ff mov -0x98(%rbx),%rcx
13: 44 89 e2 mov %r12d,%edx
16: 44 8b 83 60 ff ff ff mov -0xa0(%rbx),%r8d
1d: 4c 89 ee mov %r13,%rsi
20: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
23: R_X86_64_32S .rodata.str1.8+0x28
27: e8 00 00 00 00 callq 2c <__kstrtab_creds_are_invalid+0x3>
28: R_X86_64_PC32 panic-0x4
Arnd
[toc] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-08-23 14:50 +0200 |
| Message-ID | <uhDgB-Aw-15@gated-at.bofh.it> |
| In reply to | #1718316 |
On Wed, Aug 23, 2017 at 02:22:34PM +0200, Arnd Bergmann wrote: > Hi Josh, > > I upgraded the compiler to gcc-8.0.0 (from earlier this week) and into one > configuration that produces countless objtool warnings, starting with > > kernel/panic.o: warning: objtool: nmi_panic()+0x31: sibling call from > callable instruction with modified stack frame > kernel/panic.o: warning: objtool: __warn()+0x26: sibling call from > callable instruction with modified stack frame > kernel/panic.o: warning: objtool: nmi_panic.cold.0()+0x0: call without > frame pointer save/setup > kernel/panic.o: warning: objtool: __warn.cold.1()+0x0: call without > frame pointer save/setup > arch/x86/kernel/irq_64.o: warning: objtool: handle_irq()+0x8a: sibling > call from callable instruction with modified stack frame > arch/x86/kernel/dumpstack.o: warning: objtool: oops_end()+0x83: > sibling call from callable instruction with modified stack frame > kernel/exit.o: warning: objtool: do_exit()+0x39: sibling call from > callable instruction with modified stack frame > kernel/exit.o: warning: objtool: do_exit.cold.1()+0x0: call without > frame pointer save/setup > kernel/cred.o: warning: objtool: put_cred_rcu()+0x26: sibling call > from callable instruction with modified stack frame > kernel/cred.o: warning: objtool: put_cred_rcu.cold.1()+0x0: call > without frame pointer save/setup > mm/oom_kill.o: warning: objtool: oom_kill_process.isra.4()+0xfe: > sibling call from callable instruction with modified stack frame > mm/oom_kill.o: warning: objtool: out_of_memory()+0x556: sibling call > from callable instruction with modified stack frame > mm/oom_kill.o: warning: objtool: oom_kill_process.isra.4.cold.5()+0x0: > call without frame pointer save/setup > mm/oom_kill.o: warning: objtool: out_of_memory.cold.6()+0x0: call > without frame pointer save/setup > mm/page_alloc.o: warning: objtool: warn_alloc()+0x2f: sibling call > from callable instruction with modified stack frame > mm/page_alloc.o: warning: objtool: warn_alloc.cold.20()+0x9: call > without frame pointer save/setup > arch/x86/kernel/apic/io_apic.o: warning: objtool: mp_save_irq()+0x6c: > sibling call from callable instruction with modified stack frame > > See https://pastebin.com/1EehhrcP for the .config file. > > I looked at a few examples, and they all seem to involve calling panic() > at the end of a a function in .text.unlikely, e.g. > > 0000000000000390 <put_cred_rcu>: > 390: 55 push %rbp > 391: 48 89 e5 mov %rsp,%rbp > 394: 41 55 push %r13 > 396: 41 54 push %r12 > 398: 53 push %rbx > 399: 48 89 fb mov %rdi,%rbx > 39c: 4c 8d ab 60 ff ff ff lea -0xa0(%rbx),%r13 > 3a3: e8 00 00 00 00 callq 3a8 <put_cred_rcu+0x18> > 3a4: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 3a8: 44 8b a3 70 ff ff ff mov -0x90(%rbx),%r12d > 3af: 41 81 fc 44 61 65 44 cmp $0x44656144,%r12d > 3b6: 0f 85 00 00 00 00 jne 3bc <put_cred_rcu+0x2c> > 3b8: R_X86_64_PC32 .text.unlikely-0x4 > > ... > > 0000000000000000 <put_cred_rcu.cold.1>: > 0: e8 00 00 00 00 callq 5 <put_cred_rcu.cold.1+0x5> > 1: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 5: 44 8b 8b 64 ff ff ff mov -0x9c(%rbx),%r9d > c: 48 8b 8b 68 ff ff ff mov -0x98(%rbx),%rcx > 13: 44 89 e2 mov %r12d,%edx > 16: 44 8b 83 60 ff ff ff mov -0xa0(%rbx),%r8d > 1d: 4c 89 ee mov %r13,%rsi > 20: 48 c7 c7 00 00 00 00 mov $0x0,%rdi > 23: R_X86_64_32S .rodata.str1.8+0x28 > 27: e8 00 00 00 00 callq 2c <__kstrtab_creds_are_invalid+0x3> > 28: R_X86_64_PC32 panic-0x4 Thanks. Can you send me one of the .o files? -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-08-23 15:40 +0200 |
| Message-ID | <uhE30-17N-7@gated-at.bofh.it> |
| In reply to | #1718330 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 23, 2017 at 2:48 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Aug 23, 2017 at 02:22:34PM +0200, Arnd Bergmann wrote:
>> ...
>>
>> 0000000000000000 <put_cred_rcu.cold.1>:
>> 0: e8 00 00 00 00 callq 5 <put_cred_rcu.cold.1+0x5>
>> 1: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4
>> 5: 44 8b 8b 64 ff ff ff mov -0x9c(%rbx),%r9d
>> c: 48 8b 8b 68 ff ff ff mov -0x98(%rbx),%rcx
>> 13: 44 89 e2 mov %r12d,%edx
>> 16: 44 8b 83 60 ff ff ff mov -0xa0(%rbx),%r8d
>> 1d: 4c 89 ee mov %r13,%rsi
>> 20: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
>> 23: R_X86_64_32S .rodata.str1.8+0x28
>> 27: e8 00 00 00 00 callq 2c <__kstrtab_creds_are_invalid+0x3>
>> 28: R_X86_64_PC32 panic-0x4
>
> Thanks. Can you send me one of the .o files?
Attached here now.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-08-23 18:10 +0200 |
| Message-ID | <uhGoa-2JU-21@gated-at.bofh.it> |
| In reply to | #1718356 |
On Wed, Aug 23, 2017 at 03:38:02PM +0200, Arnd Bergmann wrote: > On Wed, Aug 23, 2017 at 2:48 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Wed, Aug 23, 2017 at 02:22:34PM +0200, Arnd Bergmann wrote: > >> ... > >> > >> 0000000000000000 <put_cred_rcu.cold.1>: > >> 0: e8 00 00 00 00 callq 5 <put_cred_rcu.cold.1+0x5> > >> 1: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > >> 5: 44 8b 8b 64 ff ff ff mov -0x9c(%rbx),%r9d > >> c: 48 8b 8b 68 ff ff ff mov -0x98(%rbx),%rcx > >> 13: 44 89 e2 mov %r12d,%edx > >> 16: 44 8b 83 60 ff ff ff mov -0xa0(%rbx),%r8d > >> 1d: 4c 89 ee mov %r13,%rsi > >> 20: 48 c7 c7 00 00 00 00 mov $0x0,%rdi > >> 23: R_X86_64_32S .rodata.str1.8+0x28 > >> 27: e8 00 00 00 00 callq 2c <__kstrtab_creds_are_invalid+0x3> > >> 28: R_X86_64_PC32 panic-0x4 > > > > Thanks. Can you send me one of the .o files? > > Attached here now. Ok, looks like I'll need to add support for this new pattern (jumping to a .cold section in .text.unlikely). I'm also about to start work on fixing that other issue you found with GCC's inefficient update of the stack pointer. I really appreciate your finding all these warnings (and getting advance GCC 8 testing). Thanks again! -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-08-24 12:20 +0200 |
| Message-ID | <uhXp0-563-29@gated-at.bofh.it> |
| In reply to | #1718488 |
On Wed, Aug 23, 2017 at 6:01 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Aug 23, 2017 at 03:38:02PM +0200, Arnd Bergmann wrote:
>> On Wed, Aug 23, 2017 at 2:48 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> > On Wed, Aug 23, 2017 at 02:22:34PM +0200, Arnd Bergmann wrote:
>> >> ...
>> >>
>> >> 0000000000000000 <put_cred_rcu.cold.1>:
>> >> 0: e8 00 00 00 00 callq 5 <put_cred_rcu.cold.1+0x5>
>> >> 1: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4
>> >> 5: 44 8b 8b 64 ff ff ff mov -0x9c(%rbx),%r9d
>> >> c: 48 8b 8b 68 ff ff ff mov -0x98(%rbx),%rcx
>> >> 13: 44 89 e2 mov %r12d,%edx
>> >> 16: 44 8b 83 60 ff ff ff mov -0xa0(%rbx),%r8d
>> >> 1d: 4c 89 ee mov %r13,%rsi
>> >> 20: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
>> >> 23: R_X86_64_32S .rodata.str1.8+0x28
>> >> 27: e8 00 00 00 00 callq 2c <__kstrtab_creds_are_invalid+0x3>
>> >> 28: R_X86_64_PC32 panic-0x4
>> >
>> > Thanks. Can you send me one of the .o files?
>>
>> Attached here now.
>
> Ok, looks like I'll need to add support for this new pattern (jumping to
> a .cold section in .text.unlikely).
>
> I'm also about to start work on fixing that other issue you found with
> GCC's inefficient update of the stack pointer.
>
> I really appreciate your finding all these warnings (and getting advance
> GCC 8 testing). Thanks again!
No worries. I've disabled the four warnings in objtool that triggered now
and almost all are gone, but I still get a few warnings after doing additional
randconfig builds.
find rand*/ -name \*success -mtime -2 -size +0c | xargs grep -v
"sibling call from callable instruction" | grep -v "call without frame
pointer save/setup" | grep -v "return with modified stack frame" | cut
-f 3- -d: | sort | uniq -c
1 warning: objtool: adv76xx_log_status.cold.34()+0x338: stack
state mismatch: reg1[14]=-1+0 reg2[14]=-2-24
1 warning: objtool: bnx2x_panic_dump.cold.125()+0x1c6:
unsupported stack register modification
1 warning: objtool: cx25840_probe.cold.9()+0x189: stack state
mismatch: reg1[3]=-2-16 reg2[3]=-1+0
1 warning: objtool: hpet_alloc.cold.6()+0x39d: stack state
mismatch: reg1[14]=-1+0 reg2[14]=-2-16
1 warning: objtool: init_test_probes.cold.6()+0x3ae: leave
instruction with modified stack frame
4 warning: objtool: lkdtm_ATOMIC_TIMING.cold.0()+0x0: leave
instruction with modified stack frame
4 warning: objtool: lkdtm_ATOMIC_TIMING.cold.4()+0x0: leave
instruction with modified stack frame
2 warning: objtool: lkdtm_REFCOUNT_TIMING.cold.1()+0x0: leave
instruction with modified stack frame
3 warning: objtool: lkdtm_REFCOUNT_TIMING.cold.5()+0x0: leave
instruction with modified stack frame
1 warning: objtool: mgsl_add_device.cold.27()+0x82: stack state
mismatch: reg1[15]=-2-16 reg2[15]=-1+0
1 warning: objtool: mgsl_add_device.cold.41()+0x82: stack state
mismatch: reg1[15]=-2-16 reg2[15]=-1+0
1 warning: objtool: mlx5_eswitch_disable_sriov.cold.19()+0xd0:
stack state mismatch: reg1[13]=-1+0 reg2[13]=-2-16
1 warning: objtool: __oom_reap_task_mm.cold.10()+0x0: stack
state mismatch: cfa1=6+16 cfa2=7+8
1 warning: objtool: __oom_reap_task_mm.cold.12()+0x0: stack
state mismatch: cfa1=6+16 cfa2=7+8
2 warning: objtool: __oom_reap_task_mm.cold.3()+0x0: stack
state mismatch: cfa1=6+16 cfa2=7+8
1 warning: objtool: __oom_reap_task_mm.cold.31()+0x0: stack
state mismatch: cfa1=6+16 cfa2=7+8
1 warning: objtool: __oom_reap_task_mm.cold.7()+0x0: stack
state mismatch: cfa1=6+16 cfa2=7+8
2 warning: objtool: ppp_print_buffer.cold.7()+0x0: stack state
mismatch: cfa1=6+16 cfa2=7+8
1 warning: objtool: rndis_msg_parser.cold.6()+0x0: stack state
mismatch: cfa1=6+16 cfa2=7+8
6 warning: objtool: rodata_test.cold.0()+0x31: leave
instruction with modified stack frame
3 warning: objtool: rodata_test.cold.0()+0x4a: leave
instruction with modified stack frame
1 warning: objtool: show_free_areas.cold.110()+0x394: stack
state mismatch: reg1[15]=-1+0 reg2[15]=-2-32
1 warning: objtool: show_free_areas.cold.116()+0x239: stack
state mismatch: reg1[12]=-1+0 reg2[12]=-2-32
1 warning: objtool: show_free_areas.cold.129()+0x23a: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-16
1 warning: objtool: show_free_areas.cold.18()+0x3aa: stack
state mismatch: reg1[12]=-1+0 reg2[12]=-2-16
1 warning: objtool: show_free_areas.cold.21()+0x3c1: stack
state mismatch: reg1[12]=-1+0 reg2[12]=-2-32
1 warning: objtool: show_free_areas.cold.21()+0x3e2: stack
state mismatch: reg1[12]=-1+0 reg2[12]=-2-32
1 warning: objtool: show_free_areas.cold.24()+0x3c1: stack
state mismatch: reg1[12]=-1+0 reg2[12]=-2-32
1 warning: objtool: show_free_areas.cold.27()+0x239: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-16
1 warning: objtool: show_free_areas.cold.30()+0x239: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-16
1 warning: objtool: show_free_areas.cold.31()+0x23c: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-56
1 warning: objtool: show_free_areas.cold.34()+0x24b: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-16
1 warning: objtool: snic_alloc_vnic_res.cold.2()+0x2b1: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-24
1 warning: objtool: snic_alloc_vnic_res.cold.2()+0x352: stack
state mismatch: reg1[13]=-1+0 reg2[13]=-2-24
1 warning: objtool: syscon_poweroff.cold.0()+0x10: leave
instruction with modified stack frame
1 warning: objtool: tda18271c2_rf_cal_init.cold.13()+0x156:
stack state mismatch: reg1[13]=-1+0 reg2[13]=-2-24
3 warning: objtool: tda18271c2_rf_cal_init.cold.6()+0x156:
stack state mismatch: reg1[13]=-1+0 reg2[13]=-2-24
1 warning: objtool: tea5761_autodetection.cold.1()+0x3c: leave
instruction with modified stack frame
1 warning: objtool: tea5761_autodetection.cold.5()+0x3c: leave
instruction with modified stack frame
1 warning: objtool: test.cold.1()+0x7e: stack state mismatch:
reg1[3]=-1+0 reg2[3]=-2-32
1 warning: objtool: test.cold.3()+0x6e: stack state mismatch:
reg1[12]=-1+0 reg2[12]=-2-32
1 warning: objtool: test.cold.4()+0x7e: stack state mismatch:
reg1[3]=-1+0 reg2[3]=-2-32
1 warning: objtool: test.cold.5()+0x90: stack state mismatch:
reg1[3]=-1+0 reg2[3]=-2-32
1 warning: objtool: .text.unlikely: unexpected end of section
1 warning: objtool: __twa_shutdown.cold.26()+0x1a: leave
instruction with modified stack frame
1 warning: objtool: __twl_shutdown.cold.10()+0x1c: leave
instruction with modified stack frame
1 warning: objtool: __twl_shutdown.cold.19()+0x1c: leave
instruction with modified stack frame
1 warning: objtool: vc4_hvs_dump_state.cold.1()+0x1e: stack
state mismatch: reg1[14]=-1+0 reg2[14]=-2-16
1 warning: objtool: vt596_dump_regs.cold.1()+0x1a: stack state
mismatch: cfa1=6+16 cfa2=7+8
1 warning: objtool: wcn36xx_smd_start.cold.20()+0x32:
unsupported stack register modification
1 warning: objtool: wsm_cmd_send.cold.18()+0x48: stack state
mismatch: reg1[14]=-1+0 reg2[14]=-2-24
1 warning: objtool: xen_callback_vector.cold.18()+0x92: leave
instruction with modified stack frame
1 warning: objtool: xlvbd_flush.cold.20()+0x3c: leave
instruction with modified stack frame
1 warning: objtool: xlvbd_flush.cold.5()+0x3f: leave
instruction with modified stack frame
I'll send you some more object files as private mail.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-08-24 21:20 +0200 |
| Message-ID | <ui5PA-28K-11@gated-at.bofh.it> |
| In reply to | #1719122 |
On Thu, Aug 24, 2017 at 12:14:27PM +0200, Arnd Bergmann wrote:
> On Wed, Aug 23, 2017 at 6:01 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Aug 23, 2017 at 03:38:02PM +0200, Arnd Bergmann wrote:
> >> On Wed, Aug 23, 2017 at 2:48 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >> > On Wed, Aug 23, 2017 at 02:22:34PM +0200, Arnd Bergmann wrote:
> >> >> ...
> >> >>
> >> >> 0000000000000000 <put_cred_rcu.cold.1>:
> >> >> 0: e8 00 00 00 00 callq 5 <put_cred_rcu.cold.1+0x5>
> >> >> 1: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4
> >> >> 5: 44 8b 8b 64 ff ff ff mov -0x9c(%rbx),%r9d
> >> >> c: 48 8b 8b 68 ff ff ff mov -0x98(%rbx),%rcx
> >> >> 13: 44 89 e2 mov %r12d,%edx
> >> >> 16: 44 8b 83 60 ff ff ff mov -0xa0(%rbx),%r8d
> >> >> 1d: 4c 89 ee mov %r13,%rsi
> >> >> 20: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
> >> >> 23: R_X86_64_32S .rodata.str1.8+0x28
> >> >> 27: e8 00 00 00 00 callq 2c <__kstrtab_creds_are_invalid+0x3>
> >> >> 28: R_X86_64_PC32 panic-0x4
> >> >
> >> > Thanks. Can you send me one of the .o files?
> >>
> >> Attached here now.
> >
> > Ok, looks like I'll need to add support for this new pattern (jumping to
> > a .cold section in .text.unlikely).
> >
> > I'm also about to start work on fixing that other issue you found with
> > GCC's inefficient update of the stack pointer.
> >
> > I really appreciate your finding all these warnings (and getting advance
> > GCC 8 testing). Thanks again!
>
> No worries. I've disabled the four warnings in objtool that triggered now
> and almost all are gone, but I still get a few warnings after doing additional
> randconfig builds.
Ok, I've got a fix for *most* of them below.
Still need to fix the gc.o warning, which at first glance looks like a
new switch statement pattern.
The only one I don't understand is smscoreapi.o.
smscore_set_device_mode() branches to smscore_set_device_mode.cold.13(),
which just falls off the edge of the world. I don't know if it's a GCC
bug, or a sancov GCC plugin bug, or if it's another "undefined behavior"
issue. Would you mind rebuilding that one with CONFIG_DEBUG_INFO?
Maybe that will give some more clues.
Also, a question about the GCC 8 development cycle. Since GCC 8 hasn't
been released yet, I don't know whether it makes sense to "fix" objtool
for it yet. Do you have any idea about when GCC 8 will be released?
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 7841e5d31973..0e8c8ec4fd4e 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -86,8 +86,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
struct insn insn;
int x86_64, sign;
unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0,
- modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0,
- sib = 0;
+ rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
+ modrm_reg = 0, sib = 0;
x86_64 = is_x86_64(elf);
if (x86_64 == -1)
@@ -114,6 +114,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
rex = insn.rex_prefix.bytes[0];
rex_w = X86_REX_W(rex) >> 3;
rex_r = X86_REX_R(rex) >> 2;
+ rex_x = X86_REX_X(rex) >> 1;
rex_b = X86_REX_B(rex);
}
@@ -217,6 +218,18 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
op->dest.reg = CFI_BP;
break;
}
+
+ if (rex_w && !rex_b && modrm_mod == 3 && modrm_rm == 4) {
+
+ /* mov reg, %rsp */
+ *type = INSN_STACK;
+ op->src.type = OP_SRC_REG;
+ op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ break;
+ }
+
/* fallthrough */
case 0x88:
if (!rex_b &&
@@ -269,80 +282,28 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
break;
case 0x8d:
- if (rex == 0x48 && modrm == 0x65) {
+ if (sib == 0x24 && rex_w && !rex_b && !rex_x) {
- /* lea disp(%rbp), %rsp */
+ /* lea disp(%rsp), reg */
*type = INSN_STACK;
op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_BP;
+ op->src.reg = CFI_SP;
op->src.offset = insn.displacement.value;
op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
- break;
- }
+ op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
- if (rex == 0x48 && (modrm == 0xa4 || modrm == 0x64) &&
- sib == 0x24) {
+ } else if (rex == 0x48 && modrm == 0x65) {
- /* lea disp(%rsp), %rsp */
+ /* lea disp(%rbp), %rsp */
*type = INSN_STACK;
op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
+ op->src.reg = CFI_BP;
op->src.offset = insn.displacement.value;
op->dest.type = OP_DEST_REG;
op->dest.reg = CFI_SP;
- break;
- }
- if (rex == 0x48 && modrm == 0x2c && sib == 0x24) {
-
- /* lea (%rsp), %rbp */
- *type = INSN_STACK;
- op->src.type = OP_SRC_REG;
- op->src.reg = CFI_SP;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_BP;
- break;
- }
-
- if (rex == 0x4c && modrm == 0x54 && sib == 0x24 &&
- insn.displacement.value == 8) {
-
- /*
- * lea 0x8(%rsp), %r10
- *
- * Here r10 is the "drap" pointer, used as a stack
- * pointer helper when the stack gets realigned.
- */
- *type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
- op->src.offset = 8;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_R10;
- break;
- }
-
- if (rex == 0x4c && modrm == 0x6c && sib == 0x24 &&
- insn.displacement.value == 16) {
-
- /*
- * lea 0x10(%rsp), %r13
- *
- * Here r13 is the "drap" pointer, used as a stack
- * pointer helper when the stack gets realigned.
- */
- *type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
- op->src.offset = 16;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_R13;
- break;
- }
-
- if (rex == 0x49 && modrm == 0x62 &&
- insn.displacement.value == -8) {
+ } else if (rex == 0x49 && modrm == 0x62 &&
+ insn.displacement.value == -8) {
/*
* lea -0x8(%r10), %rsp
@@ -356,11 +317,9 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
op->src.offset = -8;
op->dest.type = OP_DEST_REG;
op->dest.reg = CFI_SP;
- break;
- }
- if (rex == 0x49 && modrm == 0x65 &&
- insn.displacement.value == -16) {
+ } else if (rex == 0x49 && modrm == 0x65 &&
+ insn.displacement.value == -16) {
/*
* lea -0x10(%r13), %rsp
@@ -374,7 +333,6 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
op->src.offset = -16;
op->dest.type = OP_DEST_REG;
op->dest.reg = CFI_SP;
- break;
}
break;
diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h
index 443ab2c69992..2fe883c665c7 100644
--- a/tools/objtool/cfi.h
+++ b/tools/objtool/cfi.h
@@ -40,7 +40,7 @@
#define CFI_R14 14
#define CFI_R15 15
#define CFI_RA 16
-#define CFI_NUM_REGS 17
+#define CFI_NUM_REGS 17
struct cfi_reg {
int base;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3dffeb944523..5d6a3be10aee 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -102,124 +102,16 @@ static bool ignore_func(struct objtool_file *file, struct symbol *func)
return false;
}
-/*
- * This checks to see if the given function is a "noreturn" function.
- *
- * For global functions which are outside the scope of this object file, we
- * have to keep a manual list of them.
- *
- * For local functions, we have to detect them manually by simply looking for
- * the lack of a return instruction.
- *
- * Returns:
- * -1: error
- * 0: no dead end
- * 1: dead end
- */
-static int __dead_end_function(struct objtool_file *file, struct symbol *func,
- int recursion)
-{
- int i;
- struct instruction *insn;
- bool empty = true;
-
- /*
- * Unfortunately these have to be hard coded because the noreturn
- * attribute isn't provided in ELF data.
- */
- static const char * const global_noreturns[] = {
- "__stack_chk_fail",
- "panic",
- "do_exit",
- "do_task_dead",
- "__module_put_and_exit",
- "complete_and_exit",
- "kvm_spurious_fault",
- "__reiserfs_panic",
- "lbug_with_loc",
- "fortify_panic",
- };
-
- if (func->bind == STB_WEAK)
- return 0;
-
- if (func->bind == STB_GLOBAL)
- for (i = 0; i < ARRAY_SIZE(global_noreturns); i++)
- if (!strcmp(func->name, global_noreturns[i]))
- return 1;
-
- if (!func->sec)
- return 0;
-
- func_for_each_insn(file, func, insn) {
- empty = false;
-
- if (insn->type == INSN_RETURN)
- return 0;
- }
-
- if (empty)
- return 0;
-
- /*
- * A function can have a sibling call instead of a return. In that
- * case, the function's dead-end status depends on whether the target
- * of the sibling call returns.
- */
- func_for_each_insn(file, func, insn) {
- if (insn->sec != func->sec ||
- insn->offset >= func->offset + func->len)
- break;
-
- if (insn->type == INSN_JUMP_UNCONDITIONAL) {
- struct instruction *dest = insn->jump_dest;
- struct symbol *dest_func;
-
- if (!dest)
- /* sibling call to another file */
- return 0;
-
- if (dest->sec != func->sec ||
- dest->offset < func->offset ||
- dest->offset >= func->offset + func->len) {
- /* local sibling call */
- dest_func = find_symbol_by_offset(dest->sec,
- dest->offset);
- if (!dest_func)
- continue;
-
- if (recursion == 5) {
- WARN_FUNC("infinite recursion (objtool bug!)",
- dest->sec, dest->offset);
- return -1;
- }
-
- return __dead_end_function(file, dest_func,
- recursion + 1);
- }
- }
-
- if (insn->type == INSN_JUMP_DYNAMIC && list_empty(&insn->alts))
- /* sibling call */
- return 0;
- }
-
- return 1;
-}
-
-static int dead_end_function(struct objtool_file *file, struct symbol *func)
-{
- return __dead_end_function(file, func, 0);
-}
-
static void clear_insn_state(struct insn_state *state)
{
int i;
memset(state, 0, sizeof(*state));
state->cfa.base = CFI_UNDEFINED;
- for (i = 0; i < CFI_NUM_REGS; i++)
+ for (i = 0; i < CFI_NUM_REGS; i++) {
state->regs[i].base = CFI_UNDEFINED;
+ state->vals[i].base = CFI_UNDEFINED;
+ }
state->drap_reg = CFI_UNDEFINED;
state->drap_offset = -1;
}
@@ -234,6 +126,7 @@ static int decode_instructions(struct objtool_file *file)
struct symbol *func;
unsigned long offset;
struct instruction *insn;
+ bool unlikely, cold;
int ret;
for_each_sec(file, sec) {
@@ -246,6 +139,8 @@ static int decode_instructions(struct objtool_file *file)
strncmp(sec->name, ".discard.", 9))
sec->text = true;
+ unlikely = (!(strcmp(sec->name, ".text.unlikely")));
+
for (offset = 0; offset < sec->len; offset += insn->len) {
insn = malloc(sizeof(*insn));
if (!insn) {
@@ -258,6 +153,8 @@ static int decode_instructions(struct objtool_file *file)
insn->sec = sec;
insn->offset = offset;
+ if (unlikely)
+ insn->subfunc = true;
ret = arch_decode_instruction(file->elf, sec, offset,
sec->len - offset,
@@ -287,9 +184,16 @@ static int decode_instructions(struct objtool_file *file)
return -1;
}
- func_for_each_insn(file, func, insn)
+ cold = (strstr(func->name, ".cold."));
+
+ func_for_each_insn(file, func, insn) {
+
if (!insn->func)
insn->func = func;
+
+ if (unlikely && !cold)
+ insn->subfunc = false;
+ }
}
}
@@ -1007,6 +911,131 @@ static int read_unwind_hints(struct objtool_file *file)
return 0;
}
+/*
+ * This checks to see if the given function is a "noreturn" function.
+ *
+ * For global functions which are outside the scope of this object file, we
+ * have to keep a manual list of them.
+ *
+ * For local functions, we have to detect them manually by simply looking for
+ * the lack of a return instruction.
+ *
+ * Returns:
+ * -1: error
+ * 0: no dead end
+ * 1: dead end
+ */
+static int find_dead_end_function(struct objtool_file *file, struct symbol *func,
+ int recursion)
+{
+ int i;
+ struct instruction *insn;
+ bool empty = true;
+
+ /*
+ * Unfortunately these have to be hard coded because the noreturn
+ * attribute isn't provided in ELF data.
+ *
+ * Eventually this should be implemented with a GCC plugin instead.
+ */
+ static const char * const global_noreturns[] = {
+ "__stack_chk_fail",
+ "panic",
+ "do_exit",
+ "do_task_dead",
+ "__module_put_and_exit",
+ "complete_and_exit",
+ "kvm_spurious_fault",
+ "__reiserfs_panic",
+ "lbug_with_loc",
+ "fortify_panic",
+ };
+
+ if (func->bind == STB_GLOBAL)
+ for (i = 0; i < ARRAY_SIZE(global_noreturns); i++)
+ if (!strcmp(func->name, global_noreturns[i]))
+ goto dead_end;
+
+ if (func->bind == STB_WEAK || func->type != STT_FUNC)
+ return 0;
+
+ func_for_each_insn(file, func, insn) {
+ empty = false;
+
+ if (insn->type == INSN_RETURN)
+ return 0;
+ }
+
+ if (empty)
+ return 0;
+
+ /*
+ * A function can have a sibling call instead of a return. In that
+ * case, the function's dead-end status depends on whether the target
+ * of the sibling call returns.
+ *
+ * A function can also have "cold" subfunction code in .text.unlikely
+ * which needs to be checked.
+ */
+ func_for_each_insn(file, func, insn) {
+ if (insn->sec != func->sec ||
+ insn->offset >= func->offset + func->len)
+ break;
+
+ if (insn->type == INSN_JUMP_UNCONDITIONAL ||
+ (insn->jump_dest && insn->jump_dest->subfunc)) {
+ struct instruction *dest = insn->jump_dest;
+ struct symbol *dest_func;
+
+ if (!dest) {
+ /* sibling call to another file */
+ return 0;
+ }
+
+ if (dest->sec != func->sec ||
+ dest->offset < func->offset ||
+ dest->offset >= func->offset + func->len) {
+ /* local sibling call */
+ dest_func = find_symbol_by_offset(dest->sec,
+ dest->offset);
+ if (!dest_func)
+ continue;
+
+ if (recursion == 5) {
+ WARN_FUNC("infinite recursion (objtool bug!)",
+ dest->sec, dest->offset);
+ return -1;
+ }
+
+ return find_dead_end_function(file, dest_func,
+ recursion + 1);
+ }
+ }
+
+ if (insn->type == INSN_JUMP_DYNAMIC && list_empty(&insn->alts)) {
+ /* sibling call */
+ return 0;
+ }
+ }
+
+dead_end:
+ func->dead_end = true;
+ return 0;
+}
+
+static int find_dead_end_functions(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 (find_dead_end_function(file, func, 0))
+ return -1;
+
+ return 0;
+}
+
static int decode_sections(struct objtool_file *file)
{
int ret;
@@ -1041,6 +1070,10 @@ static int decode_sections(struct objtool_file *file)
if (ret)
return ret;
+ ret = find_dead_end_functions(file);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -1201,24 +1234,46 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
switch (op->src.type) {
case OP_SRC_REG:
- if (cfa->base == op->src.reg && cfa->base == CFI_SP &&
- op->dest.reg == CFI_BP && regs[CFI_BP].base == CFI_CFA &&
- regs[CFI_BP].offset == -cfa->offset) {
-
- /* mov %rsp, %rbp */
- cfa->base = op->dest.reg;
- state->bp_scratch = false;
- } else if (state->drap) {
-
- /* drap: mov %rsp, %rbp */
- regs[CFI_BP].base = CFI_BP;
- regs[CFI_BP].offset = -state->stack_size;
- state->bp_scratch = false;
- } else if (!no_fp) {
-
- WARN_FUNC("unknown stack-related register move",
- insn->sec, insn->offset);
- return -1;
+ if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP) {
+
+ if (cfa->base == CFI_SP &&
+ regs[CFI_BP].base == CFI_CFA &&
+ regs[CFI_BP].offset == -cfa->offset) {
+
+ /* mov %rsp, %rbp */
+ cfa->base = op->dest.reg;
+ state->bp_scratch = false;
+ }
+
+ else if (state->drap) {
+
+ /* drap: mov %rsp, %rbp */
+ regs[CFI_BP].base = CFI_BP;
+ regs[CFI_BP].offset = -state->stack_size;
+ state->bp_scratch = false;
+ }
+ }
+
+ else if (op->dest.reg == cfa->base) {
+
+ /* mov %reg, %rsp */
+ if (cfa->base == CFI_SP &&
+ state->vals[op->src.reg].base == CFI_CFA) {
+
+ /*
+ * This is needed for the rare case
+ * where GCC does something dumb like:
+ *
+ * lea 0x8(%rsp),%rcx
+ * mov %rcx,%rsp
+ */
+ cfa->offset = -state->vals[op->src.reg].offset;
+ state->stack_size = cfa->offset;
+
+ } else {
+ cfa->base = CFI_UNDEFINED;
+ cfa->offset = 0;
+ }
}
break;
@@ -1245,6 +1300,20 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
/* drap: lea disp(%rsp), %drap */
state->drap_reg = op->dest.reg;
+
+ /*
+ * lea disp(%rsp), %reg
+ *
+ * This is needed for the rare case where GCC
+ * does something dumb like:
+ *
+ * lea 0x8(%rsp),%rcx
+ * mov %rcx,%rsp
+ */
+ state->vals[op->dest.reg].base = CFI_CFA;
+ state->vals[op->dest.reg].offset = \
+ -state->stack_size + op->src.offset;
+
break;
}
@@ -1536,7 +1605,6 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
while (1) {
next_insn = next_insn_same_sec(file, insn);
-
if (file->c_file && func && insn->func && func != insn->func) {
WARN("%s() falls through to next function %s()",
func->name, insn->func->name);
@@ -1631,11 +1699,9 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
if (is_fentry_call(insn))
break;
- ret = dead_end_function(file, insn->call_dest);
- if (ret == 1)
+ if (insn->call_dest &&
+ insn->call_dest->dead_end)
return 0;
- if (ret == -1)
- return 1;
/* fallthrough */
case INSN_CALL_DYNAMIC:
@@ -1650,7 +1716,8 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
case INSN_JUMP_UNCONDITIONAL:
if (insn->jump_dest &&
(!func || !insn->jump_dest->func ||
- func == insn->jump_dest->func)) {
+ func == insn->jump_dest->func ||
+ insn->subfunc || insn->jump_dest->subfunc)) {
ret = validate_branch(file, insn->jump_dest,
state);
if (ret)
@@ -1808,7 +1875,7 @@ static int validate_functions(struct objtool_file *file)
continue;
insn = find_insn(file, sec, func->offset);
- if (!insn || insn->ignore)
+ if (!insn || insn->ignore || insn->subfunc)
continue;
ret = validate_branch(file, insn, state);
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 9f113016bf8c..fc5570583b88 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -33,6 +33,7 @@ struct insn_state {
bool bp_scratch;
bool drap;
int drap_reg, drap_offset;
+ struct cfi_reg vals[CFI_NUM_REGS];
};
struct instruction {
@@ -43,7 +44,7 @@ struct instruction {
unsigned int len;
unsigned char type;
unsigned long immediate;
- bool alt_group, visited, dead_end, ignore, hint, save, restore;
+ bool alt_group, visited, dead_end, ignore, subfunc, hint, save, restore;
struct symbol *call_dest;
struct instruction *jump_dest;
struct list_head alts;
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index d86e2ff14466..639c419edba3 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -61,6 +61,7 @@ struct symbol {
unsigned char bind, type;
unsigned long offset;
unsigned int len;
+ bool dead_end;
};
struct rela {
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web