linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] objtool: a couple of minor fixes
@ 2016-10-13 21:22 Josh Poimboeuf
  2016-10-13 21:22 ` [PATCH 1/2] objtool: improve rare switch jump table pattern detection Josh Poimboeuf
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-13 21:22 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Arnd Bergmann

Fix a couple of issues:

- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernels

Josh Poimboeuf (2):
  objtool: improve rare switch jump table pattern detection
  objtool: skip all "unreachable instruction" warnings for gcov kernels

 tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 33 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] objtool: improve rare switch jump table pattern detection
  2016-10-13 21:22 [PATCH 0/2] objtool: a couple of minor fixes Josh Poimboeuf
@ 2016-10-13 21:22 ` Josh Poimboeuf
  2016-10-16 11:18   ` [tip:core/urgent] objtool: Improve " tip-bot for Josh Poimboeuf
  2016-10-13 21:22 ` [PATCH 2/2] objtool: skip all "unreachable instruction" warnings for gcov kernels Josh Poimboeuf
  2016-10-26  7:58 ` [PATCH 0/2] objtool: a couple of minor fixes Arnd Bergmann
  2 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-13 21:22 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Arnd Bergmann

gcc 6 added a new switch statement jump table optimization which makes
objtool's life harder.  It looks like:

  mov [rodata addr],%reg1
  ... some instructions ...
  jmpq *(%reg1,%reg2,8)

The optimization is quite rare, but objtool still needs to be able to
identify the pattern so that it can follow all possible control flow
paths related to the switch statement.

In order to detect the pattern, objtool starts from the indirect jump
and scans backwards through the function until it finds the first
instruction in the pattern.  If it encounters an unconditional jump
along the way, it stops and considers the pattern to be not found.

As it turns out, unconditional jumps can happen, as long as they are
small forward jumps within the range being scanned.

This fixes the following warnings:

  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

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/builtin-check.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 143b6cd..a00a05d 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -713,6 +713,7 @@ static struct rela *find_switch_table(struct objtool_file *file,
 				      struct instruction *insn)
 {
 	struct rela *text_rela, *rodata_rela;
+	struct instruction *orig_insn = insn;
 
 	text_rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
 	if (text_rela && text_rela->sym == file->rodata->sym) {
@@ -733,10 +734,16 @@ static struct rela *find_switch_table(struct objtool_file *file,
 
 	/* case 3 */
 	func_for_each_insn_continue_reverse(file, func, insn) {
-		if (insn->type == INSN_JUMP_UNCONDITIONAL ||
-		    insn->type == INSN_JUMP_DYNAMIC)
+		if (insn->type == INSN_JUMP_DYNAMIC)
 			break;
 
+		/* allow small jumps within the range */
+		if (insn->type == INSN_JUMP_UNCONDITIONAL &&
+		    insn->jump_dest &&
+		    (insn->jump_dest->offset <= insn->offset ||
+		     insn->jump_dest->offset >= orig_insn->offset))
+		    break;
+
 		text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
 						    insn->len);
 		if (text_rela && text_rela->sym == file->rodata->sym)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] objtool: skip all "unreachable instruction" warnings for gcov kernels
  2016-10-13 21:22 [PATCH 0/2] objtool: a couple of minor fixes Josh Poimboeuf
  2016-10-13 21:22 ` [PATCH 1/2] objtool: improve rare switch jump table pattern detection Josh Poimboeuf
