new objtool unreachable instruction warnings

new objtool unreachable instruction warnings

[Arnd Bergmann]

I only saw these warnings once, they are either very rare, or were introduced
recently:

arch/x86/kvm/vmx.o: warning: objtool: .altinstr_replacement+0x0:
unreachable instruction
arch/x86/kvm/svm.o: warning: objtool: .altinstr_replacement+0x6e:
unreachable instruction

I see this with gcc-4.6 though gcc-7, but not with gcc-4.3.

The configuration file that triggered it is
https://pastebin.com/aMn45GYP

I managed to trace the problem down to the CC_HAVE_ASM_GOTO
macro, without that, we don't run into the problem.

       Arnd

Re: new objtool unreachable instruction warnings

[Josh Poimboeuf]

On Wed, Jul 26, 2017 at 04:46:40PM +0200, Arnd Bergmann wrote:
> I only saw these warnings once, they are either very rare, or were introduced
> recently:
> 
> arch/x86/kvm/vmx.o: warning: objtool: .altinstr_replacement+0x0:
> unreachable instruction
> arch/x86/kvm/svm.o: warning: objtool: .altinstr_replacement+0x6e:
> unreachable instruction
> 
> I see this with gcc-4.6 though gcc-7, but not with gcc-4.3.
> 
> The configuration file that triggered it is
> https://pastebin.com/aMn45GYP
> 
> I managed to trace the problem down to the CC_HAVE_ASM_GOTO
> macro, without that, we don't run into the problem.

Thanks for reporting all these!

This should fix all the warnings you reported.  I'll split it up into
real patches and submit them soon.  Let me know if you find anything
else.

diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index 21aeca874edb..b0d7dc3d71b5 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -31,8 +31,9 @@
 #define INSN_RETURN		6
 #define INSN_CONTEXT_SWITCH	7
 #define INSN_STACK		8
