From: Peter Zijlstra <peterz@infradead.org>
To: linux-kernel@vger.kernel.org
Cc: linux-tip-commits@vger.kernel.org,
Julien Thierry <jthierry@redhat.com>,
Miroslav Benes <mbenes@suse.cz>,
Alexandre Chartre <alexandre.chartre@oracle.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Ingo Molnar <mingo@kernel.org>, x86 <x86@kernel.org>
Subject: Re: [tip: objtool/core] objtool: Support multiple stack_op per instruction
Date: Thu, 23 Apr 2020 13:15:24 +0200 [thread overview]
Message-ID: <20200423111524.GS20730@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <158762818246.28353.13419513995701103731.tip-bot2@tip-bot2>
On Thu, Apr 23, 2020 at 07:49:42AM -0000, tip-bot2 for Julien Thierry wrote:
> diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
> index 7ce8650..199b408 100644
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -80,13 +80,15 @@ unsigned long arch_jump_destination(struct instruction *insn)
> int arch_decode_instruction(struct elf *elf, struct section *sec,
> unsigned long offset, unsigned int maxlen,
> unsigned int *len, enum insn_type *type,
> - unsigned long *immediate, struct stack_op *op)
> + unsigned long *immediate,
> + struct list_head *ops_list)
> {
> struct insn insn;
> int x86_64, sign;
> unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0,
> rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
> modrm_reg = 0, sib = 0;
> + struct stack_op *op;
>
> x86_64 = is_x86_64(elf);
> if (x86_64 == -1)
> @@ -127,6 +129,10 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
> if (insn.sib.nbytes)
> sib = insn.sib.bytes[0];
>
> + op = calloc(1, sizeof(*op));
> + if (!op)
> + return -1;
> +
> switch (op1) {
>
> case 0x1:
> @@ -488,6 +494,11 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
>
> *immediate = insn.immediate.nbytes ? insn.immediate.value : 0;
>
> + if (*type == INSN_STACK)
> + list_add_tail(&op->list, ops_list);
> + else
> + free(op);
> +
> return 0;
> }
So I was playing around with the intra-function thing, trying to address
Josh's comments from last time, but that also got me staring at this
again because it adds yet another type with a stack-op.
How do people feel about something like so?
---
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -77,6 +77,17 @@ unsigned long arch_jump_destination(stru
return insn->offset + insn->len + insn->immediate;
}
+#define PUSH_OP(op) \
+({ \
+ list_add_tail(&op->list, ops_list); \
+ NULL; \
+})
+
+#define ADD_OP(op) \
+ if (!(op = calloc(1, sizeof(*op)))) \
+ return -1; \
+ else for (; op; op = PUSH_OP(op))
+
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
@@ -88,7 +99,7 @@ int arch_decode_instruction(struct elf *
unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0,
rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
modrm_reg = 0, sib = 0;
- struct stack_op *op;
+ struct stack_op *op = NULL;
x86_64 = is_x86_64(elf);
if (x86_64 == -1)
@@ -129,10 +140,6 @@ int arch_decode_instruction(struct elf *
if (insn.sib.nbytes)
sib = insn.sib.bytes[0];
- op = calloc(1, sizeof(*op));
- if (!op)
- return -1;
-
switch (op1) {
case 0x1:
@@ -141,10 +148,12 @@ int arch_decode_instruction(struct elf *
/* add/sub reg, %rsp */
*type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
}
break;
@@ -152,9 +161,11 @@ int arch_decode_instruction(struct elf *
/* push reg */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG;
- op->src.reg = op_to_cfi_reg[op1 & 0x7][rex_b];
- op->dest.type = OP_DEST_PUSH;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG;
+ op->src.reg = op_to_cfi_reg[op1 & 0x7][rex_b];
+ op->dest.type = OP_DEST_PUSH;
+ }
break;
@@ -162,9 +173,11 @@ int arch_decode_instruction(struct elf *
/* pop reg */
*type = INSN_STACK;
- op->src.type = OP_SRC_POP;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = op_to_cfi_reg[op1 & 0x7][rex_b];
+ ADD_OP(op) {
+ op->src.type = OP_SRC_POP;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = op_to_cfi_reg[op1 & 0x7][rex_b];
+ }
break;
@@ -172,8 +185,10 @@ int arch_decode_instruction(struct elf *
case 0x6a:
/* push immediate */
*type = INSN_STACK;
- op->src.type = OP_SRC_CONST;
- op->dest.type = OP_DEST_PUSH;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_CONST;
+ op->dest.type = OP_DEST_PUSH;
+ }
break;
case 0x70 ... 0x7f:
@@ -188,11 +203,13 @@ int arch_decode_instruction(struct elf *
if (modrm == 0xe4) {
/* and imm, %rsp */
*type = INSN_STACK;
- op->src.type = OP_SRC_AND;
- op->src.reg = CFI_SP;
- op->src.offset = insn.immediate.value;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_AND;
+ op->src.reg = CFI_SP;
+ op->src.offset = insn.immediate.value;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
break;
}
@@ -205,11 +222,13 @@ int arch_decode_instruction(struct elf *
/* add/sub imm, %rsp */
*type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
- op->src.offset = insn.immediate.value * sign;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_SP;
+ op->src.offset = insn.immediate.value * sign;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
break;
case 0x89:
@@ -217,10 +236,12 @@ int arch_decode_instruction(struct elf *
/* mov %rsp, reg */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG;
- op->src.reg = CFI_SP;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG;
+ op->src.reg = CFI_SP;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
+ }
break;
}
@@ -228,10 +249,12 @@ int arch_decode_instruction(struct elf *
/* mov reg, %rsp */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG;
- op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG;
+ op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
break;
}
@@ -242,21 +265,25 @@ int arch_decode_instruction(struct elf *
/* mov reg, disp(%rbp) */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG;
- op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
- op->dest.type = OP_DEST_REG_INDIRECT;
- op->dest.reg = CFI_BP;
- op->dest.offset = insn.displacement.value;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG;
+ op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ op->dest.type = OP_DEST_REG_INDIRECT;
+ op->dest.reg = CFI_BP;
+ op->dest.offset = insn.displacement.value;
+ }
} else if (rex_w && !rex_b && modrm_rm == 4 && sib == 0x24) {
/* mov reg, disp(%rsp) */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG;
- op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
- op->dest.type = OP_DEST_REG_INDIRECT;
- op->dest.reg = CFI_SP;
- op->dest.offset = insn.displacement.value;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG;
+ op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ op->dest.type = OP_DEST_REG_INDIRECT;
+ op->dest.reg = CFI_SP;
+ op->dest.offset = insn.displacement.value;
+ }
}
break;
@@ -266,22 +293,26 @@ int arch_decode_instruction(struct elf *
/* mov disp(%rbp), reg */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG_INDIRECT;
- op->src.reg = CFI_BP;
- op->src.offset = insn.displacement.value;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG_INDIRECT;
+ op->src.reg = CFI_BP;
+ op->src.offset = insn.displacement.value;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ }
} else if (rex_w && !rex_b && sib == 0x24 &&
modrm_mod != 3 && modrm_rm == 4) {
/* mov disp(%rsp), reg */
*type = INSN_STACK;
- op->src.type = OP_SRC_REG_INDIRECT;
- op->src.reg = CFI_SP;
- op->src.offset = insn.displacement.value;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ ADD_OP(op) {
+ op->src.type = OP_SRC_REG_INDIRECT;
+ op->src.reg = CFI_SP;
+ op->src.offset = insn.displacement.value;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
+ }
}
break;
@@ -290,27 +321,31 @@ int arch_decode_instruction(struct elf *
if (sib == 0x24 && rex_w && !rex_b && !rex_x) {
*type = INSN_STACK;
- if (!insn.displacement.value) {
- /* lea (%rsp), reg */
- op->src.type = OP_SRC_REG;
- } else {
- /* lea disp(%rsp), reg */
- op->src.type = OP_SRC_ADD;
- op->src.offset = insn.displacement.value;
+ ADD_OP(op) {
+ if (!insn.displacement.value) {
+ /* lea (%rsp), reg */
+ op->src.type = OP_SRC_REG;
+ } else {
+ /* lea disp(%rsp), reg */
+ op->src.type = OP_SRC_ADD;
+ op->src.offset = insn.displacement.value;
+ }
+ op->src.reg = CFI_SP;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
}
- op->src.reg = CFI_SP;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r];
} else if (rex == 0x48 && modrm == 0x65) {
/* lea disp(%rbp), %rsp */
*type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_BP;
- op->src.offset = insn.displacement.value;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_BP;
+ op->src.offset = insn.displacement.value;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
} else if (rex == 0x49 && modrm == 0x62 &&
insn.displacement.value == -8) {
@@ -322,11 +357,13 @@ int arch_decode_instruction(struct elf *
* stack realignment.
*/
*type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_R10;
- op->src.offset = -8;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_R10;
+ op->src.offset = -8;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
} else if (rex == 0x49 && modrm == 0x65 &&
insn.displacement.value == -16) {
@@ -338,11 +375,13 @@ int arch_decode_instruction(struct elf *
* stack realignment.
*/
*type = INSN_STACK;
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_R13;
- op->src.offset = -16;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_R13;
+ op->src.offset = -16;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
}
break;
@@ -350,8 +389,10 @@ int arch_decode_instruction(struct elf *
case 0x8f:
/* pop to mem */
*type = INSN_STACK;
- op->src.type = OP_SRC_POP;
- op->dest.type = OP_DEST_MEM;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_POP;
+ op->dest.type = OP_DEST_MEM;
+ }
break;
case 0x90:
@@ -361,15 +402,19 @@ int arch_decode_instruction(struct elf *
case 0x9c:
/* pushf */
*type = INSN_STACK;
- op->src.type = OP_SRC_CONST;
- op->dest.type = OP_DEST_PUSHF;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_CONST;
+ op->dest.type = OP_DEST_PUSHF;
+ }
break;
case 0x9d:
/* popf */
*type = INSN_STACK;
- op->src.type = OP_SRC_POPF;
- op->dest.type = OP_DEST_MEM;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_POPF;
+ op->dest.type = OP_DEST_MEM;
+ }
break;
case 0x0f:
@@ -405,15 +450,19 @@ int arch_decode_instruction(struct elf *
/* push fs/gs */
*type = INSN_STACK;
- op->src.type = OP_SRC_CONST;
- op->dest.type = OP_DEST_PUSH;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_CONST;
+ op->dest.type = OP_DEST_PUSH;
+ }
} else if (op2 == 0xa1 || op2 == 0xa9) {
/* pop fs/gs */
*type = INSN_STACK;
- op->src.type = OP_SRC_POP;
- op->dest.type = OP_DEST_MEM;
+ ADD_OP(op) {
+ op->src.type = OP_SRC_POP;
+ op->dest.type = OP_DEST_MEM;
+ }
}
break;
@@ -427,7 +476,8 @@ int arch_decode_instruction(struct elf *
* pop bp
*/
*type = INSN_STACK;
- op->dest.type = OP_DEST_LEAVE;
+ ADD_OP(op)
+ op->dest.type = OP_DEST_LEAVE;
break;
@@ -449,12 +499,14 @@ int arch_decode_instruction(struct elf *
case 0xcf: /* iret */
*type = INSN_EXCEPTION_RETURN;
- /* add $40, %rsp */
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
- op->src.offset = 5*8;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ ADD_OP(op) {
+ /* add $40, %rsp */
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_SP;
+ op->src.offset = 5*8;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
break;
case 0xca: /* retf */
@@ -504,11 +556,6 @@ int arch_decode_instruction(struct elf *
*immediate = insn.immediate.nbytes ? insn.immediate.value : 0;
- if (*type == INSN_STACK || *type == INSN_EXCEPTION_RETURN)
- list_add_tail(&op->list, ops_list);
- else
- free(op);
-
return 0;
}
next prev parent reply other threads:[~2020-04-23 11:15 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-27 15:28 [PATCH v2 00/10] Objtool updates for easier portability Julien Thierry
2020-03-27 15:28 ` [PATCH v2 01/10] objtool: Move header sync-check ealier in build Julien Thierry
2020-04-01 12:32 ` Miroslav Benes
2020-04-01 12:44 ` Julien Thierry
2020-04-01 12:55 ` Miroslav Benes
2020-04-02 17:12 ` Josh Poimboeuf
2020-04-02 17:17 ` Josh Poimboeuf
2020-03-27 15:28 ` [PATCH v2 02/10] objtool: check: Remove redundant checks on operand type Julien Thierry
2020-03-27 15:28 ` [PATCH v2 03/10] objtool: check: Clean instruction state before each function validation Julien Thierry
2020-03-27 15:28 ` [PATCH v2 04/10] objtool: check: Ignore empty alternative groups Julien Thierry
2020-04-01 12:53 ` Miroslav Benes
2020-04-01 13:43 ` Julien Thierry
2020-04-01 13:48 ` Miroslav Benes
2020-03-27 15:28 ` [PATCH v2 05/10] objtool: check: Remove check preventing branches within alternative Julien Thierry
2020-05-01 18:22 ` [tip: objtool/core] objtool: " tip-bot2 for Julien Thierry
2020-03-27 15:28 ` [PATCH v2 06/10] objtool: check: Use arch specific values in restore_reg() Julien Thierry
2020-03-27 15:28 ` [PATCH v2 07/10] objtool: check: Allow save/restore hint in non standard function symbols Julien Thierry
2020-04-01 13:54 ` Miroslav Benes
2020-04-01 14:06 ` Julien Thierry
2020-03-27 15:28 ` [PATCH v2 08/10] objtool: Add abstraction for computation of symbols offsets Julien Thierry
2020-03-27 15:28 ` [PATCH v2 09/10] objtool: Split generic and arch specific CFI definitions Julien Thierry
2020-03-27 15:28 ` [PATCH v2 10/10] objtool: Support multiple stack_op per instruction Julien Thierry
2020-04-02 17:54 ` Josh Poimboeuf
2020-04-02 18:25 ` Peter Zijlstra
2020-04-03 8:01 ` Julien Thierry
2020-04-03 14:55 ` Josh Poimboeuf
2020-04-22 22:24 ` [tip: objtool/core] " tip-bot2 for Julien Thierry
2020-04-23 7:49 ` tip-bot2 for Julien Thierry
2020-04-23 11:15 ` Peter Zijlstra [this message]
2020-04-02 17:58 ` [PATCH v2 00/10] Objtool updates for easier portability Josh Poimboeuf
2020-04-02 18:30 ` Peter Zijlstra
2020-04-03 7:17 ` Miroslav Benes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200423111524.GS20730@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=alexandre.chartre@oracle.com \
--cc=jpoimboe@redhat.com \
--cc=jthierry@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=mingo@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.