@ 2016-10-13 21:22 ` Josh Poimboeuf
  2016-10-16 11:18   ` [tip:core/urgent] objtool: Skip " tip-bot for Josh Poimboeuf
  2016-10-26  7:58 ` [PATCH 0/2] objtool: a couple of minor fixes Arnd Bergmann
  2 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-13 21:22 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Arnd Bergmann

Recently objtool has started reporting a few "unreachable instruction"
warnings when CONFIG_GCOV is enabled for newer versions of gcc.  Usually
this warning means there's some new control flow that objtool doesn't
understand.  But in this case, objtool is correct and the instructions
really are inaccessible.  It's an annoying quirk of gcov, but it's
harmless, so it's ok to just silence the warnings.

With older versions of gcc, it was relatively easy to detect
gcov-specific instructions and to skip any unreachable warnings produced
by them.  But gcc 6 has gotten craftier.

Instead of continuing to play whack-a-mole with gcov, just use a bigger,
more permanent hammer and disable unreachable warnings for the whole
file when gcov is enabled.  This is fine to do because a) unreachable
warnings are usually of questionable value; and b) gcov isn't used for
production kernels and we can relax the checks a bit there.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/builtin-check.c | 57 ++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 31 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index a00a05d..4490601 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -97,6 +97,19 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,
 	return next;
 }
 
+static bool gcov_enabled(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *sym;
+
+	list_for_each_entry(sec, &file->elf->sections, list)
+		list_for_each_entry(sym, &sec->symbol_list, list)
+			if (!strncmp(sym->name, "__gcov_.", 8))
+				return true;
+
+	return false;
+}
+
 #define for_each_insn(file, insn)					\
 	list_for_each_entry(insn, &file->insn_list, list)
 
@@ -1041,34 +1054,6 @@ static int validate_branch(struct objtool_file *file,
 	return 0;
 }
 
-static bool is_gcov_insn(struct instruction *insn)
-{
-	struct rela *rela;
-	struct section *sec;
-	struct symbol *sym;
-	unsigned long offset;
-
-	rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
-	if (!rela)
-		return false;
-
-	if (rela->sym->type != STT_SECTION)
-		return false;
-
-	sec = rela->sym->sec;
-	offset = rela->addend + insn->offset + insn->len - rela->offset;
-
-	list_for_each_entry(sym, &sec->symbol_list, list) {
-		if (sym->type != STT_OBJECT)
-			continue;
-
-		if (offset >= sym->offset && offset < sym->offset + sym->len)
-			return (!memcmp(sym->name, "__gcov0.", 8));
-	}
-
-	return false;
-}
-
 static bool is_kasan_insn(struct instruction *insn)
 {
 	return (insn->type == INSN_CALL &&
@@ -1090,9 +1075,6 @@ static bool ignore_unreachable_insn(struct symbol *func,
 	if (insn->type == INSN_NOP)
 		return true;
 
-	if (is_gcov_insn(insn))
-		return true;
-
 	/*
 	 * Check if this (or a subsequent) instruction is related to
 	 * CONFIG_UBSAN or CONFIG_KASAN.
@@ -1153,6 +1135,19 @@ static int validate_functions(struct objtool_file *file)
 				    ignore_unreachable_insn(func, insn))
 					continue;
 
+				/*
+				 * gcov produces a lot of unreachable
+				 * instructions.  If we get an unreachable
+				 * warning and the file has gcov enabled, just
+				 * ignore it, and all other such warnings for
+				 * the file.
+				 */
+				if (!file->ignore_unreachables &&
+				    gcov_enabled(file)) {
+					file->ignore_unreachables = true;
+					continue;
+				}
+
 				WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
 				warnings++;
 			}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [tip:core/urgent] objtool: Improve rare switch jump table pattern detection
  2016-10-13 21:22 ` [PATCH 1/2] objtool: improve rare switch jump table pattern detection Josh Poimboeuf