-#define INSN_NOP		9
-#define INSN_OTHER		10
+#define INSN_BUG		9
+#define INSN_NOP		10
+#define INSN_OTHER		11
 #define INSN_LAST		INSN_OTHER
 
 enum op_dest_type {
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index a36c2eba64e7..7841e5d31973 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -271,7 +271,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
 	case 0x8d:
 		if (rex == 0x48 && modrm == 0x65) {
 
-			/* lea -disp(%rbp), %rsp */
+			/* lea disp(%rbp), %rsp */
 			*type = INSN_STACK;
 			op->src.type = OP_SRC_ADD;
 			op->src.reg = CFI_BP;
@@ -281,6 +281,30 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
 			break;
 		}
 
+		if (rex == 0x48 && (modrm == 0xa4 || modrm == 0x64) &&
+		    sib == 0x24) {
+
+			/* lea disp(%rsp), %rsp */
+			*type = INSN_STACK;
+			op->src.type = OP_SRC_ADD;
+			op->src.reg = CFI_SP;
+			op->src.offset = insn.displacement.value;
+			op->dest.type = OP_DEST_REG;
+			op->dest.reg = CFI_SP;
+			break;
+		}
+
+		if (rex == 0x48 && modrm == 0x2c && sib == 0x24) {
+
+			/* lea (%rsp), %rbp */
+			*type = INSN_STACK;
+			op->src.type = OP_SRC_REG;
+			op->src.reg = CFI_SP;
+			op->dest.type = OP_DEST_REG;
+			op->dest.reg = CFI_BP;
+			break;
+		}
+
 		if (rex == 0x4c && modrm == 0x54 && sib == 0x24 &&
 		    insn.displacement.value == 8) {
 
@@ -382,20 +406,27 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
 
 	case 0x0f:
 
-		if (op2 >= 0x80 && op2 <= 0x8f)
+		if (op2 >= 0x80 && op2 <= 0x8f) {
+
 			*type = INSN_JUMP_CONDITIONAL;
-		else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 ||
-			 op2 == 0x35)
+
+		} else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 ||
+			   op2 == 0x35) {
 
 			/* sysenter, sysret */
 			*type = INSN_CONTEXT_SWITCH;
 
-		else if (op2 == 0x0d || op2 == 0x1f)
+		} else if (op2 == 0x0b || op2 == 0xb9) {
+
+			/* ud2 */
+			*type = INSN_BUG;
+
+		} else if (op2 == 0x0d || op2 == 0x1f) {
 
 			/* nopl/nopw */
 			*type = INSN_NOP;
 
-		else if (op2 == 0xa0 || op2 == 0xa8) {
+		} else if (op2 == 0xa0 || op2 == 0xa8) {
 
 			/* push fs/gs */
 			*type = INSN_STACK;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3436a942b606..7578dd9d1148 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1632,6 +1632,9 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
 
 			break;
 
+		case INSN_BUG:
+			return 0;
+
 		default:
 			break;
 		}
@@ -1693,8 +1696,13 @@ static bool ignore_unreachable_insn(struct instruction *insn)
 	/*
 	 * Ignore any unused exceptions.  This can happen when a whitelisted
 	 * function has an exception table entry.
+	 *
+	 * Also ignore alternative replacement instructions.  This can happen
+	 * when a whitelisted function uses one of the ALTERNATIVE macros.
 	 */
-	if (!strcmp(insn->sec->name, ".fixup"))
+	if (!strcmp(insn->sec->name, ".fixup") ||
+	    !strcmp(insn->sec->name, ".altinstr_replacement") ||
+	    !strcmp(insn->sec->name, ".altinstr_aux"))
 		return true;
 
 	/*

Re: new objtool unreachable instruction warnings

[Arnd Bergmann]

On Wed, Jul 26, 2017 at 9:10 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Jul 26, 2017 at 04:46:40PM +0200, Arnd Bergmann wrote:
>> I only saw these warnings once, they are either very rare, or were introduced
>> recently:
>>
>> arch/x86/kvm/vmx.o: warning: objtool: .altinstr_replacement+0x0:
>> unreachable instruction
>> arch/x86/kvm/svm.o: warning: objtool: .altinstr_replacement+0x6e:
>> unreachable instruction
>>
>> I see this with gcc-4.6 though gcc-7, but not with gcc-4.3.
>>
>> The configuration file that triggered it is
>> https://pastebin.com/aMn45GYP
>>
>> I managed to trace the problem down to the CC_HAVE_ASM_GOTO
>> macro, without that, we don't run into the problem.
>
> Thanks for reporting all these!
>
> This should fix all the warnings you reported.  I'll split it up into
> real patches and submit them soon.  Let me know if you find anything
> else.

Looks good. I tried ten different randconfigs that found warnings in the
past few days, and all the warnings are gone now.

For the warnings that showed up only with gcc-7, I think we may want
a backport to stable kernels, the other ones seem to be recent additions
(after 4.12).

Thanks,

     Arnd

Re: new objtool unreachable instruction warnings

[Josh Poimboeuf]

On Wed, Jul 26, 2017 at 09:52:36PM +0200, Arnd Bergmann wrote:
> On Wed, Jul 26, 2017 at 9:10 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Jul 26, 2017 at 04:46:40PM +0200, Arnd Bergmann wrote:
> >> I only saw these warnings once, they are either very rare, or were introduced
> >> recently:
> >>
> >> arch/x86/kvm/vmx.o: warning: objtool: .altinstr_replacement+0x0:
> >> unreachable instruction
> >> arch/x86/kvm/svm.o: warning: objtool: .altinstr_replacement+0x6e:
> >> unreachable instruction
> >>
> >> I see this with gcc-4.6 though gcc-7, but not with gcc-4.3.
> >>
> >> The configuration file that triggered it is
> >> https://pastebin.com/aMn45GYP
> >>
> >> I managed to trace the problem down to the CC_HAVE_ASM_GOTO
> >> macro, without that, we don't run into the problem.
> >
> > Thanks for reporting all these!
> >
> > This should fix all the warnings you reported.  I'll split it up into
> > real patches and submit them soon.  Let me know if you find anything
> > else.
> 
> Looks good. I tried ten different randconfigs that found warnings in the
> past few days, and all the warnings are gone now.
> 
> For the warnings that showed up only with gcc-7, I think we may want
> a backport to stable kernels, the other ones seem to be recent additions
> (after 4.12).

Yeah, I think you're right.

The GCC 7 one (where GCC inserts 'ud2' instead of dividing by zero) was
introduced with:

  d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

The atom one is only in -next.

And the one above (arch/x86/kvm/vmx.o .altinstr_replacement unreachable
instruction) has always been possible, but has only recently been made
visible with some new objtool whitelisting in -next.

-- 
Josh