Skip to content

Commit

Permalink
x86/insn: Remove superfluous checks from instruction decoding routines
Browse files Browse the repository at this point in the history 
It's pointless checking if a particular part of an instruction is
decoded before calling the routine responsible for decoding it as this
check is duplicated in the routines itself. Streamline the code by
removing the superfluous checks. No functional difference.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20240222111636.2214523-2-nik.borisov@suse.com
  • Loading branch information
Nikolay Borisov authored and Ingo Molnar committed Feb 22, 2024
1 parent b401b62 commit 427e164
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 70 deletions.
56 changes: 21 additions & 35 deletions arch/x86/lib/insn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,9 @@ int insn_get_opcode(struct insn *insn)
if (opcode->got)
return 0;

if (!insn->prefixes.got) {
ret = insn_get_prefixes(insn);
if (ret)
return ret;
}
ret = insn_get_prefixes(insn);
if (ret)
return ret;

/* Get first opcode */
op = get_next(insn_byte_t, insn);
Expand Down Expand Up @@ -339,11 +337,9 @@ int insn_get_modrm(struct insn *insn)
if (modrm->got)
return 0;

if (!insn->opcode.got) {
ret = insn_get_opcode(insn);
if (ret)
return ret;
}
ret = insn_get_opcode(insn);
if (ret)
return ret;

if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
Expand Down Expand Up @@ -386,11 +382,9 @@ int insn_rip_relative(struct insn *insn)
if (!insn->x86_64)
return 0;

if (!modrm->got) {
ret = insn_get_modrm(insn);
if (ret)
return 0;
}
ret = insn_get_modrm(insn);
if (ret)
return 0;
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
Expand All @@ -417,11 +411,9 @@ int insn_get_sib(struct insn *insn)
if (insn->sib.got)
return 0;

if (!insn->modrm.got) {
ret = insn_get_modrm(insn);
if (ret)
return ret;
}
ret = insn_get_modrm(insn);
if (ret)
return ret;

if (insn->modrm.nbytes) {
modrm = insn->modrm.bytes[0];
Expand Down Expand Up @@ -460,11 +452,9 @@ int insn_get_displacement(struct insn *insn)
if (insn->displacement.got)
return 0;

if (!insn->sib.got) {
ret = insn_get_sib(insn);
if (ret)
return ret;
}
ret = insn_get_sib(insn);
if (ret)
return ret;

if (insn->modrm.nbytes) {
/*
Expand Down Expand Up @@ -628,11 +618,9 @@ int insn_get_immediate(struct insn *insn)
if (insn->immediate.got)
return 0;

if (!insn->displacement.got) {
ret = insn_get_displacement(insn);
if (ret)
return ret;
}
ret = insn_get_displacement(insn);
if (ret)
return ret;

if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
Expand Down Expand Up @@ -703,11 +691,9 @@ int insn_get_length(struct insn *insn)
if (insn->length)
return 0;

if (!insn->immediate.got) {
ret = insn_get_immediate(insn);
if (ret)
return ret;
}
ret = insn_get_immediate(insn);
if (ret)
return ret;

insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
Expand Down
56 changes: 21 additions & 35 deletions tools/arch/x86/lib/insn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,9 @@ int insn_get_opcode(struct insn *insn)
if (opcode->got)
return 0;

if (!insn->prefixes.got) {
ret = insn_get_prefixes(insn);
if (ret)
return ret;
}
ret = insn_get_prefixes(insn);
if (ret)
return ret;

/* Get first opcode */
op = get_next(insn_byte_t, insn);
Expand Down Expand Up @@ -339,11 +337,9 @@ int insn_get_modrm(struct insn *insn)
if (modrm->got)
return 0;

if (!insn->opcode.got) {
ret = insn_get_opcode(insn);
if (ret)
return ret;
}
ret = insn_get_opcode(insn);
if (ret)
return ret;

if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
Expand Down Expand Up @@ -386,11 +382,9 @@ int insn_rip_relative(struct insn *insn)
if (!insn->x86_64)
return 0;

if (!modrm->got) {
ret = insn_get_modrm(insn);
if (ret)
return 0;
}
ret = insn_get_modrm(insn);
if (ret)
return 0;
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
Expand All @@ -417,11 +411,9 @@ int insn_get_sib(struct insn *insn)
if (insn->sib.got)
return 0;

if (!insn->modrm.got) {
ret = insn_get_modrm(insn);
if (ret)
return ret;
}
ret = insn_get_modrm(insn);
if (ret)
return ret;

if (insn->modrm.nbytes) {
modrm = insn->modrm.bytes[0];
Expand Down Expand Up @@ -460,11 +452,9 @@ int insn_get_displacement(struct insn *insn)
if (insn->displacement.got)
return 0;

if (!insn->sib.got) {
ret = insn_get_sib(insn);
if (ret)
return ret;
}
ret = insn_get_sib(insn);
if (ret)
return ret;

if (insn->modrm.nbytes) {
/*
Expand Down Expand Up @@ -628,11 +618,9 @@ int insn_get_immediate(struct insn *insn)
if (insn->immediate.got)
return 0;

if (!insn->displacement.got) {
ret = insn_get_displacement(insn);
if (ret)
return ret;
}
ret = insn_get_displacement(insn);
if (ret)
return ret;

if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
Expand Down Expand Up @@ -703,11 +691,9 @@ int insn_get_length(struct insn *insn)
if (insn->length)
return 0;

if (!insn->immediate.got) {
ret = insn_get_immediate(insn);
if (ret)
return ret;
}
ret = insn_get_immediate(insn);
if (ret)
return ret;

insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
Expand Down

0 comments on commit 427e164

Please sign in to comment.