@ 2016-10-16 11:18   ` tip-bot for Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2016-10-16 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, torvalds, mingo, hpa, jpoimboe, luto, arnd, bp, peterz,
	linux-kernel, dvlasenk, brgerst

Commit-ID:  3732710ff6f2ce2b1b7f044937a422b717d4f953
Gitweb:     http://git.kernel.org/tip/3732710ff6f2ce2b1b7f044937a422b717d4f953
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Thu, 13 Oct 2016 16:22:52 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 16 Oct 2016 09:12:35 +0200

objtool: Improve rare switch jump table pattern detection

GCC 6 added a new switch statement jump table optimization which makes
objtool's life harder.  It looks like:

  mov [rodata addr],%reg1
  ... some instructions ...
  jmpq *(%reg1,%reg2,8)

The optimization is quite rare, but objtool still needs to be able to
identify the pattern so that it can follow all possible control flow
paths related to the switch statement.

In order to detect the pattern, objtool starts from the indirect jump
and scans backwards through the function until it finds the first
instruction in the pattern.  If it encounters an unconditional jump
along the way, it stops and considers the pattern to be not found.

As it turns out, unconditional jumps can happen, as long as they are
small forward jumps within the range being scanned.

This fixes the following warnings:

  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

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/8a9ed68ae1780e8d3963e4ee13f2f257fe3a3c33.1476393584.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/objtool/builtin-check.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 143b6cd..a00a05d 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -713,6 +713,7 @@ static struct rela *find_switch_table(struct objtool_file *file,
 				      struct instruction *insn)
 {
 	struct rela *text_rela, *rodata_rela;
+	struct instruction *orig_insn = insn;
 
 	text_rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
 	if (text_rela && text_rela->sym == file->rodata->sym) {
@@ -733,10 +734,16 @@ static struct rela *find_switch_table(struct objtool_file *file,
 
 	/* case 3 */
 	func_for_each_insn_continue_reverse(file, func, insn) {
-		if (insn->type == INSN_JUMP_UNCONDITIONAL ||
-		    insn->type == INSN_JUMP_DYNAMIC)
+		if (insn->type == INSN_JUMP_DYNAMIC)
 			break;
 
+		/* allow small jumps within the range */
+		if (insn->type == INSN_JUMP_UNCONDITIONAL &&
+		    insn->jump_dest &&
+		    (insn->jump_dest->offset <= insn->offset ||
+		     insn->jump_dest->offset >= orig_insn->offset))
+		    break;
+
 		text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
 						    insn->len);
 		if (text_rela && text_rela->sym == file->rodata->sym)

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [tip:core/urgent] objtool: Skip all "unreachable instruction" warnings for gcov kernels
  2016-10-13 21:22 ` [PATCH 2/2] objtool: skip all "unreachable instruction" warnings for gcov kernels Josh Poimboeuf
@ 2016-10-16 11:18   ` tip-bot for Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2016-10-16 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bp, dvlasenk, arnd, torvalds, jpoimboe, peterz, hpa,
	linux-kernel, fengguang.wu, brgerst, luto, tglx, mingo

Commit-ID:  9cfffb116887b1b7c51cd4e3fa5790dc52a0758f
Gitweb:     http://git.kernel.org/tip/9cfffb116887b1b7c51cd4e3fa5790dc52a0758f
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Thu, 13 Oct 2016 16:22:53 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 16 Oct 2016 09:12:36 +0200

objtool: Skip all "unreachable instruction" warnings for gcov kernels

Recently objtool has started reporting a few "unreachable instruction"
warnings when CONFIG_GCOV is enabled for newer versions of GCC.  Usually
this warning means there's some new control flow that objtool doesn't
understand.  But in this case, objtool is correct and the instructions
really are inaccessible.  It's an annoying quirk of gcov, but it's
harmless, so it's ok to just silence the warnings.

With older versions of GCC, it was relatively easy to detect
gcov-specific instructions and to skip any unreachable warnings produced
by them.  But GCC 6 has gotten craftier.

Instead of continuing to play whack-a-mole with gcov, just use a bigger,
more permanent hammer and disable unreachable warnings for the whole
file when gcov is enabled.  This is fine to do because a) unreachable
warnings are usually of questionable value; and b) gcov isn't used for
production kernels and we can relax the checks a bit there.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/38d5c87d61d9cd46486dd2c86f46603dff0df86f.1476393584.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/objtool/builtin-check.c | 57 ++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 31 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index a00a05d..4490601 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -97,6 +97,19 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,
 	return next;
 }
 
+static bool gcov_enabled(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *sym;
+
+	list_for_each_entry(sec, &file->elf->sections, list)
+		list_for_each_entry(sym, &sec->symbol_list, list)
+			if (!strncmp(sym->name, "__gcov_.", 8))
+				return true;
+
+	return false;
+}
+
 #define for_each_insn(file, insn)					\
 	list_for_each_entry(insn, &file->insn_list, list)
 
@@ -1041,34 +1054,6 @@ static int validate_branch(struct objtool_file *file,
 	return 0;
 }
 
-static bool is_gcov_insn(struct instruction *insn)
-{
-	struct rela *rela;
-	struct section *sec;
-	struct symbol *sym;
-	unsigned long offset;
-
-	rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
-	if (!rela)
-		return false;
-
-	if (rela->sym->type != STT_SECTION)
-		return false;
-
-	sec = rela->sym->sec;
-	offset = rela->addend + insn->offset + insn->len - rela->offset;
-
-	list_for_each_entry(sym, &sec->symbol_list, list) {
-		if (sym->type != STT_OBJECT)
-			continue;
-
-		if (offset >= sym->offset && offset < sym->offset + sym->len)
-			return (!memcmp(sym->name, "__gcov0.", 8));
-	}
-
-	return false;
-}
-
 static bool is_kasan_insn(struct instruction *insn)
 {
 	return (insn->type == INSN_CALL &&
@@ -1090,9 +1075,6 @@ static bool ignore_unreachable_insn(struct symbol *func,
 	if (insn->type == INSN_NOP)
 		return true;
 
-	if (is_gcov_insn(insn))
-		return true;
-
 	/*
 	 * Check if this (or a subsequent) instruction is related to
 	 * CONFIG_UBSAN or CONFIG_KASAN.
@@ -1153,6 +1135,19 @@ static int validate_functions(struct objtool_file *file)
 				    ignore_unreachable_insn(func, insn))
 					continue;
 
+				/*
+				 * gcov produces a lot of unreachable
+				 * instructions.  If we get an unreachable
+				 * warning and the file has gcov enabled, just
+				 * ignore it, and all other such warnings for
+				 * the file.
+				 */
+				if (!file->ignore_unreachables &&
+				    gcov_enabled(file)) {
+					file->ignore_unreachables = true;
+					continue;
+				}
+
 				WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
 				warnings++;
 			}

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] objtool: a couple of minor fixes
  2016-10-13 21:22 [PATCH 0/2] objtool: a couple of minor fixes Josh Poimboeuf
  2016-10-13 21:22 ` [PATCH 1/2] objtool: improve rare switch jump table pattern detection Josh Poimboeuf
  2016-10-13 21:22 ` [PATCH 2/2] objtool: skip all "unreachable instruction" warnings for gcov kernels Josh Poimboeuf
@ 2016-10-26  7:58 ` Arnd Bergmann
  2016-10-26  9:16   ` Arnd Bergmann
                     ` (2 more replies)
  2 siblings, 3 replies; 14+ messages in thread
