* [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings @ 2016-10-10 12:56 Arnd Bergmann 2016-10-10 20:23 ` Josh Poimboeuf 2016-10-11 1:53 ` [PATCH] objtool: support '-mtune=atom' stack frame setup instruction Josh Poimboeuf 0 siblings, 2 replies; 36+ messages in thread From: Arnd Bergmann @ 2016-10-10 12:56 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86 Cc: Arnd Bergmann, Josh Poimboeuf, linux-kernel I have no idea what is actually going on here, but building an x86 kernel with CONFIG_MATOM results in countless warnings from objtool, such as arch/x86/events/intel/ds.o: warning: objtool: intel_pmu_pebs_del()+0x43: call without frame pointer save/setup security/keys/keyring.o: warning: objtool: keyring_read()+0x59: call without frame pointer save/setup kernel/signal.o: warning: objtool: __dequeue_signal()+0xd8: call without frame pointer save/setup kernel/signal.o: warning: objtool: kill_pid()+0x15: call without frame pointer save/setup kernel/signal.o: warning: objtool: SyS_signal()+0x27: call without frame pointer save/setup mm/page_alloc.o: warning: objtool: zone_watermark_ok_safe()+0x27: call without frame pointer save/setup fs/exec.o: warning: objtool: read_code()+0x18: call without frame pointer save/setup mm/swap.o: warning: objtool: get_kernel_page()+0x24: call without frame pointer save/setup mm/swap.o: warning: objtool: pagevec_move_tail.constprop.25()+0x26: call without frame pointer save/setup block/bio.o: warning: objtool: bio_map_kern()+0x47: call without frame pointer save/setup arch/x86/crypto/poly1305_glue.o: warning: objtool: poly1305_simd_mult()+0x2d: call without frame pointer save/setup crypto/skcipher.o: warning: objtool: skcipher_encrypt_ablkcipher()+0x58: call without frame pointer save/setup crypto/skcipher.o: warning: objtool: skcipher_decrypt_ablkcipher()+0x58: call without frame pointer save/setup fs/inode.o: warning: objtool: ilookup()+0x5d: call without frame pointer save/setup fs/inode.o: warning: objtool: proc_nr_inodes()+0x3e: call without frame pointer save/setup fs/namei.o: warning: objtool: lookup_one_len_unlocked()+0x21: call without frame pointer save/setup block/elevator.o: warning: objtool: elv_rb_add()+0x5b: call without frame pointer save/setup crypto/shash.o: warning: objtool: shash_async_init()+0x1e: call without frame pointer save/setup crypto/shash.o: warning: objtool: shash_async_import()+0x1e: call without frame pointer save/setup mm/vmscan.o: warning: objtool: pfmemalloc_watermark_ok()+0xb9: call without frame pointer save/setup I have not looked at whether this is a bug in gcc or in objtool, however I found that not using -mtune=atom reliably avoids the problem. I could reproduce the problem with gcc versions 4.7 through 6.1. Cc: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/x86/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 2d449337a360..e1dfb37d66ad 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -110,7 +110,7 @@ else cflags-$(CONFIG_MCORE2) += \ $(call cc-option,-march=core2,$(call cc-option,-mtune=generic)) cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \ - $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic)) + $(call cc-option,-mtune=generic) cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic) KBUILD_CFLAGS += $(cflags-y) -- 2.9.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-10 12:56 [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann @ 2016-10-10 20:23 ` Josh Poimboeuf 2016-10-11 8:08 ` Arnd Bergmann 2016-10-11 1:53 ` [PATCH] objtool: support '-mtune=atom' stack frame setup instruction Josh Poimboeuf 1 sibling, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-10 20:23 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel On Mon, Oct 10, 2016 at 02:56:56PM +0200, Arnd Bergmann wrote: > I have no idea what is actually going on here, but building an x86 kernel > with CONFIG_MATOM results in countless warnings from objtool, such as > > arch/x86/events/intel/ds.o: warning: objtool: intel_pmu_pebs_del()+0x43: call without frame pointer save/setup > security/keys/keyring.o: warning: objtool: keyring_read()+0x59: call without frame pointer save/setup > kernel/signal.o: warning: objtool: __dequeue_signal()+0xd8: call without frame pointer save/setup > kernel/signal.o: warning: objtool: kill_pid()+0x15: call without frame pointer save/setup > kernel/signal.o: warning: objtool: SyS_signal()+0x27: call without frame pointer save/setup > mm/page_alloc.o: warning: objtool: zone_watermark_ok_safe()+0x27: call without frame pointer save/setup > fs/exec.o: warning: objtool: read_code()+0x18: call without frame pointer save/setup > mm/swap.o: warning: objtool: get_kernel_page()+0x24: call without frame pointer save/setup > mm/swap.o: warning: objtool: pagevec_move_tail.constprop.25()+0x26: call without frame pointer save/setup > block/bio.o: warning: objtool: bio_map_kern()+0x47: call without frame pointer save/setup > arch/x86/crypto/poly1305_glue.o: warning: objtool: poly1305_simd_mult()+0x2d: call without frame pointer save/setup > crypto/skcipher.o: warning: objtool: skcipher_encrypt_ablkcipher()+0x58: call without frame pointer save/setup > crypto/skcipher.o: warning: objtool: skcipher_decrypt_ablkcipher()+0x58: call without frame pointer save/setup > fs/inode.o: warning: objtool: ilookup()+0x5d: call without frame pointer save/setup > fs/inode.o: warning: objtool: proc_nr_inodes()+0x3e: call without frame pointer save/setup > fs/namei.o: warning: objtool: lookup_one_len_unlocked()+0x21: call without frame pointer save/setup > block/elevator.o: warning: objtool: elv_rb_add()+0x5b: call without frame pointer save/setup > crypto/shash.o: warning: objtool: shash_async_init()+0x1e: call without frame pointer save/setup > crypto/shash.o: warning: objtool: shash_async_import()+0x1e: call without frame pointer save/setup > mm/vmscan.o: warning: objtool: pfmemalloc_watermark_ok()+0xb9: call without frame pointer save/setup > > I have not looked at whether this is a bug in gcc or in objtool, however > I found that not using -mtune=atom reliably avoids the problem. I could > reproduce the problem with gcc versions 4.7 through 6.1. Thanks for reporting it. It looks like 'mtune=atom' sometimes makes a slight change to one of the stack frame setup instructions. Instead of: move rsp, rbp It sometimes does: lea (%rsp),%rbp They're two different instructions, but they have the same result. It's an easy fix for objtool. I'll post a patch soon. -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-10 20:23 ` Josh Poimboeuf @ 2016-10-11 8:08 ` Arnd Bergmann 2016-10-11 12:20 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2016-10-11 8:08 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel On Monday, October 10, 2016 3:23:22 PM CEST Josh Poimboeuf wrote: > > Thanks for reporting it. It looks like 'mtune=atom' sometimes makes a > slight change to one of the stack frame setup instructions. Instead of: > > move rsp, rbp > > It sometimes does: > > lea (%rsp),%rbp > > They're two different instructions, but they have the same result. It's > an easy fix for objtool. I'll post a patch soon. > > Ah, good to hear. I've replaced my patch with yours in my randconfig tests now and will let you know if there are any other warnings on atom. I've done a few thousand x86 randconfig builds now and done private patches for all warnings I got (I previously had fixes only for the arm warnings). I found objtool warnings for a few files in some configurations that do not involve -mtune=atom, maybe you can also look at what is going on there as I have no idea for how to address them: drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x2f3: sibling call from callable instruction with changed frame pointer drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0x10f: sibling call from callable instruction with changed frame pointer kernel/locking/rwsem.o: warning: objtool: down_write_killable()+0x16: call without frame pointer save/setup I can provide additional information for reproducing them if it's not immediately obvious what the problems are. Thanks, Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 8:08 ` Arnd Bergmann @ 2016-10-11 12:20 ` Josh Poimboeuf 2016-10-11 13:30 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-11 12:20 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel On Tue, Oct 11, 2016 at 10:08:12AM +0200, Arnd Bergmann wrote: > On Monday, October 10, 2016 3:23:22 PM CEST Josh Poimboeuf wrote: > > > > Thanks for reporting it. It looks like 'mtune=atom' sometimes makes a > > slight change to one of the stack frame setup instructions. Instead of: > > > > move rsp, rbp > > > > It sometimes does: > > > > lea (%rsp),%rbp > > > > They're two different instructions, but they have the same result. It's > > an easy fix for objtool. I'll post a patch soon. > > > > > > Ah, good to hear. I've replaced my patch with yours in my randconfig > tests now and will let you know if there are any other warnings on > atom. I've done a few thousand x86 randconfig builds now and done private > patches for all warnings I got (I previously had fixes only for the arm > warnings). I found objtool warnings for a few files in some configurations > that do not involve -mtune=atom, maybe you can also look at what > is going on there as I have no idea for how to address them: > > drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() > drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section > drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x2f3: sibling call from callable instruction with changed frame pointer > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0x10f: sibling call from callable instruction with changed frame pointer > kernel/locking/rwsem.o: warning: objtool: down_write_killable()+0x16: call without frame pointer save/setup > > I can provide additional information for reproducing them if it's > not immediately obvious what the problems are. I'm really surprised the 0-day bot didn't find these. I was under the impression that it continuously did a bunch of randconfigs. Anyway, if you could send the configs for the warnings, that would be very helpful. I also happen to be working on a significant rewrite of objtool and these configs will come in handy for making a regression suite. -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 12:20 ` Josh Poimboeuf @ 2016-10-11 13:30 ` Arnd Bergmann 2016-10-11 15:05 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2016-10-11 13:30 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2597 bytes --] On Tuesday, October 11, 2016 7:20:49 AM CEST Josh Poimboeuf wrote: > On Tue, Oct 11, 2016 at 10:08:12AM +0200, Arnd Bergmann wrote: > > On Monday, October 10, 2016 3:23:22 PM CEST Josh Poimboeuf wrote: > > > > > > Thanks for reporting it. It looks like 'mtune=atom' sometimes makes a > > > slight change to one of the stack frame setup instructions. Instead of: > > > > > > move rsp, rbp > > > > > > It sometimes does: > > > > > > lea (%rsp),%rbp > > > > > > They're two different instructions, but they have the same result. It's > > > an easy fix for objtool. I'll post a patch soon. > > > > > > > > > > Ah, good to hear. I've replaced my patch with yours in my randconfig > > tests now and will let you know if there are any other warnings on > > atom. I've done a few thousand x86 randconfig builds now and done private > > patches for all warnings I got (I previously had fixes only for the arm > > warnings). I found objtool warnings for a few files in some configurations > > that do not involve -mtune=atom, maybe you can also look at what > > is going on there as I have no idea for how to address them: > > > > drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() > > drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section > > drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x2f3: sibling call from callable instruction with changed frame pointer > > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0x10f: sibling call from callable instruction with changed frame pointer > > kernel/locking/rwsem.o: warning: objtool: down_write_killable()+0x16: call without frame pointer save/setup > > > > I can provide additional information for reproducing them if it's > > not immediately obvious what the problems are. > > I'm really surprised the 0-day bot didn't find these. I was under the > impression that it continuously did a bunch of randconfigs. > > Anyway, if you could send the configs for the warnings, that would be > very helpful. > > I also happen to be working on a significant rewrite of objtool and > these configs will come in handy for making a regression suite. I've attached the three .config files here, but due to the size I don't know if they make it to the list or your inbox. Let me know if you get them, and if you are able to reproduce the problem. The compiler version I used is gcc-6 (Ubuntu 6.2.0-3ubuntu11~16.04) 6.2.0 20160901, and this is on top of linux-next plus a few other patches. Arnd [-- Attachment #2: 0x3A1DA440-config.zip --] [-- Type: application/zip, Size: 31860 bytes --] [-- Attachment #3: 0x364C8CDB-config.zip --] [-- Type: application/zip, Size: 32190 bytes --] [-- Attachment #4: 0xFC244C03-config.zip --] [-- Type: application/zip, Size: 31679 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 13:30 ` Arnd Bergmann @ 2016-10-11 15:05 ` Josh Poimboeuf 2016-10-11 15:51 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-11 15:05 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel On Tue, Oct 11, 2016 at 03:30:20PM +0200, Arnd Bergmann wrote: > I've attached the three .config files here, but due to the size I > don't know if they make it to the list or your inbox. Let me > know if you get them, and if you are able to reproduce the problem. > > The compiler version I used is gcc-6 (Ubuntu 6.2.0-3ubuntu11~16.04) > 6.2.0 20160901, and this is on top of linux-next plus a few other > patches. Thanks, I got the configs, and I do see the warnings. Will investigate... -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 15:05 ` Josh Poimboeuf @ 2016-10-11 15:51 ` Josh Poimboeuf 2016-10-11 20:38 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-11 15:51 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Denys Vlasenko (spoiler alert: another bad gcc bug which is truncating functions...) On Tue, Oct 11, 2016 at 10:05:41AM -0500, Josh Poimboeuf wrote: > On Tue, Oct 11, 2016 at 03:30:20PM +0200, Arnd Bergmann wrote: > > I've attached the three .config files here, but due to the size I > > don't know if they make it to the list or your inbox. Let me > > know if you get them, and if you are able to reproduce the problem. > > > > The compiler version I used is gcc-6 (Ubuntu 6.2.0-3ubuntu11~16.04) > > 6.2.0 20160901, and this is on top of linux-next plus a few other > > patches. > > Thanks, I got the configs, and I do see the warnings. Will > investigate... 1) 0x364C8CDB-config: kernel/locking/rwsem.o: warning: objtool: down_write_killable()+0x16: call without frame pointer save/setup This is a bug in kernel code in the ____down_write() macro. It doesn't ensure there's a stack frame before the call instruction. Easy fix. 2) 0x3A1DA440-config: drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x2f4: sibling call from callable instruction with changed frame pointer drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0x10f: sibling call from callable instruction with changed frame pointer These are false positive warnings, caused by the bane of objtool's existence, gcc switch statement jump tables. objtool needs to be made a little smarter. 3) 0xFC244C03-config: drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section These look like another bad gcc bug which is truncating functions: 0000000000000940 <snic_log_q_error>: 940: 55 push %rbp 941: 48 89 e5 mov %rsp,%rbp 944: 53 push %rbx 945: 48 89 fb mov %rdi,%rbx 948: e8 00 00 00 00 callq 94d <snic_log_q_error+0xd> 949: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 94d: 8b 83 58 02 00 00 mov 0x258(%rbx),%eax 953: 85 c0 test %eax,%eax 955: 75 08 jne 95f <snic_log_q_error+0x1f> 957: e8 00 00 00 00 callq 95c <snic_log_q_error+0x1c> 958: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 95c: 5b pop %rbx 95d: 5d pop %rbp 95e: c3 retq 95f: e8 00 00 00 00 callq 964 <snic_log_q_error+0x24> 960: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 964: 48 8b 83 10 1c 00 00 mov 0x1c10(%rbx),%rax 96b: 48 8d 78 50 lea 0x50(%rax),%rdi 96f: e8 00 00 00 00 callq 974 <snic_log_q_error+0x34> 970: R_X86_64_PC32 ioread32-0x4 974: 83 bb 58 02 00 00 01 cmpl $0x1,0x258(%rbx) 97b: 76 da jbe 957 <snic_log_q_error+0x17> 97d: e8 00 00 00 00 callq 982 <snic_log_q_error+0x42> 97e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 [end of file] Notice how it just falls off the end of the function. We had a similar bug before: https://lkml.kernel.org/r/20160413033649.7r3msnmo3trtq47z@treble https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 I'm not sure yet if this is the same gcc bug or a different one. Maybe it's related to the new GCC_PLUGIN_SANCOV? -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 15:51 ` Josh Poimboeuf @ 2016-10-11 20:38 ` Arnd Bergmann 2016-10-12 13:01 ` Josh Poimboeuf ` (2 more replies) 0 siblings, 3 replies; 36+ messages in thread From: Arnd Bergmann @ 2016-10-11 20:38 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Denys Vlasenko On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: > > 3) 0xFC244C03-config: > drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() > drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section > > These look like another bad gcc bug which is truncating functions: Same bug for both of them? > > 0000000000000940 <snic_log_q_error>: > 940: 55 push %rbp > 941: 48 89 e5 mov %rsp,%rbp > 944: 53 push %rbx > 945: 48 89 fb mov %rdi,%rbx > 948: e8 00 00 00 00 callq 94d <snic_log_q_error+0xd> > 949: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 94d: 8b 83 58 02 00 00 mov 0x258(%rbx),%eax > 953: 85 c0 test %eax,%eax > 955: 75 08 jne 95f <snic_log_q_error+0x1f> > 957: e8 00 00 00 00 callq 95c <snic_log_q_error+0x1c> > 958: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 95c: 5b pop %rbx > 95d: 5d pop %rbp > 95e: c3 retq > 95f: e8 00 00 00 00 callq 964 <snic_log_q_error+0x24> > 960: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 964: 48 8b 83 10 1c 00 00 mov 0x1c10(%rbx),%rax > 96b: 48 8d 78 50 lea 0x50(%rax),%rdi > 96f: e8 00 00 00 00 callq 974 <snic_log_q_error+0x34> > 970: R_X86_64_PC32 ioread32-0x4 > 974: 83 bb 58 02 00 00 01 cmpl $0x1,0x258(%rbx) > 97b: 76 da jbe 957 <snic_log_q_error+0x17> > 97d: e8 00 00 00 00 callq 982 <snic_log_q_error+0x42> > 97e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > [end of file] > > Notice how it just falls off the end of the function. We had a similar > bug before: > > https://lkml.kernel.org/r/20160413033649.7r3msnmo3trtq47z@treble I remember that nightmare :( > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 > > I'm not sure yet if this is the same gcc bug or a different one. Maybe > it's related to the new GCC_PLUGIN_SANCOV? I've reduced one of the test cases to this now: /* gcc-6 -O2 -fno-strict-aliasing -fno-reorder-blocks -fno-omit-frame-pointer -Wno-pointer-sign -fsanitize-coverage=trace-pc -Wall -Werror -c snic_res.c -o snic_res.o */ typedef int spinlock_t; extern unsigned int ioread32(void *); struct vnic_wq_ctrl { unsigned int error_status; }; struct vnic_wq { struct vnic_wq_ctrl *ctrl; } mempool_t; struct snic { unsigned int wq_count; __attribute__ ((__aligned__)) struct vnic_wq wq[1]; spinlock_t wq_lock[1]; }; unsigned int snic_log_q_error_err_status; void snic_log_q_error(struct snic *snic) { unsigned int i; for (i = 0; i < snic->wq_count; i++) snic_log_q_error_err_status = ioread32(&snic->wq[i].ctrl->error_status); } which gets compiled into 0000000000000000 <snic_log_q_error>: 0: 55 push %rbp 1: 48 89 e5 mov %rsp,%rbp 4: 53 push %rbx 5: 48 89 fb mov %rdi,%rbx 8: 48 83 ec 08 sub $0x8,%rsp c: e8 00 00 00 00 callq 11 <snic_log_q_error+0x11> d: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 11: 8b 03 mov (%rbx),%eax 13: 85 c0 test %eax,%eax 15: 75 11 jne 28 <snic_log_q_error+0x28> 17: 48 83 c4 08 add $0x8,%rsp 1b: 5b pop %rbx 1c: 5d pop %rbp 1d: e9 00 00 00 00 jmpq 22 <snic_log_q_error+0x22> 1e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) 28: e8 00 00 00 00 callq 2d <snic_log_q_error+0x2d> 29: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 2d: 48 8b 7b 10 mov 0x10(%rbx),%rdi 31: e8 00 00 00 00 callq 36 <snic_log_q_error+0x36> 32: R_X86_64_PC32 ioread32-0x4 36: 89 05 00 00 00 00 mov %eax,0x0(%rip) # 3c <snic_log_q_error+0x3c> 38: R_X86_64_PC32 snic_log_q_error_err_status-0x4 3c: 83 3b 01 cmpl $0x1,(%rbx) 3f: 76 d6 jbe 17 <snic_log_q_error+0x17> 41: e8 00 00 00 00 callq 46 <snic_log_q_error+0x46> 42: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 20:38 ` Arnd Bergmann @ 2016-10-12 13:01 ` Josh Poimboeuf 2016-10-13 12:46 ` Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) Josh Poimboeuf 2017-03-01 9:34 ` [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann 2 siblings, 0 replies; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-12 13:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Denys Vlasenko On Tue, Oct 11, 2016 at 10:38:42PM +0200, Arnd Bergmann wrote: > I've reduced one of the test cases to this now: > > /* gcc-6 -O2 -fno-strict-aliasing -fno-reorder-blocks -fno-omit-frame-pointer -Wno-pointer-sign -fsanitize-coverage=trace-pc -Wall -Werror -c snic_res.c -o snic_res.o */ > typedef int spinlock_t; > extern unsigned int ioread32(void *); > struct vnic_wq_ctrl { > unsigned int error_status; > }; > struct vnic_wq { > struct vnic_wq_ctrl *ctrl; > } mempool_t; > struct snic { > unsigned int wq_count; > __attribute__ ((__aligned__)) struct vnic_wq wq[1]; > spinlock_t wq_lock[1]; > }; > unsigned int snic_log_q_error_err_status; > void snic_log_q_error(struct snic *snic) > { > unsigned int i; > for (i = 0; i < snic->wq_count; i++) > snic_log_q_error_err_status = > ioread32(&snic->wq[i].ctrl->error_status); > } > > which gets compiled into > > 0000000000000000 <snic_log_q_error>: > 0: 55 push %rbp > 1: 48 89 e5 mov %rsp,%rbp > 4: 53 push %rbx > 5: 48 89 fb mov %rdi,%rbx > 8: 48 83 ec 08 sub $0x8,%rsp > c: e8 00 00 00 00 callq 11 <snic_log_q_error+0x11> > d: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 11: 8b 03 mov (%rbx),%eax > 13: 85 c0 test %eax,%eax > 15: 75 11 jne 28 <snic_log_q_error+0x28> > 17: 48 83 c4 08 add $0x8,%rsp > 1b: 5b pop %rbx > 1c: 5d pop %rbp > 1d: e9 00 00 00 00 jmpq 22 <snic_log_q_error+0x22> > 1e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) > 28: e8 00 00 00 00 callq 2d <snic_log_q_error+0x2d> > 29: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 2d: 48 8b 7b 10 mov 0x10(%rbx),%rdi > 31: e8 00 00 00 00 callq 36 <snic_log_q_error+0x36> > 32: R_X86_64_PC32 ioread32-0x4 > 36: 89 05 00 00 00 00 mov %eax,0x0(%rip) # 3c <snic_log_q_error+0x3c> > 38: R_X86_64_PC32 snic_log_q_error_err_status-0x4 > 3c: 83 3b 01 cmpl $0x1,(%rbx) > 3f: 76 d6 jbe 17 <snic_log_q_error+0x17> > 41: e8 00 00 00 00 callq 46 <snic_log_q_error+0x46> > 42: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > Thanks! I'll open a gcc bug. -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) 2016-10-11 20:38 ` Arnd Bergmann 2016-10-12 13:01 ` Josh Poimboeuf @ 2016-10-13 12:46 ` Josh Poimboeuf 2016-10-13 17:57 ` Denys Vlasenko 2017-03-01 9:34 ` [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann 2 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-13 12:46 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Denys Vlasenko, Emese Revfy On Tue, Oct 11, 2016 at 10:38:42PM +0200, Arnd Bergmann wrote: > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: > > Notice how it just falls off the end of the function. We had a similar > > bug before: > > > > https://lkml.kernel.org/r/20160413033649.7r3msnmo3trtq47z@treble > > I remember that nightmare :( > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 > > > > I'm not sure yet if this is the same gcc bug or a different one. Maybe > > it's related to the new GCC_PLUGIN_SANCOV? > > I've reduced one of the test cases to this now: > > /* gcc-6 -O2 -fno-strict-aliasing -fno-reorder-blocks -fno-omit-frame-pointer -Wno-pointer-sign -fsanitize-coverage=trace-pc -Wall -Werror -c snic_res.c -o snic_res.o */ > typedef int spinlock_t; > extern unsigned int ioread32(void *); > struct vnic_wq_ctrl { > unsigned int error_status; > }; > struct vnic_wq { > struct vnic_wq_ctrl *ctrl; > } mempool_t; > struct snic { > unsigned int wq_count; > __attribute__ ((__aligned__)) struct vnic_wq wq[1]; > spinlock_t wq_lock[1]; > }; > unsigned int snic_log_q_error_err_status; > void snic_log_q_error(struct snic *snic) > { > unsigned int i; > for (i = 0; i < snic->wq_count; i++) > snic_log_q_error_err_status = > ioread32(&snic->wq[i].ctrl->error_status); > } > > which gets compiled into > > 0000000000000000 <snic_log_q_error>: > 0: 55 push %rbp > 1: 48 89 e5 mov %rsp,%rbp > 4: 53 push %rbx > 5: 48 89 fb mov %rdi,%rbx > 8: 48 83 ec 08 sub $0x8,%rsp > c: e8 00 00 00 00 callq 11 <snic_log_q_error+0x11> > d: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 11: 8b 03 mov (%rbx),%eax > 13: 85 c0 test %eax,%eax > 15: 75 11 jne 28 <snic_log_q_error+0x28> > 17: 48 83 c4 08 add $0x8,%rsp > 1b: 5b pop %rbx > 1c: 5d pop %rbp > 1d: e9 00 00 00 00 jmpq 22 <snic_log_q_error+0x22> > 1e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) > 28: e8 00 00 00 00 callq 2d <snic_log_q_error+0x2d> > 29: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > 2d: 48 8b 7b 10 mov 0x10(%rbx),%rdi > 31: e8 00 00 00 00 callq 36 <snic_log_q_error+0x36> > 32: R_X86_64_PC32 ioread32-0x4 > 36: 89 05 00 00 00 00 mov %eax,0x0(%rip) # 3c <snic_log_q_error+0x3c> > 38: R_X86_64_PC32 snic_log_q_error_err_status-0x4 > 3c: 83 3b 01 cmpl $0x1,(%rbx) > 3f: 76 d6 jbe 17 <snic_log_q_error+0x17> > 41: e8 00 00 00 00 callq 46 <snic_log_q_error+0x46> > 42: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 I opened a bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966 -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) 2016-10-13 12:46 ` Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) Josh Poimboeuf @ 2016-10-13 17:57 ` Denys Vlasenko 2016-10-13 20:15 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Denys Vlasenko @ 2016-10-13 17:57 UTC (permalink / raw) To: Josh Poimboeuf, Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Emese Revfy On 10/13/2016 02:46 PM, Josh Poimboeuf wrote: > On Tue, Oct 11, 2016 at 10:38:42PM +0200, Arnd Bergmann wrote: >> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: >>> Notice how it just falls off the end of the function. We had a similar >>> bug before: >>> >>> https://lkml.kernel.org/r/20160413033649.7r3msnmo3trtq47z@treble >> >> I remember that nightmare :( >> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 >>> >>> I'm not sure yet if this is the same gcc bug or a different one. Maybe >>> it's related to the new GCC_PLUGIN_SANCOV? >> >> I've reduced one of the test cases to this now: >> >> /* gcc-6 -O2 -fno-strict-aliasing -fno-reorder-blocks -fno-omit-frame-pointer -Wno-pointer-sign -fsanitize-coverage=trace-pc -Wall -Werror -c snic_res.c -o snic_res.o */ >> typedef int spinlock_t; >> extern unsigned int ioread32(void *); >> struct vnic_wq_ctrl { >> unsigned int error_status; >> }; >> struct vnic_wq { >> struct vnic_wq_ctrl *ctrl; >> } mempool_t; >> struct snic { >> unsigned int wq_count; >> __attribute__ ((__aligned__)) struct vnic_wq wq[1]; >> spinlock_t wq_lock[1]; >> }; >> unsigned int snic_log_q_error_err_status; >> void snic_log_q_error(struct snic *snic) >> { >> unsigned int i; >> for (i = 0; i < snic->wq_count; i++) >> snic_log_q_error_err_status = >> ioread32(&snic->wq[i].ctrl->error_status); >> } >> >> which gets compiled into >> >> 0000000000000000 <snic_log_q_error>: >> 0: 55 push %rbp >> 1: 48 89 e5 mov %rsp,%rbp >> 4: 53 push %rbx >> 5: 48 89 fb mov %rdi,%rbx >> 8: 48 83 ec 08 sub $0x8,%rsp >> c: e8 00 00 00 00 callq 11 <snic_log_q_error+0x11> >> d: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 >> 11: 8b 03 mov (%rbx),%eax >> 13: 85 c0 test %eax,%eax >> 15: 75 11 jne 28 <snic_log_q_error+0x28> >> 17: 48 83 c4 08 add $0x8,%rsp >> 1b: 5b pop %rbx >> 1c: 5d pop %rbp >> 1d: e9 00 00 00 00 jmpq 22 <snic_log_q_error+0x22> >> 1e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 >> 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) >> 28: e8 00 00 00 00 callq 2d <snic_log_q_error+0x2d> >> 29: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 >> 2d: 48 8b 7b 10 mov 0x10(%rbx),%rdi >> 31: e8 00 00 00 00 callq 36 <snic_log_q_error+0x36> >> 32: R_X86_64_PC32 ioread32-0x4 >> 36: 89 05 00 00 00 00 mov %eax,0x0(%rip) # 3c <snic_log_q_error+0x3c> >> 38: R_X86_64_PC32 snic_log_q_error_err_status-0x4 >> 3c: 83 3b 01 cmpl $0x1,(%rbx) >> 3f: 76 d6 jbe 17 <snic_log_q_error+0x17> >> 41: e8 00 00 00 00 callq 46 <snic_log_q_error+0x46> >> 42: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > I opened a bug: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966 > Surprisingly, it's really "not a bug". The only way you can end up in this branch is if you have a bug and run off the end of wq[1] array member: i.e. if snic->wq_count >= 2. (See gcc BZ for smaller example) It's debatable whether it's okay for gcc to just let buggy code to run off and execute something random. It is surely surprising, and not debug-friendly. An option to emit a crashing instruction (HLT, INT3, that sort of thing) instead of just stopping code generation might be useful. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) 2016-10-13 17:57 ` Denys Vlasenko @ 2016-10-13 20:15 ` Josh Poimboeuf 0 siblings, 0 replies; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-13 20:15 UTC (permalink / raw) To: Denys Vlasenko Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel, Emese Revfy On Thu, Oct 13, 2016 at 07:57:41PM +0200, Denys Vlasenko wrote: > On 10/13/2016 02:46 PM, Josh Poimboeuf wrote: > > On Tue, Oct 11, 2016 at 10:38:42PM +0200, Arnd Bergmann wrote: > > > 0000000000000000 <snic_log_q_error>: > > > 0: 55 push %rbp > > > 1: 48 89 e5 mov %rsp,%rbp > > > 4: 53 push %rbx > > > 5: 48 89 fb mov %rdi,%rbx > > > 8: 48 83 ec 08 sub $0x8,%rsp > > > c: e8 00 00 00 00 callq 11 <snic_log_q_error+0x11> > > > d: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > > 11: 8b 03 mov (%rbx),%eax > > > 13: 85 c0 test %eax,%eax > > > 15: 75 11 jne 28 <snic_log_q_error+0x28> > > > 17: 48 83 c4 08 add $0x8,%rsp > > > 1b: 5b pop %rbx > > > 1c: 5d pop %rbp > > > 1d: e9 00 00 00 00 jmpq 22 <snic_log_q_error+0x22> > > > 1e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > > 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) > > > 28: e8 00 00 00 00 callq 2d <snic_log_q_error+0x2d> > > > 29: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > > 2d: 48 8b 7b 10 mov 0x10(%rbx),%rdi > > > 31: e8 00 00 00 00 callq 36 <snic_log_q_error+0x36> > > > 32: R_X86_64_PC32 ioread32-0x4 > > > 36: 89 05 00 00 00 00 mov %eax,0x0(%rip) # 3c <snic_log_q_error+0x3c> > > > 38: R_X86_64_PC32 snic_log_q_error_err_status-0x4 > > > 3c: 83 3b 01 cmpl $0x1,(%rbx) > > > 3f: 76 d6 jbe 17 <snic_log_q_error+0x17> > > > 41: e8 00 00 00 00 callq 46 <snic_log_q_error+0x46> > > > 42: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > > > I opened a bug: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966 > > > > Surprisingly, it's really "not a bug". The only way you can end up in this branch > is if you have a bug and run off the end of wq[1] array member: i.e. > if snic->wq_count >= 2. (See gcc BZ for smaller example) > > It's debatable whether it's okay for gcc to just let buggy code to run off > and execute something random. It is surely surprising, and not debug-friendly. > > An option to emit a crashing instruction (HLT, INT3, that sort of thing) > instead of just stopping code generation might be useful. Ah, you're right. IMO it's still a gcc bug though. Instead of following a bad pointer, it would instead start executing some random function. That takes "undefined behavior" to a new level. -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2016-10-11 20:38 ` Arnd Bergmann 2016-10-12 13:01 ` Josh Poimboeuf 2016-10-13 12:46 ` Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) Josh Poimboeuf @ 2017-03-01 9:34 ` Arnd Bergmann 2017-03-01 9:45 ` Arnd Bergmann 2017-03-01 14:31 ` Josh Poimboeuf 2 siblings, 2 replies; 36+ messages in thread From: Arnd Bergmann @ 2017-03-01 9:34 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: >> >> 3) 0xFC244C03-config: >> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() >> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section >> >> These look like another bad gcc bug which is truncating functions: > > Same bug for both of them? I ran into this one again today, after updating to the latest gcc-7.0.1: drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0xfe: sibling call from callable instruction with changed frame pointer Josh, did you get around to updating objtool the last time I reported it, or is it still the same problem? If this is a new variation, I can provide more details about the failure, otherwise I'll just ignore it for now. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 9:34 ` [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann @ 2017-03-01 9:45 ` Arnd Bergmann 2017-03-01 14:40 ` Josh Poimboeuf 2017-03-01 14:31 ` Josh Poimboeuf 1 sibling, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-01 9:45 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 1, 2017 at 10:34 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: >>> >>> 3) 0xFC244C03-config: >>> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() >>> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section >>> >>> These look like another bad gcc bug which is truncating functions: >> >> Same bug for both of them? > > I ran into this one again today, after updating to the latest gcc-7.0.1: > > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: > rxe_responder()+0xfe: sibling call from callable instruction with > changed frame pointer > > Josh, did you get around to updating objtool the last time I reported it, or > is it still the same problem? If this is a new variation, I can provide more > details about the failure, otherwise I'll just ignore it for now. Actually, something must have changed in gcc since last month, I also just got a report in another file: drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe() falls through to next function img_i2c_read_fifo() See below for the relevant snippet from the assembler output. Arnd # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176: if (i2c->bitrate <= timings[i].max_bitrate) { movl 1648(%rbx), %edx # MEM[(struct img_i2c *)_29].bitrate, _99 cmpl timings+8(%rip), %edx # timings[0].max_bitrate, _99 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1171: i2c->need_wr_rd_fence = true; movb $1, 1652(%rbx) #, MEM[(struct img_i2c *)_29].need_wr_rd_fence movl timings+48(%rip), %ecx # timings[1].max_bitrate, pretmp_260 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176: if (i2c->bitrate <= timings[i].max_bitrate) { jbe .L59 #, cmpl %ecx, %edx # pretmp_260, _99 jbe .L60 #, .L61: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1182: dev_warn(i2c->adap.dev.parent, movq 240(%rbx), %rdi # MEM[(struct img_i2c *)_29].adap.dev.parent, MEM[(struct img_i2c *)_29].adap.dev.parent movq $.LC12, %rsi #, call dev_warn # # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1187: i2c->bitrate = timing.max_bitrate; movl timings+48(%rip), %eax # MEM[(struct img_i2c_timings *)&timings + 48B], MEM[(struct img_i2c_timings *)&timings + 48B] movl %eax, 1648(%rbx) # MEM[(struct img_i2c_timings *)&timings + 48B], MEM[(struct img_i2c *)_29].bitrate .L60: ud2 .L66: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1360: dev_err(&pdev->dev, "can't request irq %d\n", irq); movl %r13d, %edx # <retval>, movq $.LC6, %rsi #, movq %r14, %rdi # _1, call dev_err # # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1361: return ret; movl -52(%rbp), %eax # %sfp, _62 movl %eax, %r13d # _62, <retval> jmp .L52 # .L65: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1341: dev_err(&pdev->dev, "can't get irq number\n"); movq $.LC5, %rsi #, movq %r14, %rdi # _1, call dev_err # # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1342: return irq; jmp .L52 # .L67: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1162: dev_info(i2c->adap.dev.parent, movzbl %ah, %ecx # ret, tmp150 movq 240(%rbx), %rdi # MEM[(struct img_i2c *)_29].adap.dev.parent, MEM[(struct img_i2c *)_29].adap.dev.parent movl %eax, %edx # ret, tmp153 movl %ecx, %r8d # tmp150, tmp150 movl %eax, %ecx # ret, tmp151 movzbl %al, %r9d # ret, shrl $16, %ecx #, tmp151 shrl $24, %edx #, tmp153 movq $.LC11, %rsi #, movzbl %cl, %ecx # tmp151, tmp152 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1404: return ret; movl $-22, %r13d #, <retval> # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1162: dev_info(i2c->adap.dev.parent, call _dev_info # # /git/arm-soc/include/linux/clk.h:210: might_sleep(); xorl %edx, %edx # movl $210, %esi #, movq $.LC0, %rdi #, call __might_sleep # xorl %edx, %edx # movl $210, %esi #, movq $.LC0, %rdi #, call __might_sleep # # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1404: return ret; jmp .L52 # .L62: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1332: return -ENOMEM; movl $-12, %r13d #, <retval> jmp .L52 # .L59: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1181: if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) { cmpl %ecx, %edx # pretmp_260, _99 ja .L61 #, ud2 .size img_i2c_probe, .-img_i2c_probe .p2align 4,,15 .type img_i2c_read_fifo, @function img_i2c_read_fifo: 1: call __fentry__ # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:545: while (i2c->msg.len) { cmpw $0, 1828(%rdi) #, i2c_10(D)->msg.len # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:544: { pushq %rbp # movq %rsp, %rbp #, # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:545: while (i2c->msg.len) { je .L68 #, # /git/arm-soc/arch/x86/include/asm/io.h:66: build_mmio_write(writel, "l", unsigned int, "r", :"memory") xorl %esi, %esi # tmp118 movl $255, %ecx #, tmp119 jmp .L71 # ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 9:45 ` Arnd Bergmann @ 2017-03-01 14:40 ` Josh Poimboeuf 2017-03-01 15:27 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-01 14:40 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote: > On Wed, Mar 1, 2017 at 10:34 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: > >> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: > >>> > >>> 3) 0xFC244C03-config: > >>> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() > >>> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section > >>> > >>> These look like another bad gcc bug which is truncating functions: > >> > >> Same bug for both of them? > > > > I ran into this one again today, after updating to the latest gcc-7.0.1: > > > > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: > > rxe_responder()+0xfe: sibling call from callable instruction with > > changed frame pointer > > > > Josh, did you get around to updating objtool the last time I reported it, or > > is it still the same problem? If this is a new variation, I can provide more > > details about the failure, otherwise I'll just ignore it for now. > > Actually, something must have changed in gcc since last month, I also > just got a report in another file: > > drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe() > falls through to next function img_i2c_read_fifo() This one looks like it could be related to some recent objtool changes which affect how it interprets 'ud2'. Which commit were you testing with? Can you provide the .config file, and the object file if it's not too big? -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 14:40 ` Josh Poimboeuf @ 2017-03-01 15:27 ` Arnd Bergmann 2017-03-01 16:53 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-01 15:27 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko [-- Attachment #1: Type: text/plain, Size: 810 bytes --] On Wed, Mar 1, 2017 at 3:40 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote: >> Actually, something must have changed in gcc since last month, I also >> just got a report in another file: >> >> drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe() >> falls through to next function img_i2c_read_fifo() > > This one looks like it could be related to some recent objtool changes > which affect how it interprets 'ud2'. Which commit were you testing > with? Can you provide the .config file, and the object file if it's not > too big? This is with my randconfig test series on top of latest linux-next. I see it with the latest gcc-7.0.1 snapshot as well as an earlier gcc-7.0.0 build (20161201), but not with gcc-6.3.1 Arnd [-- Attachment #2: i2c-img-scb.o --] [-- Type: application/x-object, Size: 15088 bytes --] [-- Attachment #3: 0xE4B9EB4B_defconfig.gz --] [-- Type: application/x-gzip, Size: 23060 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 15:27 ` Arnd Bergmann @ 2017-03-01 16:53 ` Josh Poimboeuf 2017-03-01 22:05 ` Arnd Bergmann 2017-03-01 22:42 ` Arnd Bergmann 0 siblings, 2 replies; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-01 16:53 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > On Wed, Mar 1, 2017 at 3:40 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote: > > >> Actually, something must have changed in gcc since last month, I also > >> just got a report in another file: > >> > >> drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe() > >> falls through to next function img_i2c_read_fifo() > > > > This one looks like it could be related to some recent objtool changes > > which affect how it interprets 'ud2'. Which commit were you testing > > with? Can you provide the .config file, and the object file if it's not > > too big? > > This is with my randconfig test series on top of latest linux-next. > I see it with the latest gcc-7.0.1 snapshot as well as an earlier gcc-7.0.0 > build (20161201), but not with gcc-6.3.1 I wonder if this is another gcc bug. gcc inserted two ud2 instructions in img_i2c_probe() for no apparent reason. Here's one of them: 5c3: e8 00 00 00 00 callq 5c8 <img_i2c_probe+0x298> 5c4: R_X86_64_PC32 dev_warn-0x4 5c8: 8b 05 00 00 00 00 mov 0x0(%rip),%eax # 5ce <img_i2c_probe+0x29e> 5ca: R_X86_64_PC32 .data+0xec 5ce: 89 83 70 06 00 00 mov %eax,0x670(%rbx) 5d4: 0f 0b ud2 Which corresponds to the following code block: if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) { dev_warn(i2c->adap.dev.parent, "requested bitrate (%u) is higher than the max bitrate supported (%u)\n", i2c->bitrate, timings[ARRAY_SIZE(timings) - 1].max_bitrate); timing = timings[ARRAY_SIZE(timings) - 1]; i2c->bitrate = timing.max_bitrate; } I see no apparent reason for the ud2. Can you rebuild the object with CONFIG_DEBUG_INFO and use addr2line to see what code lines are associated with the ud2's? -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 16:53 ` Josh Poimboeuf @ 2017-03-01 22:05 ` Arnd Bergmann 2017-03-01 22:42 ` Arnd Bergmann 1 sibling, 0 replies; 36+ messages in thread From: Arnd Bergmann @ 2017-03-01 22:05 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: >> On Wed, Mar 1, 2017 at 3:40 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote: >> >> >> Actually, something must have changed in gcc since last month, I also >> >> just got a report in another file: >> >> >> >> drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe() >> >> falls through to next function img_i2c_read_fifo() >> > >> > This one looks like it could be related to some recent objtool changes >> > which affect how it interprets 'ud2'. Which commit were you testing >> > with? Can you provide the .config file, and the object file if it's not >> > too big? >> >> This is with my randconfig test series on top of latest linux-next. >> I see it with the latest gcc-7.0.1 snapshot as well as an earlier gcc-7.0.0 >> build (20161201), but not with gcc-6.3.1 > > I wonder if this is another gcc bug. gcc inserted two ud2 instructions > in img_i2c_probe() for no apparent reason. Here's one of them: > > 5c3: e8 00 00 00 00 callq 5c8 <img_i2c_probe+0x298> > 5c4: R_X86_64_PC32 dev_warn-0x4 > 5c8: 8b 05 00 00 00 00 mov 0x0(%rip),%eax # 5ce <img_i2c_probe+0x29e> > 5ca: R_X86_64_PC32 .data+0xec > 5ce: 89 83 70 06 00 00 mov %eax,0x670(%rbx) > 5d4: 0f 0b ud2 > > Which corresponds to the following code block: > > if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) { > dev_warn(i2c->adap.dev.parent, > "requested bitrate (%u) is higher than the max bitrate supported (%u)\n", > i2c->bitrate, > timings[ARRAY_SIZE(timings) - 1].max_bitrate); > timing = timings[ARRAY_SIZE(timings) - 1]; > i2c->bitrate = timing.max_bitrate; > } > > I see no apparent reason for the ud2. > > Can you rebuild the object with CONFIG_DEBUG_INFO and use addr2line to > see what code lines are associated with the ud2's? $ addr2line -e drivers/i2c/busses/i2c-img-scb.o 0x5bc /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1187: i2c->bitrate = timing.max_bitrate; 0x65d /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1181: if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) { and from the .s file with line numbers: .type img_i2c_probe, @function img_i2c_probe: .LFB1968: .loc 1 1323 0 .cfi_startproc .LVL40: 1: call __fentry__ pushq %rbp # .cfi_def_cfa_offset 16 .cfi_offset 6, -16 .LBB757: .LBB758: # /git/arm-soc/include/linux/device.h:668: return devm_kmalloc(dev, size, gfp | __GFP_ZERO); .file 5 "/git/arm-soc/include/linux/device.h" .loc 5 668 0 movl $21004480, %edx #, movl $1992, %esi #, .LBE758: .LBE757: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1323: { .loc 1 1323 0 movq %rsp, %rbp #, .cfi_def_cfa_register 6 pushq %r15 # pushq %r14 # .cfi_offset 15, -24 .cfi_offset 14, -32 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1330: i2c = devm_kzalloc(&pdev->dev, sizeof(struct img_i2c), GFP_KERNEL); .loc 1 1330 0 leaq 16(%rdi), %r14 #, _1 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1323: { .loc 1 1323 0 pushq %r13 # pushq %r12 # pushq %rbx # .cfi_offset 13, -40 .cfi_offset 12, -48 .cfi_offset 3, -56 movq %rdi, %r12 # pdev, pdev subq $24, %rsp #, # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1324: struct device_node *node = pdev->dev.of_node; .loc 1 1324 0 movq 864(%rdi), %r15 # pdev_19(D)->dev.of_node, node .LVL41: .LBB760: .LBB759: # /git/arm-soc/include/linux/device.h:668: return devm_kmalloc(dev, size, gfp | __GFP_ZERO); .loc 5 668 0 movq %r14, %rdi # _1, .LVL42: call devm_kmalloc # .LVL43: .LBE759: .LBE760: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1331: if (!i2c) .loc 1 1331 0 testq %rax, %rax # _29 je .L62 #, # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1334: res = platform_get_resource(pdev, IORESOURCE_MEM, 0); .loc 1 1334 0 xorl %edx, %edx # movl $512, %esi #, movq %r12, %rdi # pdev, movq %rax, %rbx #, _29 call platform_get_resource # .LVL44: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1335: i2c->base = devm_ioremap_resource(&pdev->dev, res); .loc 1 1335 0 movq %r14, %rdi # _1, .LVL45: movq %rax, %rsi # res, call devm_ioremap_resource # .LVL46: .LBB761: .LBB762: # /git/arm-soc/include/linux/err.h:35: return IS_ERR_VALUE((unsigned long)ptr); .file 6 "/git/arm-soc/include/linux/err.h" .loc 6 35 0 xorl %esi, %esi # tmp128 cmpq $-4096, %rax #, _2 .LBE762: .LBE761: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1335: i2c->base = devm_ioremap_resource(&pdev->dev, res); .loc 1 1335 0 movq %rax, %r13 #, _2 .LBB766: .LBB764: # /git/arm-soc/include/linux/err.h:35: return IS_ERR_VALUE((unsigned long)ptr); .loc 6 35 0 seta %sil #, tmp128 .LBE764: .LBE766: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1335: i2c->base = devm_ioremap_resource(&pdev->dev, res); .loc 1 1335 0 movq %rax, 1624(%rbx) # _2, MEM[(struct img_i2c *)_29].base .LBB767: .LBB765: .LBB763: # /git/arm-soc/include/linux/err.h:35: return IS_ERR_VALUE((unsigned long)ptr); .loc 6 35 0 xorl %edx, %edx # movq $______f.2078, %rdi #, call ftrace_likely_update # .LVL47: .LBE763: .LBE765: .LBE767: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1336: if (IS_ERR(i2c->base)) .loc 1 1336 0 cmpq $-4096, %r13 #, _2 jbe .L54 #, # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1337: return PTR_ERR(i2c->base); .loc 1 1337 0 movl 1624(%rbx), %r13d # MEM[(struct img_i2c *)_29].base, <retval> .LVL48: .L52: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1405: } .loc 1 1405 0 addq $24, %rsp #, movl %r13d, %eax # <retval>, popq %rbx # .cfi_remember_state .cfi_restore 3 popq %r12 # .cfi_restore 12 .LVL49: popq %r13 # .cfi_restore 13 popq %r14 # .cfi_restore 14 popq %r15 # .cfi_restore 15 .LVL50: popq %rbp # .cfi_restore 6 .cfi_def_cfa 7, 8 ret .LVL51: .L54: .cfi_restore_state # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1339: irq = platform_get_irq(pdev, 0); .loc 1 1339 0 xorl %esi, %esi # movq %r12, %rdi # pdev, call platform_get_irq # .LVL52: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1340: if (irq < 0) { .loc 1 1340 0 testl %eax, %eax # <retval> # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1339: irq = platform_get_irq(pdev, 0); .loc 1 1339 0 movl %eax, %r13d #, <retval> # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1340: if (irq < 0) { .loc 1 1340 0 js .L65 #, .LBB768: .LBB769: # /git/arm-soc/include/linux/interrupt.h:173: return devm_request_threaded_irq(dev, irq, handler, NULL, irqflags, .file 7 "/git/arm-soc/include/linux/interrupt.h" .loc 7 173 0 movq (%r12), %r9 # pdev_19(D)->name, .LBE769: .LBE768: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1345: i2c->sys_clk = devm_clk_get(&pdev->dev, "sys"); .loc 1 1345 0 movq $0, 1640(%rbx) #, MEM[(struct img_i2c *)_29].sys_clk .LBB772: .LBB770: # /git/arm-soc/include/linux/interrupt.h:173: return devm_request_threaded_irq(dev, irq, handler, NULL, irqflags, .loc 7 173 0 xorl %r8d, %r8d # .LBE770: .LBE772: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1351: i2c->scb_clk = devm_clk_get(&pdev->dev, "scb"); .loc 1 1351 0 movq $0, 1632(%rbx) #, MEM[(struct img_i2c *)_29].scb_clk .LBB773: .LBB771: # /git/arm-soc/include/linux/interrupt.h:173: return devm_request_threaded_irq(dev, irq, handler, NULL, irqflags, .loc 7 173 0 xorl %ecx, %ecx # movq %rbx, (%rsp) # _29, movq $img_i2c_isr, %rdx #, movl %eax, %esi # <retval>, movq %r14, %rdi # _1, call devm_request_threaded_irq # .LVL53: .LBE771: .LBE773: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1359: if (ret) { .loc 1 1359 0 testl %eax, %eax # _62 movl %eax, -52(%rbp) # _62, %sfp jne .L66 #, .LBB774: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1365: init_timer(&i2c->check_timer); .loc 1 1365 0 leaq 1864(%rbx), %rdi #, tmp131 xorl %esi, %esi # movq $__key.25244, %rcx #, movq $.LC7, %rdx #, call init_timer_key # .LVL54: .LBE774: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1369: i2c->bitrate = timings[0].max_bitrate; .loc 1 1369 0 movl timings+8(%rip), %eax # timings[0].max_bitrate, timings[0].max_bitrate .LBB775: .LBB776: .LBB777: # /git/arm-soc/include/linux/of.h:458: int ret = of_property_read_variable_u32_array(np, propname, out_values, .file 8 "/git/arm-soc/include/linux/of.h" .loc 8 458 0 leaq -44(%rbp), %rdx #, tmp161 xorl %r8d, %r8d # .LBE777: .LBE776: .LBE775: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1366: i2c->check_timer.function = img_i2c_check_timer; .loc 1 1366 0 movq $img_i2c_check_timer, 1888(%rbx) #, MEM[(struct img_i2c *)_29].check_timer.function # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1367: i2c->check_timer.data = (unsigned long)i2c; .loc 1 1367 0 movq %rbx, 1896(%rbx) # _29, MEM[(struct img_i2c *)_29].check_timer.data .LBB782: .LBB780: .LBB778: # /git/arm-soc/include/linux/of.h:458: int ret = of_property_read_variable_u32_array(np, propname, out_values, .loc 8 458 0 movl $1, %ecx #, movq $.LC8, %rsi #, movq %r15, %rdi # node, .LBE778: .LBE780: .LBE782: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1369: i2c->bitrate = timings[0].max_bitrate; .loc 1 1369 0 movl %eax, 1648(%rbx) # timings[0].max_bitrate, MEM[(struct img_i2c *)_29].bitrate .LBB783: .LBB781: .LBB779: # /git/arm-soc/include/linux/of.h:458: int ret = of_property_read_variable_u32_array(np, propname, out_values, .loc 8 458 0 call of_property_read_variable_u32_array # .LVL55: # /git/arm-soc/include/linux/of.h:460: if (ret >= 0) .loc 8 460 0 testl %eax, %eax # ret js .L57 #, .LVL56: .LBE779: .LBE781: .LBE783: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1371: i2c->bitrate = val; .loc 1 1371 0 movl -44(%rbp), %eax # val, val .LVL57: movl %eax, 1648(%rbx) # val, MEM[(struct img_i2c *)_29].bitrate .LVL58: .L57: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1379: i2c->adap.nr = pdev->id; .loc 1 1379 0 movl 8(%r12), %eax # pdev_19(D)->id, pdev_19(D)->id .LVL59: .LBB784: .LBB785: # /git/arm-soc/include/linux/spinlock.h:288: return &lock->rlock; .loc 4 288 0 leaq 1752(%rbx), %rdi #, tmp142 .LBE785: .LBE784: .LBB786: .LBB787: .LBB788: # /git/arm-soc/include/linux/device.h:1033: dev->driver_data = data; .loc 5 1033 0 movq %rbx, 512(%rbx) # _29, MEM[(struct device *)_29 + 240B].driver_data .LBE788: .LBE787: .LBE786: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1374: i2c->adap.dev.parent = &pdev->dev; .loc 1 1374 0 movq %r14, 240(%rbx) # _1, MEM[(struct img_i2c *)_29].adap.dev.parent # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1375: i2c->adap.dev.of_node = node; .loc 1 1375 0 movq %r15, 1088(%rbx) # node, MEM[(struct img_i2c *)_29].adap.dev.of_node .LBB789: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1383: spin_lock_init(&i2c->lock); .loc 1 1383 0 movq $__key.25245, %rdx #, .LBE789: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1376: i2c->adap.owner = THIS_MODULE; .loc 1 1376 0 movq $__this_module, (%rbx) #, MEM[(struct img_i2c *)_29].adap.owner # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1377: i2c->adap.algo = &img_i2c_algo; .loc 1 1377 0 movq $img_i2c_algo, 16(%rbx) #, MEM[(struct img_i2c *)_29].adap.algo .LBB790: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1383: spin_lock_init(&i2c->lock); .loc 1 1383 0 movq $.LC9, %rsi #, .LBE790: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1379: i2c->adap.nr = pdev->id; .loc 1 1379 0 movl %eax, 1280(%rbx) # pdev_19(D)->id, MEM[(struct img_i2c *)_29].adap.nr # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1380: snprintf(i2c->adap.name, sizeof(i2c->adap.name), "IMG SCB I2C"); .loc 1 1380 0 movabsq $2324494381979487561, %rax #, tmp162 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1378: i2c->adap.retries = 5; .loc 1 1378 0 movl $5, 236(%rbx) #, MEM[(struct img_i2c *)_29].adap.retries # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1380: snprintf(i2c->adap.name, sizeof(i2c->adap.name), "IMG SCB I2C"); .loc 1 1380 0 movq %rax, 1284(%rbx) # tmp162, MEM[(void *)_29 + 1284B] movl $4403785, 1292(%rbx) #, MEM[(void *)_29 + 1284B] .LBB791: .LBB792: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:443: i2c->mode = mode; .loc 1 443 0 movl $0, 1848(%rbx) #, MEM[(struct img_i2c *)_29].mode # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:444: i2c->int_enable = img_i2c_int_enable_by_mode[mode]; .loc 1 444 0 movq $0, 1852(%rbx) #, MEM[(unsigned int *)_29 + 1852B] .LBE792: .LBE791: .LBB793: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1383: spin_lock_init(&i2c->lock); .loc 1 1383 0 call __raw_spin_lock_init # .LVL60: .LBE793: .LBB794: .LBB795: .LBB796: # /git/arm-soc/include/linux/completion.h:79: init_waitqueue_head(&x->wait); .file 9 "/git/arm-soc/include/linux/completion.h" .loc 9 79 0 leaq 1664(%rbx), %rdi #, tmp144 .LBE796: # /git/arm-soc/include/linux/completion.h:78: x->done = 0; .loc 9 78 0 movl $0, 1656(%rbx) #, MEM[(struct completion *)_29 + 1656B].done .LBB797: # /git/arm-soc/include/linux/completion.h:79: init_waitqueue_head(&x->wait); .loc 9 79 0 movq $__key.8818, %rdx #, movq $.LC10, %rsi #, call __init_waitqueue_head # .LVL61: .LBE797: .LBE795: .LBE794: .LBB798: .LBB799: .LBB800: .LBB801: # /git/arm-soc/include/linux/clk.h:191: might_sleep(); .loc 2 191 0 xorl %edx, %edx # .LBE801: .LBE800: .LBE799: .LBE798: .LBB805: .LBB806: .LBB807: # /git/arm-soc/include/linux/device.h:1033: dev->driver_data = data; .loc 5 1033 0 movq %rbx, 288(%r12) # _29, MEM[(struct device *)pdev_19(D) + 16B].driver_data .LBE807: .LBE806: .LBE805: .LBB808: .LBB804: .LBB803: .LBB802: # /git/arm-soc/include/linux/clk.h:191: might_sleep(); .loc 2 191 0 movl $191, %esi #, movq $.LC0, %rdi #, call __might_sleep # .LVL62: .LBE802: .LBE803: .LBE804: .LBE808: .LBB809: .LBB810: .LBB811: .LBB812: .LBB813: .LBB814: xorl %edx, %edx # movl $191, %esi #, movq $.LC0, %rdi #, call __might_sleep # .LVL63: .LBE814: .LBE813: .LBE812: .LBE811: .LBB815: .LBB816: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:418: return readl(i2c->base + offset); .loc 1 418 0 movq 1624(%rbx), %rax # MEM[(struct img_i2c *)_29].base, MEM[(struct img_i2c *)_29].base .LBB817: .LBB818: # /git/arm-soc/arch/x86/include/asm/io.h:58: build_mmio_read(readl, "l", unsigned int, "=r", :"memory") .loc 3 58 0 #APP # 58 "/git/arm-soc/arch/x86/include/asm/io.h" 1 movl 128(%rax),%eax # MEM[(volatile unsigned int *)_81], ret # 0 "" 2 .LVL64: #NO_APP .LBE818: .LBE817: .LBE816: .LBE815: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1161: if ((rev & 0x00ffffff) < 0x00020200) { .loc 1 1161 0 movl %eax, %edx # ret, tmp147 andl $16777215, %edx #, tmp147 cmpl $131583, %edx #, tmp147 jbe .L67 #, # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176: if (i2c->bitrate <= timings[i].max_bitrate) { .loc 1 1176 0 movl 1648(%rbx), %edx # MEM[(struct img_i2c *)_29].bitrate, _99 cmpl timings+8(%rip), %edx # timings[0].max_bitrate, _99 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1171: i2c->need_wr_rd_fence = true; .loc 1 1171 0 movb $1, 1652(%rbx) #, MEM[(struct img_i2c *)_29].need_wr_rd_fence movl timings+48(%rip), %ecx # timings[1].max_bitrate, pretmp_260 # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176: if (i2c->bitrate <= timings[i].max_bitrate) { .loc 1 1176 0 jbe .L59 #, cmpl %ecx, %edx # pretmp_260, _99 jbe .L60 #, .L61: .LBB819: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1182: dev_warn(i2c->adap.dev.parent, .loc 1 1182 0 movq 240(%rbx), %rdi # MEM[(struct img_i2c *)_29].adap.dev.parent, MEM[(struct img_i2c *)_29].adap.dev.parent movq $.LC12, %rsi #, call dev_warn # .LVL65: # /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1187: i2c->bitrate = timing.max_bitrate; .loc 1 1187 0 movl timings+48(%rip), %eax # MEM[(struct img_i2c_timings *)&timings + 48B], MEM[(struct img_i2c_timings *)&timings + 48B] movl %eax, 1648(%rbx) # MEM[(struct img_i2c_timings *)&timings + 48B], MEM[(struct img_i2c *)_29].bitrate .LVL66: .L60: ud2 .LVL67: .L66: .LBE819: .LBE810: .LBE809: ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 16:53 ` Josh Poimboeuf 2017-03-01 22:05 ` Arnd Bergmann @ 2017-03-01 22:42 ` Arnd Bergmann 2017-03-02 1:03 ` Josh Poimboeuf 1 sibling, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-01 22:42 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > I see no apparent reason for the ud2. It's the possible division by zero. This change would avoid the ud2: diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c index db8e8b40569d..a2b09c518225 100644 --- a/drivers/i2c/busses/i2c-img-scb.c +++ b/drivers/i2c/busses/i2c-img-scb.c @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) clk_khz /= prescale; /* Setup the clock increment value */ + if (clk_khz < 1) + clk_khz = 1; inc = (256 * 16 * bitrate_khz) / clk_khz; /* Arnd ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 22:42 ` Arnd Bergmann @ 2017-03-02 1:03 ` Josh Poimboeuf 2017-03-02 6:31 ` Ingo Molnar 2017-03-02 22:49 ` Arnd Bergmann 0 siblings, 2 replies; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-02 1:03 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > > > I see no apparent reason for the ud2. > > It's the possible division by zero. This change would avoid the ud2: > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > index db8e8b40569d..a2b09c518225 100644 > --- a/drivers/i2c/busses/i2c-img-scb.c > +++ b/drivers/i2c/busses/i2c-img-scb.c > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > clk_khz /= prescale; > > /* Setup the clock increment value */ > + if (clk_khz < 1) > + clk_khz = 1; > inc = (256 * 16 * bitrate_khz) / clk_khz; > > /* Ok, I see what gcc is doing. clk_khz = clk_get_rate(i2c->scb_clk) / 1000; ... inc = (256 * 16 * bitrate_khz) / clk_khz; Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means clk_khz is always zero, so the last statement *always* results in a divide-by-zero. So that looks like a bug in the code. However, I'm baffled by how gcc handles it. Instead of: a) reporting a compile-time warning/error; or b) letting the #DE (divide error) exception happen; it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 1:03 ` Josh Poimboeuf @ 2017-03-02 6:31 ` Ingo Molnar 2017-03-02 12:49 ` Josh Poimboeuf 2017-03-02 22:49 ` Arnd Bergmann 1 sibling, 1 reply; 36+ messages in thread From: Ingo Molnar @ 2017-03-02 6:31 UTC (permalink / raw) To: Josh Poimboeuf Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > > > > > I see no apparent reason for the ud2. > > > > It's the possible division by zero. This change would avoid the ud2: > > > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > > index db8e8b40569d..a2b09c518225 100644 > > --- a/drivers/i2c/busses/i2c-img-scb.c > > +++ b/drivers/i2c/busses/i2c-img-scb.c > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > > clk_khz /= prescale; > > > > /* Setup the clock increment value */ > > + if (clk_khz < 1) > > + clk_khz = 1; > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > /* > > Ok, I see what gcc is doing. > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > ... > inc = (256 * 16 * bitrate_khz) / clk_khz; > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > clk_khz is always zero, so the last statement *always* results in a > divide-by-zero. So that looks like a bug in the code. > > However, I'm baffled by how gcc handles it. Instead of: > > a) reporting a compile-time warning/error; or > > b) letting the #DE (divide error) exception happen; > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? Well, technically an invalid opcode is shorter code than generating an (integer) division by zero exception, right? Thanks, Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 6:31 ` Ingo Molnar @ 2017-03-02 12:49 ` Josh Poimboeuf 2017-03-02 13:46 ` Ingo Molnar 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-02 12:49 UTC (permalink / raw) To: Ingo Molnar Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Thu, Mar 02, 2017 at 07:31:39AM +0100, Ingo Molnar wrote: > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > > > > > > > I see no apparent reason for the ud2. > > > > > > It's the possible division by zero. This change would avoid the ud2: > > > > > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > > > index db8e8b40569d..a2b09c518225 100644 > > > --- a/drivers/i2c/busses/i2c-img-scb.c > > > +++ b/drivers/i2c/busses/i2c-img-scb.c > > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > > > clk_khz /= prescale; > > > > > > /* Setup the clock increment value */ > > > + if (clk_khz < 1) > > > + clk_khz = 1; > > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > > > /* > > > > Ok, I see what gcc is doing. > > > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > > ... > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > > clk_khz is always zero, so the last statement *always* results in a > > divide-by-zero. So that looks like a bug in the code. > > > > However, I'm baffled by how gcc handles it. Instead of: > > > > a) reporting a compile-time warning/error; or > > > > b) letting the #DE (divide error) exception happen; > > > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? > > Well, technically an invalid opcode is shorter code than generating an (integer) > division by zero exception, right? What does that matter if it's the wrong behavior? -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 12:49 ` Josh Poimboeuf @ 2017-03-02 13:46 ` Ingo Molnar 2017-03-02 14:08 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Ingo Molnar @ 2017-03-02 13:46 UTC (permalink / raw) To: Josh Poimboeuf Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Thu, Mar 02, 2017 at 07:31:39AM +0100, Ingo Molnar wrote: > > > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > > > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > > > > > > > > > I see no apparent reason for the ud2. > > > > > > > > It's the possible division by zero. This change would avoid the ud2: > > > > > > > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > > > > index db8e8b40569d..a2b09c518225 100644 > > > > --- a/drivers/i2c/busses/i2c-img-scb.c > > > > +++ b/drivers/i2c/busses/i2c-img-scb.c > > > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > > > > clk_khz /= prescale; > > > > > > > > /* Setup the clock increment value */ > > > > + if (clk_khz < 1) > > > > + clk_khz = 1; > > > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > > > > > /* > > > > > > Ok, I see what gcc is doing. > > > > > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > > > ... > > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > > > clk_khz is always zero, so the last statement *always* results in a > > > divide-by-zero. So that looks like a bug in the code. > > > > > > However, I'm baffled by how gcc handles it. Instead of: > > > > > > a) reporting a compile-time warning/error; or > > > > > > b) letting the #DE (divide error) exception happen; > > > > > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? > > > > Well, technically an invalid opcode is shorter code than generating an (integer) > > division by zero exception, right? > > What does that matter if it's the wrong behavior? Well, both terminate the program, and it's obvious if you look at it with a debugger what happened, right? Thanks, Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 13:46 ` Ingo Molnar @ 2017-03-02 14:08 ` Josh Poimboeuf 2017-03-02 14:46 ` Ingo Molnar 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-02 14:08 UTC (permalink / raw) To: Ingo Molnar Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Thu, Mar 02, 2017 at 02:46:29PM +0100, Ingo Molnar wrote: > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > On Thu, Mar 02, 2017 at 07:31:39AM +0100, Ingo Molnar wrote: > > > > > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > > > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > > > > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > > > > > > > > > > > I see no apparent reason for the ud2. > > > > > > > > > > It's the possible division by zero. This change would avoid the ud2: > > > > > > > > > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > > > > > index db8e8b40569d..a2b09c518225 100644 > > > > > --- a/drivers/i2c/busses/i2c-img-scb.c > > > > > +++ b/drivers/i2c/busses/i2c-img-scb.c > > > > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > > > > > clk_khz /= prescale; > > > > > > > > > > /* Setup the clock increment value */ > > > > > + if (clk_khz < 1) > > > > > + clk_khz = 1; > > > > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > > > > > > > /* > > > > > > > > Ok, I see what gcc is doing. > > > > > > > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > > > > ... > > > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > > > > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > > > > clk_khz is always zero, so the last statement *always* results in a > > > > divide-by-zero. So that looks like a bug in the code. > > > > > > > > However, I'm baffled by how gcc handles it. Instead of: > > > > > > > > a) reporting a compile-time warning/error; or > > > > > > > > b) letting the #DE (divide error) exception happen; > > > > > > > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? > > > > > > Well, technically an invalid opcode is shorter code than generating an (integer) > > > division by zero exception, right? > > > > What does that matter if it's the wrong behavior? > > Well, both terminate the program, and it's obvious if you look at it with a > debugger what happened, right? If it were obvious, we wouldn't be having this discussion :-) The only thing obvious to me was that gcc mysteriously removed a bunch of code and replaced it with a 'ud2' instruction in the middle of the function for no apparent reason. -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 14:08 ` Josh Poimboeuf @ 2017-03-02 14:46 ` Ingo Molnar 0 siblings, 0 replies; 36+ messages in thread From: Ingo Molnar @ 2017-03-02 14:46 UTC (permalink / raw) To: Josh Poimboeuf Cc: Arnd Bergmann, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > Well, technically an invalid opcode is shorter code than generating an > > > > (integer) division by zero exception, right? > > > > > > What does that matter if it's the wrong behavior? > > > > Well, both terminate the program, and it's obvious if you look at it with a > > debugger what happened, right? > > If it were obvious, we wouldn't be having this discussion :-) Touche ;-) > The only thing obvious to me was that gcc mysteriously removed a bunch of code > and replaced it with a 'ud2' instruction in the middle of the function for no > apparent reason. I don't know what their motivation was, but if it's not a bug, if it was done intentionally, then I'd guess it's roughly the argument I made: in simple testcases it can be argued to be a code size improvement, plus it's probably allowed by the letter of the compiler standards (program termination behavior is notoriously platform dependent and thus vaguely specified) - but for real-life code I very much agree that it's a step backward in generated code quality... Thanks, Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 1:03 ` Josh Poimboeuf 2017-03-02 6:31 ` Ingo Molnar @ 2017-03-02 22:49 ` Arnd Bergmann 2017-03-02 23:05 ` Josh Poimboeuf 1 sibling, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-02 22:49 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Thu, Mar 2, 2017 at 2:03 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: >> On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: >> >> > I see no apparent reason for the ud2. >> >> It's the possible division by zero. This change would avoid the ud2: >> >> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c >> index db8e8b40569d..a2b09c518225 100644 >> --- a/drivers/i2c/busses/i2c-img-scb.c >> +++ b/drivers/i2c/busses/i2c-img-scb.c >> @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) >> clk_khz /= prescale; >> >> /* Setup the clock increment value */ >> + if (clk_khz < 1) >> + clk_khz = 1; >> inc = (256 * 16 * bitrate_khz) / clk_khz; >> >> /* > > Ok, I see what gcc is doing. > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > ... > inc = (256 * 16 * bitrate_khz) / clk_khz; > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > clk_khz is always zero, so the last statement *always* results in a > divide-by-zero. So that looks like a bug in the code. > > However, I'm baffled by how gcc handles it. Instead of: > > a) reporting a compile-time warning/error; or > > b) letting the #DE (divide error) exception happen; > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? Just FYI, I found another one like this: 0000000000000000 <hibvt_pwm_get_state>: 0: e8 00 00 00 00 callq 5 <hibvt_pwm_get_state+0x5> 1: R_X86_64_PC32 __fentry__-0x4 5: 8b 46 10 mov 0x10(%rsi),%eax 8: 55 push %rbp 9: 48 89 e5 mov %rsp,%rbp c: c1 e0 05 shl $0x5,%eax f: 48 03 47 48 add 0x48(%rdi),%rax 13: 8b 00 mov (%rax),%eax 15: 0f 0b ud2 17: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1) 1e: 00 00 static inline unsigned long clk_get_rate(struct clk *clk) { return 0; } static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip); void __iomem *base; u32 freq, value; freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000); base = hi_pwm_chip->base; value = readl(base + PWM_CFG0_ADDR(pwm->hwpwm)); state->period = div_u64(value * 1000, freq); value = readl(base + PWM_CFG1_ADDR(pwm->hwpwm)); state->duty_cycle = div_u64(value * 1000, freq); value = readl(base + PWM_CTRL_ADDR(pwm->hwpwm)); state->enabled = (PWM_ENABLE_MASK & value); } Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 22:49 ` Arnd Bergmann @ 2017-03-02 23:05 ` Josh Poimboeuf 2017-03-03 8:58 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-02 23:05 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Thu, Mar 02, 2017 at 11:49:49PM +0100, Arnd Bergmann wrote: > On Thu, Mar 2, 2017 at 2:03 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote: > >> On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote: > >> > >> > I see no apparent reason for the ud2. > >> > >> It's the possible division by zero. This change would avoid the ud2: > >> > >> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c > >> index db8e8b40569d..a2b09c518225 100644 > >> --- a/drivers/i2c/busses/i2c-img-scb.c > >> +++ b/drivers/i2c/busses/i2c-img-scb.c > >> @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c) > >> clk_khz /= prescale; > >> > >> /* Setup the clock increment value */ > >> + if (clk_khz < 1) > >> + clk_khz = 1; > >> inc = (256 * 16 * bitrate_khz) / clk_khz; > >> > >> /* > > > > Ok, I see what gcc is doing. > > > > clk_khz = clk_get_rate(i2c->scb_clk) / 1000; > > ... > > inc = (256 * 16 * bitrate_khz) / clk_khz; > > > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means > > clk_khz is always zero, so the last statement *always* results in a > > divide-by-zero. So that looks like a bug in the code. > > > > However, I'm baffled by how gcc handles it. Instead of: > > > > a) reporting a compile-time warning/error; or > > > > b) letting the #DE (divide error) exception happen; > > > > it inserts a 'ud2', resulting in a #UD (invalid opcode). Why?!? > > Just FYI, I found another one like this: > > 0000000000000000 <hibvt_pwm_get_state>: > 0: e8 00 00 00 00 callq 5 <hibvt_pwm_get_state+0x5> > 1: R_X86_64_PC32 __fentry__-0x4 > 5: 8b 46 10 mov 0x10(%rsi),%eax > 8: 55 push %rbp > 9: 48 89 e5 mov %rsp,%rbp > c: c1 e0 05 shl $0x5,%eax > f: 48 03 47 48 add 0x48(%rdi),%rax > 13: 8b 00 mov (%rax),%eax > 15: 0f 0b ud2 > 17: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1) > 1e: 00 00 > > static inline unsigned long clk_get_rate(struct clk *clk) > { > return 0; > } > > static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > struct pwm_state *state) > { > struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip); > void __iomem *base; > u32 freq, value; > > freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000); > base = hi_pwm_chip->base; > > value = readl(base + PWM_CFG0_ADDR(pwm->hwpwm)); > state->period = div_u64(value * 1000, freq); > > value = readl(base + PWM_CFG1_ADDR(pwm->hwpwm)); > state->duty_cycle = div_u64(value * 1000, freq); > > value = readl(base + PWM_CTRL_ADDR(pwm->hwpwm)); > state->enabled = (PWM_ENABLE_MASK & value); > } I assume '-Wdiv-by-zero' is enabled and gcc isn't showing the "division by zero" warning for either of these? The 'ud2' is guaranteed to trigger since the function has no branches. Surely at least the missing warning is a gcc bug. The good news is objtool is flushing these out, albeit with a confusing message. -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 23:05 ` Josh Poimboeuf @ 2017-03-03 8:58 ` Arnd Bergmann 2017-03-03 11:27 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-03 8:58 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Fri, Mar 3, 2017 at 12:05 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > I assume '-Wdiv-by-zero' is enabled and gcc isn't showing the "division > by zero" warning for either of these? The 'ud2' is guaranteed to > trigger since the function has no branches. Surely at least the missing > warning is a gcc bug. > > The good news is objtool is flushing these out, albeit with a confusing > message. I'm still not sure if it's intentional or not. I've reduced the test case to the simple static inline int return0(void) { return 0; } int provoke_div0_warning(void) { return 1 / return0(); } which does not generate a compile-time warning, but will generate an unconditional runtime warning if built with -fsanitze=integer-divide-by-zero. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-03 8:58 ` Arnd Bergmann @ 2017-03-03 11:27 ` Arnd Bergmann 0 siblings, 0 replies; 36+ messages in thread From: Arnd Bergmann @ 2017-03-03 11:27 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko I opened requests on both gcc and llvm, but it looks like there is no easy way to get a warning here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79828 https://bugs.llvm.org/show_bug.cgi?id=32126 Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 9:34 ` [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann 2017-03-01 9:45 ` Arnd Bergmann @ 2017-03-01 14:31 ` Josh Poimboeuf 2017-03-01 15:21 ` Arnd Bergmann 1 sibling, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-01 14:31 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 01, 2017 at 10:34:42AM +0100, Arnd Bergmann wrote: > On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: > >> > >> 3) 0xFC244C03-config: > >> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() > >> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section > >> > >> These look like another bad gcc bug which is truncating functions: > > > > Same bug for both of them? > > I ran into this one again today, after updating to the latest gcc-7.0.1: > > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: > rxe_responder()+0xfe: sibling call from callable instruction with > changed frame pointer > > Josh, did you get around to updating objtool the last time I reported it, or > is it still the same problem? If this is a new variation, I can provide more > details about the failure, otherwise I'll just ignore it for now. This one should have been fixed with: 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection") Can you attach the object file? -- Josh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 14:31 ` Josh Poimboeuf @ 2017-03-01 15:21 ` Arnd Bergmann 2017-03-02 18:25 ` Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-01 15:21 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko [-- Attachment #1: Type: text/plain, Size: 1377 bytes --] On Wed, Mar 1, 2017 at 3:31 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Wed, Mar 01, 2017 at 10:34:42AM +0100, Arnd Bergmann wrote: >> On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: >> >> >> >> 3) 0xFC244C03-config: >> >> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() >> >> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section >> >> >> >> These look like another bad gcc bug which is truncating functions: >> > >> > Same bug for both of them? >> >> I ran into this one again today, after updating to the latest gcc-7.0.1: >> >> drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: >> rxe_responder()+0xfe: sibling call from callable instruction with >> changed frame pointer >> >> Josh, did you get around to updating objtool the last time I reported it, or >> is it still the same problem? If this is a new variation, I can provide more >> details about the failure, otherwise I'll just ignore it for now. > > This one should have been fixed with: > > 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection") It was on the current linux-next, so that commit should certainly be included. > Can you attach the object file? done. Arnd [-- Attachment #2: rxe_resp.o --] [-- Type: application/x-object, Size: 20248 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-01 15:21 ` Arnd Bergmann @ 2017-03-02 18:25 ` Josh Poimboeuf 2017-03-02 22:43 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-02 18:25 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Wed, Mar 01, 2017 at 04:21:36PM +0100, Arnd Bergmann wrote: > On Wed, Mar 1, 2017 at 3:31 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Wed, Mar 01, 2017 at 10:34:42AM +0100, Arnd Bergmann wrote: > >> On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote: > >> > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: > >> >> > >> >> 3) 0xFC244C03-config: > >> >> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event() > >> >> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section > >> >> > >> >> These look like another bad gcc bug which is truncating functions: > >> > > >> > Same bug for both of them? > >> > >> I ran into this one again today, after updating to the latest gcc-7.0.1: > >> > >> drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: > >> rxe_responder()+0xfe: sibling call from callable instruction with > >> changed frame pointer > >> > >> Josh, did you get around to updating objtool the last time I reported it, or > >> is it still the same problem? If this is a new variation, I can provide more > >> details about the failure, otherwise I'll just ignore it for now. > > > > This one should have been fixed with: > > > > 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection") > > It was on the current linux-next, so that commit should certainly be included. > > > Can you attach the object file? Here's the preliminary fix for this one (still needs more testing): diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index bd12eb1..7b718bb 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -806,11 +806,20 @@ static struct rela *find_switch_table(struct objtool_file *file, insn->jump_dest->offset > orig_insn->offset)) break; + /* look for a rela which references .rodata */ text_rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len); - if (text_rela && text_rela->sym == file->rodata->sym) - return find_rela_by_dest(file->rodata, - text_rela->addend); + if (!text_rela || text_rela->sym != file->rodata->sym) + continue; + + /* + * Make sure the .rodata address isn't associated with a + * symbol. gcc jump tables are anonymous data. + */ + if (find_symbol_containing(file->rodata, text_rela->addend)) + continue; + + return find_rela_by_dest(file->rodata, text_rela->addend); } return NULL; diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 0d7983a..d897702 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -85,6 +85,18 @@ struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset) return NULL; } +struct symbol *find_symbol_containing(struct section *sec, unsigned long offset) +{ + struct symbol *sym; + + list_for_each_entry(sym, &sec->symbol_list, list) + if (sym->type != STT_SECTION && + offset >= sym->offset && offset < sym->offset + sym->len) + return sym; + + return NULL; +} + struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset, unsigned int len) { diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index aa1ff65..731973e 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -79,6 +79,7 @@ struct elf { struct elf *elf_open(const char *name); struct section *find_section_by_name(struct elf *elf, const char *name); struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset); +struct symbol *find_symbol_containing(struct section *sec, unsigned long offset); struct rela *find_rela_by_dest(struct section *sec, unsigned long offset); struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset, unsigned int len); ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings 2017-03-02 18:25 ` Josh Poimboeuf @ 2017-03-02 22:43 ` Arnd Bergmann 2017-03-02 22:57 ` [PATCH] objtool: fix another gcc jump table detection issue Josh Poimboeuf 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2017-03-02 22:43 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Thu, Mar 2, 2017 at 7:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> It was on the current linux-next, so that commit should certainly be included. >> >> > Can you attach the object file? > > Here's the preliminary fix for this one (still needs more testing): It fixes the warning for me. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] objtool: fix another gcc jump table detection issue 2017-03-02 22:43 ` Arnd Bergmann @ 2017-03-02 22:57 ` Josh Poimboeuf 2017-03-02 23:01 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Josh Poimboeuf @ 2017-03-02 22:57 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko Arnd Bergmann reported a (false positive) objtool warning: drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0xfe: sibling call from callable instruction with changed frame pointer The issue is in find_switch_table(). It tries to find a switch statement's jump table by walking backwards from an indirect jump instruction, looking for a relocation to the .rodata section. In this case it stopped walking prematurely: the first .rodata relocation it encountered was for a variable (resp_state_name) instead of a jump table, so it just assumed there wasn't a jump table. The fix is to ignore any .rodata relocation which refers to an ELF object symbol. This works because the jump tables are anonymous and have no symbols associated with them. Reported-by: Arnd Bergmann <arnd@arndb.de> Fixes: 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection") Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- tools/objtool/builtin-check.c | 15 ++++++++++++--- tools/objtool/elf.c | 12 ++++++++++++ tools/objtool/elf.h | 1 + 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 5fc52ee..c2a8518 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -805,11 +805,20 @@ static struct rela *find_switch_table(struct objtool_file *file, insn->jump_dest->offset > orig_insn->offset)) break; + /* look for a relocation which references .rodata */ text_rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len); - if (text_rela && text_rela->sym == file->rodata->sym) - return find_rela_by_dest(file->rodata, - text_rela->addend); + if (!text_rela || text_rela->sym != file->rodata->sym) + continue; + + /* + * Make sure the .rodata address isn't associated with a + * symbol. gcc jump tables are anonymous data. + */ + if (find_symbol_containing(file->rodata, text_rela->addend)) + continue; + + return find_rela_by_dest(file->rodata, text_rela->addend); } return NULL; diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 0d7983a..d897702 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -85,6 +85,18 @@ struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset) return NULL; } +struct symbol *find_symbol_containing(struct section *sec, unsigned long offset) +{ + struct symbol *sym; + + list_for_each_entry(sym, &sec->symbol_list, list) + if (sym->type != STT_SECTION && + offset >= sym->offset && offset < sym->offset + sym->len) + return sym; + + return NULL; +} + struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset, unsigned int len) { diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index aa1ff65..731973e 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -79,6 +79,7 @@ struct elf { struct elf *elf_open(const char *name); struct section *find_section_by_name(struct elf *elf, const char *name); struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset); +struct symbol *find_symbol_containing(struct section *sec, unsigned long offset); struct rela *find_rela_by_dest(struct section *sec, unsigned long offset); struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset, unsigned int len); -- 2.7.4 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] objtool: fix another gcc jump table detection issue 2017-03-02 22:57 ` [PATCH] objtool: fix another gcc jump table detection issue Josh Poimboeuf @ 2017-03-02 23:01 ` Arnd Bergmann 0 siblings, 0 replies; 36+ messages in thread From: Arnd Bergmann @ 2017-03-02 23:01 UTC (permalink / raw) To: Josh Poimboeuf Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Linux Kernel Mailing List, Denys Vlasenko On Thu, Mar 2, 2017 at 11:57 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > Arnd Bergmann reported a (false positive) objtool warning: > > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0xfe: sibling call from callable instruction with changed frame pointer > > The issue is in find_switch_table(). It tries to find a switch > statement's jump table by walking backwards from an indirect jump > instruction, looking for a relocation to the .rodata section. In this > case it stopped walking prematurely: the first .rodata relocation it > encountered was for a variable (resp_state_name) instead of a jump > table, so it just assumed there wasn't a jump table. > > The fix is to ignore any .rodata relocation which refers to an ELF > object symbol. This works because the jump tables are anonymous and > have no symbols associated with them. > > Reported-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection") > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Tested-by: Arnd Bergmann <arnd@arndb.de> ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] objtool: support '-mtune=atom' stack frame setup instruction 2016-10-10 12:56 [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann 2016-10-10 20:23 ` Josh Poimboeuf @ 2016-10-11 1:53 ` Josh Poimboeuf 1 sibling, 0 replies; 36+ messages in thread From: Josh Poimboeuf @ 2016-10-11 1:53 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel >From 60c982d4d04014adb3bde1ebee6ca95320ffb213 Mon Sep 17 00:00:00 2001 Message-Id: <60c982d4d04014adb3bde1ebee6ca95320ffb213.1476150736.git.jpoimboe@redhat.com> From: Josh Poimboeuf <jpoimboe@redhat.com> Date: Mon, 10 Oct 2016 15:24:01 -0500 Subject: [PATCH] objtool: support '-mtune=atom' stack frame setup instruction Arnd reported that enabling CONFIG_MATOM results in a bunch of objtool false positive frame pointer warnings: arch/x86/events/intel/ds.o: warning: objtool: intel_pmu_pebs_del()+0x43: call without frame pointer save/setup security/keys/keyring.o: warning: objtool: keyring_read()+0x59: call without frame pointer save/setup kernel/signal.o: warning: objtool: __dequeue_signal()+0xd8: call without frame pointer save/setup ... objtool gets confused by the fact that the '-mtune=atom' gcc option sometimes uses 'lea (%rsp),%rbp' instead of 'mov %rsp,%rbp'. The instructions are effectively the same, but objtool doesn't know about the 'lea' variant. Fix the false warnings by adding support for 'lea (%rsp),%rbp' in the objtool decoder. Reported-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- tools/objtool/arch/x86/decode.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index c0c0b26..b63a31b 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -98,6 +98,15 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, *type = INSN_FP_SETUP; break; + case 0x8d: + if (insn.rex_prefix.bytes && + insn.rex_prefix.bytes[0] == 0x48 && + insn.modrm.nbytes && insn.modrm.bytes[0] == 0x2c && + insn.sib.nbytes && insn.sib.bytes[0] == 0x24) + /* lea %(rsp), %rbp */ + *type = INSN_FP_SETUP; + break; + case 0x90: *type = INSN_NOP; break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 36+ messages in thread
end of thread, other threads:[~2017-03-03 15:39 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-10 12:56 [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann 2016-10-10 20:23 ` Josh Poimboeuf 2016-10-11 8:08 ` Arnd Bergmann 2016-10-11 12:20 ` Josh Poimboeuf 2016-10-11 13:30 ` Arnd Bergmann 2016-10-11 15:05 ` Josh Poimboeuf 2016-10-11 15:51 ` Josh Poimboeuf 2016-10-11 20:38 ` Arnd Bergmann 2016-10-12 13:01 ` Josh Poimboeuf 2016-10-13 12:46 ` Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) Josh Poimboeuf 2016-10-13 17:57 ` Denys Vlasenko 2016-10-13 20:15 ` Josh Poimboeuf 2017-03-01 9:34 ` [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann 2017-03-01 9:45 ` Arnd Bergmann 2017-03-01 14:40 ` Josh Poimboeuf 2017-03-01 15:27 ` Arnd Bergmann 2017-03-01 16:53 ` Josh Poimboeuf 2017-03-01 22:05 ` Arnd Bergmann 2017-03-01 22:42 ` Arnd Bergmann 2017-03-02 1:03 ` Josh Poimboeuf 2017-03-02 6:31 ` Ingo Molnar 2017-03-02 12:49 ` Josh Poimboeuf 2017-03-02 13:46 ` Ingo Molnar 2017-03-02 14:08 ` Josh Poimboeuf 2017-03-02 14:46 ` Ingo Molnar 2017-03-02 22:49 ` Arnd Bergmann 2017-03-02 23:05 ` Josh Poimboeuf 2017-03-03 8:58 ` Arnd Bergmann 2017-03-03 11:27 ` Arnd Bergmann 2017-03-01 14:31 ` Josh Poimboeuf 2017-03-01 15:21 ` Arnd Bergmann 2017-03-02 18:25 ` Josh Poimboeuf 2017-03-02 22:43 ` Arnd Bergmann 2017-03-02 22:57 ` [PATCH] objtool: fix another gcc jump table detection issue Josh Poimboeuf 2017-03-02 23:01 ` Arnd Bergmann 2016-10-11 1:53 ` [PATCH] objtool: support '-mtune=atom' stack frame setup instruction Josh Poimboeuf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).