* gcc-8 objtool warnings
@ 2017-08-23 12:22 Arnd Bergmann
2017-08-23 12:48 ` Josh Poimboeuf
0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2017-08-23 12:22 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-23 12:22 gcc-8 objtool warnings Arnd Bergmann
@ 2017-08-23 12:48 ` Josh Poimboeuf
2017-08-23 13:38 ` Arnd Bergmann
0 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-08-23 12:48 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-23 12:48 ` Josh Poimboeuf
@ 2017-08-23 13:38 ` Arnd Bergmann
2017-08-23 16:01 ` Josh Poimboeuf
0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2017-08-23 13:38 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 972 bytes --]
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
[-- Attachment #2: cred.o --]
[-- Type: application/x-object, Size: 27472 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-23 13:38 ` Arnd Bergmann
@ 2017-08-23 16:01 ` Josh Poimboeuf
2017-08-24 10:14 ` Arnd Bergmann
0 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-08-23 16:01 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-23 16:01 ` Josh Poimboeuf
@ 2017-08-24 10:14 ` Arnd Bergmann
2017-08-24 19:19 ` Josh Poimboeuf
0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2017-08-24 10:14 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-24 10:14 ` Arnd Bergmann
@ 2017-08-24 19:19 ` Josh Poimboeuf
2017-08-24 21:14 ` Arnd Bergmann
2017-09-11 14:34 ` Arnd Bergmann
0 siblings, 2 replies; 12+ messages in thread
From: Josh Poimboeuf @ 2017-08-24 19:19 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Linux Kernel Mailing List
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 {
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-24 19:19 ` Josh Poimboeuf
@ 2017-08-24 21:14 ` Arnd Bergmann
2017-09-11 14:34 ` Arnd Bergmann
1 sibling, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2017-08-24 21:14 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 3032 bytes --]
On Thu, Aug 24, 2017 at 9:19 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 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.
attached now.
> 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?
They do major releases once a year, some time in spring, and bugfix
releases inbetween. 8.0.0 is the name of the prerelease snapshots, the
first official gcc-8 release will be 8.1.1.
I'll be on vacation for all of next week, starting tomorrow, so I won't
be able to do more testing for a while with your patch.
Arnd
[-- Attachment #2: smscoreapi.o --]
[-- Type: application/x-object, Size: 292904 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-08-24 19:19 ` Josh Poimboeuf
2017-08-24 21:14 ` Arnd Bergmann
@ 2017-09-11 14:34 ` Arnd Bergmann
2017-09-19 19:52 ` Josh Poimboeuf
1 sibling, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2017-09-11 14:34 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
On Thu, Aug 24, 2017 at 9:19 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 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.
I now got around to testing your patch, and ran into a few remaining
problems:
1. one configuration causing tons of warnings, on most compiler
versions (4.9 and newer), and even a couple of "unreachable
instruction warnings on gcc-4.3.
arch/x86/mm/pageattr.o: warning: objtool: set_memory_x()+0x3a: call
without frame pointer save/setup
security/keys/keyring.o: warning: objtool: keyring_read()+0x70: leave
instruction with modified stack frame
arch/x86/events/intel/pt.o: warning: objtool:
pt_event_addr_filters_sync uses BP as a scratch register
2. one configuration showing two warnings in a file that is compile
with "-O0", reproducible with gcc-4.9 and above (gcc-6 and above show
slightly different results from older versions):
crypto/jitterentropy.o: warning: objtool: jent_stuck()+0xbc: leave
instruction with modified stack frame
crypto/jitterentropy.o: warning: objtool:
jent_entropy_collector_free()+0x64: leave instruction with modified
stack frame
or
crypto/jitterentropy.o: warning: objtool: jent_fold_time()+0x3b: call
without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_stuck()+0x1d: call
without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_unbiased_bit()+0x15:
call without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_read_entropy()+0x32:
call without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool:
jent_entropy_collector_free()+0x19: call without frame pointer
save/setup
3. A few dozen "sibling call from callable instruction with modified
stack frame" warnings in one configuration using gcc-8, e.g.
drivers/infiniband/hw/cxgb4/ev.o: warning: objtool:
c4iw_ev_dispatch()+0xff: sibling call from callable instruction with
modified stack frame
drivers/infiniband/ulp/srpt/ib_srpt.o: warning: objtool:
srpt_rdma_read_done.cold.40()+0xa3: sibling call from callable
instruction with modified stack frame
drivers/isdn/hardware/avm/b1.o: warning: objtool:
b1_parse_version()+0x234: sibling call from callable instruction with
modified stack frame
drivers/isdn/hardware/mISDN/mISDNisar.o: warning: objtool:
isar_fill_fifo()+0x17f: sibling call from callable instruction with
modified stack frame
4. a small number of other warnings in various configurations, also using gcc-8:
fs/btrfs/extent_io.o: warning: objtool: __clear_extent_bit() falls
through to next function endio_readpage_release_extent()
drivers/mtd/nand/nand_base.o: warning: objtool:
nand_scan_tail()+0x25f: sibling call from callable instruction with
modified stack frame
fs/fat/inode.o: warning: objtool: parse_options()+0x26c: unreachable instruction
drivers/media/tuners/tuner-simple.o: warning: objtool:
simple_set_params.cold.20()+0x0: unreachable instruction
drivers/media/tuners/tda8290.o: warning: objtool:
tda829x_attach.cold.18()+0x0: unreachable instruction
drivers/media/tuners/xc5000.o: warning: objtool:
xc5000_set_digital_params.cold.32()+0xe5: unreachable instruction
drivers/mtd/nand/nand_base.o: warning: objtool:
nand_scan_tail()+0x231: sibling call from callable instruction with
modified stack frame
fs/btrfs/extent_io.o: warning: objtool: __clear_extent_bit() falls
through to next function set_extent_bit()
drivers/edac/mce_amd.o: warning: objtool: f16h_mc2_mce()+0x32: sibling
call from callable instruction with modified stack frame
drivers/gpu/drm/radeon/cik.o: warning: objtool:
cik_init_microcode()+0x4d: sibling call from callable instruction with
modified stack frame
drivers/media/dvb-frontends/drxk_hard.o: warning: objtool:
set_qam.constprop.9()+0x778: sibling call from callable instruction
with modified stack frame
drivers/message/fusion/mptbase.o: warning: objtool:
mpt_HardResetHandler() falls through to next function
mpt_Soft_Hard_ResetHandler()
drivers/message/fusion/mptbase.o: warning: objtool:
mpt_Soft_Hard_ResetHandler() falls through to next function
mptbase_sas_persist_operation()
drivers/video/fbdev/tridentfb.o: warning: objtool:
trident_pci_probe.part.9()+0x1e1: sibling call from callable
instruction with modified stack frame
drivers/message/fusion/mptbase.o: warning: objtool:
mpt_Soft_Hard_ResetHandler() falls through to next function
mptbase_sas_persist_operation()
I've uploaded the relevant files for you to
https://drive.google.com/file/d/0B_XQwQ5KlfJAbUtrTVJaRmJIZUk/view?usp=sharing
Arnd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-09-11 14:34 ` Arnd Bergmann
@ 2017-09-19 19:52 ` Josh Poimboeuf
2017-09-19 20:43 ` Arnd Bergmann
0 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-09-19 19:52 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Linux Kernel Mailing List
On Mon, Sep 11, 2017 at 04:34:52PM +0200, Arnd Bergmann wrote:
> On Thu, Aug 24, 2017 at 9:19 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 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.
>
> I now got around to testing your patch, and ran into a few remaining
> problems:
>
> 1. one configuration causing tons of warnings, on most compiler
> versions (4.9 and newer), and even a couple of "unreachable
> instruction warnings on gcc-4.3.
>
> arch/x86/mm/pageattr.o: warning: objtool: set_memory_x()+0x3a: call
> without frame pointer save/setup
> security/keys/keyring.o: warning: objtool: keyring_read()+0x70: leave
> instruction with modified stack frame
> arch/x86/events/intel/pt.o: warning: objtool:
> pt_event_addr_filters_sync uses BP as a scratch register
I downloaded the tarball, and I see configs/logs for the other problems,
but not for this one. Did you forget to attach it, or did I miss it?
--
Josh
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-09-19 19:52 ` Josh Poimboeuf
@ 2017-09-19 20:43 ` Arnd Bergmann
2017-09-19 21:39 ` Josh Poimboeuf
0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2017-09-19 20:43 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
On Tue, Sep 19, 2017 at 9:52 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Mon, Sep 11, 2017 at 04:34:52PM +0200, Arnd Bergmann wrote:
>> On Thu, Aug 24, 2017 at 9:19 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> 1. one configuration causing tons of warnings, on most compiler
>> versions (4.9 and newer), and even a couple of "unreachable
>> instruction warnings on gcc-4.3.
>>
>> arch/x86/mm/pageattr.o: warning: objtool: set_memory_x()+0x3a: call
>> without frame pointer save/setup
>> security/keys/keyring.o: warning: objtool: keyring_read()+0x70: leave
>> instruction with modified stack frame
>> arch/x86/events/intel/pt.o: warning: objtool:
>> pt_event_addr_filters_sync uses BP as a scratch register
>
> I downloaded the tarball, and I see configs/logs for the other problems,
> but not for this one. Did you forget to attach it, or did I miss it?
I think what I ended up doing was to take the smallest files for that
configuration and packed them up, but the warnings I listed in the
mail don't match the ones for those files. However, it seems the
0xEBFDB964_defconfig/log file got corrupted.
I've uploaded a new copy to http://paste.ubuntu.com/25574959/ now.
I've also experimented with other compiler versions
in the meantime and ran into similar problems on those version but did
not bother to take detailed notes about them since they are likely just
variants of the ones I already reported. If you can work around the
warnings I reported here, I can rerun with all compilers I have to
see if anything else shows up.
Arnd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-09-19 20:43 ` Arnd Bergmann
@ 2017-09-19 21:39 ` Josh Poimboeuf
2017-09-21 13:27 ` Arnd Bergmann
0 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-09-19 21:39 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Linux Kernel Mailing List
On Tue, Sep 19, 2017 at 10:43:31PM +0200, Arnd Bergmann wrote:
> On Tue, Sep 19, 2017 at 9:52 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Mon, Sep 11, 2017 at 04:34:52PM +0200, Arnd Bergmann wrote:
> >> On Thu, Aug 24, 2017 at 9:19 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >> 1. one configuration causing tons of warnings, on most compiler
> >> versions (4.9 and newer), and even a couple of "unreachable
> >> instruction warnings on gcc-4.3.
> >>
> >> arch/x86/mm/pageattr.o: warning: objtool: set_memory_x()+0x3a: call
> >> without frame pointer save/setup
> >> security/keys/keyring.o: warning: objtool: keyring_read()+0x70: leave
> >> instruction with modified stack frame
> >> arch/x86/events/intel/pt.o: warning: objtool:
> >> pt_event_addr_filters_sync uses BP as a scratch register
> >
> > I downloaded the tarball, and I see configs/logs for the other problems,
> > but not for this one. Did you forget to attach it, or did I miss it?
>
> I think what I ended up doing was to take the smallest files for that
> configuration and packed them up, but the warnings I listed in the
> mail don't match the ones for those files. However, it seems the
> 0xEBFDB964_defconfig/log file got corrupted.
>
> I've uploaded a new copy to http://paste.ubuntu.com/25574959/ now.
>
> I've also experimented with other compiler versions
> in the meantime and ran into similar problems on those version but did
> not bother to take detailed notes about them since they are likely just
> variants of the ones I already reported. If you can work around the
> warnings I reported here, I can rerun with all compilers I have to
> see if anything else shows up.
Here's the fix for #1 and #2. I still need to work on the GCC 8
warnings.
-----
From: Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH] objtool: Support unoptimized frame pointer setup
Arnd Bergmann reported a bunch of warnings like:
crypto/jitterentropy.o: warning: objtool: jent_fold_time()+0x3b: call without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_stuck()+0x1d: call without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_unbiased_bit()+0x15: call without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_read_entropy()+0x32: call without frame pointer save/setup
crypto/jitterentropy.o: warning: objtool: jent_entropy_collector_free()+0x19: call without frame pointer save/setup
and
arch/x86/events/core.o: warning: objtool: collect_events uses BP as a scratch register
arch/x86/events/core.o: warning: objtool: events_ht_sysfs_show()+0x22: call without frame pointer save/setup
With certain rare configurations, GCC sometimes sets up the frame
pointer with:
lea (%rsp),%rbp
instead of:
mov %rsp,%rbp
The instructions are equivalent, so treat the former like the latter.
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
tools/objtool/arch/x86/decode.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 0e8c8ec4fd4e..97a1fdb3db63 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -284,11 +284,16 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
case 0x8d:
if (sib == 0x24 && rex_w && !rex_b && !rex_x) {
- /* lea disp(%rsp), reg */
*type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
+ if (!insn.displacement.value) {
+ /* lea (%rsp), reg */
+ op->src.type = OP_SRC_REG;
+ } else {
+ /* lea disp(%rsp), reg */
+ op->src.type = OP_SRC_ADD;
+ op->src.offset = insn.displacement.value;
+ }
op->src.reg = CFI_SP;
- op->src.offset = insn.displacement.value;
op->dest.type = OP_DEST_REG;
op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
--
2.13.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: gcc-8 objtool warnings
2017-09-19 21:39 ` Josh Poimboeuf
@ 2017-09-21 13:27 ` Arnd Bergmann
0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2017-09-21 13:27 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Linux Kernel Mailing List
On Tue, Sep 19, 2017 at 11:39 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Tue, Sep 19, 2017 at 10:43:31PM +0200, Arnd Bergmann wrote:
> -----
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] objtool: Support unoptimized frame pointer setup
>
> Arnd Bergmann reported a bunch of warnings like:
>
> crypto/jitterentropy.o: warning: objtool: jent_fold_time()+0x3b: call without frame pointer save/setup
> crypto/jitterentropy.o: warning: objtool: jent_stuck()+0x1d: call without frame pointer save/setup
> crypto/jitterentropy.o: warning: objtool: jent_unbiased_bit()+0x15: call without frame pointer save/setup
> crypto/jitterentropy.o: warning: objtool: jent_read_entropy()+0x32: call without frame pointer save/setup
> crypto/jitterentropy.o: warning: objtool: jent_entropy_collector_free()+0x19: call without frame pointer save/setup
>
> and
>
> arch/x86/events/core.o: warning: objtool: collect_events uses BP as a scratch register
> arch/x86/events/core.o: warning: objtool: events_ht_sysfs_show()+0x22: call without frame pointer save/setup
>
> With certain rare configurations, GCC sometimes sets up the frame
> pointer with:
>
> lea (%rsp),%rbp
>
> instead of:
>
> mov %rsp,%rbp
>
> The instructions are equivalent, so treat the former like the latter.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Confirmed, this fixes many configurations that had warnings before,
currently testing with gcc-6.3.
Arnd
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-09-21 13:27 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-23 12:22 gcc-8 objtool warnings Arnd Bergmann
2017-08-23 12:48 ` Josh Poimboeuf
2017-08-23 13:38 ` Arnd Bergmann
2017-08-23 16:01 ` Josh Poimboeuf
2017-08-24 10:14 ` Arnd Bergmann
2017-08-24 19:19 ` Josh Poimboeuf
2017-08-24 21:14 ` Arnd Bergmann
2017-09-11 14:34 ` Arnd Bergmann
2017-09-19 19:52 ` Josh Poimboeuf
2017-09-19 20:43 ` Arnd Bergmann
2017-09-19 21:39 ` Josh Poimboeuf
2017-09-21 13:27 ` Arnd Bergmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.