From: Arnd Bergmann @ 2016-10-26  7:58 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Thursday, October 13, 2016 4:22:51 PM CEST Josh Poimboeuf wrote:
> Fix a couple of issues:
> 
> - fix a false positive warning related to switch statement jump tables
> - get rid of useless "unreachable instruction" warnings for gcov kernels
> 
> Josh Poimboeuf (2):
>   objtool: improve rare switch jump table pattern detection
>   objtool: skip all "unreachable instruction" warnings for gcov kernels
> 
>  tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 33 deletions(-)

I got another warning today with linux-next, but have not looked into it:

drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer

.config and object file attached.

	Arnd

[-- Attachment #2: .config.zip --]
[-- Type: application/zip, Size: 35623 bytes --]

[-- Attachment #3: rxe_comp.o.zip --]
[-- Type: application/zip, Size: 3304 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] objtool: a couple of minor fixes
  2016-10-26  7:58 ` [PATCH 0/2] objtool: a couple of minor fixes Arnd Bergmann
@ 2016-10-26  9:16   ` Arnd Bergmann
  2016-10-26 12:37   ` Josh Poimboeuf
  2016-10-26 15:34   ` [PATCH] objtool: fix rare switch jump table pattern detection Josh Poimboeuf
  2 siblings, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2016-10-26  9:16 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel

On Wednesday, October 26, 2016 9:58:29 AM CEST Arnd Bergmann wrote:
> On Thursday, October 13, 2016 4:22:51 PM CEST Josh Poimboeuf wrote:
> > Fix a couple of issues:
> > 
> > - fix a false positive warning related to switch statement jump tables
> > - get rid of useless "unreachable instruction" warnings for gcov kernels
> > 
> > Josh Poimboeuf (2):
> >   objtool: improve rare switch jump table pattern detection
> >   objtool: skip all "unreachable instruction" warnings for gcov kernels
> > 
> >  tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++---------------------
> >  1 file changed, 35 insertions(+), 33 deletions(-)
> 
> I got another warning today with linux-next, but have not looked into it:
> 
> drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
> 
> .config and object file attached.


And another one, let me know if you want detailed information about
this one too, I suspect they are related.

drivers/media/usb/ttusb-dec/ttusb_dec.o: warning: objtool: ttusb_dec_process_urb_frame_list()+0x112: sibling call from callable instruction with changed frame pointer

	Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] objtool: a couple of minor fixes
  2016-10-26  7:58 ` [PATCH 0/2] objtool: a couple of minor fixes Arnd Bergmann
  2016-10-26  9:16   ` Arnd Bergmann
@ 2016-10-26 12:37   ` Josh Poimboeuf
  2016-10-26 12:43     ` Josh Poimboeuf
  2016-10-26 15:34   ` [PATCH] objtool: fix rare switch jump table pattern detection Josh Poimboeuf
  2 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-26 12:37 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ingo Molnar, linux-kernel

On Wed, Oct 26, 2016 at 09:58:29AM +0200, Arnd Bergmann wrote:
> On Thursday, October 13, 2016 4:22:51 PM CEST Josh Poimboeuf wrote:
> > Fix a couple of issues:
> > 
> > - fix a false positive warning related to switch statement jump tables
> > - get rid of useless "unreachable instruction" warnings for gcov kernels
> > 
> > Josh Poimboeuf (2):
> >   objtool: improve rare switch jump table pattern detection
> >   objtool: skip all "unreachable instruction" warnings for gcov kernels
> > 
> >  tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++---------------------
> >  1 file changed, 35 insertions(+), 33 deletions(-)
> 
> I got another warning today with linux-next, but have not looked into it:
> 
> drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
> 
> .config and object file attached.

I think this is the same as one of the warnings you reported before (for
which I opened the gcc bug):

  https://lkml.kernel.org/r/1855683.1QnG4Fe4Hq@wuerfel

-- 
Josh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] objtool: a couple of minor fixes
  2016-10-26 12:37   ` Josh Poimboeuf
@ 2016-10-26 12:43     ` Josh Poimboeuf
  2016-10-26 13:18       ` Josh Poimboeuf
  0 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-26 12:43 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ingo Molnar, linux-kernel

On Wed, Oct 26, 2016 at 07:37:39AM -0500, Josh Poimboeuf wrote:
> On Wed, Oct 26, 2016 at 09:58:29AM +0200, Arnd Bergmann wrote:
> > On Thursday, October 13, 2016 4:22:51 PM CEST Josh Poimboeuf wrote:
> > > Fix a couple of issues:
> > > 
> > > - fix a false positive warning related to switch statement jump tables
> > > - get rid of useless "unreachable instruction" warnings for gcov kernels
> > > 
> > > Josh Poimboeuf (2):
> > >   objtool: improve rare switch jump table pattern detection
> > >   objtool: skip all "unreachable instruction" warnings for gcov kernels
> > > 
> > >  tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++---------------------
> > >  1 file changed, 35 insertions(+), 33 deletions(-)
> > 
> > I got another warning today with linux-next, but have not looked into it:
> > 
> > drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
> > 
> > .config and object file attached.
> 
> I think this is the same as one of the warnings you reported before (for
> which I opened the gcc bug):
> 
>   https://lkml.kernel.org/r/1855683.1QnG4Fe4Hq@wuerfel

Er, scratch that.  Actually this is similar to one I supposedly fixed
with commit 3732710ff6f2ce2b1b7f044937a422b717d4f953.  And the other
warning looks similar.  Will take a look.

-- 
Josh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] objtool: a couple of minor fixes
  2016-10-26 12:43     ` Josh Poimboeuf
@ 2016-10-26 13:18       ` Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-26 13:18 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ingo Molnar, linux-kernel

