[PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

[-- 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 --]

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

(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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings)

[Josh Poimboeuf]

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

Re: Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings)

[Denys Vlasenko]

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.

Re: Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings)

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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    #

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

[-- 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 --]

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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:

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Ingo Molnar]

* 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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Ingo Molnar]

* 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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Ingo Molnar]

* 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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

[-- 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 --]

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Josh Poimboeuf]

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);

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

[Arnd Bergmann]

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

[PATCH] objtool: fix another gcc jump table detection issue

[Josh Poimboeuf]

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

Re: [PATCH] objtool: fix another gcc jump table detection issue

[Arnd Bergmann]

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>

[PATCH] objtool: support '-mtune=atom' stack frame setup instruction

[Josh Poimboeuf]

>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