On Wed, Oct 26, 2016 at 07:43:41AM -0500, Josh Poimboeuf wrote:
> On Wed, Oct 26, 2016 at 07:37:39AM -0500, Josh Poimboeuf wrote:
> > On Wed, Oct 26, 2016 at 09:58:29AM +0200, Arnd Bergmann wrote:
> > > On Thursday, October 13, 2016 4:22:51 PM CEST Josh Poimboeuf wrote:
> > > > Fix a couple of issues:
> > > > 
> > > > - fix a false positive warning related to switch statement jump tables
> > > > - get rid of useless "unreachable instruction" warnings for gcov kernels
> > > > 
> > > > Josh Poimboeuf (2):
> > > >   objtool: improve rare switch jump table pattern detection
> > > >   objtool: skip all "unreachable instruction" warnings for gcov kernels
> > > > 
> > > >  tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++---------------------
> > > >  1 file changed, 35 insertions(+), 33 deletions(-)
> > > 
> > > I got another warning today with linux-next, but have not looked into it:
> > > 
> > > drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
> > > 
> > > .config and object file attached.
> > 
> > I think this is the same as one of the warnings you reported before (for
> > which I opened the gcc bug):
> > 
> >   https://lkml.kernel.org/r/1855683.1QnG4Fe4Hq@wuerfel
> 
> Er, scratch that.  Actually this is similar to one I supposedly fixed
> with commit 3732710ff6f2ce2b1b7f044937a422b717d4f953.  And the other
> warning looks similar.  Will take a look.

As it turns out, this one is a simple fix to the above mentioned commit.

But as for the other warning...  the code generation is just crazy.
It's another issue with switch statement jump tables, which as it turns
out are really hard to detect.

I may ask the gcc folks for help.  If they could mark the jump tables
somehow, like maybe associating them with ELF symbols, it would make
objtool's life (and mine!) so much easier.

-- 
Josh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] objtool: fix rare switch jump table pattern detection
  2016-10-26  7:58 ` [PATCH 0/2] objtool: a couple of minor fixes Arnd Bergmann
  2016-10-26  9:16   ` Arnd Bergmann
  2016-10-26 12:37   ` Josh Poimboeuf
@ 2016-10-26 15:34   ` Josh Poimboeuf
  2016-10-26 16:03     ` Arnd Bergmann
  2016-10-27  7:37     ` [tip:core/urgent] objtool: Fix " tip-bot for Josh Poimboeuf
  2 siblings, 2 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-26 15:34 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ingo Molnar, linux-kernel

The following commit:

  3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")

... improved objtool's ability to detect gcc switch statement jump
tables for gcc 6.  However the check to allow short jumps with the
scanned range of instructions wasn't quite right.  The pattern detection
should allow jumps to the indirect jump instruction itself.

This fixes the following warning:

  drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 4490601..e8a1f69 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -754,7 +754,7 @@ static struct rela *find_switch_table(struct objtool_file *file,
 		if (insn->type == INSN_JUMP_UNCONDITIONAL &&
 		    insn->jump_dest &&
 		    (insn->jump_dest->offset <= insn->offset ||
-		     insn->jump_dest->offset >= orig_insn->offset))
+		     insn->jump_dest->offset > orig_insn->offset))
 		    break;
 
 		text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] objtool: fix rare switch jump table pattern detection
  2016-10-26 15:34   ` [PATCH] objtool: fix rare switch jump table pattern detection Josh Poimboeuf
@ 2016-10-26 16:03     ` Arnd Bergmann
  2016-10-26 16:45       ` Josh Poimboeuf
  2016-10-27  7:37     ` [tip:core/urgent] objtool: Fix " tip-bot for Josh Poimboeuf
  1 sibling, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2016-10-26 16:03 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel

On Wednesday, October 26, 2016 10:34:08 AM CEST Josh Poimboeuf wrote:
> The following commit:
> 
>   3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
> 
> ... improved objtool's ability to detect gcc switch statement jump
> tables for gcc 6.  However the check to allow short jumps with the
> scanned range of instructions wasn't quite right.  The pattern detection
> should allow jumps to the indirect jump instruction itself.
> 
> This fixes the following warning:
> 
>   drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
> 
> 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>

It fixes the mlx4/resource_tracker.o problem, but not the other
one for ttusb-dec/ttusb_dec.o, as you mentioned.

Do you need any more help creating a testcase for that one?

	Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] objtool: fix rare switch jump table pattern detection
  2016-10-26 16:03     ` Arnd Bergmann
@ 2016-10-26 16:45       ` Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2016-10-26 16:45 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ingo Molnar, linux-kernel

On Wed, Oct 26, 2016 at 06:03:44PM +0200, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 10:34:08 AM CEST Josh Poimboeuf wrote:
> > The following commit:
> > 
> >   3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
> > 
> > ... improved objtool's ability to detect gcc switch statement jump
> > tables for gcc 6.  However the check to allow short jumps with the
> > scanned range of instructions wasn't quite right.  The pattern detection
> > should allow jumps to the indirect jump instruction itself.
> > 
> > This fixes the following warning:
> > 
> >   drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
> > 
> > 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>
> 
> It fixes the mlx4/resource_tracker.o problem, but not the other
> one for ttusb-dec/ttusb_dec.o, as you mentioned.
> 
> Do you need any more help creating a testcase for that one?

No, I can recreate the ttusb_dec.o warning with the config you gave me
for this one.  Thanks!

-- 
Josh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tip:core/urgent] objtool: Fix rare switch jump table pattern detection
  2016-10-26 15:34   ` [PATCH] objtool: fix rare switch jump table pattern detection Josh Poimboeuf
  2016-10-26 16:03     ` Arnd Bergmann
@ 2016-10-27  7:37     ` tip-bot for Josh Poimboeuf
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2016-10-27  7:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, tglx, torvalds, jpoimboe, mingo, peterz, hpa, arnd

Commit-ID:  56fb2d6eb63acd48b50437b415b6f7d2fcffe75d
Gitweb:     http://git.kernel.org/tip/56fb2d6eb63acd48b50437b415b6f7d2fcffe75d
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Wed, 26 Oct 2016 10:34:08 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 27 Oct 2016 08:20:27 +0200

objtool: Fix rare switch jump table pattern detection

The following commit:

  3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")

... improved objtool's ability to detect GCC switch statement jump
tables for GCC 6.  However the check to allow short jumps with the
scanned range of instructions wasn't quite right.  The pattern detection
should allow jumps to the indirect jump instruction itself.

This fixes the following warning:

  drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
Link: http://lkml.kernel.org/r/20161026153408.2rifnw7bvoc5sex7@treble
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/objtool/builtin-check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 4490601..e8a1f69 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -754,7 +754,7 @@ static struct rela *find_switch_table(struct objtool_file *file,
 		if (insn->type == INSN_JUMP_UNCONDITIONAL &&
 		    insn->jump_dest &&
 		    (insn->jump_dest->offset <= insn->offset ||
-		     insn->jump_dest->offset >= orig_insn->offset))
+		     insn->jump_dest->offset > orig_insn->offset))
 		    break;
 
 		text_rela = find_rela_by_dest_range(insn->sec, insn->offset,

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-10-27  7:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-13 21:22 [PATCH 0/2] objtool: a couple of minor fixes Josh Poimboeuf
2016-10-13 21:22 ` [PATCH 1/2] objtool: improve rare switch jump table pattern detection Josh Poimboeuf
2016-10-16 11:18   ` [tip:core/urgent] objtool: Improve " tip-bot for Josh Poimboeuf
2016-10-13 21:22 ` [PATCH 2/2] objtool: skip all "unreachable instruction" warnings for gcov kernels Josh Poimboeuf
2016-10-16 11:18   ` [tip:core/urgent] objtool: Skip " tip-bot for Josh Poimboeuf
2016-10-26  7:58 ` [PATCH 0/2] objtool: a couple of minor fixes Arnd Bergmann
2016-10-26  9:16   ` Arnd Bergmann
2016-10-26 12:37   ` Josh Poimboeuf
2016-10-26 12:43     ` Josh Poimboeuf
2016-10-26 13:18       ` Josh Poimboeuf
2016-10-26 15:34   ` [PATCH] objtool: fix rare switch jump table pattern detection Josh Poimboeuf
2016-10-26 16:03     ` Arnd Bergmann
2016-10-26 16:45       ` Josh Poimboeuf
2016-10-27  7:37     ` [tip:core/urgent] objtool: Fix " tip-bot for Josh Poimboeuf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).